feat(linter): eslint-plugin-unicorn no-unreadable-array-destructuring…

… (style) (#1594)

Try to implement `no-unreadable-array-destructuring` for #684

crates/oxc_linter/src/rules.rs

@@ -172,6 +172,7 @@ mod unicorn {
     pub mod no_this_assignment;
     pub mod no_typeof_undefined;
     pub mod no_unnecessary_await;
+    pub mod no_unreadable_array_destructuring;
     pub mod no_unreadable_iife;
     pub mod no_useless_fallback_in_spread;
     pub mod no_useless_length_check;
@@ -352,6 +353,7 @@ oxc_macros::declare_all_lint_rules! {
     unicorn::no_this_assignment,
     unicorn::no_typeof_undefined,
     unicorn::no_unnecessary_await,
+    unicorn::no_unreadable_array_destructuring,
     unicorn::no_unreadable_iife,
     unicorn::no_useless_fallback_in_spread,
     unicorn::no_useless_length_check,

crates/oxc_linter/src/rules/unicorn/no_unreadable_array_destructuring.rs

@@ -0,0 +1,143 @@
+use oxc_allocator::Vec;
+use oxc_ast::{
+    ast::{AssignmentTarget, AssignmentTargetPattern},
+    AstKind,
+};
+use oxc_diagnostics::{
+    miette::{self, Diagnostic},
+    thiserror::Error,
+};
+use oxc_macros::declare_oxc_lint;
+use oxc_span::Span;
+
+use crate::{context::LintContext, rule::Rule, AstNode};
+
+#[derive(Debug, Error, Diagnostic)]
+#[error("eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring")]
+#[diagnostic(
+    severity(warning),
+    help("Array destructuring may not contain consecutive ignored values.")
+)]
+struct NoUnreadableArrayDestructuringDiagnostic(#[label] pub Span);
+
+#[derive(Debug, Default, Clone)]
+pub struct NoUnreadableArrayDestructuring;
+
+declare_oxc_lint!(
+    /// ### What it does
+    ///
+    /// Disallow unreadable array destructuring
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Destructuring is very useful, but it can also make some code harder to read.
+    /// This rule prevents ignoring consecutive values when destructuring from an array.
+    ///
+    /// ### Example
+    /// ```javascript
+    /// // Bad
+    /// const [,, foo] = parts;
+    ///
+    /// // Good
+    /// const [foo] = parts;
+    /// ```
+    NoUnreadableArrayDestructuring,
+    style
+);
+
+fn is_unreadable_array_destructuring<T, U>(elements: &Vec<Option<T>>, rest: &Option<U>) -> bool {
+    if elements.len() >= 3 && elements.windows(2).any(|window| window.iter().all(Option::is_none)) {
+        return true;
+    }
+
+    if elements.len() == 2 && elements.iter().all(std::option::Option::is_none) && rest.is_some() {
+        return true;
+    }
+
+    false
+}
+
+impl Rule for NoUnreadableArrayDestructuring {
+    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
+        if let AstKind::ArrayPattern(array_pattern) = node.kind() {
+            if is_unreadable_array_destructuring(&array_pattern.elements, &array_pattern.rest) {
+                ctx.diagnostic(NoUnreadableArrayDestructuringDiagnostic(array_pattern.span));
+            }
+        }
+
+        if let AstKind::AssignmentTarget(AssignmentTarget::AssignmentTargetPattern(
+            AssignmentTargetPattern::ArrayAssignmentTarget(array_pattern),
+        )) = node.kind()
+        {
+            if is_unreadable_array_destructuring(&array_pattern.elements, &array_pattern.rest) {
+                ctx.diagnostic(NoUnreadableArrayDestructuringDiagnostic(array_pattern.span));
+            }
+        }
+    }
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = vec![
+        (r" [, foo] = parts;", None),
+        (r" [foo] = parts;", None),
+        (r" [foo,,bar] = parts;", None),
+        (r" [foo,   ,     bar] = parts;", None),
+        (r" [foo,] = parts;", None),
+        (r" [foo,,] = parts;", None),
+        (r" [foo,, bar,, baz] = parts;", None),
+        (r"[,foo] = bar;", None),
+        (r"({parts: [,foo]} = bar);", None),
+        (r"function foo([, bar]) {}", None),
+        (r"function foo([bar]) {}", None),
+        (r"function foo([bar,,baz]) {}", None),
+        (r"function foo([bar,   ,     baz]) {}", None),
+        (r"function foo([bar,]) {}", None),
+        (r"function foo([bar,,]) {}", None),
+        (r"function foo([bar,, baz,, qux]) {}", None),
+        (r" [, ...rest] = parts;", None),
+        // This is stupid, but valid code
+        (r" [,,] = parts;", None),
+    ];
+
+    let fail = vec![
+        (r"const [,, foo] = parts;", None),
+        (r"const [foo,,, bar] = parts;", None),
+        (r"const [foo,,,] = parts;", None),
+        (r"const [foo, bar,, baz ,,, qux] = parts;", None),
+        (r"[,, foo] = bar;", None),
+        (r"({parts: [,, foo]} = bar);", None),
+        (r"function foo([,, bar]) {}", None),
+        (r"function foo([bar,,, baz]) {}", None),
+        (r"function foo([bar,,,]) {}", None),
+        (r"function foo([bar, baz,, qux ,,, quux]) {}", None),
+        (r"const [,,...rest] = parts;", None),
+        // This is stupid, but valid code
+        (r"const [,,,] = parts;", None),
+        // Should add parentheses to array
+        (r"const [,,...rest] = new Array;", None),
+        (r"const [,,...rest] = (0, foo);", None),
+        (r"let [,,thirdElement] = new Array;", None),
+        (r"var [,,thirdElement] = (((0, foo)));", None),
+        // Variable is not `Identifier`
+        (r"let [,,[,,thirdElementInThirdElement]] = foo", None),
+        (r"let [,,{propertyOfThirdElement}] = foo", None),
+        // Multiple declarations
+        (r"let [,,thirdElement] = foo, anotherVariable = bar;", None),
+        // Default value
+        (r"let [,,thirdElement = {}] = foo;", None),
+        (r"for (const [, , id] of shuffle(list)) {}", None),
+        // Space after keyword
+        (r"let[,,thirdElement] = foo;", None),
+        (r"let[,,...thirdElement] = foo;", None),
+        (r"const[,,thirdElement] = foo;", None),
+        (r"const[,,...thirdElement] = foo;", None),
+        (r"var[,,thirdElement] = foo;", None),
+        (r"var[,,...thirdElement] = foo;", None),
+        (r"let[]=[],[,,thirdElement] = foo;", None),
+    ];
+
+    Tester::new(NoUnreadableArrayDestructuring::NAME, pass, fail).test_and_snapshot();
+}

crates/oxc_linter/src/snapshots/no_unreadable_array_destructuring.snap

@@ -0,0 +1,208 @@
+---
+source: crates/oxc_linter/src/tester.rs
+expression: no_unreadable_array_destructuring
+---
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ const [,, foo] = parts;
+   ·       ────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ const [foo,,, bar] = parts;
+   ·       ────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ const [foo,,,] = parts;
+   ·       ────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ const [foo, bar,, baz ,,, qux] = parts;
+   ·       ────────────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ [,, foo] = bar;
+   · ────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ ({parts: [,, foo]} = bar);
+   ·          ────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ function foo([,, bar]) {}
+   ·              ────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ function foo([bar,,, baz]) {}
+   ·              ────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ function foo([bar,,,]) {}
+   ·              ────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ function foo([bar, baz,, qux ,,, quux]) {}
+   ·              ─────────────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ const [,,...rest] = parts;
+   ·       ───────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ const [,,,] = parts;
+   ·       ─────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ const [,,...rest] = new Array;
+   ·       ───────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ const [,,...rest] = (0, foo);
+   ·       ───────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ let [,,thirdElement] = new Array;
+   ·     ────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ var [,,thirdElement] = (((0, foo)));
+   ·     ────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ let [,,[,,thirdElementInThirdElement]] = foo
+   ·     ──────────────────────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ let [,,[,,thirdElementInThirdElement]] = foo
+   ·        ──────────────────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ let [,,{propertyOfThirdElement}] = foo
+   ·     ────────────────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ let [,,thirdElement] = foo, anotherVariable = bar;
+   ·     ────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ let [,,thirdElement = {}] = foo;
+   ·     ─────────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ for (const [, , id] of shuffle(list)) {}
+   ·            ────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ let[,,thirdElement] = foo;
+   ·    ────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ let[,,...thirdElement] = foo;
+   ·    ───────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ const[,,thirdElement] = foo;
+   ·      ────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ const[,,...thirdElement] = foo;
+   ·      ───────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ var[,,thirdElement] = foo;
+   ·    ────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ var[,,...thirdElement] = foo;
+   ·    ───────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+  ⚠ eslint-plugin-unicorn(no-unreadable-array-destructuring): Disallow unreadable array destructuring
+   ╭─[no_unreadable_array_destructuring.tsx:1:1]
+ 1 │ let[]=[],[,,thirdElement] = foo;
+   ·          ────────────────
+   ╰────
+  help: Array destructuring may not contain consecutive ignored values.
+
+