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

feat(rome_analyze): noSparseArray #2650

Merged
merged 25 commits into from
Jun 13, 2022
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
db2f504
chore: 🤖 checkpoint
IWANABETHATGUY Jun 3, 2022
1c44ec4
test: 💍 add code action
IWANABETHATGUY Jun 3, 2022
658985e
test: 💍 add more test case
IWANABETHATGUY Jun 3, 2022
b26aff3
test: 💍 add more test case
IWANABETHATGUY Jun 3, 2022
bb15811
test: 💍 tweak test case
IWANABETHATGUY Jun 3, 2022
ccfc457
Merge branch 'main' into feat/no-sparse-array
IWANABETHATGUY Jun 8, 2022
ed1943f
fix: 🐛 test
IWANABETHATGUY Jun 8, 2022
72d9cd5
test: 💍 fix test case
IWANABETHATGUY Jun 8, 2022
360fa2c
test: 💍 update test case
IWANABETHATGUY Jun 8, 2022
abd57e3
chore: 🤖 remove test main
IWANABETHATGUY Jun 8, 2022
86cf2ba
fix: 🐛 tweak text
IWANABETHATGUY Jun 9, 2022
49afe36
chore: 🤖 shapshot
IWANABETHATGUY Jun 9, 2022
b9f8754
Merge branch 'main' into feat/no-sparse-array
IWANABETHATGUY Jun 9, 2022
631e5f7
Merge branch 'feat/no-sparse-array' of github.com:IWANABETHATGUY/tool…
IWANABETHATGUY Jun 9, 2022
757c437
fix: 🐛 compile error
IWANABETHATGUY Jun 9, 2022
c967605
fix: 🐛 address issue
IWANABETHATGUY Jun 9, 2022
452926b
fix: 🐛 address cr issue
IWANABETHATGUY Jun 9, 2022
cbd9a8b
Merge branch 'main' into feat/no-sparse-array
IWANABETHATGUY Jun 9, 2022
d614331
Merge branch 'main' into feat/no-sparse-array
IWANABETHATGUY Jun 9, 2022
af4d6ea
Merge branch 'main' into feat/no-sparse-array
IWANABETHATGUY Jun 10, 2022
a43d17a
fix: 🐛 address cr
IWANABETHATGUY Jun 10, 2022
4aff0c0
Merge branch 'main' into feat/no-sparse-array
IWANABETHATGUY Jun 13, 2022
b483b75
fix: 🐛 snapshot
IWANABETHATGUY Jun 13, 2022
604f8d5
Merge branch 'feat/no-sparse-array' of github.com:IWANABETHATGUY/tool…
IWANABETHATGUY Jun 13, 2022
701a6d9
Merge branch 'main' into feat/no-sparse-array
leops Jun 13, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/rome_analyze/src/analyzers.rs

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

83 changes: 83 additions & 0 deletions crates/rome_analyze/src/analyzers/no_sparse_array.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
use rome_console::markup;
use rome_diagnostics::Applicability;
use rome_js_factory::make;
use rome_js_syntax::{
JsAnyArrayElement, JsAnyExpression, JsAnyRoot, JsArrayExpression, TriviaPieceKind,
};
use rome_rowan::{AstNode, AstNodeExt, AstSeparatedList};

use crate::registry::{JsRuleAction, Rule, 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<Self::State> {
// We defer collect `JsHole` index until user want to apply code action.
node.elements().iter().find_map(|element| {
if matches!(element.ok()?, JsAnyArrayElement::JsArrayHole(_),) {
Some(())
} else {
None
}
})
}

fn diagnostic(node: &Self::Query, _state: &Self::State) -> Option<RuleDiagnostic> {
Some(RuleDiagnostic::warning(
node.syntax().text_trimmed_range(),
markup! {
"This "<Emphasis>"array"</Emphasis>" contains an "<Emphasis>"empty slot"</Emphasis>"."
}
.to_owned()
))
}

fn action(root: JsAnyRoot, node: &Self::Query, _state: &Self::State) -> Option<JsRuleAction> {
let mut final_array_element_list = node.elements();

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()?,
final_array_element_list,
node.r_brack_token().ok()?,
),
)?;

Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::MaybeIncorrect,
message: markup! { "Replace hole with undefined" }.to_owned(),
root,
})
}
}
1 change: 1 addition & 0 deletions crates/rome_analyze/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ impl_registry_builders!(
NoDelete,
NoDoubleEquals,
NoNegationElse,
NoSparseArray,
UseSingleCaseStatement,
UseSingleVarDeclarator,
UseValidTypeof,
Expand Down
10 changes: 10 additions & 0 deletions crates/rome_analyze/tests/specs/noSparseArray.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// valid
const a = [ 1, 2, ];

// invalid
const b = [/**test*/,];
const c = [,,];
const d = [,2];

const e = [1,,]
const f = [1,,2]
122 changes: 122 additions & 0 deletions crates/rome_analyze/tests/specs/noSparseArray.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
---
source: crates/rome_analyze/tests/spec_tests.rs
assertion_line: 98
expression: noSparseArray.js
---
# Input
```js
// valid
const a = [ 1, 2, ];

// invalid
const b = [/**test*/,];
const c = [,,];
const d = [,2];

const e = [1,,]
const f = [1,,2]

```

# Diagnostics
```
warning[noSparseArray]: This array contains an empty slot.
┌─ noSparseArray.js:5:11
5 │ const b = [/**test*/,];
│ ------------

Suggested fix: Replace hole with undefined
| @@ -2,7 +2,7 @@
1 1 | const a = [ 1, 2, ];
2 2 |
3 3 | // invalid
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:11
6 │ const c = [,,];
│ ----

Suggested fix: Replace hole with undefined
| @@ -3,7 +3,7 @@
2 2 |
3 3 | // invalid
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,,]


```

```
warning[noSparseArray]: This array contains an empty slot.
┌─ noSparseArray.js:7:11
7 │ const d = [,2];
│ ----

Suggested fix: Replace hole with undefined
| @@ -4,7 +4,7 @@
3 3 | // invalid
4 4 | const b = [/**test*/,];
5 5 | const c = [,,];
6 | - const d = [,2];
6 | + const d = [undefined,2];
7 7 |
8 8 | const e = [1,,]
9 9 | const f = [1,,2]


```

```
warning[noSparseArray]: This array contains an empty slot.
┌─ noSparseArray.js:9:11
9 │ const e = [1,,]
│ -----

Suggested fix: Replace hole with undefined
| @@ -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]
│ ------

Suggested fix: Replace hole 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]

IWANABETHATGUY marked this conversation as resolved.
Show resolved Hide resolved

```