From fb2d3e64497db0aeb7218771089363d5cc1d7704 Mon Sep 17 00:00:00 2001 From: Victorien ELVINGER Date: Wed, 21 Jun 2023 10:58:19 +0200 Subject: [PATCH] refactor(rome_js_analyze): relax noConfusingArrow (#4593) --- CHANGELOG.md | 15 ++++++++++++ .../analyzers/nursery/no_confusing_arrow.rs | 17 ++++++++------ .../specs/nursery/noConfusingArrow/invalid.js | 1 - .../nursery/noConfusingArrow/invalid.js.snap | 18 ++------------- .../specs/nursery/noConfusingArrow/valid.js | 9 +++++--- .../nursery/noConfusingArrow/valid.js.snap | 10 +++++--- .../src/pages/lint/rules/noConfusingArrow.md | 23 +++++-------------- 7 files changed, 46 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab541f29c2a..8357c12552a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,6 +75,21 @@ parameter decorators: The code action now removes any whitespace between the parameter name and its initialization. +- Relax [noConfusingArrow](https://docs.rome.tools/lint/rules/noconfusingarrow/) + + All arrow functions that enclose its parameter with parenthesis are allowed. + Thus, the following snippet no longer trigger the rule: + + ```js + var x = (a) => 1 ? 2 : 3; + ``` + + The following snippet still triggers the rule: + + ```js + var x = a => 1 ? 2 : 3; + ``` + - The rules [`useExhaustiveDependencies`](https://docs.rome.tools/lint/rules/useexhaustivedependencies/) and [`useHookAtTopLevel`](https://docs.rome.tools/lint/rules/usehookattoplevel/) accept a different shape of options Old configuration diff --git a/crates/rome_js_analyze/src/analyzers/nursery/no_confusing_arrow.rs b/crates/rome_js_analyze/src/analyzers/nursery/no_confusing_arrow.rs index 21f3b89fdc3..124ad218fbd 100644 --- a/crates/rome_js_analyze/src/analyzers/nursery/no_confusing_arrow.rs +++ b/crates/rome_js_analyze/src/analyzers/nursery/no_confusing_arrow.rs @@ -17,19 +17,19 @@ declare_rule! { /// ```js,expect_diagnostic /// var x = a => 1 ? 2 : 3; /// ``` - /// ```js,expect_diagnostic - /// var x = (a) => 1 ? 2 : 3; - /// ``` /// /// ## Valid /// /// ```js + /// var x = (a) => 1 ? 2 : 3; + /// /// var x = a => (1 ? 2 : 3); + /// /// var x = (a) => (1 ? 2 : 3); - /// var x = (a) => { - /// return 1 ? 2 : 3; - /// }; + /// /// var x = a => { return 1 ? 2 : 3; }; + /// + /// var x = (a) => { return 1 ? 2 : 3; }; /// ``` /// pub(crate) NoConfusingArrow { @@ -47,7 +47,10 @@ impl Rule for NoConfusingArrow { fn run(ctx: &RuleContext) -> Self::Signals { let arrow_fn = ctx.query(); - + if arrow_fn.parameters().ok()?.as_js_parameters().is_some() { + // Don't report arrow functions that enclose its parameters with parenthesis. + return None; + } arrow_fn .body() .ok()? diff --git a/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/invalid.js b/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/invalid.js index 76e008c97d0..7d2e3938fcf 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/invalid.js +++ b/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/invalid.js @@ -1,2 +1 @@ var x = a => 1 ? 2 : 3; -var x = (a) => 1 ? 2 : 3; diff --git a/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/invalid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/invalid.js.snap index 010d883eb20..62dc1a731f2 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/invalid.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/invalid.js.snap @@ -1,11 +1,11 @@ --- source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 100 expression: invalid.js --- # Input ```js var x = a => 1 ? 2 : 3; -var x = (a) => 1 ? 2 : 3; ``` @@ -17,21 +17,7 @@ invalid.js:1:11 lint/nursery/noConfusingArrow ━━━━━━━━━━━ > 1 │ var x = a => 1 ? 2 : 3; │ ^^ - 2 │ var x = (a) => 1 ? 2 : 3; - 3 │ - - -``` - -``` -invalid.js:2:13 lint/nursery/noConfusingArrow ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Fat arrows can be confused with some comparison operators (<, >, <=, >=). - - 1 │ var x = a => 1 ? 2 : 3; - > 2 │ var x = (a) => 1 ? 2 : 3; - │ ^^ - 3 │ + 2 │ ``` diff --git a/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/valid.js b/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/valid.js index 5e82e06a7d6..796f16862d9 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/valid.js +++ b/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/valid.js @@ -1,8 +1,11 @@ /* should not generate diagnostics */ var x = a => (1 ? 2 : 3); + +var x = (a) => 1 ? 2 : 3; + var x = (a) => (1 ? 2 : 3); -var x = (a) => { - return 1 ? 2 : 3; -}; + +var x = (a) => { return 1 ? 2 : 3; }; + var x = a => { return 1 ? 2 : 3; }; diff --git a/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/valid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/valid.js.snap index cf6fa725a1b..4b33c1e8167 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/valid.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noConfusingArrow/valid.js.snap @@ -1,5 +1,6 @@ --- source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 100 expression: valid.js --- # Input @@ -7,10 +8,13 @@ expression: valid.js /* should not generate diagnostics */ var x = a => (1 ? 2 : 3); + +var x = (a) => 1 ? 2 : 3; + var x = (a) => (1 ? 2 : 3); -var x = (a) => { - return 1 ? 2 : 3; -}; + +var x = (a) => { return 1 ? 2 : 3; }; + var x = a => { return 1 ? 2 : 3; }; ``` diff --git a/website/src/pages/lint/rules/noConfusingArrow.md b/website/src/pages/lint/rules/noConfusingArrow.md index 68524534494..0e34679d1e3 100644 --- a/website/src/pages/lint/rules/noConfusingArrow.md +++ b/website/src/pages/lint/rules/noConfusingArrow.md @@ -30,29 +30,18 @@ var x = a => 1 ? 2 : 3; -```jsx -var x = (a) => 1 ? 2 : 3; -``` - -
nursery/noConfusingArrow.js:1:13 lint/nursery/noConfusingArrow ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
-
-  ⚠ Fat arrows can be confused with some comparison operators (<, >, <=, >=).
-  
-  > 1 │ var x = (a) => 1 ? 2 : 3;
-      │             ^^
-    2 │ 
-  
-
- ## Valid ```jsx +var x = (a) => 1 ? 2 : 3; + var x = a => (1 ? 2 : 3); + var x = (a) => (1 ? 2 : 3); -var x = (a) => { - return 1 ? 2 : 3; -}; + var x = a => { return 1 ? 2 : 3; }; + +var x = (a) => { return 1 ? 2 : 3; }; ``` ## Related links