From db2f504e9375fc6fa67097da62f85ab32afcbb09 Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Fri, 3 Jun 2022 18:01:22 +0800 Subject: [PATCH 01/16] =?UTF-8?q?chore:=20=F0=9F=A4=96=20checkpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/rome_analyze/src/analyzers.rs | 2 + .../src/analyzers/no_sparse_array.rs | 62 +++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 crates/rome_analyze/src/analyzers/no_sparse_array.rs diff --git a/crates/rome_analyze/src/analyzers.rs b/crates/rome_analyze/src/analyzers.rs index 9168a2af90d..6e04d3c398b 100644 --- a/crates/rome_analyze/src/analyzers.rs +++ b/crates/rome_analyze/src/analyzers.rs @@ -1,9 +1,11 @@ mod no_delete; mod no_double_equals; +mod no_sparse_array; mod use_single_var_declarator; mod use_while; pub(crate) use no_delete::NoDelete; pub(crate) use no_double_equals::NoDoubleEquals; +pub(crate) use no_sparse_array::NoSparseArray; pub(crate) use use_single_var_declarator::UseSingleVarDeclarator; pub(crate) use use_while::UseWhile; diff --git a/crates/rome_analyze/src/analyzers/no_sparse_array.rs b/crates/rome_analyze/src/analyzers/no_sparse_array.rs new file mode 100644 index 00000000000..6a8c9d0dcdb --- /dev/null +++ b/crates/rome_analyze/src/analyzers/no_sparse_array.rs @@ -0,0 +1,62 @@ +use rome_console::markup; +use rome_diagnostics::{Applicability, Severity}; +use rome_js_factory::make; +use rome_js_syntax::{ + JsAnyAssignment, JsAnyAssignmentPattern, JsAnyExpression, JsAnyRoot, JsArrayExpression, + JsComputedMemberExpression, JsComputedMemberExpressionFields, JsStaticMemberExpression, + JsStaticMemberExpressionFields, JsUnaryExpression, JsUnaryOperator, T, +}; +use rome_rowan::{AstNode, AstNodeExt, AstSeparatedList}; + +use crate::registry::{Rule, RuleAction, RuleDiagnostic}; +use crate::{ActionCategory, RuleCategory}; + +pub(crate) enum NoSparseArray {} + +impl Rule for NoSparseArray { + const NAME: &'static str = "noSparseArray"; + const CATEGORY: RuleCategory = RuleCategory::Lint; + + type Query = JsArrayExpression; + type State = (); + + fn run(node: &Self::Query) -> Option { + // We defer collect `JsHole` index until user want to fix this issue. + node.elements() + .iter() + .filter_map(|item| item.ok()) + .position(|element| matches!(element, JsArrayHole)) + .map(|_| ()) + } + + fn diagnostic(node: &Self::Query, _state: &Self::State) -> Option { + Some(RuleDiagnostic { + severity: Severity::Warning, + range: node.syntax().text_trimmed_range(), + message: markup! { + "This ""array"" contains an ""empty slot""." + } + .to_owned(), + }) + } + + fn action(root: JsAnyRoot, node: &Self::Query, state: &Self::State) -> Option { + let root = root.replace_node( + JsAnyExpression::from(node.clone()), + JsAnyExpression::from(make::js_assignment_expression( + state.clone().try_into().ok()?, + make::token_decorated_with_space(T![=]), + JsAnyExpression::from(make::js_identifier_expression( + make::js_reference_identifier(make::ident("undefined")), + )), + )), + )?; + + Some(RuleAction { + category: ActionCategory::QuickFix, + applicability: Applicability::MaybeIncorrect, + message: markup! { "Replace with undefined assignment" }.to_owned(), + root, + }) + } +} From 1c44ec49c436ebfd6a862dc20fbc950d0985afca Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Fri, 3 Jun 2022 21:19:50 +0800 Subject: [PATCH 02/16] =?UTF-8?q?test:=20=F0=9F=92=8D=20add=20code=20actio?= =?UTF-8?q?n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/analyzers/no_sparse_array.rs | 52 +++++++++++++------ crates/rome_analyze/src/registry.rs | 1 + .../rome_analyze/tests/specs/noSparseArray.js | 6 +++ .../tests/specs/noSparseArray.js.snap | 41 +++++++++++++++ 4 files changed, 85 insertions(+), 15 deletions(-) create mode 100644 crates/rome_analyze/tests/specs/noSparseArray.js create mode 100644 crates/rome_analyze/tests/specs/noSparseArray.js.snap diff --git a/crates/rome_analyze/src/analyzers/no_sparse_array.rs b/crates/rome_analyze/src/analyzers/no_sparse_array.rs index 6a8c9d0dcdb..b8087ca7525 100644 --- a/crates/rome_analyze/src/analyzers/no_sparse_array.rs +++ b/crates/rome_analyze/src/analyzers/no_sparse_array.rs @@ -2,11 +2,12 @@ use rome_console::markup; use rome_diagnostics::{Applicability, Severity}; use rome_js_factory::make; use rome_js_syntax::{ - JsAnyAssignment, JsAnyAssignmentPattern, JsAnyExpression, JsAnyRoot, JsArrayExpression, - JsComputedMemberExpression, JsComputedMemberExpressionFields, JsStaticMemberExpression, - JsStaticMemberExpressionFields, JsUnaryExpression, JsUnaryOperator, T, + JsAnyArrayElement, JsAnyAssignment, JsAnyAssignmentPattern, JsAnyExpression, JsAnyRoot, + JsArrayExpression, JsArrayHole, JsComputedMemberExpression, JsComputedMemberExpressionFields, + JsStaticMemberExpression, JsStaticMemberExpressionFields, JsSyntaxKind, JsUnaryExpression, + JsUnaryOperator, T, }; -use rome_rowan::{AstNode, AstNodeExt, AstSeparatedList}; +use rome_rowan::{AstNode, AstNodeExt, AstSeparatedList, SyntaxElement}; use crate::registry::{Rule, RuleAction, RuleDiagnostic}; use crate::{ActionCategory, RuleCategory}; @@ -25,7 +26,7 @@ impl Rule for NoSparseArray { node.elements() .iter() .filter_map(|item| item.ok()) - .position(|element| matches!(element, JsArrayHole)) + .position(|element| matches!(element, JsAnyArrayElement::JsArrayHole(_),)) .map(|_| ()) } @@ -41,16 +42,37 @@ impl Rule for NoSparseArray { } fn action(root: JsAnyRoot, node: &Self::Query, state: &Self::State) -> Option { - let root = root.replace_node( - JsAnyExpression::from(node.clone()), - JsAnyExpression::from(make::js_assignment_expression( - state.clone().try_into().ok()?, - make::token_decorated_with_space(T![=]), - JsAnyExpression::from(make::js_identifier_expression( - make::js_reference_identifier(make::ident("undefined")), - )), - )), - )?; + let mut syntax = node.clone().into_syntax(); + let hole_index_iter = syntax.children().enumerate().filter_map(|(i, a)| { + if matches!(a.kind(), JsSyntaxKind::JS_ARRAY_HOLE) { + Some(i) + } else { + None + } + }); + + let ident_expr = + make::js_identifier_expression(make::js_reference_identifier(make::ident("undefined"))); + let ident_expr_syntax = ident_expr.into_syntax(); + for index in hole_index_iter { + syntax = syntax.splice_slots( + index..=index, + [Some(SyntaxElement::Node(ident_expr_syntax.clone()))].into_iter(), + ); + } + + let root = root.replace_node(node.clone(), JsArrayExpression::unwrap_cast(syntax))?; + // syntax.splice_slots(range, replace_with) + // let root = root.replace_node( + // JsAnyExpression::from(node.clone()), + // JsAnyExpression::from(make::js_assignment_expression( + // state.clone().try_into().ok()?, + // make::token_decorated_with_space(T![=]), + // JsAnyExpression::from(make::js_identifier_expression( + // make::js_reference_identifier(make::ident("undefined")), + // )), + // )), + // )?; Some(RuleAction { category: ActionCategory::QuickFix, diff --git a/crates/rome_analyze/src/registry.rs b/crates/rome_analyze/src/registry.rs index 0a5139cca2f..8336ca5209a 100644 --- a/crates/rome_analyze/src/registry.rs +++ b/crates/rome_analyze/src/registry.rs @@ -39,6 +39,7 @@ impl_registry_builders!( NoDoubleEquals, UseSingleVarDeclarator, UseWhile, + NoSparseArray, // Assists FlipBinExp ); diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js b/crates/rome_analyze/tests/specs/noSparseArray.js new file mode 100644 index 00000000000..7823de430df --- /dev/null +++ b/crates/rome_analyze/tests/specs/noSparseArray.js @@ -0,0 +1,6 @@ +// valid +var a = [ 1, 2, ]; + +// invalid +var a = [,]; +var a = [ 1,, 2]; \ No newline at end of file diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js.snap b/crates/rome_analyze/tests/specs/noSparseArray.js.snap new file mode 100644 index 00000000000..307a7ed1e50 --- /dev/null +++ b/crates/rome_analyze/tests/specs/noSparseArray.js.snap @@ -0,0 +1,41 @@ +--- +source: crates/rome_analyze/tests/spec_tests.rs +assertion_line: 96 +expression: noSparseArray.js +--- +# Input +```js +// valid +var a = [ 1, 2, ]; + +// invalid +var a = [,]; +var a = [ 1,, 2]; +``` + +# Diagnostics +``` +warning[noSparseArray]: This array contains an empty slot. + ┌─ noSparseArray.js:5:9 + │ +5 │ var a = [,]; + │ --- This array contains an empty slot. + +Replace with undefined assignment + + +``` + +``` +warning[noSparseArray]: This array contains an empty slot. + ┌─ noSparseArray.js:6:9 + │ +6 │ var a = [ 1,, 2]; + │ -------- This array contains an empty slot. + +Replace with undefined assignment + + +``` + + From 658985e15e3b01cb7da39a6393558c11717b33e5 Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Fri, 3 Jun 2022 22:25:12 +0800 Subject: [PATCH 03/16] =?UTF-8?q?test:=20=F0=9F=92=8D=20add=20more=20test?= =?UTF-8?q?=20case?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/analyzers/no_sparse_array.rs | 54 ++++++++++--------- .../rome_analyze/tests/specs/noSparseArray.js | 5 +- .../tests/specs/noSparseArray.js.snap | 40 +++++++++++--- 3 files changed, 65 insertions(+), 34 deletions(-) diff --git a/crates/rome_analyze/src/analyzers/no_sparse_array.rs b/crates/rome_analyze/src/analyzers/no_sparse_array.rs index b8087ca7525..ff450c9e3d1 100644 --- a/crates/rome_analyze/src/analyzers/no_sparse_array.rs +++ b/crates/rome_analyze/src/analyzers/no_sparse_array.rs @@ -3,9 +3,9 @@ use rome_diagnostics::{Applicability, Severity}; use rome_js_factory::make; use rome_js_syntax::{ JsAnyArrayElement, JsAnyAssignment, JsAnyAssignmentPattern, JsAnyExpression, JsAnyRoot, - JsArrayExpression, JsArrayHole, JsComputedMemberExpression, JsComputedMemberExpressionFields, - JsStaticMemberExpression, JsStaticMemberExpressionFields, JsSyntaxKind, JsUnaryExpression, - JsUnaryOperator, T, + JsArrayElementList, JsArrayExpression, JsArrayHole, JsComputedMemberExpression, + JsComputedMemberExpressionFields, JsStaticMemberExpression, JsStaticMemberExpressionFields, + JsSyntaxKind, JsUnaryExpression, JsUnaryOperator, T, }; use rome_rowan::{AstNode, AstNodeExt, AstSeparatedList, SyntaxElement}; @@ -42,37 +42,39 @@ impl Rule for NoSparseArray { } fn action(root: JsAnyRoot, node: &Self::Query, state: &Self::State) -> Option { - let mut syntax = node.clone().into_syntax(); - let hole_index_iter = syntax.children().enumerate().filter_map(|(i, a)| { - if matches!(a.kind(), JsSyntaxKind::JS_ARRAY_HOLE) { - Some(i) - } else { - None - } - }); + let mut syntax = node.elements().clone().into_syntax(); + let mut hole_index_iter = syntax + .children_with_tokens() + .enumerate() + .filter_map(|(i, a)| { + if matches!(a.kind(), JsSyntaxKind::JS_ARRAY_HOLE) { + Some(i) + } else { + None + } + }); - let ident_expr = - make::js_identifier_expression(make::js_reference_identifier(make::ident("undefined"))); - let ident_expr_syntax = ident_expr.into_syntax(); for index in hole_index_iter { + let ident_expr = make::js_identifier_expression(make::js_reference_identifier( + make::ident("undefined"), + )); + let ident_expr_syntax = ident_expr.into_syntax(); syntax = syntax.splice_slots( index..=index, [Some(SyntaxElement::Node(ident_expr_syntax.clone()))].into_iter(), ); } - let root = root.replace_node(node.clone(), JsArrayExpression::unwrap_cast(syntax))?; - // syntax.splice_slots(range, replace_with) - // let root = root.replace_node( - // JsAnyExpression::from(node.clone()), - // JsAnyExpression::from(make::js_assignment_expression( - // state.clone().try_into().ok()?, - // make::token_decorated_with_space(T![=]), - // JsAnyExpression::from(make::js_identifier_expression( - // make::js_reference_identifier(make::ident("undefined")), - // )), - // )), - // )?; + let root = root + .replace_node( + node.clone(), + make::js_array_expression( + node.l_brack_token().ok()?, + JsArrayElementList::unwrap_cast(syntax), + node.r_brack_token().ok()?, + ), + ) + .unwrap(); Some(RuleAction { category: ActionCategory::QuickFix, diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js b/crates/rome_analyze/tests/specs/noSparseArray.js index 7823de430df..21aac08b1f7 100644 --- a/crates/rome_analyze/tests/specs/noSparseArray.js +++ b/crates/rome_analyze/tests/specs/noSparseArray.js @@ -2,5 +2,6 @@ var a = [ 1, 2, ]; // invalid -var a = [,]; -var a = [ 1,, 2]; \ No newline at end of file +var a = [/**test*/,]; +var a = [,,]; +var a = [1,,2]; \ No newline at end of file diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js.snap b/crates/rome_analyze/tests/specs/noSparseArray.js.snap index 307a7ed1e50..93a10fc8593 100644 --- a/crates/rome_analyze/tests/specs/noSparseArray.js.snap +++ b/crates/rome_analyze/tests/specs/noSparseArray.js.snap @@ -9,8 +9,9 @@ expression: noSparseArray.js var a = [ 1, 2, ]; // invalid -var a = [,]; -var a = [ 1,, 2]; +var a = [/**test*/,]; +var a = [,,]; +var a = [1,,2]; ``` # Diagnostics @@ -18,10 +19,18 @@ var a = [ 1,, 2]; warning[noSparseArray]: This array contains an empty slot. ┌─ noSparseArray.js:5:9 │ -5 │ var a = [,]; - │ --- This array contains an empty slot. +5 │ var a = [/**test*/,]; + │ ------------ This array contains an empty slot. Replace with undefined assignment + | @@ -2,6 +2,6 @@ +1 1 | var a = [ 1, 2, ]; +2 2 | +3 3 | // invalid +4 | - var a = [/**test*/,]; + 4 | + var a = [/**test*/undefined,]; +5 5 | var a = [,,]; +6 6 | var a = [1,,2]; ``` @@ -30,8 +39,27 @@ Replace with undefined assignment warning[noSparseArray]: This array contains an empty slot. ┌─ noSparseArray.js:6:9 │ -6 │ var a = [ 1,, 2]; - │ -------- This array contains an empty slot. +6 │ var a = [,,]; + │ ---- This array contains an empty slot. + +Replace with undefined assignment + | @@ -3,5 +3,5 @@ +2 2 | +3 3 | // invalid +4 4 | var a = [/**test*/,]; +5 | - var a = [,,]; + 5 | + var a = [undefined,undefined,]; +6 6 | var a = [1,,2]; + + +``` + +``` +warning[noSparseArray]: This array contains an empty slot. + ┌─ noSparseArray.js:7:9 + │ +7 │ var a = [1,,2]; + │ ------ This array contains an empty slot. Replace with undefined assignment From b26aff38f22c47e764e9c39027c5086488fbe920 Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Fri, 3 Jun 2022 23:27:32 +0800 Subject: [PATCH 04/16] =?UTF-8?q?test:=20=F0=9F=92=8D=20add=20more=20test?= =?UTF-8?q?=20case?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/analyzers/no_sparse_array.rs | 6 +- .../rome_analyze/tests/specs/noSparseArray.js | 10 ++- .../tests/specs/noSparseArray.js.snap | 79 ++++++++++++------- 3 files changed, 61 insertions(+), 34 deletions(-) diff --git a/crates/rome_analyze/src/analyzers/no_sparse_array.rs b/crates/rome_analyze/src/analyzers/no_sparse_array.rs index ff450c9e3d1..dbf4dcaa552 100644 --- a/crates/rome_analyze/src/analyzers/no_sparse_array.rs +++ b/crates/rome_analyze/src/analyzers/no_sparse_array.rs @@ -22,7 +22,7 @@ impl Rule for NoSparseArray { type State = (); fn run(node: &Self::Query) -> Option { - // We defer collect `JsHole` index until user want to fix this issue. + // We defer collect `JsHole` index until user want to apply code action. node.elements() .iter() .filter_map(|item| item.ok()) @@ -53,7 +53,7 @@ impl Rule for NoSparseArray { None } }); - + // syntax. for index in hole_index_iter { let ident_expr = make::js_identifier_expression(make::js_reference_identifier( make::ident("undefined"), @@ -79,7 +79,7 @@ impl Rule for NoSparseArray { Some(RuleAction { category: ActionCategory::QuickFix, applicability: Applicability::MaybeIncorrect, - message: markup! { "Replace with undefined assignment" }.to_owned(), + message: markup! { "Replace with undefined" }.to_owned(), root, }) } diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js b/crates/rome_analyze/tests/specs/noSparseArray.js index 21aac08b1f7..f3d4f7d9bba 100644 --- a/crates/rome_analyze/tests/specs/noSparseArray.js +++ b/crates/rome_analyze/tests/specs/noSparseArray.js @@ -1,7 +1,9 @@ // valid -var a = [ 1, 2, ]; +const a = [ 1, 2, ]; // invalid -var a = [/**test*/,]; -var a = [,,]; -var a = [1,,2]; \ No newline at end of file +const b = [/**test*/,]; +const c = [,,]; +const d = [,,2]; + +const e = [1,,2] \ No newline at end of file diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js.snap b/crates/rome_analyze/tests/specs/noSparseArray.js.snap index 93a10fc8593..ccd6472b332 100644 --- a/crates/rome_analyze/tests/specs/noSparseArray.js.snap +++ b/crates/rome_analyze/tests/specs/noSparseArray.js.snap @@ -6,62 +6,87 @@ expression: noSparseArray.js # Input ```js // valid -var a = [ 1, 2, ]; +const a = [ 1, 2, ]; // invalid -var a = [/**test*/,]; -var a = [,,]; -var a = [1,,2]; +const b = [/**test*/,]; +const c = [,,]; +const d = [,,2]; + +const e = [1,,2] ``` # Diagnostics ``` warning[noSparseArray]: This array contains an empty slot. - ┌─ noSparseArray.js:5:9 + ┌─ noSparseArray.js:5:11 │ -5 │ var a = [/**test*/,]; - │ ------------ This array contains an empty slot. +5 │ const b = [/**test*/,]; + │ ------------ This array contains an empty slot. -Replace with undefined assignment - | @@ -2,6 +2,6 @@ -1 1 | var a = [ 1, 2, ]; +Replace with undefined + | @@ -2,7 +2,7 @@ +1 1 | const a = [ 1, 2, ]; 2 2 | 3 3 | // invalid -4 | - var a = [/**test*/,]; - 4 | + var a = [/**test*/undefined,]; -5 5 | var a = [,,]; -6 6 | var a = [1,,2]; +4 | - const b = [/**test*/,]; + 4 | + const b = [/**test*/undefined,]; +5 5 | const c = [,,]; +6 6 | const d = [,,2]; +7 7 | ``` ``` warning[noSparseArray]: This array contains an empty slot. - ┌─ noSparseArray.js:6:9 + ┌─ noSparseArray.js:6:11 │ -6 │ var a = [,,]; - │ ---- This array contains an empty slot. +6 │ const c = [,,]; + │ ---- This array contains an empty slot. -Replace with undefined assignment - | @@ -3,5 +3,5 @@ +Replace with undefined + | @@ -3,7 +3,7 @@ 2 2 | 3 3 | // invalid -4 4 | var a = [/**test*/,]; -5 | - var a = [,,]; - 5 | + var a = [undefined,undefined,]; -6 6 | var a = [1,,2]; +4 4 | const b = [/**test*/,]; +5 | - const c = [,,]; + 5 | + const c = [undefined,undefined,]; +6 6 | const d = [,,2]; +7 7 | +8 8 | const e = [1,,2] + + +``` + +``` +warning[noSparseArray]: This array contains an empty slot. + ┌─ noSparseArray.js:7:11 + │ +7 │ const d = [,,2]; + │ ----- This array contains an empty slot. + +Replace with undefined + | @@ -4,6 +4,6 @@ +3 3 | // invalid +4 4 | const b = [/**test*/,]; +5 5 | const c = [,,]; +6 | - const d = [,,2]; + 6 | + const d = [undefined,undefined,2]; +7 7 | +8 8 | const e = [1,,2] ``` ``` warning[noSparseArray]: This array contains an empty slot. - ┌─ noSparseArray.js:7:9 + ┌─ noSparseArray.js:9:11 │ -7 │ var a = [1,,2]; - │ ------ This array contains an empty slot. +9 │ const e = [1,,2] + │ ------ This array contains an empty slot. -Replace with undefined assignment +Replace with undefined ``` From bb15811d153c15890413e7bb6921738e99ff6044 Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Sat, 4 Jun 2022 00:22:45 +0800 Subject: [PATCH 05/16] =?UTF-8?q?test:=20=F0=9F=92=8D=20tweak=20test=20cas?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/analyzers/no_sparse_array.rs | 36 +++++++----- crates/rome_analyze/src/main.rs | 56 +++++++++++++++++++ .../rome_analyze/tests/specs/noSparseArray.js | 4 +- .../tests/specs/noSparseArray.js.snap | 25 +++++---- 4 files changed, 92 insertions(+), 29 deletions(-) create mode 100644 crates/rome_analyze/src/main.rs diff --git a/crates/rome_analyze/src/analyzers/no_sparse_array.rs b/crates/rome_analyze/src/analyzers/no_sparse_array.rs index dbf4dcaa552..2c4b8cbb2fe 100644 --- a/crates/rome_analyze/src/analyzers/no_sparse_array.rs +++ b/crates/rome_analyze/src/analyzers/no_sparse_array.rs @@ -1,3 +1,5 @@ +use std::iter::once; + use rome_console::markup; use rome_diagnostics::{Applicability, Severity}; use rome_js_factory::make; @@ -7,7 +9,7 @@ use rome_js_syntax::{ JsComputedMemberExpressionFields, JsStaticMemberExpression, JsStaticMemberExpressionFields, JsSyntaxKind, JsUnaryExpression, JsUnaryOperator, T, }; -use rome_rowan::{AstNode, AstNodeExt, AstSeparatedList, SyntaxElement}; +use rome_rowan::{AstNode, AstNodeExt, AstSeparatedList, SyntaxElement, SyntaxNode}; use crate::registry::{Rule, RuleAction, RuleDiagnostic}; use crate::{ActionCategory, RuleCategory}; @@ -41,36 +43,40 @@ impl Rule for NoSparseArray { }) } - fn action(root: JsAnyRoot, node: &Self::Query, state: &Self::State) -> Option { - let mut syntax = node.elements().clone().into_syntax(); - let mut hole_index_iter = syntax - .children_with_tokens() + fn action(root: JsAnyRoot, node: &Self::Query, _state: &Self::State) -> Option { + let mut array_element_list = node.elements(); + let hole_index_iter = array_element_list + .iter() .enumerate() - .filter_map(|(i, a)| { - if matches!(a.kind(), JsSyntaxKind::JS_ARRAY_HOLE) { + .filter_map(|(i, item)| { + if matches!(item, Ok(JsAnyArrayElement::JsArrayHole(_))) { + println!("{}", i); Some(i) } else { None } }); - // syntax. + for index in hole_index_iter { + // println!("{}", index); let ident_expr = make::js_identifier_expression(make::js_reference_identifier( make::ident("undefined"), )); - let ident_expr_syntax = ident_expr.into_syntax(); - syntax = syntax.splice_slots( - index..=index, - [Some(SyntaxElement::Node(ident_expr_syntax.clone()))].into_iter(), - ); - } + let n_element = array_element_list.iter().skip(index).next()?.ok()?; + array_element_list = array_element_list.replace_node( + n_element, + JsAnyArrayElement::JsAnyExpression(JsAnyExpression::JsIdentifierExpression( + ident_expr, + )), + )?; + } let root = root .replace_node( node.clone(), make::js_array_expression( node.l_brack_token().ok()?, - JsArrayElementList::unwrap_cast(syntax), + array_element_list, node.r_brack_token().ok()?, ), ) diff --git a/crates/rome_analyze/src/main.rs b/crates/rome_analyze/src/main.rs new file mode 100644 index 00000000000..47aa68e7c97 --- /dev/null +++ b/crates/rome_analyze/src/main.rs @@ -0,0 +1,56 @@ +use core::slice; + +use rome_analyze::AnalysisFilter; +use rome_js_parser::parse; +use rome_js_syntax::SourceType; + +fn main() { + let input_code = r#" + let b = [1, ,2]; + + + "#; + println!("{}", input_code); + let source_type = SourceType::js_module(); + let parsed = parse(&input_code, 0, source_type); + let root = parsed.tree(); + let filter = AnalysisFilter { + rules: Some(slice::from_ref(&"noSparseArray")), + ..AnalysisFilter::default() + }; + + let mut diagnostics = Vec::new(); + let mut code_fixes = Vec::new(); + + rome_analyze::analyze(0, &root, filter, |event| { + if let Some(mut diag) = event.diagnostic() { + if let Some(action) = event.action() { + diag.suggestions.push(action.into()); + } + + diagnostics.push(diag); + return; + } + + if let Some(action) = event.action() { + code_fixes.push(action); + } + }); +} + +// fn diagnostic_to_string(name: &str, source: &str, diag: Diagnostic) -> String { +// let file = SimpleFile::new(name.into(), source.into()); +// let text = markup_to_string(markup! { +// {diag.display(&file)} +// }); + +// text +// } + +// fn code_fix_to_string(source: &str, action: AnalyzerAction) -> String { +// let output = action.root.syntax().to_string(); + +// markup_to_string(markup! { +// {Diff { mode: DiffMode::Unified, left: source, right: &output }} +// }) +// } diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js b/crates/rome_analyze/tests/specs/noSparseArray.js index f3d4f7d9bba..4a04cdff908 100644 --- a/crates/rome_analyze/tests/specs/noSparseArray.js +++ b/crates/rome_analyze/tests/specs/noSparseArray.js @@ -4,6 +4,6 @@ const a = [ 1, 2, ]; // invalid const b = [/**test*/,]; const c = [,,]; -const d = [,,2]; +const d = [,2]; -const e = [1,,2] \ No newline at end of file +const e = [1,,] diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js.snap b/crates/rome_analyze/tests/specs/noSparseArray.js.snap index ccd6472b332..28cd994d219 100644 --- a/crates/rome_analyze/tests/specs/noSparseArray.js.snap +++ b/crates/rome_analyze/tests/specs/noSparseArray.js.snap @@ -11,9 +11,10 @@ const a = [ 1, 2, ]; // invalid const b = [/**test*/,]; const c = [,,]; -const d = [,,2]; +const d = [,2]; + +const e = [1,,] -const e = [1,,2] ``` # Diagnostics @@ -32,7 +33,7 @@ Replace with undefined 4 | - const b = [/**test*/,]; 4 | + const b = [/**test*/undefined,]; 5 5 | const c = [,,]; -6 6 | const d = [,,2]; +6 6 | const d = [,2]; 7 7 | @@ -52,9 +53,9 @@ Replace with undefined 4 4 | const b = [/**test*/,]; 5 | - const c = [,,]; 5 | + const c = [undefined,undefined,]; -6 6 | const d = [,,2]; +6 6 | const d = [,2]; 7 7 | -8 8 | const e = [1,,2] +8 8 | const e = [1,,] ``` @@ -63,18 +64,18 @@ Replace with undefined warning[noSparseArray]: This array contains an empty slot. ┌─ noSparseArray.js:7:11 │ -7 │ const d = [,,2]; - │ ----- This array contains an empty slot. +7 │ const d = [,2]; + │ ---- This array contains an empty slot. Replace with undefined | @@ -4,6 +4,6 @@ 3 3 | // invalid 4 4 | const b = [/**test*/,]; 5 5 | const c = [,,]; -6 | - const d = [,,2]; - 6 | + const d = [undefined,undefined,2]; +6 | - const d = [,2]; + 6 | + const d = [undefined,2]; 7 7 | -8 8 | const e = [1,,2] +8 8 | const e = [1,,] ``` @@ -83,8 +84,8 @@ Replace with undefined warning[noSparseArray]: This array contains an empty slot. ┌─ noSparseArray.js:9:11 │ -9 │ const e = [1,,2] - │ ------ This array contains an empty slot. +9 │ const e = [1,,] + │ ----- This array contains an empty slot. Replace with undefined From ed1943fa91375d7d68fe3a70c1f7a80a5b935540 Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Wed, 8 Jun 2022 20:51:21 +0800 Subject: [PATCH 06/16] =?UTF-8?q?fix:=20=F0=9F=90=9B=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/rome_analyze/tests/specs/noSparseArray.js.snap | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js.snap b/crates/rome_analyze/tests/specs/noSparseArray.js.snap index 28cd994d219..63471c8b7a0 100644 --- a/crates/rome_analyze/tests/specs/noSparseArray.js.snap +++ b/crates/rome_analyze/tests/specs/noSparseArray.js.snap @@ -88,6 +88,12 @@ warning[noSparseArray]: This array contains an empty slot. │ ----- This array contains an empty slot. Replace with undefined + | @@ -6,4 +6,4 @@ +5 5 | const c = [,,]; +6 6 | const d = [,2]; +7 7 | +8 | - const e = [1,,] + 8 | + const e = [1,undefined,] ``` From 72d9cd56741de534d8e4410d2e71473af8fb15d2 Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Wed, 8 Jun 2022 20:54:32 +0800 Subject: [PATCH 07/16] =?UTF-8?q?test:=20=F0=9F=92=8D=20fix=20test=20case?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rome_analyze/tests/specs/noSparseArray.js | 1 + .../tests/specs/noSparseArray.js.snap | 25 +++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js b/crates/rome_analyze/tests/specs/noSparseArray.js index 4a04cdff908..035cb44cf04 100644 --- a/crates/rome_analyze/tests/specs/noSparseArray.js +++ b/crates/rome_analyze/tests/specs/noSparseArray.js @@ -7,3 +7,4 @@ const c = [,,]; const d = [,2]; const e = [1,,] +const f = [1,,2] diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js.snap b/crates/rome_analyze/tests/specs/noSparseArray.js.snap index 63471c8b7a0..1eb80a16695 100644 --- a/crates/rome_analyze/tests/specs/noSparseArray.js.snap +++ b/crates/rome_analyze/tests/specs/noSparseArray.js.snap @@ -14,6 +14,7 @@ const c = [,,]; const d = [,2]; const e = [1,,] +const f = [1,,2] ``` @@ -68,7 +69,7 @@ warning[noSparseArray]: This array contains an empty slot. │ ---- This array contains an empty slot. Replace with undefined - | @@ -4,6 +4,6 @@ + | @@ -4,7 +4,7 @@ 3 3 | // invalid 4 4 | const b = [/**test*/,]; 5 5 | const c = [,,]; @@ -76,6 +77,7 @@ Replace with undefined 6 | + const d = [undefined,2]; 7 7 | 8 8 | const e = [1,,] +9 9 | const f = [1,,2] ``` @@ -88,12 +90,31 @@ warning[noSparseArray]: This array contains an empty slot. │ ----- This array contains an empty slot. Replace with undefined - | @@ -6,4 +6,4 @@ + | @@ -6,5 +6,5 @@ 5 5 | const c = [,,]; 6 6 | const d = [,2]; 7 7 | 8 | - const e = [1,,] 8 | + const e = [1,undefined,] +9 9 | const f = [1,,2] + + +``` + +``` +warning[noSparseArray]: This array contains an empty slot. + ┌─ noSparseArray.js:10:11 + │ +10 │ const f = [1,,2] + │ ------ This array contains an empty slot. + +Replace with undefined + | @@ -7,4 +7,4 @@ + 6 6 | const d = [,2]; + 7 7 | + 8 8 | const e = [1,,] + 9 | - const f = [1,,2] + 9 | + const f = [1,undefined,2] ``` From 360fa2c4f6a018bc80e2a017690f2a34629d911f Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Wed, 8 Jun 2022 21:05:12 +0800 Subject: [PATCH 08/16] =?UTF-8?q?test:=20=F0=9F=92=8D=20update=20test=20ca?= =?UTF-8?q?se?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/analyzers/no_sparse_array.rs | 23 +++++++++---------- .../tests/specs/noSparseArray.js.snap | 6 ++--- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/crates/rome_analyze/src/analyzers/no_sparse_array.rs b/crates/rome_analyze/src/analyzers/no_sparse_array.rs index 2c4b8cbb2fe..5d594a88410 100644 --- a/crates/rome_analyze/src/analyzers/no_sparse_array.rs +++ b/crates/rome_analyze/src/analyzers/no_sparse_array.rs @@ -1,15 +1,10 @@ -use std::iter::once; - use rome_console::markup; use rome_diagnostics::{Applicability, Severity}; use rome_js_factory::make; use rome_js_syntax::{ - JsAnyArrayElement, JsAnyAssignment, JsAnyAssignmentPattern, JsAnyExpression, JsAnyRoot, - JsArrayElementList, JsArrayExpression, JsArrayHole, JsComputedMemberExpression, - JsComputedMemberExpressionFields, JsStaticMemberExpression, JsStaticMemberExpressionFields, - JsSyntaxKind, JsUnaryExpression, JsUnaryOperator, T, + JsAnyArrayElement, JsAnyExpression, JsAnyRoot, JsArrayExpression, TriviaPieceKind, }; -use rome_rowan::{AstNode, AstNodeExt, AstSeparatedList, SyntaxElement, SyntaxNode}; +use rome_rowan::{AstNode, AstNodeExt, AstSeparatedList}; use crate::registry::{Rule, RuleAction, RuleDiagnostic}; use crate::{ActionCategory, RuleCategory}; @@ -58,12 +53,16 @@ impl Rule for NoSparseArray { }); for index in hole_index_iter { - // println!("{}", index); - let ident_expr = make::js_identifier_expression(make::js_reference_identifier( - make::ident("undefined"), - )); + let undefine_indent = if index == 0 { + make::ident("undefined") + } else { + make::ident("undefined") + .with_leading_trivia(std::iter::once((TriviaPieceKind::Whitespace, " "))) + }; + let ident_expr = + make::js_identifier_expression(make::js_reference_identifier(undefine_indent)); - let n_element = array_element_list.iter().skip(index).next()?.ok()?; + let n_element = array_element_list.iter().nth(index)?.ok()?; array_element_list = array_element_list.replace_node( n_element, JsAnyArrayElement::JsAnyExpression(JsAnyExpression::JsIdentifierExpression( diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js.snap b/crates/rome_analyze/tests/specs/noSparseArray.js.snap index 1eb80a16695..de6850e10d2 100644 --- a/crates/rome_analyze/tests/specs/noSparseArray.js.snap +++ b/crates/rome_analyze/tests/specs/noSparseArray.js.snap @@ -53,7 +53,7 @@ Replace with undefined 3 3 | // invalid 4 4 | const b = [/**test*/,]; 5 | - const c = [,,]; - 5 | + const c = [undefined,undefined,]; + 5 | + const c = [undefined, undefined,]; 6 6 | const d = [,2]; 7 7 | 8 8 | const e = [1,,] @@ -95,7 +95,7 @@ Replace with undefined 6 6 | const d = [,2]; 7 7 | 8 | - const e = [1,,] - 8 | + const e = [1,undefined,] + 8 | + const e = [1, undefined,] 9 9 | const f = [1,,2] @@ -114,7 +114,7 @@ Replace with undefined 7 7 | 8 8 | const e = [1,,] 9 | - const f = [1,,2] - 9 | + const f = [1,undefined,2] + 9 | + const f = [1, undefined,2] ``` From abd57e3f0b2f90c3cbff5efc0881258a3f0fee7f Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Wed, 8 Jun 2022 21:06:39 +0800 Subject: [PATCH 09/16] =?UTF-8?q?chore:=20=F0=9F=A4=96=20remove=20test=20m?= =?UTF-8?q?ain?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/rome_analyze/src/main.rs | 56 --------------------------------- 1 file changed, 56 deletions(-) delete mode 100644 crates/rome_analyze/src/main.rs diff --git a/crates/rome_analyze/src/main.rs b/crates/rome_analyze/src/main.rs deleted file mode 100644 index 47aa68e7c97..00000000000 --- a/crates/rome_analyze/src/main.rs +++ /dev/null @@ -1,56 +0,0 @@ -use core::slice; - -use rome_analyze::AnalysisFilter; -use rome_js_parser::parse; -use rome_js_syntax::SourceType; - -fn main() { - let input_code = r#" - let b = [1, ,2]; - - - "#; - println!("{}", input_code); - let source_type = SourceType::js_module(); - let parsed = parse(&input_code, 0, source_type); - let root = parsed.tree(); - let filter = AnalysisFilter { - rules: Some(slice::from_ref(&"noSparseArray")), - ..AnalysisFilter::default() - }; - - let mut diagnostics = Vec::new(); - let mut code_fixes = Vec::new(); - - rome_analyze::analyze(0, &root, filter, |event| { - if let Some(mut diag) = event.diagnostic() { - if let Some(action) = event.action() { - diag.suggestions.push(action.into()); - } - - diagnostics.push(diag); - return; - } - - if let Some(action) = event.action() { - code_fixes.push(action); - } - }); -} - -// fn diagnostic_to_string(name: &str, source: &str, diag: Diagnostic) -> String { -// let file = SimpleFile::new(name.into(), source.into()); -// let text = markup_to_string(markup! { -// {diag.display(&file)} -// }); - -// text -// } - -// fn code_fix_to_string(source: &str, action: AnalyzerAction) -> String { -// let output = action.root.syntax().to_string(); - -// markup_to_string(markup! { -// {Diff { mode: DiffMode::Unified, left: source, right: &output }} -// }) -// } From 86cf2ba0accbc72b39dfafed7a34fdeb4bff8481 Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Thu, 9 Jun 2022 14:20:37 +0800 Subject: [PATCH 10/16] =?UTF-8?q?fix:=20=F0=9F=90=9B=20tweak=20text?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/rome_analyze/src/analyzers/no_sparse_array.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rome_analyze/src/analyzers/no_sparse_array.rs b/crates/rome_analyze/src/analyzers/no_sparse_array.rs index 5d594a88410..0c79e7375ff 100644 --- a/crates/rome_analyze/src/analyzers/no_sparse_array.rs +++ b/crates/rome_analyze/src/analyzers/no_sparse_array.rs @@ -84,7 +84,7 @@ impl Rule for NoSparseArray { Some(RuleAction { category: ActionCategory::QuickFix, applicability: Applicability::MaybeIncorrect, - message: markup! { "Replace with undefined" }.to_owned(), + message: markup! { "Replace hole with undefined" }.to_owned(), root, }) } From 49afe368e2b128b60089b78a726a779d29829c7f Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Thu, 9 Jun 2022 14:22:28 +0800 Subject: [PATCH 11/16] =?UTF-8?q?chore:=20=F0=9F=A4=96=20shapshot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/rome_analyze/src/analyzers/no_sparse_array.rs | 1 - crates/rome_analyze/tests/specs/noSparseArray.js.snap | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/rome_analyze/src/analyzers/no_sparse_array.rs b/crates/rome_analyze/src/analyzers/no_sparse_array.rs index 0c79e7375ff..ce811c03797 100644 --- a/crates/rome_analyze/src/analyzers/no_sparse_array.rs +++ b/crates/rome_analyze/src/analyzers/no_sparse_array.rs @@ -45,7 +45,6 @@ impl Rule for NoSparseArray { .enumerate() .filter_map(|(i, item)| { if matches!(item, Ok(JsAnyArrayElement::JsArrayHole(_))) { - println!("{}", i); Some(i) } else { None diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js.snap b/crates/rome_analyze/tests/specs/noSparseArray.js.snap index de6850e10d2..52e21c6be26 100644 --- a/crates/rome_analyze/tests/specs/noSparseArray.js.snap +++ b/crates/rome_analyze/tests/specs/noSparseArray.js.snap @@ -26,7 +26,7 @@ warning[noSparseArray]: This array contains an empty slot. 5 │ const b = [/**test*/,]; │ ------------ This array contains an empty slot. -Replace with undefined +Replace hole with undefined | @@ -2,7 +2,7 @@ 1 1 | const a = [ 1, 2, ]; 2 2 | @@ -47,7 +47,7 @@ warning[noSparseArray]: This array contains an empty slot. 6 │ const c = [,,]; │ ---- This array contains an empty slot. -Replace with undefined +Replace hole with undefined | @@ -3,7 +3,7 @@ 2 2 | 3 3 | // invalid @@ -68,7 +68,7 @@ warning[noSparseArray]: This array contains an empty slot. 7 │ const d = [,2]; │ ---- This array contains an empty slot. -Replace with undefined +Replace hole with undefined | @@ -4,7 +4,7 @@ 3 3 | // invalid 4 4 | const b = [/**test*/,]; @@ -89,7 +89,7 @@ warning[noSparseArray]: This array contains an empty slot. 9 │ const e = [1,,] │ ----- This array contains an empty slot. -Replace with undefined +Replace hole with undefined | @@ -6,5 +6,5 @@ 5 5 | const c = [,,]; 6 6 | const d = [,2]; @@ -108,7 +108,7 @@ warning[noSparseArray]: This array contains an empty slot. 10 │ const f = [1,,2] │ ------ This array contains an empty slot. -Replace with undefined +Replace hole with undefined | @@ -7,4 +7,4 @@ 6 6 | const d = [,2]; 7 7 | From 757c4376db28c079549fa73cb64fb1b52f3b0df0 Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Thu, 9 Jun 2022 23:49:08 +0800 Subject: [PATCH 12/16] =?UTF-8?q?fix:=20=F0=9F=90=9B=20compile=20error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/rome_analyze/src/analyzers/no_sparse_array.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/rome_analyze/src/analyzers/no_sparse_array.rs b/crates/rome_analyze/src/analyzers/no_sparse_array.rs index ce811c03797..bebb9cfdd5a 100644 --- a/crates/rome_analyze/src/analyzers/no_sparse_array.rs +++ b/crates/rome_analyze/src/analyzers/no_sparse_array.rs @@ -6,7 +6,7 @@ use rome_js_syntax::{ }; use rome_rowan::{AstNode, AstNodeExt, AstSeparatedList}; -use crate::registry::{Rule, RuleAction, RuleDiagnostic}; +use crate::registry::{Rule, RuleAction, RuleDiagnostic, JsRuleAction}; use crate::{ActionCategory, RuleCategory}; pub(crate) enum NoSparseArray {} @@ -38,7 +38,7 @@ impl Rule for NoSparseArray { }) } - fn action(root: JsAnyRoot, node: &Self::Query, _state: &Self::State) -> Option { + fn action(root: JsAnyRoot, node: &Self::Query, _state: &Self::State) -> Option { let mut array_element_list = node.elements(); let hole_index_iter = array_element_list .iter() From c967605ccc560166cc623ea7c4dea433a098ec41 Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Thu, 9 Jun 2022 23:54:56 +0800 Subject: [PATCH 13/16] =?UTF-8?q?fix:=20=F0=9F=90=9B=20address=20issue?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/analyzers/no_sparse_array.rs | 16 +++++++++++----- crates/rome_analyze/src/registry.rs | 4 ++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/crates/rome_analyze/src/analyzers/no_sparse_array.rs b/crates/rome_analyze/src/analyzers/no_sparse_array.rs index bebb9cfdd5a..986ed50a3fe 100644 --- a/crates/rome_analyze/src/analyzers/no_sparse_array.rs +++ b/crates/rome_analyze/src/analyzers/no_sparse_array.rs @@ -6,7 +6,7 @@ use rome_js_syntax::{ }; use rome_rowan::{AstNode, AstNodeExt, AstSeparatedList}; -use crate::registry::{Rule, RuleAction, RuleDiagnostic, JsRuleAction}; +use crate::registry::{JsRuleAction, Rule, RuleDiagnostic}; use crate::{ActionCategory, RuleCategory}; pub(crate) enum NoSparseArray {} @@ -22,9 +22,15 @@ impl Rule for NoSparseArray { // We defer collect `JsHole` index until user want to apply code action. node.elements() .iter() - .filter_map(|item| item.ok()) - .position(|element| matches!(element, JsAnyArrayElement::JsArrayHole(_),)) - .map(|_| ()) + // .filter_map(|item| item.ok()) + .find_map(|element| { + if matches!(element.ok()?, JsAnyArrayElement::JsArrayHole(_),) { + Some(()) + } else { + None + } + }) + // .map(|_| ()) } fn diagnostic(node: &Self::Query, _state: &Self::State) -> Option { @@ -80,7 +86,7 @@ impl Rule for NoSparseArray { ) .unwrap(); - Some(RuleAction { + Some(JsRuleAction { category: ActionCategory::QuickFix, applicability: Applicability::MaybeIncorrect, message: markup! { "Replace hole with undefined" }.to_owned(), diff --git a/crates/rome_analyze/src/registry.rs b/crates/rome_analyze/src/registry.rs index 78042b3cb6f..a841e9ac0b7 100644 --- a/crates/rome_analyze/src/registry.rs +++ b/crates/rome_analyze/src/registry.rs @@ -36,11 +36,11 @@ macro_rules! impl_registry_builders { impl_registry_builders!( // Analyzers + UseWhile, + NoSparseArray, NoDelete, NoDoubleEquals, UseSingleVarDeclarator, - UseWhile, - NoSparseArray, // Assists FlipBinExp, ); From 452926b8b1f4298dd740299dc9bd0ab0184827d7 Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Fri, 10 Jun 2022 00:38:43 +0800 Subject: [PATCH 14/16] =?UTF-8?q?fix:=20=F0=9F=90=9B=20address=20cr=20issu?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/analyzers/no_sparse_array.rs | 69 ++++++++----------- 1 file changed, 30 insertions(+), 39 deletions(-) diff --git a/crates/rome_analyze/src/analyzers/no_sparse_array.rs b/crates/rome_analyze/src/analyzers/no_sparse_array.rs index 986ed50a3fe..5e4aead6d14 100644 --- a/crates/rome_analyze/src/analyzers/no_sparse_array.rs +++ b/crates/rome_analyze/src/analyzers/no_sparse_array.rs @@ -30,7 +30,6 @@ impl Rule for NoSparseArray { None } }) - // .map(|_| ()) } fn diagnostic(node: &Self::Query, _state: &Self::State) -> Option { @@ -45,46 +44,38 @@ impl Rule for NoSparseArray { } fn action(root: JsAnyRoot, node: &Self::Query, _state: &Self::State) -> Option { - let mut array_element_list = node.elements(); - let hole_index_iter = array_element_list - .iter() - .enumerate() - .filter_map(|(i, item)| { - if matches!(item, Ok(JsAnyArrayElement::JsArrayHole(_))) { - Some(i) - } else { - None - } - }); + let mut final_array_element_list = node.elements(); - for index in hole_index_iter { - let undefine_indent = if index == 0 { - make::ident("undefined") - } else { - make::ident("undefined") - .with_leading_trivia(std::iter::once((TriviaPieceKind::Whitespace, " "))) - }; - let ident_expr = - make::js_identifier_expression(make::js_reference_identifier(undefine_indent)); - - let n_element = array_element_list.iter().nth(index)?.ok()?; - array_element_list = array_element_list.replace_node( - n_element, - JsAnyArrayElement::JsAnyExpression(JsAnyExpression::JsIdentifierExpression( - ident_expr, - )), - )?; + for (i, item) in final_array_element_list.iter().enumerate() { + if matches!(item, Ok(JsAnyArrayElement::JsArrayHole(_))) { + let undefine_indent = if i == 0 { + make::ident("undefined") + } else { + make::ident("undefined") + .with_leading_trivia(std::iter::once((TriviaPieceKind::Whitespace, " "))) + }; + let ident_expr = + make::js_identifier_expression(make::js_reference_identifier(undefine_indent)); + // Why we need to use `final_array_element_list.iter().nth(i)` instead of `item`, because every time we + // call `replace_node` the previous iteration `item` is not the descent child of current `final_array_element_list` any more. + let n_element = final_array_element_list.iter().nth(i)?.ok()?; + final_array_element_list = final_array_element_list.replace_node( + n_element, + JsAnyArrayElement::JsAnyExpression(JsAnyExpression::JsIdentifierExpression( + ident_expr, + )), + )?; + } } - let root = root - .replace_node( - node.clone(), - make::js_array_expression( - node.l_brack_token().ok()?, - array_element_list, - node.r_brack_token().ok()?, - ), - ) - .unwrap(); + + let root = root.replace_node( + node.clone(), + make::js_array_expression( + node.l_brack_token().ok()?, + final_array_element_list, + node.r_brack_token().ok()?, + ), + )?; Some(JsRuleAction { category: ActionCategory::QuickFix, From a43d17ac6dac3fda83c9618f25c2e8eceee9a1a6 Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Fri, 10 Jun 2022 21:37:43 +0800 Subject: [PATCH 15/16] =?UTF-8?q?fix:=20=F0=9F=90=9B=20address=20cr?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/analyzers/no_sparse_array.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/crates/rome_analyze/src/analyzers/no_sparse_array.rs b/crates/rome_analyze/src/analyzers/no_sparse_array.rs index 5e4aead6d14..2db73542cfe 100644 --- a/crates/rome_analyze/src/analyzers/no_sparse_array.rs +++ b/crates/rome_analyze/src/analyzers/no_sparse_array.rs @@ -20,16 +20,13 @@ impl Rule for NoSparseArray { fn run(node: &Self::Query) -> Option { // We defer collect `JsHole` index until user want to apply code action. - node.elements() - .iter() - // .filter_map(|item| item.ok()) - .find_map(|element| { - if matches!(element.ok()?, JsAnyArrayElement::JsArrayHole(_),) { - Some(()) - } else { - None - } - }) + node.elements().iter().find_map(|element| { + if matches!(element.ok()?, JsAnyArrayElement::JsArrayHole(_),) { + Some(()) + } else { + None + } + }) } fn diagnostic(node: &Self::Query, _state: &Self::State) -> Option { From b483b7576f556d12e7776ca6c7283634c891dac9 Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Tue, 14 Jun 2022 00:54:20 +0800 Subject: [PATCH 16/16] =?UTF-8?q?fix:=20=F0=9F=90=9B=20snapshot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/analyzers/no_sparse_array.rs | 13 +++++------ .../tests/specs/noSparseArray.js.snap | 22 +++++++++---------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/crates/rome_analyze/src/analyzers/no_sparse_array.rs b/crates/rome_analyze/src/analyzers/no_sparse_array.rs index 5e4aead6d14..db21177e716 100644 --- a/crates/rome_analyze/src/analyzers/no_sparse_array.rs +++ b/crates/rome_analyze/src/analyzers/no_sparse_array.rs @@ -1,5 +1,5 @@ use rome_console::markup; -use rome_diagnostics::{Applicability, Severity}; +use rome_diagnostics::Applicability; use rome_js_factory::make; use rome_js_syntax::{ JsAnyArrayElement, JsAnyExpression, JsAnyRoot, JsArrayExpression, TriviaPieceKind, @@ -33,14 +33,13 @@ impl Rule for NoSparseArray { } fn diagnostic(node: &Self::Query, _state: &Self::State) -> Option { - Some(RuleDiagnostic { - severity: Severity::Warning, - range: node.syntax().text_trimmed_range(), - message: markup! { + Some(RuleDiagnostic::warning( + node.syntax().text_trimmed_range(), +markup! { "This ""array"" contains an ""empty slot""." } - .to_owned(), - }) + .to_owned() + )) } fn action(root: JsAnyRoot, node: &Self::Query, _state: &Self::State) -> Option { diff --git a/crates/rome_analyze/tests/specs/noSparseArray.js.snap b/crates/rome_analyze/tests/specs/noSparseArray.js.snap index 52e21c6be26..45446939ae7 100644 --- a/crates/rome_analyze/tests/specs/noSparseArray.js.snap +++ b/crates/rome_analyze/tests/specs/noSparseArray.js.snap @@ -1,6 +1,6 @@ --- source: crates/rome_analyze/tests/spec_tests.rs -assertion_line: 96 +assertion_line: 98 expression: noSparseArray.js --- # Input @@ -24,9 +24,9 @@ warning[noSparseArray]: This array contains an empty slot. ┌─ noSparseArray.js:5:11 │ 5 │ const b = [/**test*/,]; - │ ------------ This array contains an empty slot. + │ ------------ -Replace hole with undefined +Suggested fix: Replace hole with undefined | @@ -2,7 +2,7 @@ 1 1 | const a = [ 1, 2, ]; 2 2 | @@ -45,9 +45,9 @@ warning[noSparseArray]: This array contains an empty slot. ┌─ noSparseArray.js:6:11 │ 6 │ const c = [,,]; - │ ---- This array contains an empty slot. + │ ---- -Replace hole with undefined +Suggested fix: Replace hole with undefined | @@ -3,7 +3,7 @@ 2 2 | 3 3 | // invalid @@ -66,9 +66,9 @@ warning[noSparseArray]: This array contains an empty slot. ┌─ noSparseArray.js:7:11 │ 7 │ const d = [,2]; - │ ---- This array contains an empty slot. + │ ---- -Replace hole with undefined +Suggested fix: Replace hole with undefined | @@ -4,7 +4,7 @@ 3 3 | // invalid 4 4 | const b = [/**test*/,]; @@ -87,9 +87,9 @@ warning[noSparseArray]: This array contains an empty slot. ┌─ noSparseArray.js:9:11 │ 9 │ const e = [1,,] - │ ----- This array contains an empty slot. + │ ----- -Replace hole with undefined +Suggested fix: Replace hole with undefined | @@ -6,5 +6,5 @@ 5 5 | const c = [,,]; 6 6 | const d = [,2]; @@ -106,9 +106,9 @@ warning[noSparseArray]: This array contains an empty slot. ┌─ noSparseArray.js:10:11 │ 10 │ const f = [1,,2] - │ ------ This array contains an empty slot. + │ ------ -Replace hole with undefined +Suggested fix: Replace hole with undefined | @@ -7,4 +7,4 @@ 6 6 | const d = [,2]; 7 7 |