Skip to content

feat(no-new-array): add no-new-array rule - #1522

Open
bartlomieju wants to merge 1 commit into
mainfrom
oxlint-port-no-new-array
Open

feat(no-new-array): add no-new-array rule#1522
bartlomieju wants to merge 1 commit into
mainfrom
oxlint-port-no-new-array

Conversation

@bartlomieju

Copy link
Copy Markdown
Member

Ports the oxlint unicorn/no-new-array rule to deno_lint as a native Rust
rule.

The rule flags new Array(...) when it is called with exactly one argument,
since in that case it is ambiguous whether the argument is meant to be the
length of the array or its only element. When the argument is the array's
length, Array.from({ length: n }) is clearer; when it is the only element,
an array literal [element] is clearer. Calls with zero or multiple arguments
(e.g. new Array(), new Array(1, 2)) and plain calls without new are left
alone, matching oxc's behavior.

The diagnostic message and hint text, along with the complete pass/fail test
fixtures, are ported verbatim from the oxc reference:
https://github.com/oxc-project/oxc/blob/main/crates/oxc_linter/src/rules/unicorn/no_new_array.rs

Tagged RECOMMENDED (on by default) — flagging for maintainer sign-off.

@bartlomieju

Copy link
Copy Markdown
Member Author

Verdict: needs a spread check verified (likely false positive) — otherwise LGTM. Not merging (new rule + this open question).

Clean port; flags new Array(x) with exactly one argument, correctly leaving new Array(), new Array (no parens), new Array(1, 2), and plain Array(1) alone.

Correctness — one gap to confirm

  • Spread argument isn't excluded. new_expr.args.len() == 1 is true for new Array(...xs) (one ExprOrSpread that happens to be a spread), so it gets flagged. But a spread isn't the "is this the length or the only element?" ambiguity this rule targets — new Array(...xs) is unambiguously spreading elements. ESLint's unicorn/no-new-array excludes SpreadElement for exactly this reason. Please verify against the oxc reference; if oxc also excludes spread (I believe it does), add a guard:
    if args[0].spread.is_some() { return; }
    and a valid-case test new Array(...xs). If oxc intentionally flags spreads, keep it but add the test to document the choice.

Otherwise correct

  • Name check + new-only (Array(1) without new is correctly ignored since it's a CallExpr, not NewExpr). Invalid tests cover number/string/null/Number("1")/parenthesized single args well.
  • Name-based matching means a shadowed local Array would also be flagged — consistent with oxc's name-based approach; fine to leave.

Perf: trivial (per new_expr check).

Housekeeping: RECOMMENDED tagging is the maintainer call you flagged. mergeable: UNKNOWN → rebase; docs .md not in the diff.

Only real item: confirm/handle the spread case (new Array(...xs)) + add a test. Then tagging sign-off + rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant