From 731ce10de2a4d4a3e6ca85f60cfe6d4db2700d46 Mon Sep 17 00:00:00 2001 From: Victorien ELVINGER Date: Mon, 5 Dec 2022 11:58:58 +0100 Subject: [PATCH] feat(rome_js_analyze): `useDefaultParameterLast` (#3873) Co-authored-by: Micha Reiser --- .../src/categories.rs | 1 + .../rome_js_analyze/src/analyzers/nursery.rs | 3 +- .../nursery/use_default_parameter_last.rs | 176 ++++++++++++ .../useDefaultParameterLast/invalid.js | 11 + .../useDefaultParameterLast/invalid.js.snap | 107 ++++++++ .../useDefaultParameterLast/invalid.ts | 11 + .../useDefaultParameterLast/invalid.ts.snap | 254 ++++++++++++++++++ .../nursery/useDefaultParameterLast/valid.js | 1 + .../useDefaultParameterLast/valid.js.snap | 11 + .../nursery/useDefaultParameterLast/valid.ts | 3 + .../useDefaultParameterLast/valid.ts.snap | 13 + .../src/configuration/linter/rules.rs | 15 +- editors/vscode/configuration_schema.json | 11 + npm/backend-jsonrpc/src/workspace.ts | 5 + npm/rome/configuration_schema.json | 11 + website/src/pages/lint/rules/index.mdx | 6 + .../lint/rules/useDefaultParameterLast.md | 139 ++++++++++ 17 files changed, 772 insertions(+), 6 deletions(-) create mode 100644 crates/rome_js_analyze/src/analyzers/nursery/use_default_parameter_last.rs create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.js create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.js.snap create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.ts create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.ts.snap create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.js create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.js.snap create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.ts create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.ts.snap create mode 100644 website/src/pages/lint/rules/useDefaultParameterLast.md diff --git a/crates/rome_diagnostics_categories/src/categories.rs b/crates/rome_diagnostics_categories/src/categories.rs index 32af1938935..0f989971623 100644 --- a/crates/rome_diagnostics_categories/src/categories.rs +++ b/crates/rome_diagnostics_categories/src/categories.rs @@ -97,6 +97,7 @@ define_dategories! { "lint/nursery/noVoidTypeReturn": "https://docs.rome.tools/lint/rules/noVoidTypeReturn", "lint/nursery/useCamelCase": "https://docs.rome.tools/lint/rules/useCamelCase", "lint/nursery/useConst":"https://docs.rome.tools/lint/rules/useConst", + "lint/nursery/useDefaultParameterLast":"https://docs.rome.tools/lint/rules/useDefaultParameterLast", "lint/nursery/useDefaultSwitchClauseLast":"https://docs.rome.tools/lint/rules/useDefaultSwitchClauseLast", "lint/nursery/useEnumInitializers":"https://docs.rome.tools/lint/rules/useEnumInitializers", "lint/nursery/useExhaustiveDependencies": "https://docs.rome.tools/lint/rules/useExhaustiveDependencies", diff --git a/crates/rome_js_analyze/src/analyzers/nursery.rs b/crates/rome_js_analyze/src/analyzers/nursery.rs index e438714d3f3..845e0559530 100644 --- a/crates/rome_js_analyze/src/analyzers/nursery.rs +++ b/crates/rome_js_analyze/src/analyzers/nursery.rs @@ -20,10 +20,11 @@ mod no_setter_return; mod no_string_case_mismatch; mod no_unsafe_finally; mod no_void_type_return; +mod use_default_parameter_last; mod use_default_switch_clause_last; mod use_enum_initializers; mod use_exponentiation_operator; mod use_flat_map; mod use_numeric_literals; mod use_valid_for_direction; -declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_access_key :: NoAccessKey , self :: no_banned_types :: NoBannedTypes , self :: no_conditional_assignment :: NoConditionalAssignment , self :: no_const_enum :: NoConstEnum , self :: no_constructor_return :: NoConstructorReturn , self :: no_distracting_elements :: NoDistractingElements , self :: no_dupe_keys :: NoDupeKeys , self :: no_empty_interface :: NoEmptyInterface , self :: no_explicit_any :: NoExplicitAny , self :: no_extra_non_null_assertion :: NoExtraNonNullAssertion , self :: no_header_scope :: NoHeaderScope , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: no_non_null_assertion :: NoNonNullAssertion , self :: no_precision_loss :: NoPrecisionLoss , self :: no_redundant_use_strict :: NoRedundantUseStrict , self :: no_setter_return :: NoSetterReturn , self :: no_string_case_mismatch :: NoStringCaseMismatch , self :: no_unsafe_finally :: NoUnsafeFinally , self :: no_void_type_return :: NoVoidTypeReturn , self :: use_default_switch_clause_last :: UseDefaultSwitchClauseLast , self :: use_enum_initializers :: UseEnumInitializers , self :: use_exponentiation_operator :: UseExponentiationOperator , self :: use_flat_map :: UseFlatMap , self :: use_numeric_literals :: UseNumericLiterals , self :: use_valid_for_direction :: UseValidForDirection ,] } } +declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_access_key :: NoAccessKey , self :: no_banned_types :: NoBannedTypes , self :: no_conditional_assignment :: NoConditionalAssignment , self :: no_const_enum :: NoConstEnum , self :: no_constructor_return :: NoConstructorReturn , self :: no_distracting_elements :: NoDistractingElements , self :: no_dupe_keys :: NoDupeKeys , self :: no_empty_interface :: NoEmptyInterface , self :: no_explicit_any :: NoExplicitAny , self :: no_extra_non_null_assertion :: NoExtraNonNullAssertion , self :: no_header_scope :: NoHeaderScope , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: no_non_null_assertion :: NoNonNullAssertion , self :: no_precision_loss :: NoPrecisionLoss , self :: no_redundant_use_strict :: NoRedundantUseStrict , self :: no_setter_return :: NoSetterReturn , self :: no_string_case_mismatch :: NoStringCaseMismatch , self :: no_unsafe_finally :: NoUnsafeFinally , self :: no_void_type_return :: NoVoidTypeReturn , self :: use_default_parameter_last :: UseDefaultParameterLast , self :: use_default_switch_clause_last :: UseDefaultSwitchClauseLast , self :: use_enum_initializers :: UseEnumInitializers , self :: use_exponentiation_operator :: UseExponentiationOperator , self :: use_flat_map :: UseFlatMap , self :: use_numeric_literals :: UseNumericLiterals , self :: use_valid_for_direction :: UseValidForDirection ,] } } diff --git a/crates/rome_js_analyze/src/analyzers/nursery/use_default_parameter_last.rs b/crates/rome_js_analyze/src/analyzers/nursery/use_default_parameter_last.rs new file mode 100644 index 00000000000..3c2a04a6a08 --- /dev/null +++ b/crates/rome_js_analyze/src/analyzers/nursery/use_default_parameter_last.rs @@ -0,0 +1,176 @@ +use rome_analyze::context::RuleContext; +use rome_analyze::{declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic}; +use rome_console::markup; +use rome_diagnostics::Applicability; +use rome_js_syntax::{JsFormalParameter, JsInitializerClause, JsSyntaxToken, TsPropertyParameter}; +use rome_rowan::{declare_node_union, AstNode, BatchMutationExt, Direction}; + +use crate::JsRuleAction; + +declare_rule! { + /// Enforce default function parameters and optional parameters to be last. + /// + /// Default and optional parameters that precede a required parameter cannot be omitted at call site. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```js,expect_diagnostic + /// function f(a = 0, b) {} + /// ``` + /// + /// ```js,expect_diagnostic + /// function f(a, b = 0, c) {} + /// ``` + /// + /// ```ts,expect_diagnostic + /// function f(a: number, b?: number, c: number) {} + /// ``` + /// + /// ```ts,expect_diagnostic + /// class Foo { + /// constructor(readonly a = 10, readonly b: number) {} + /// } + /// ``` + /// + /// ### Valid + /// + /// ```js + /// function f(a, b = 0) {} + /// ``` + /// + /// ```ts + /// function f(a: number, b?: number, c = 0) {} + /// ``` + /// + /// ```ts + /// function f(a: number, b = 0, c?: number) {} + /// ``` + /// + pub(crate) UseDefaultParameterLast { + version: "11.0.0", + name: "useDefaultParameterLast", + recommended: true, + } +} + +declare_node_union! { + pub(crate) AnyFormalParameter = JsFormalParameter | TsPropertyParameter +} + +impl AnyFormalParameter { + pub(crate) fn is_optional(&self) -> bool { + self.question_mark_token().is_some() || self.initializer().is_some() + } + + pub(crate) fn initializer(&self) -> Option { + match self { + AnyFormalParameter::JsFormalParameter(x) => x.initializer(), + AnyFormalParameter::TsPropertyParameter(x) => x + .formal_parameter() + .ok()? + .as_js_formal_parameter()? + .initializer(), + } + } + + pub(crate) fn question_mark_token(&self) -> Option { + match self { + AnyFormalParameter::JsFormalParameter(x) => x.question_mark_token(), + AnyFormalParameter::TsPropertyParameter(x) => x + .formal_parameter() + .ok()? + .as_js_formal_parameter()? + .question_mark_token(), + } + } +} + +impl Rule for UseDefaultParameterLast { + type Query = Ast; + type State = AnyFormalParameter; + type Signals = Option; + type Options = (); + + fn run(ctx: &RuleContext) -> Self::Signals { + let formal_param = ctx.query(); + if formal_param.is_optional() { + let next_required_param = formal_param + .syntax() + .siblings(Direction::Next) + .filter_map(AnyFormalParameter::cast) + .find(|x| !x.is_optional()); + next_required_param + } else { + None + } + } + + fn diagnostic(ctx: &RuleContext, required_param: &Self::State) -> Option { + let formal_param = ctx.query(); + let param_kind = if formal_param.question_mark_token().is_some() { + "optional" + } else { + "default" + }; + Some(RuleDiagnostic::new( + rule_category!(), + formal_param.range(), + markup! { + "The "{param_kind}" parameter"" should follow the ""required parameter"" or should be a ""required parameter""." + }, + ).detail( + required_param.range(), + markup! { + "The ""required parameter"" is here:" + }, + ).note( + markup! { + "A "{param_kind}" parameter"" that precedes a ""required parameter"" cannot be omitted at call site." + } + )) + } + + fn action(ctx: &RuleContext, _: &Self::State) -> Option { + let opt_param = ctx.query(); + let mut mutation = ctx.root().begin(); + if opt_param.question_mark_token().is_some() { + let question_mark = opt_param.question_mark_token()?; + let next_token = question_mark.next_token()?; + let new_next_token = next_token.with_leading_trivia_pieces( + question_mark + .leading_trivia() + .pieces() + .chain(question_mark.trailing_trivia().pieces()) + .chain(next_token.leading_trivia().pieces()) + .collect::>(), + ); + mutation.replace_token_discard_trivia(next_token, new_next_token); + mutation.remove_token(question_mark); + } else { + let initializer = opt_param.initializer()?; + let first_token = initializer.syntax().first_token()?; + let last_token = initializer.syntax().last_token()?; + let prev_token = first_token.prev_token()?; + let new_prev_token = prev_token.with_trailing_trivia_pieces( + prev_token + .trailing_trivia() + .pieces() + .chain(first_token.leading_trivia().pieces()) + .chain(last_token.trailing_trivia().pieces()) + .collect::>(), + ); + mutation.replace_token_discard_trivia(prev_token, new_prev_token); + mutation.remove_node(initializer); + } + Some(JsRuleAction { + mutation, + message: + markup! {"Turn the parameter into a ""required parameter""."} + .to_owned(), + category: ActionCategory::QuickFix, + applicability: Applicability::MaybeIncorrect, + }) + } +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.js b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.js new file mode 100644 index 00000000000..95e3cc94423 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.js @@ -0,0 +1,11 @@ +export function f(a = 0, b) {} + +export function g(a, b = 0, c) {} + +export function g(a, b /* before */ = /* mid */ 0/* after */) {} + +export function g( + a, + b /* before */ = /* mid */ 0/* after */,/* after coma */ + c +) {} \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.js.snap new file mode 100644 index 00000000000..44a97caca83 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.js.snap @@ -0,0 +1,107 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 86 +expression: invalid.js +--- +# Input +```js +export function f(a = 0, b) {} + +export function g(a, b = 0, c) {} + +export function g(a, b /* before */ = /* mid */ 0/* after */) {} + +export function g( + a, + b /* before */ = /* mid */ 0/* after */,/* after coma */ + c +) {} +``` + +# Diagnostics +``` +invalid.js:1:19 lint/nursery/useDefaultParameterLast FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The default parameter should follow the required parameter or should be a required parameter. + + > 1 │ export function f(a = 0, b) {} + │ ^^^^^ + 2 │ + 3 │ export function g(a, b = 0, c) {} + + i The required parameter is here: + + > 1 │ export function f(a = 0, b) {} + │ ^ + 2 │ + 3 │ export function g(a, b = 0, c) {} + + i A default parameter that precedes a required parameter cannot be omitted at call site. + + i Suggested fix: Turn the parameter into a required parameter. + + 1 │ export·function·f(a·=·0,·b)·{} + │ --- + +``` + +``` +invalid.js:3:22 lint/nursery/useDefaultParameterLast FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The default parameter should follow the required parameter or should be a required parameter. + + 1 │ export function f(a = 0, b) {} + 2 │ + > 3 │ export function g(a, b = 0, c) {} + │ ^^^^^ + 4 │ + 5 │ export function g(a, b /* before */ = /* mid */ 0/* after */) {} + + i The required parameter is here: + + 1 │ export function f(a = 0, b) {} + 2 │ + > 3 │ export function g(a, b = 0, c) {} + │ ^ + 4 │ + 5 │ export function g(a, b /* before */ = /* mid */ 0/* after */) {} + + i A default parameter that precedes a required parameter cannot be omitted at call site. + + i Suggested fix: Turn the parameter into a required parameter. + + 3 │ export·function·g(a,·b·=·0,·c)·{} + │ --- + +``` + +``` +invalid.js:9:2 lint/nursery/useDefaultParameterLast FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The default parameter should follow the required parameter or should be a required parameter. + + 7 │ export function g( + 8 │ a, + > 9 │ b /* before */ = /* mid */ 0/* after */,/* after coma */ + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 10 │ c + 11 │ ) {} + + i The required parameter is here: + + 8 │ a, + 9 │ b /* before */ = /* mid */ 0/* after */,/* after coma */ + > 10 │ c + │ ^ + 11 │ ) {} + + i A default parameter that precedes a required parameter cannot be omitted at call site. + + i Suggested fix: Turn the parameter into a required parameter. + + 9 │ → b·/*·before·*/·=·/*·mid·*/·0/*·after·*/,/*·after·coma·*/ + │ ------------- + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.ts b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.ts new file mode 100644 index 00000000000..5d8700abf42 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.ts @@ -0,0 +1,11 @@ +export function h(a: number, b?: number, c: number) {} + +export function l(a = 0, b?: number, c: number) {} + +export function f(a = 0, b = 0, c?: string, c: string) {} + +export function h(a/* before */?/* after */: number, b: number) {} + +export class Foo { + constructor(readonly a = 10, readonly b: number) {} +} \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.ts.snap new file mode 100644 index 00000000000..0fcfd3c6a59 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.ts.snap @@ -0,0 +1,254 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 86 +expression: invalid.ts +--- +# Input +```js +export function h(a: number, b?: number, c: number) {} + +export function l(a = 0, b?: number, c: number) {} + +export function f(a = 0, b = 0, c?: string, c: string) {} + +export function h(a/* before */?/* after */: number, b: number) {} + +export class Foo { + constructor(readonly a = 10, readonly b: number) {} +} +``` + +# Diagnostics +``` +invalid.ts:1:30 lint/nursery/useDefaultParameterLast FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The optional parameter should follow the required parameter or should be a required parameter. + + > 1 │ export function h(a: number, b?: number, c: number) {} + │ ^^^^^^^^^^ + 2 │ + 3 │ export function l(a = 0, b?: number, c: number) {} + + i The required parameter is here: + + > 1 │ export function h(a: number, b?: number, c: number) {} + │ ^^^^^^^^^ + 2 │ + 3 │ export function l(a = 0, b?: number, c: number) {} + + i A optional parameter that precedes a required parameter cannot be omitted at call site. + + i Suggested fix: Turn the parameter into a required parameter. + + 1 │ export·function·h(a:·number,·b?:·number,·c:·number)·{} + │ - + +``` + +``` +invalid.ts:3:19 lint/nursery/useDefaultParameterLast FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The default parameter should follow the required parameter or should be a required parameter. + + 1 │ export function h(a: number, b?: number, c: number) {} + 2 │ + > 3 │ export function l(a = 0, b?: number, c: number) {} + │ ^^^^^ + 4 │ + 5 │ export function f(a = 0, b = 0, c?: string, c: string) {} + + i The required parameter is here: + + 1 │ export function h(a: number, b?: number, c: number) {} + 2 │ + > 3 │ export function l(a = 0, b?: number, c: number) {} + │ ^^^^^^^^^ + 4 │ + 5 │ export function f(a = 0, b = 0, c?: string, c: string) {} + + i A default parameter that precedes a required parameter cannot be omitted at call site. + + i Suggested fix: Turn the parameter into a required parameter. + + 3 │ export·function·l(a·=·0,·b?:·number,·c:·number)·{} + │ --- + +``` + +``` +invalid.ts:3:26 lint/nursery/useDefaultParameterLast FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The optional parameter should follow the required parameter or should be a required parameter. + + 1 │ export function h(a: number, b?: number, c: number) {} + 2 │ + > 3 │ export function l(a = 0, b?: number, c: number) {} + │ ^^^^^^^^^^ + 4 │ + 5 │ export function f(a = 0, b = 0, c?: string, c: string) {} + + i The required parameter is here: + + 1 │ export function h(a: number, b?: number, c: number) {} + 2 │ + > 3 │ export function l(a = 0, b?: number, c: number) {} + │ ^^^^^^^^^ + 4 │ + 5 │ export function f(a = 0, b = 0, c?: string, c: string) {} + + i A optional parameter that precedes a required parameter cannot be omitted at call site. + + i Suggested fix: Turn the parameter into a required parameter. + + 3 │ export·function·l(a·=·0,·b?:·number,·c:·number)·{} + │ - + +``` + +``` +invalid.ts:5:19 lint/nursery/useDefaultParameterLast FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The default parameter should follow the required parameter or should be a required parameter. + + 3 │ export function l(a = 0, b?: number, c: number) {} + 4 │ + > 5 │ export function f(a = 0, b = 0, c?: string, c: string) {} + │ ^^^^^ + 6 │ + 7 │ export function h(a/* before */?/* after */: number, b: number) {} + + i The required parameter is here: + + 3 │ export function l(a = 0, b?: number, c: number) {} + 4 │ + > 5 │ export function f(a = 0, b = 0, c?: string, c: string) {} + │ ^^^^^^^^^ + 6 │ + 7 │ export function h(a/* before */?/* after */: number, b: number) {} + + i A default parameter that precedes a required parameter cannot be omitted at call site. + + i Suggested fix: Turn the parameter into a required parameter. + + 5 │ export·function·f(a·=·0,·b·=·0,·c?:·string,·c:·string)·{} + │ --- + +``` + +``` +invalid.ts:5:26 lint/nursery/useDefaultParameterLast FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The default parameter should follow the required parameter or should be a required parameter. + + 3 │ export function l(a = 0, b?: number, c: number) {} + 4 │ + > 5 │ export function f(a = 0, b = 0, c?: string, c: string) {} + │ ^^^^^ + 6 │ + 7 │ export function h(a/* before */?/* after */: number, b: number) {} + + i The required parameter is here: + + 3 │ export function l(a = 0, b?: number, c: number) {} + 4 │ + > 5 │ export function f(a = 0, b = 0, c?: string, c: string) {} + │ ^^^^^^^^^ + 6 │ + 7 │ export function h(a/* before */?/* after */: number, b: number) {} + + i A default parameter that precedes a required parameter cannot be omitted at call site. + + i Suggested fix: Turn the parameter into a required parameter. + + 5 │ export·function·f(a·=·0,·b·=·0,·c?:·string,·c:·string)·{} + │ --- + +``` + +``` +invalid.ts:5:33 lint/nursery/useDefaultParameterLast FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The optional parameter should follow the required parameter or should be a required parameter. + + 3 │ export function l(a = 0, b?: number, c: number) {} + 4 │ + > 5 │ export function f(a = 0, b = 0, c?: string, c: string) {} + │ ^^^^^^^^^^ + 6 │ + 7 │ export function h(a/* before */?/* after */: number, b: number) {} + + i The required parameter is here: + + 3 │ export function l(a = 0, b?: number, c: number) {} + 4 │ + > 5 │ export function f(a = 0, b = 0, c?: string, c: string) {} + │ ^^^^^^^^^ + 6 │ + 7 │ export function h(a/* before */?/* after */: number, b: number) {} + + i A optional parameter that precedes a required parameter cannot be omitted at call site. + + i Suggested fix: Turn the parameter into a required parameter. + + 5 │ export·function·f(a·=·0,·b·=·0,·c?:·string,·c:·string)·{} + │ - + +``` + +``` +invalid.ts:7:19 lint/nursery/useDefaultParameterLast FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The optional parameter should follow the required parameter or should be a required parameter. + + 5 │ export function f(a = 0, b = 0, c?: string, c: string) {} + 6 │ + > 7 │ export function h(a/* before */?/* after */: number, b: number) {} + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 8 │ + 9 │ export class Foo { + + i The required parameter is here: + + 5 │ export function f(a = 0, b = 0, c?: string, c: string) {} + 6 │ + > 7 │ export function h(a/* before */?/* after */: number, b: number) {} + │ ^^^^^^^^^ + 8 │ + 9 │ export class Foo { + + i A optional parameter that precedes a required parameter cannot be omitted at call site. + + i Suggested fix: Turn the parameter into a required parameter. + + 7 │ export·function·h(a/*·before·*/?/*·after·*/:·number,·b:·number)·{} + │ - + +``` + +``` +invalid.ts:10:17 lint/nursery/useDefaultParameterLast FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The default parameter should follow the required parameter or should be a required parameter. + + 9 │ export class Foo { + > 10 │ constructor(readonly a = 10, readonly b: number) {} + │ ^^^^^^^^^^^^^^^ + 11 │ } + + i The required parameter is here: + + 9 │ export class Foo { + > 10 │ constructor(readonly a = 10, readonly b: number) {} + │ ^^^^^^^^^^^^^^^^^^ + 11 │ } + + i A default parameter that precedes a required parameter cannot be omitted at call site. + + i Suggested fix: Turn the parameter into a required parameter. + + 10 │ ····constructor(readonly·a·=·10,·readonly·b:·number)·{} + │ ---- + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.js b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.js new file mode 100644 index 00000000000..d25d2f391e8 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.js @@ -0,0 +1 @@ +export function f(a, b = 0) {} \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.js.snap new file mode 100644 index 00000000000..2623b529d2e --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.js.snap @@ -0,0 +1,11 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 73 +expression: valid.js +--- +# Input +```js +export function f(a, b = 0) {} +``` + + diff --git a/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.ts b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.ts new file mode 100644 index 00000000000..26ed62c15aa --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.ts @@ -0,0 +1,3 @@ +export function g(a: number, b?: number, c = 0) {} + +export function h(a: number, b = 0, c?: number) {} \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.ts.snap new file mode 100644 index 00000000000..3bd1e1bdbf4 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/valid.ts.snap @@ -0,0 +1,13 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 73 +expression: valid.ts +--- +# Input +```js +export function g(a: number, b?: number, c = 0) {} + +export function h(a: number, b = 0, c?: number) {} +``` + + diff --git a/crates/rome_service/src/configuration/linter/rules.rs b/crates/rome_service/src/configuration/linter/rules.rs index 70b83d3dabe..ec3d5091f8c 100644 --- a/crates/rome_service/src/configuration/linter/rules.rs +++ b/crates/rome_service/src/configuration/linter/rules.rs @@ -779,6 +779,8 @@ struct NurserySchema { use_camel_case: Option, #[doc = "Require const declarations for variables that are never reassigned after declared."] use_const: Option, + #[doc = "Enforce default function parameters and optional parameters to be last."] + use_default_parameter_last: Option, #[doc = "Enforce default clauses in switch statements to be last"] use_default_switch_clause_last: Option, #[doc = "Require that each enum member value be explicitly initialized."] @@ -796,7 +798,7 @@ struct NurserySchema { } impl Nursery { const CATEGORY_NAME: &'static str = "nursery"; - pub(crate) const CATEGORY_RULES: [&'static str; 32] = [ + pub(crate) const CATEGORY_RULES: [&'static str; 33] = [ "noAccessKey", "noBannedTypes", "noConditionalAssignment", @@ -822,6 +824,7 @@ impl Nursery { "useAriaPropsForRole", "useCamelCase", "useConst", + "useDefaultParameterLast", "useDefaultSwitchClauseLast", "useEnumInitializers", "useExhaustiveDependencies", @@ -830,7 +833,7 @@ impl Nursery { "useNumericLiterals", "useValidForDirection", ]; - const RECOMMENDED_RULES: [&'static str; 26] = [ + const RECOMMENDED_RULES: [&'static str; 27] = [ "noAccessKey", "noBannedTypes", "noConditionalAssignment", @@ -851,6 +854,7 @@ impl Nursery { "noVoidTypeReturn", "useAriaPropsForRole", "useConst", + "useDefaultParameterLast", "useDefaultSwitchClauseLast", "useEnumInitializers", "useExhaustiveDependencies", @@ -858,7 +862,7 @@ impl Nursery { "useNumericLiterals", "useValidForDirection", ]; - const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 26] = [ + const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 27] = [ RuleFilter::Rule("nursery", Self::CATEGORY_RULES[0]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[1]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[2]), @@ -882,9 +886,10 @@ impl Nursery { RuleFilter::Rule("nursery", Self::CATEGORY_RULES[25]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[26]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[27]), - RuleFilter::Rule("nursery", Self::CATEGORY_RULES[29]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[28]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[30]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[31]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[32]), ]; pub(crate) fn is_recommended(&self) -> bool { !matches!(self.recommended, Some(false)) } pub(crate) fn get_enabled_rules(&self) -> IndexSet { @@ -911,7 +916,7 @@ impl Nursery { pub(crate) fn is_recommended_rule(rule_name: &str) -> bool { Self::RECOMMENDED_RULES.contains(&rule_name) } - pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 26] { + pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 27] { Self::RECOMMENDED_RULES_AS_FILTERS } } diff --git a/editors/vscode/configuration_schema.json b/editors/vscode/configuration_schema.json index 779180c5288..18898f7ca15 100644 --- a/editors/vscode/configuration_schema.json +++ b/editors/vscode/configuration_schema.json @@ -1035,6 +1035,17 @@ } ] }, + "useDefaultParameterLast": { + "description": "Enforce default function parameters and optional parameters to be last.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "useDefaultSwitchClauseLast": { "description": "Enforce default clauses in switch statements to be last", "anyOf": [ diff --git a/npm/backend-jsonrpc/src/workspace.ts b/npm/backend-jsonrpc/src/workspace.ts index e37ff47ba02..6d9ce26b152 100644 --- a/npm/backend-jsonrpc/src/workspace.ts +++ b/npm/backend-jsonrpc/src/workspace.ts @@ -451,6 +451,10 @@ export interface Nursery { * Require const declarations for variables that are never reassigned after declared. */ useConst?: RuleConfiguration; + /** + * Enforce default function parameters and optional parameters to be last. + */ + useDefaultParameterLast?: RuleConfiguration; /** * Enforce default clauses in switch statements to be last */ @@ -705,6 +709,7 @@ export type Category = | "lint/nursery/noVoidTypeReturn" | "lint/nursery/useCamelCase" | "lint/nursery/useConst" + | "lint/nursery/useDefaultParameterLast" | "lint/nursery/useDefaultSwitchClauseLast" | "lint/nursery/useEnumInitializers" | "lint/nursery/useExhaustiveDependencies" diff --git a/npm/rome/configuration_schema.json b/npm/rome/configuration_schema.json index 779180c5288..18898f7ca15 100644 --- a/npm/rome/configuration_schema.json +++ b/npm/rome/configuration_schema.json @@ -1035,6 +1035,17 @@ } ] }, + "useDefaultParameterLast": { + "description": "Enforce default function parameters and optional parameters to be last.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "useDefaultSwitchClauseLast": { "description": "Enforce default clauses in switch statements to be last", "anyOf": [ diff --git a/website/src/pages/lint/rules/index.mdx b/website/src/pages/lint/rules/index.mdx index 4b176e24d01..3216c51fe71 100644 --- a/website/src/pages/lint/rules/index.mdx +++ b/website/src/pages/lint/rules/index.mdx @@ -596,6 +596,12 @@ Enforce camel case naming convention. Require const declarations for variables that are never reassigned after declared.
+

+ useDefaultParameterLast +

+Enforce default function parameters and optional parameters to be last. +
+

useDefaultSwitchClauseLast

diff --git a/website/src/pages/lint/rules/useDefaultParameterLast.md b/website/src/pages/lint/rules/useDefaultParameterLast.md new file mode 100644 index 00000000000..cf875c90762 --- /dev/null +++ b/website/src/pages/lint/rules/useDefaultParameterLast.md @@ -0,0 +1,139 @@ +--- +title: Lint Rule useDefaultParameterLast +parent: lint/rules/index +--- + +# useDefaultParameterLast (since v11.0.0) + +Enforce default function parameters and optional parameters to be last. + +Default and optional parameters that precede a required parameter cannot be omitted at call site. + +## Examples + +### Invalid + +```jsx +function f(a = 0, b) {} +``` + +
nursery/useDefaultParameterLast.js:1:12 lint/nursery/useDefaultParameterLast  FIXABLE  ━━━━━━━━━━━━━
+
+  ✖ The default parameter should follow the required parameter or should be a required parameter.
+  
+  > 1 │ function f(a = 0, b) {}
+      │            ^^^^^
+    2 │ 
+  
+  ℹ The required parameter is here:
+  
+  > 1 │ function f(a = 0, b) {}
+      │                   ^
+    2 │ 
+  
+  ℹ A default parameter that precedes a required parameter cannot be omitted at call site.
+  
+  ℹ Suggested fix: Turn the parameter into a required parameter.
+  
+    1 │ function·f(a·=·0,·b)·{}
+      │              ---       
+
+ +```jsx +function f(a, b = 0, c) {} +``` + +
nursery/useDefaultParameterLast.js:1:15 lint/nursery/useDefaultParameterLast  FIXABLE  ━━━━━━━━━━━━━
+
+  ✖ The default parameter should follow the required parameter or should be a required parameter.
+  
+  > 1 │ function f(a, b = 0, c) {}
+      │               ^^^^^
+    2 │ 
+  
+  ℹ The required parameter is here:
+  
+  > 1 │ function f(a, b = 0, c) {}
+      │                      ^
+    2 │ 
+  
+  ℹ A default parameter that precedes a required parameter cannot be omitted at call site.
+  
+  ℹ Suggested fix: Turn the parameter into a required parameter.
+  
+    1 │ function·f(a,·b·=·0,·c)·{}
+      │                 ---       
+
+ +```ts +function f(a: number, b?: number, c: number) {} +``` + +
nursery/useDefaultParameterLast.js:1:23 lint/nursery/useDefaultParameterLast  FIXABLE  ━━━━━━━━━━━━━
+
+  ✖ The optional parameter should follow the required parameter or should be a required parameter.
+  
+  > 1 │ function f(a: number, b?: number, c: number) {}
+      │                       ^^^^^^^^^^
+    2 │ 
+  
+  ℹ The required parameter is here:
+  
+  > 1 │ function f(a: number, b?: number, c: number) {}
+      │                                   ^^^^^^^^^
+    2 │ 
+  
+  ℹ A optional parameter that precedes a required parameter cannot be omitted at call site.
+  
+  ℹ Suggested fix: Turn the parameter into a required parameter.
+  
+    1 │ function·f(a:·number,·b?:·number,·c:·number)·{}
+      │                        -                       
+
+ +```ts +class Foo { + constructor(readonly a = 10, readonly b: number) {} +} +``` + +
nursery/useDefaultParameterLast.js:2:17 lint/nursery/useDefaultParameterLast  FIXABLE  ━━━━━━━━━━━━━
+
+  ✖ The default parameter should follow the required parameter or should be a required parameter.
+  
+    1 │ class Foo {
+  > 2 │     constructor(readonly a = 10, readonly b: number) {}
+      │                 ^^^^^^^^^^^^^^^
+    3 │ }
+    4 │ 
+  
+  ℹ The required parameter is here:
+  
+    1 │ class Foo {
+  > 2 │     constructor(readonly a = 10, readonly b: number) {}
+      │                                  ^^^^^^^^^^^^^^^^^^
+    3 │ }
+    4 │ 
+  
+  ℹ A default parameter that precedes a required parameter cannot be omitted at call site.
+  
+  ℹ Suggested fix: Turn the parameter into a required parameter.
+  
+    2 │ ····constructor(readonly·a·=·10,·readonly·b:·number)·{}
+      │                            ----                        
+
+ +### Valid + +```jsx +function f(a, b = 0) {} +``` + +```ts +function f(a: number, b?: number, c = 0) {} +``` + +```ts +function f(a: number, b = 0, c?: number) {} +``` +