Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
feat(rome_js_analyze): useDefaultParameterLast (#3873)
Browse files Browse the repository at this point in the history 
Co-authored-by: Micha Reiser <[email protected]>
  • Loading branch information
@Conaclos @MichaReiser
Conaclos and MichaReiser authored Dec 5, 2022
1 parent 58a8e38 commit 731ce10
Show file tree
Hide file tree
Showing 17 changed files with 772 additions and 6 deletions.
1 change: 1 addition & 0 deletions crates/rome_diagnostics_categories/src/categories.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion crates/rome_js_analyze/src/analyzers/nursery.rs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

176 changes: 176 additions & 0 deletions crates/rome_js_analyze/src/analyzers/nursery/use_default_parameter_last.rs
View file
Original file line number Diff line number Diff line change
@@ -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<JsInitializerClause> {
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<JsSyntaxToken> {
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<AnyFormalParameter>;
type State = AnyFormalParameter;
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> 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<Self>, required_param: &Self::State) -> Option<RuleDiagnostic> {
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 "<Emphasis>{param_kind}" parameter"</Emphasis>" should follow the "<Emphasis>"required parameter"</Emphasis>" or should be a "<Emphasis>"required parameter"</Emphasis>"."
},
).detail(
required_param.range(),
markup! {
"The "<Emphasis>"required parameter"</Emphasis>" is here:"
},
).note(
markup! {
"A "<Emphasis>{param_kind}" parameter"</Emphasis>" that precedes a "<Emphasis>"required parameter"</Emphasis>" cannot be omitted at call site."
}
))
}

fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
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::<Vec<_>>(),
);
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::<Vec<_>>(),
);
mutation.replace_token_discard_trivia(prev_token, new_prev_token);
mutation.remove_node(initializer);
}
Some(JsRuleAction {
mutation,
message:
markup! {"Turn the parameter into a "<Emphasis>"required parameter"</Emphasis>"."}
.to_owned(),
category: ActionCategory::QuickFix,
applicability: Applicability::MaybeIncorrect,
})
}
}
11 changes: 11 additions & 0 deletions crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.js
View file
Original file line number Diff line number Diff line change
@@ -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
) {}
107 changes: 107 additions & 0 deletions crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.js.snap
View file
Original file line number Diff line number Diff line change
@@ -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·*/
│ -------------
```


11 changes: 11 additions & 0 deletions crates/rome_js_analyze/tests/specs/nursery/useDefaultParameterLast/invalid.ts
View file
Original file line number Diff line number Diff line change
@@ -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) {}
}
Loading

0 comments on commit 731ce10

Please sign in to comment.