From 2febb7bf5f2f6c683610c35aa6ab9718eecc1788 Mon Sep 17 00:00:00 2001 From: Denis Bezrukov <6227442+denbezrukov@users.noreply.github.com> Date: Fri, 23 Jun 2023 22:27:52 +0300 Subject: [PATCH] fix(rome_js_analyze): Add constructor support --- CHANGELOG.md | 2 + .../suspicious/no_duplicate_parameters.rs | 53 +-- .../noDuplicateParameters/invalid.jsonc | 13 - .../noDuplicateParameters/invalid.jsonc.snap | 214 --------- .../noDuplicateParameters/invalid.ts | 28 ++ .../noDuplicateParameters/invalid.ts.snap | 308 +++++++++++++ .../src/js/bindings/parameters.rs | 6 +- .../js/lists/constructor_parameter_list.rs | 3 +- .../src/js/lists/parameter_list.rs | 100 +---- crates/rome_js_syntax/src/lib.rs | 1 + crates/rome_js_syntax/src/parameter_ext.rs | 412 ++++++++++++++++++ 11 files changed, 775 insertions(+), 365 deletions(-) delete mode 100644 crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.jsonc delete mode 100644 crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.jsonc.snap create mode 100644 crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.ts create mode 100644 crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.ts.snap create mode 100644 crates/rome_js_syntax/src/parameter_ext.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a1489a1613..39b93e8b8e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,8 @@ multiple files: ### Linter +- [`noDuplicateParameters`](https://docs.rome.tools/lint/rules/noduplicateparameters/): enhanced rule to manage constructor parameters. + #### BREAKING CHANGES - Remove `lint/complexity/noExtraSemicolon` ([#4553](https://github.com/rome/tools/issues/4553)) diff --git a/crates/rome_js_analyze/src/semantic_analyzers/suspicious/no_duplicate_parameters.rs b/crates/rome_js_analyze/src/semantic_analyzers/suspicious/no_duplicate_parameters.rs index ab5426b39c7..6ef8f5c0836 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/suspicious/no_duplicate_parameters.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/suspicious/no_duplicate_parameters.rs @@ -1,12 +1,11 @@ use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic}; use rome_console::markup; +use rome_js_syntax::parameter_ext::{AnyJsParameterList, AnyJsParameters, AnyParameter}; use rome_js_syntax::{ - AnyJsArrayBindingPatternElement, AnyJsBinding, AnyJsBindingPattern, AnyJsFormalParameter, - AnyJsObjectBindingPatternMember, AnyJsParameter, JsArrowFunctionExpression, - JsFunctionDeclaration, JsFunctionExportDefaultDeclaration, JsFunctionExpression, - JsIdentifierBinding, JsMethodClassMember, JsMethodObjectMember, + AnyJsArrayBindingPatternElement, AnyJsBinding, AnyJsBindingPattern, + AnyJsObjectBindingPatternMember, JsIdentifierBinding, }; -use rome_rowan::{declare_node_union, AstNode}; +use rome_rowan::AstNode; use rustc_hash::FxHashSet; declare_rule! { @@ -47,28 +46,26 @@ declare_rule! { } impl Rule for NoDuplicateParameters { - type Query = Ast; + type Query = Ast; type State = JsIdentifierBinding; type Signals = Option; type Options = (); fn run(ctx: &RuleContext) -> Option { - let function = ctx.query(); - let args = match function { - AnyJsFunctionAndMethod::JsArrowFunctionExpression(func) => { - func.parameters().ok()?.as_js_parameters()?.clone() + let parameters = ctx.query(); + + let list = match parameters { + AnyJsParameters::JsParameters(parameters) => { + AnyJsParameterList::from(parameters.items()) } - AnyJsFunctionAndMethod::JsFunctionDeclaration(func) => func.parameters().ok()?, - AnyJsFunctionAndMethod::JsFunctionExportDefaultDeclaration(func) => { - func.parameters().ok()? + AnyJsParameters::JsConstructorParameters(parameters) => { + AnyJsParameterList::from(parameters.parameters()) } - AnyJsFunctionAndMethod::JsFunctionExpression(func) => func.parameters().ok()?, - AnyJsFunctionAndMethod::JsMethodClassMember(member) => member.parameters().ok()?, - AnyJsFunctionAndMethod::JsMethodObjectMember(member) => member.parameters().ok()?, }; + let mut set = FxHashSet::default(); // Traversing the parameters of the function in preorder and checking for duplicates, - args.items().into_iter().find_map(|parameter| { + list.iter().find_map(|parameter| { let parameter = parameter.ok()?; traverse_parameter(parameter, &mut set) }) @@ -92,21 +89,12 @@ impl Rule for NoDuplicateParameters { /// Traverse the parameter recursively and check if it is duplicated. /// Return `Some(JsIdentifierBinding)` if it is duplicated. fn traverse_parameter( - parameter: AnyJsParameter, + parameter: AnyParameter, tracked_bindings: &mut FxHashSet, ) -> Option { - match parameter { - AnyJsParameter::AnyJsFormalParameter(p) => match p { - AnyJsFormalParameter::JsFormalParameter(parameter) => { - traverse_binding(parameter.binding().ok()?, tracked_bindings) - } - AnyJsFormalParameter::JsBogusParameter(_) => None, - }, - AnyJsParameter::JsRestParameter(rest_parameter) => { - traverse_binding(rest_parameter.binding().ok()?, tracked_bindings) - } - AnyJsParameter::TsThisParameter(_) => None, - } + parameter + .binding() + .and_then(|binding| traverse_binding(binding, tracked_bindings)) } /// Traverse a [JsAnyBindingPattern] in preorder and check if the name of [JsIdentifierBinding] has seem before. @@ -198,8 +186,3 @@ fn track_binding( false } } - -declare_node_union! { - /// A union of all possible FunctionLike `JsAstNode` in the JS grammar. - pub(crate) AnyJsFunctionAndMethod = JsArrowFunctionExpression| JsFunctionDeclaration| JsFunctionExportDefaultDeclaration | JsFunctionExpression | JsMethodClassMember | JsMethodObjectMember -} diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.jsonc b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.jsonc deleted file mode 100644 index eb67d9e5dc1..00000000000 --- a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.jsonc +++ /dev/null @@ -1,13 +0,0 @@ -[ - "function b(a, b, b) {}", - "function c(a, a, a) {}", - "const d = (a, b, a) => {};", - "function e(a, b, a, b) {}", - "var f = function (a, b, b) {};", - "class G { ggg(a, a, a) {} }", - "let objectMethods = { method(a, b, c, c) {} }", - "var h = function (a, b, a) {};", - "export default function (a, b, a, a) {}", - "function f({ test: res = 3 }, res) {}", - "export function f2(a, b, c = (a, b, b) => {}) {}" -] diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.jsonc.snap b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.jsonc.snap deleted file mode 100644 index d2455749b80..00000000000 --- a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.jsonc.snap +++ /dev/null @@ -1,214 +0,0 @@ ---- -source: crates/rome_js_analyze/tests/spec_tests.rs -expression: invalid.jsonc ---- -# Input -```js -function b(a, b, b) {} -``` - -# Diagnostics -``` -invalid.jsonc:1:18 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Duplicate parameter name. - - > 1 │ function b(a, b, b) {} - │ ^ - - i The parameter overrides a preceding parameter by using the same name. - - -``` - -# Input -```js -function c(a, a, a) {} -``` - -# Diagnostics -``` -invalid.jsonc:1:15 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Duplicate parameter name. - - > 1 │ function c(a, a, a) {} - │ ^ - - i The parameter overrides a preceding parameter by using the same name. - - -``` - -# Input -```js -const d = (a, b, a) => {}; -``` - -# Diagnostics -``` -invalid.jsonc:1:18 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Duplicate parameter name. - - > 1 │ const d = (a, b, a) => {}; - │ ^ - - i The parameter overrides a preceding parameter by using the same name. - - -``` - -# Input -```js -function e(a, b, a, b) {} -``` - -# Diagnostics -``` -invalid.jsonc:1:18 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Duplicate parameter name. - - > 1 │ function e(a, b, a, b) {} - │ ^ - - i The parameter overrides a preceding parameter by using the same name. - - -``` - -# Input -```js -var f = function (a, b, b) {}; -``` - -# Diagnostics -``` -invalid.jsonc:1:25 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Duplicate parameter name. - - > 1 │ var f = function (a, b, b) {}; - │ ^ - - i The parameter overrides a preceding parameter by using the same name. - - -``` - -# Input -```js -class G { ggg(a, a, a) {} } -``` - -# Diagnostics -``` -invalid.jsonc:1:18 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Duplicate parameter name. - - > 1 │ class G { ggg(a, a, a) {} } - │ ^ - - i The parameter overrides a preceding parameter by using the same name. - - -``` - -# Input -```js -let objectMethods = { method(a, b, c, c) {} } -``` - -# Diagnostics -``` -invalid.jsonc:1:39 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Duplicate parameter name. - - > 1 │ let objectMethods = { method(a, b, c, c) {} } - │ ^ - - i The parameter overrides a preceding parameter by using the same name. - - -``` - -# Input -```js -var h = function (a, b, a) {}; -``` - -# Diagnostics -``` -invalid.jsonc:1:25 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Duplicate parameter name. - - > 1 │ var h = function (a, b, a) {}; - │ ^ - - i The parameter overrides a preceding parameter by using the same name. - - -``` - -# Input -```js -export default function (a, b, a, a) {} -``` - -# Diagnostics -``` -invalid.jsonc:1:32 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Duplicate parameter name. - - > 1 │ export default function (a, b, a, a) {} - │ ^ - - i The parameter overrides a preceding parameter by using the same name. - - -``` - -# Input -```js -function f({ test: res = 3 }, res) {} -``` - -# Diagnostics -``` -invalid.jsonc:1:31 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Duplicate parameter name. - - > 1 │ function f({ test: res = 3 }, res) {} - │ ^^^ - - i The parameter overrides a preceding parameter by using the same name. - - -``` - -# Input -```js -export function f2(a, b, c = (a, b, b) => {}) {} -``` - -# Diagnostics -``` -invalid.jsonc:1:37 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Duplicate parameter name. - - > 1 │ export function f2(a, b, c = (a, b, b) => {}) {} - │ ^ - - i The parameter overrides a preceding parameter by using the same name. - - -``` - - diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.ts b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.ts new file mode 100644 index 00000000000..f40b32b6567 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.ts @@ -0,0 +1,28 @@ +function b(a, b, b) {} +function c(a, a, a) {} +const d = (a, b, a) => {}; +function e(a, b, a, b) {} +var f = function (a, b, b) {}; +class G { + ggg(a, a, a) {} +} +let objectMethods = { method(a, b, c, c) {} }; +var h = function (a, b, a) {}; +export default function (a, b, a, a) {} +function f({ test: res = 3 }, res) {} +export function f2(a, b, c = (a, b, b) => {}) {} +class A { + constructor(a, a) {} +} +class A { + constructor(private a, a) {} +} +class A { + constructor(a, readonly a) {} +} +class A { + constructor(private a, private a) {} +} +class A { + constructor(readonly a, private a) {} +} diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.ts.snap b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.ts.snap new file mode 100644 index 00000000000..1376ee864d7 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateParameters/invalid.ts.snap @@ -0,0 +1,308 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: invalid.ts +--- +# Input +```js +function b(a, b, b) {} +function c(a, a, a) {} +const d = (a, b, a) => {}; +function e(a, b, a, b) {} +var f = function (a, b, b) {}; +class G { + ggg(a, a, a) {} +} +let objectMethods = { method(a, b, c, c) {} }; +var h = function (a, b, a) {}; +export default function (a, b, a, a) {} +function f({ test: res = 3 }, res) {} +export function f2(a, b, c = (a, b, b) => {}) {} +class A { + constructor(a, a) {} +} +class A { + constructor(private a, a) {} +} +class A { + constructor(a, readonly a) {} +} +class A { + constructor(private a, private a) {} +} +class A { + constructor(readonly a, private a) {} +} + +``` + +# Diagnostics +``` +invalid.ts:1:18 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + > 1 │ function b(a, b, b) {} + │ ^ + 2 │ function c(a, a, a) {} + 3 │ const d = (a, b, a) => {}; + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:2:15 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 1 │ function b(a, b, b) {} + > 2 │ function c(a, a, a) {} + │ ^ + 3 │ const d = (a, b, a) => {}; + 4 │ function e(a, b, a, b) {} + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:3:18 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 1 │ function b(a, b, b) {} + 2 │ function c(a, a, a) {} + > 3 │ const d = (a, b, a) => {}; + │ ^ + 4 │ function e(a, b, a, b) {} + 5 │ var f = function (a, b, b) {}; + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:4:18 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 2 │ function c(a, a, a) {} + 3 │ const d = (a, b, a) => {}; + > 4 │ function e(a, b, a, b) {} + │ ^ + 5 │ var f = function (a, b, b) {}; + 6 │ class G { + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:5:25 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 3 │ const d = (a, b, a) => {}; + 4 │ function e(a, b, a, b) {} + > 5 │ var f = function (a, b, b) {}; + │ ^ + 6 │ class G { + 7 │ ggg(a, a, a) {} + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:7:9 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 5 │ var f = function (a, b, b) {}; + 6 │ class G { + > 7 │ ggg(a, a, a) {} + │ ^ + 8 │ } + 9 │ let objectMethods = { method(a, b, c, c) {} }; + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:9:39 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 7 │ ggg(a, a, a) {} + 8 │ } + > 9 │ let objectMethods = { method(a, b, c, c) {} }; + │ ^ + 10 │ var h = function (a, b, a) {}; + 11 │ export default function (a, b, a, a) {} + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:10:25 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 8 │ } + 9 │ let objectMethods = { method(a, b, c, c) {} }; + > 10 │ var h = function (a, b, a) {}; + │ ^ + 11 │ export default function (a, b, a, a) {} + 12 │ function f({ test: res = 3 }, res) {} + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:11:32 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 9 │ let objectMethods = { method(a, b, c, c) {} }; + 10 │ var h = function (a, b, a) {}; + > 11 │ export default function (a, b, a, a) {} + │ ^ + 12 │ function f({ test: res = 3 }, res) {} + 13 │ export function f2(a, b, c = (a, b, b) => {}) {} + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:12:31 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 10 │ var h = function (a, b, a) {}; + 11 │ export default function (a, b, a, a) {} + > 12 │ function f({ test: res = 3 }, res) {} + │ ^^^ + 13 │ export function f2(a, b, c = (a, b, b) => {}) {} + 14 │ class A { + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:13:37 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 11 │ export default function (a, b, a, a) {} + 12 │ function f({ test: res = 3 }, res) {} + > 13 │ export function f2(a, b, c = (a, b, b) => {}) {} + │ ^ + 14 │ class A { + 15 │ constructor(a, a) {} + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:15:17 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 13 │ export function f2(a, b, c = (a, b, b) => {}) {} + 14 │ class A { + > 15 │ constructor(a, a) {} + │ ^ + 16 │ } + 17 │ class A { + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:18:25 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 16 │ } + 17 │ class A { + > 18 │ constructor(private a, a) {} + │ ^ + 19 │ } + 20 │ class A { + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:21:26 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 19 │ } + 20 │ class A { + > 21 │ constructor(a, readonly a) {} + │ ^ + 22 │ } + 23 │ class A { + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:24:33 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 22 │ } + 23 │ class A { + > 24 │ constructor(private a, private a) {} + │ ^ + 25 │ } + 26 │ class A { + + i The parameter overrides a preceding parameter by using the same name. + + +``` + +``` +invalid.ts:27:34 lint/suspicious/noDuplicateParameters ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Duplicate parameter name. + + 25 │ } + 26 │ class A { + > 27 │ constructor(readonly a, private a) {} + │ ^ + 28 │ } + 29 │ + + i The parameter overrides a preceding parameter by using the same name. + + +``` + + diff --git a/crates/rome_js_formatter/src/js/bindings/parameters.rs b/crates/rome_js_formatter/src/js/bindings/parameters.rs index 700a03ad000..3f2413bd6d4 100644 --- a/crates/rome_js_formatter/src/js/bindings/parameters.rs +++ b/crates/rome_js_formatter/src/js/bindings/parameters.rs @@ -1,11 +1,9 @@ use crate::prelude::*; use rome_formatter::{write, CstFormatContext}; -use crate::js::lists::parameter_list::{ - AnyJsParameterList, AnyParameter, FormatJsAnyParameterList, -}; - +use crate::js::lists::parameter_list::FormatJsAnyParameterList; use crate::utils::test_call::is_test_call_argument; +use rome_js_syntax::parameter_ext::{AnyJsParameterList, AnyParameter}; use rome_js_syntax::{ AnyJsConstructorParameter, AnyJsFormalParameter, AnyTsType, JsConstructorParameters, JsParameters, JsSyntaxToken, diff --git a/crates/rome_js_formatter/src/js/lists/constructor_parameter_list.rs b/crates/rome_js_formatter/src/js/lists/constructor_parameter_list.rs index 0e14925d765..2580ab4f989 100644 --- a/crates/rome_js_formatter/src/js/lists/constructor_parameter_list.rs +++ b/crates/rome_js_formatter/src/js/lists/constructor_parameter_list.rs @@ -1,6 +1,7 @@ use crate::js::bindings::parameters::ParameterLayout; -use crate::js::lists::parameter_list::{AnyJsParameterList, FormatJsAnyParameterList}; +use crate::js::lists::parameter_list::FormatJsAnyParameterList; use crate::prelude::*; +use rome_js_syntax::parameter_ext::AnyJsParameterList; use rome_js_syntax::JsConstructorParameterList; #[derive(Debug, Clone, Default)] diff --git a/crates/rome_js_formatter/src/js/lists/parameter_list.rs b/crates/rome_js_formatter/src/js/lists/parameter_list.rs index ce94bb5f453..e2bfb64036a 100644 --- a/crates/rome_js_formatter/src/js/lists/parameter_list.rs +++ b/crates/rome_js_formatter/src/js/lists/parameter_list.rs @@ -2,11 +2,8 @@ use crate::js::bindings::parameters::ParameterLayout; use crate::prelude::*; use crate::context::trailing_comma::FormatTrailingComma; -use rome_js_syntax::{ - AnyJsConstructorParameter, AnyJsParameter, JsConstructorParameterList, JsLanguage, - JsParameterList, -}; -use rome_rowan::{declare_node_union, AstSeparatedListNodesIterator, SyntaxResult}; +use rome_js_syntax::parameter_ext::{AnyJsParameterList, AnyParameter}; +use rome_js_syntax::{AnyJsConstructorParameter, AnyJsParameter, JsParameterList}; #[derive(Debug, Clone, Default)] pub(crate) struct FormatJsParameterList; @@ -106,96 +103,3 @@ impl Format for FormatJsAnyParameterList<'_> { } } } - -#[derive(Debug)] -pub(crate) enum AnyJsParameterList { - JsParameterList(JsParameterList), - JsConstructorParameterList(JsConstructorParameterList), -} - -impl From for AnyJsParameterList { - fn from(list: JsParameterList) -> Self { - AnyJsParameterList::JsParameterList(list) - } -} - -impl From for AnyJsParameterList { - fn from(list: JsConstructorParameterList) -> Self { - AnyJsParameterList::JsConstructorParameterList(list) - } -} - -impl AnyJsParameterList { - pub fn len(&self) -> usize { - match self { - AnyJsParameterList::JsParameterList(parameters) => parameters.len(), - AnyJsParameterList::JsConstructorParameterList(parameters) => parameters.len(), - } - } - - pub fn is_empty(&self) -> bool { - match self { - AnyJsParameterList::JsParameterList(parameters) => parameters.is_empty(), - AnyJsParameterList::JsConstructorParameterList(parameters) => parameters.is_empty(), - } - } - - pub fn first(&self) -> Option> { - Some(match self { - AnyJsParameterList::JsParameterList(parameters) => { - parameters.first()?.map(|parameter| parameter.into()) - } - AnyJsParameterList::JsConstructorParameterList(parameters) => { - parameters.first()?.map(|parameter| parameter.into()) - } - }) - } - - pub fn iter(&self) -> AnyJsParameterListNodeIter { - match self { - AnyJsParameterList::JsParameterList(list) => { - AnyJsParameterListNodeIter::JsParameterList(list.iter()) - } - AnyJsParameterList::JsConstructorParameterList(list) => { - AnyJsParameterListNodeIter::JsConstructorParameterList(list.iter()) - } - } - } - - pub fn last(&self) -> Option> { - Some(match self { - AnyJsParameterList::JsParameterList(parameters) => { - parameters.last()?.map(|parameter| parameter.into()) - } - AnyJsParameterList::JsConstructorParameterList(parameters) => { - parameters.last()?.map(|parameter| parameter.into()) - } - }) - } -} - -pub(crate) enum AnyJsParameterListNodeIter { - JsParameterList(AstSeparatedListNodesIterator), - JsConstructorParameterList( - AstSeparatedListNodesIterator, - ), -} - -impl Iterator for AnyJsParameterListNodeIter { - type Item = SyntaxResult; - - fn next(&mut self) -> Option { - Some(match self { - AnyJsParameterListNodeIter::JsParameterList(inner) => { - inner.next()?.map(AnyParameter::from) - } - AnyJsParameterListNodeIter::JsConstructorParameterList(inner) => { - inner.next()?.map(AnyParameter::from) - } - }) - } -} - -declare_node_union! { - pub(crate) AnyParameter = AnyJsConstructorParameter | AnyJsParameter -} diff --git a/crates/rome_js_syntax/src/lib.rs b/crates/rome_js_syntax/src/lib.rs index 4a487b57737..4661eadf710 100644 --- a/crates/rome_js_syntax/src/lib.rs +++ b/crates/rome_js_syntax/src/lib.rs @@ -13,6 +13,7 @@ pub mod import_ext; pub mod jsx_ext; pub mod modifier_ext; pub mod numbers; +pub mod parameter_ext; pub mod static_value; pub mod stmt_ext; pub mod suppression; diff --git a/crates/rome_js_syntax/src/parameter_ext.rs b/crates/rome_js_syntax/src/parameter_ext.rs new file mode 100644 index 00000000000..d343b28ea83 --- /dev/null +++ b/crates/rome_js_syntax/src/parameter_ext.rs @@ -0,0 +1,412 @@ +use crate::{ + AnyJsBindingPattern, AnyJsConstructorParameter, AnyJsParameter, JsConstructorParameterList, + JsConstructorParameters, JsLanguage, JsParameterList, JsParameters, +}; +use rome_rowan::{ + declare_node_union, AstSeparatedList, AstSeparatedListNodesIterator, SyntaxResult, +}; + +/// An enumeration representing different types of JavaScript/TypeScript parameter lists. +/// +/// This enum can represent a regular JavaScript/TypeScript parameter list (i.e., for functions) +/// or a JavaScript/TypeScript constructor parameter list (i.e., for class constructors). +/// +/// # Examples +/// +/// ``` +/// use rome_js_factory::make; +/// use rome_js_syntax::{AnyJsBinding, AnyJsBindingPattern, AnyJsConstructorParameter, AnyJsFormalParameter, AnyJsParameter}; +/// use rome_js_syntax::parameter_ext::AnyJsParameterList; +/// +/// // Create a function parameter list +/// let parameter_list = make::js_parameter_list( +/// Some(AnyJsParameter::AnyJsFormalParameter( +/// AnyJsFormalParameter::JsFormalParameter( +/// make::js_formal_parameter( +/// make::js_decorator_list(std::iter::empty()), +/// AnyJsBindingPattern::AnyJsBinding(AnyJsBinding::JsIdentifierBinding( +/// make::js_identifier_binding(make::ident("params")), +/// )), +/// ) +/// .build(), +/// ), +/// )), +/// None, +/// ); +/// let function_params = AnyJsParameterList::JsParameterList(parameter_list); +/// +/// // Create a constructor parameter list +/// let constructor_parameter_list = make::js_constructor_parameter_list( +/// Some(AnyJsConstructorParameter::AnyJsFormalParameter( +/// AnyJsFormalParameter::JsFormalParameter( +/// make::js_formal_parameter( +/// make::js_decorator_list(std::iter::empty()), +/// AnyJsBindingPattern::AnyJsBinding(AnyJsBinding::JsIdentifierBinding( +/// make::js_identifier_binding(make::ident("params")), +/// )), +/// ) +/// .build(), +/// ), +/// )), +/// None, +/// ); +/// +/// let constructor_params = AnyJsParameterList::JsConstructorParameterList(constructor_parameter_list); +/// ``` +/// +/// # Variants +/// +/// * `JsParameterList` - A list of parameters for a JavaScript function. +/// * `JsConstructorParameterList` - A list of parameters for a JavaScript constructor. +#[derive(Debug)] +pub enum AnyJsParameterList { + JsParameterList(JsParameterList), + JsConstructorParameterList(JsConstructorParameterList), +} + +impl From for AnyJsParameterList { + fn from(list: JsParameterList) -> Self { + AnyJsParameterList::JsParameterList(list) + } +} + +impl From for AnyJsParameterList { + fn from(list: JsConstructorParameterList) -> Self { + AnyJsParameterList::JsConstructorParameterList(list) + } +} + +impl AnyJsParameterList { + /// + /// This method allows to get the length of a parameter list, regardless + /// of whether it's a standard parameter list or a constructor parameter list. + /// + /// # Examples + /// + /// ``` + /// use rome_js_factory::make; + /// use rome_js_syntax::parameter_ext::AnyJsParameterList; + /// use rome_js_syntax::{ + /// AnyJsBinding, AnyJsBindingPattern, AnyJsConstructorParameter, AnyJsFormalParameter, + /// AnyJsParameter, T, + /// }; + /// + /// let parameter_list = make::js_parameter_list( + /// Some(AnyJsParameter::AnyJsFormalParameter( + /// AnyJsFormalParameter::JsFormalParameter( + /// make::js_formal_parameter( + /// make::js_decorator_list(std::iter::empty()), + /// AnyJsBindingPattern::AnyJsBinding(AnyJsBinding::JsIdentifierBinding( + /// make::js_identifier_binding(make::ident("params")), + /// )), + /// ) + /// .build(), + /// ), + /// )), + /// None, + /// ); + /// + /// let params = AnyJsParameterList::JsParameterList(parameter_list); + /// assert_eq!(params.len(), 1); + /// + /// let constructor_parameter_list = make::js_constructor_parameter_list( + /// Some(AnyJsConstructorParameter::AnyJsFormalParameter( + /// AnyJsFormalParameter::JsFormalParameter( + /// make::js_formal_parameter( + /// make::js_decorator_list(std::iter::empty()), + /// AnyJsBindingPattern::AnyJsBinding(AnyJsBinding::JsIdentifierBinding( + /// make::js_identifier_binding(make::ident("params")), + /// )), + /// ) + /// .build(), + /// ), + /// )), + /// None, + /// ); + /// + /// let params = AnyJsParameterList::JsConstructorParameterList(constructor_parameter_list); + /// assert_eq!(params.len(), 1); + /// ``` + /// + /// # Returns + /// + /// Returns the length of the parameter list. + pub fn len(&self) -> usize { + match self { + AnyJsParameterList::JsParameterList(parameters) => parameters.len(), + AnyJsParameterList::JsConstructorParameterList(parameters) => parameters.len(), + } + } + + /// + /// This method checks if a parameter list is empty. + /// + /// # Examples + /// + /// ``` + /// use rome_js_factory::make; + /// use rome_js_syntax::parameter_ext::AnyJsParameterList; + /// use rome_js_syntax::{ + /// AnyJsBinding, AnyJsBindingPattern, AnyJsConstructorParameter, AnyJsFormalParameter, + /// AnyJsParameter, T, + /// }; + /// + /// let parameter_list = make::js_parameter_list( + /// Some(AnyJsParameter::AnyJsFormalParameter( + /// AnyJsFormalParameter::JsFormalParameter( + /// make::js_formal_parameter( + /// make::js_decorator_list(std::iter::empty()), + /// AnyJsBindingPattern::AnyJsBinding(AnyJsBinding::JsIdentifierBinding( + /// make::js_identifier_binding(make::ident("params")), + /// )), + /// ) + /// .build(), + /// ), + /// )), + /// None, + /// ); + /// + /// let params = AnyJsParameterList::JsParameterList(parameter_list); + /// assert_eq!(params.is_empty(), false); + /// + /// let constructor_parameter_list = make::js_constructor_parameter_list( + /// None, + /// None, + /// ); + /// + /// let params = AnyJsParameterList::JsConstructorParameterList(constructor_parameter_list); + /// assert!(params.is_empty()); + /// ``` + /// + /// # Returns + /// + /// Returns true if the parameter list contains no parameters, false otherwise. + pub fn is_empty(&self) -> bool { + match self { + AnyJsParameterList::JsParameterList(parameters) => parameters.is_empty(), + AnyJsParameterList::JsConstructorParameterList(parameters) => parameters.is_empty(), + } + } + + /// + /// This method allows to get the first parameter in the parameter list. + /// + /// # Examples + /// + /// ``` + /// use rome_js_factory::make; + /// use rome_js_syntax::parameter_ext::{AnyJsParameterList, AnyParameter}; + /// use rome_js_syntax::{ + /// AnyJsBinding, AnyJsBindingPattern, AnyJsConstructorParameter, AnyJsFormalParameter, + /// AnyJsParameter, T, + /// }; + /// use rome_rowan::SyntaxResult; + /// + /// let parameter_list = make::js_parameter_list( + /// Some(AnyJsParameter::AnyJsFormalParameter( + /// AnyJsFormalParameter::JsFormalParameter( + /// make::js_formal_parameter( + /// make::js_decorator_list(std::iter::empty()), + /// AnyJsBindingPattern::AnyJsBinding(AnyJsBinding::JsIdentifierBinding( + /// make::js_identifier_binding(make::ident("param1")), + /// )), + /// ) + /// .build(), + /// ), + /// )), + /// None, + /// ); + /// + /// let params = AnyJsParameterList::JsParameterList(parameter_list); + /// let first_param = params.first().unwrap(); + /// assert_eq!(first_param.is_ok(), true); + /// + /// let empty_parameter_list = make::js_constructor_parameter_list(None, None); + /// let empty_params = AnyJsParameterList::JsConstructorParameterList(empty_parameter_list); + /// assert!(empty_params.first().is_none()); + /// ``` + /// + /// # Returns + /// + /// Returns the first parameter in the parameter list if it exists. + pub fn first(&self) -> Option> { + Some(match self { + AnyJsParameterList::JsParameterList(parameters) => { + parameters.first()?.map(|parameter| parameter.into()) + } + AnyJsParameterList::JsConstructorParameterList(parameters) => { + parameters.first()?.map(|parameter| parameter.into()) + } + }) + } + + /// + /// This method allows you to iterate over the parameters in a `JsParameterList` or a `JsConstructorParameterList`, + /// depending on the variant of the `AnyJsParameterList` enum. + /// + /// # Examples + /// + /// ``` + /// use rome_js_factory::make; + /// use rome_js_syntax::parameter_ext::AnyJsParameterList; + /// use rome_js_syntax::{ + /// AnyJsBinding, AnyJsBindingPattern, AnyJsConstructorParameter, AnyJsFormalParameter, + /// AnyJsParameter, T, + /// }; + /// + /// let parameter_list = make::js_parameter_list( + /// Some(AnyJsParameter::AnyJsFormalParameter( + /// AnyJsFormalParameter::JsFormalParameter( + /// make::js_formal_parameter( + /// make::js_decorator_list(std::iter::empty()), + /// AnyJsBindingPattern::AnyJsBinding(AnyJsBinding::JsIdentifierBinding( + /// make::js_identifier_binding(make::ident("param1")), + /// )), + /// ) + /// .build(), + /// ), + /// )), + /// None, + /// ); + /// + /// let params = AnyJsParameterList::JsParameterList(parameter_list); + /// let mut iter = params.iter(); + /// + /// assert_eq!(iter.next().is_some(), true); + /// assert_eq!(iter.next().is_none(), true); + /// ``` + /// + /// # Returns + /// + /// Returns an iterator over the parameters in the list. + /// + pub fn iter(&self) -> AnyJsParameterListNodeIter { + match self { + AnyJsParameterList::JsParameterList(list) => { + AnyJsParameterListNodeIter::JsParameterList(list.iter()) + } + AnyJsParameterList::JsConstructorParameterList(list) => { + AnyJsParameterListNodeIter::JsConstructorParameterList(list.iter()) + } + } + } + + /// + /// This method allows to get the last parameter in the parameter list. + /// + /// # Examples + /// + /// ``` + /// use rome_js_factory::make; + /// use rome_js_syntax::parameter_ext::AnyJsParameterList; + /// use rome_js_syntax::{ + /// AnyJsBinding, AnyJsBindingPattern, AnyJsConstructorParameter, AnyJsFormalParameter, + /// AnyJsParameter, T, + /// }; + /// + /// let parameter_list = make::js_parameter_list( + /// Some(AnyJsParameter::AnyJsFormalParameter( + /// AnyJsFormalParameter::JsFormalParameter( + /// make::js_formal_parameter( + /// make::js_decorator_list(std::iter::empty()), + /// AnyJsBindingPattern::AnyJsBinding(AnyJsBinding::JsIdentifierBinding( + /// make::js_identifier_binding(make::ident("param1")), + /// )), + /// ) + /// .build(), + /// ), + /// )), + /// None, + /// ); + /// + /// let params = AnyJsParameterList::JsParameterList(parameter_list); + /// let last_param = params.last().unwrap(); + /// assert_eq!(last_param.is_ok(), true); + /// + /// let empty_parameter_list = make::js_parameter_list(None, None); + /// let empty_params = AnyJsParameterList::JsParameterList(empty_parameter_list); + /// assert!(empty_params.last().is_none()); + /// ``` + /// + /// # Returns + /// + /// Returns the last parameter in the parameter list if it exists. + /// + pub fn last(&self) -> Option> { + Some(match self { + AnyJsParameterList::JsParameterList(parameters) => { + parameters.last()?.map(|parameter| parameter.into()) + } + AnyJsParameterList::JsConstructorParameterList(parameters) => { + parameters.last()?.map(|parameter| parameter.into()) + } + }) + } +} + +/// An iterator over the parameters in an `AnyJsParameterList`. +/// +/// This iterator can traverse a regular JavaScript/TypeScript parameter list (i.e., for functions) +/// or a JavaScript/TypeScript constructor parameter list (i.e., for class constructors), depending +/// on the variant of the `AnyJsParameterListNodeIter` enum. +pub enum AnyJsParameterListNodeIter { + JsParameterList(AstSeparatedListNodesIterator), + JsConstructorParameterList( + AstSeparatedListNodesIterator, + ), +} + +impl Iterator for AnyJsParameterListNodeIter { + type Item = SyntaxResult; + + fn next(&mut self) -> Option { + Some(match self { + AnyJsParameterListNodeIter::JsParameterList(inner) => { + inner.next()?.map(AnyParameter::from) + } + AnyJsParameterListNodeIter::JsConstructorParameterList(inner) => { + inner.next()?.map(AnyParameter::from) + } + }) + } +} + +declare_node_union! { + /// The `AnyParameter` union can represent either a standard JavaScript/TypeScript parameter + /// or a JavaScript/TypeScript constructor parameter. This is useful in contexts where a + /// function could accept either type of parameter. + pub AnyParameter = AnyJsConstructorParameter | AnyJsParameter +} + +impl AnyParameter { + pub fn binding(&self) -> Option { + match self { + AnyParameter::AnyJsConstructorParameter(parameter) => match parameter { + AnyJsConstructorParameter::AnyJsFormalParameter(parameter) => { + parameter.as_js_formal_parameter()?.binding().ok() + } + AnyJsConstructorParameter::JsRestParameter(parameter) => parameter.binding().ok(), + AnyJsConstructorParameter::TsPropertyParameter(parameter) => parameter + .formal_parameter() + .ok()? + .as_js_formal_parameter()? + .binding() + .ok(), + }, + AnyParameter::AnyJsParameter(parameter) => match parameter { + AnyJsParameter::AnyJsFormalParameter(parameter) => { + parameter.as_js_formal_parameter()?.binding().ok() + } + AnyJsParameter::JsRestParameter(parameter) => parameter.binding().ok(), + AnyJsParameter::TsThisParameter(_) => None, + }, + } + } +} + +declare_node_union! { + /// The `AnyJsParameters` union can represent either a standard JavaScript/TypeScript parameters + /// or a JavaScript/TypeScript constructor parameters. This is useful in contexts where a + /// function could accept either type of parameters. + pub AnyJsParameters = JsParameters | JsConstructorParameters +}