Skip to content

feat(linter): add suggestion for unicorn/no-new-array#22682

Merged
graphite-app[bot] merged 1 commit into
mainfrom
05-23-feat_linter_add_suggestion_for_unicorn_no-new-array_
May 26, 2026
Merged

feat(linter): add suggestion for unicorn/no-new-array#22682
graphite-app[bot] merged 1 commit into
mainfrom
05-23-feat_linter_add_suggestion_for_unicorn_no-new-array_

Conversation

@Sysix

@Sysix Sysix commented May 22, 2026

Copy link
Copy Markdown
Member

The implementation of the upstream rule is difficult to implement.

At first, it looks for a numeric value, this part can be migrated to rust. But terminating if one or two suggestions should be disabled, is depending on the static (resolved) value.

https://github.com/eslint-community/eslint-utils/blob/main/src/get-static-value.mjs

Instead, we just provide both suggestions and mark both as dangerous.

`isNumber` migrated rust code, which is not used anymore
const MATH_METHODS: &[&str] = &[
    "abs",
    "acos",
    "acosh",
    "asin",
    "asinh",
    "atan",
    "atan2",
    "atanh",
    "cbrt",
    "ceil",
    "clz32",
    "cos",
    "cosh",
    "exp",
    "expm1",
    "floor",
    "f16round",
    "fround",
    "hypot",
    "imul",
    "log",
    "log10",
    "log1p",
    "log2",
    "max",
    "min",
    "pow",
    "random",
    "round",
    "sign",
    "sin",
    "sinh",
    "sqrt",
    "sumPrecise",
    "tan",
    "tanh",
    "trunc",
];

const NUMBER_PROPERTIES: &[&str] = &[
    "EPSILON",
    "MAX_SAFE_INTEGER",
    "MAX_VALUE",
    "MIN_SAFE_INTEGER",
    "MIN_VALUE",
    "NaN",
    "NEGATIVE_INFINITY",
    "POSITIVE_INFINITY",
];

const MATH_PROPERTIES: &[&str] = &["E", "LN10", "LN2", "LOG10E", "LOG2E", "PI", "SQRT1_2", "SQRT2"];


fn is_number_expression(expression: &Expression, ctx: &LintContext) -> bool {
    if is_known_number_property(expression) {
        return true;
    }
    if is_known_number_return_value(expression) {
        return true;
    }
    if is_length_property(expression) {
        return true;
    }
    match expression {
        Expression::NumericLiteral(_) | Expression::UpdateExpression(_) => true,
        // Expression::UpdateExpression(update) => is_number_expression(&update.argument),
        Expression::UnaryExpression(unary) => {
            // `+` can't use on `BigInt`
            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt#operators
            if matches!(unary.operator, UnaryOperator::UnaryPlus) {
                return true;
            }

            if !matches!(unary.operator, UnaryOperator::UnaryNegation | UnaryOperator::BitwiseNot) {
                return false;
            }

            is_number_expression(&unary.argument, ctx)
        }
        Expression::AssignmentExpression(assign) => {
            if assign.operator == AssignmentOperator::Assign {
                return is_number_expression(&assign.right, ctx);
            }

            // `>>>=` (zero-fill right shift) can't use on `BigInt`
            if assign.operator == AssignmentOperator::ShiftRightZeroFill {
                return true;
            }

            if assign.operator == AssignmentOperator::Addition {
                let Some(left) = assign.left.get_expression() else {
                    return false;
                };
                // `+=` can be used for both string concatenation and numeric addition, so we need
                return is_number_expression(&assign.right, ctx) && is_number_expression(left, ctx);
            }

            // `a * b` can be `BigInt`, we need make sure at least one side is number
            if !matches!(
                assign.operator,
                AssignmentOperator::Subtraction
                    | AssignmentOperator::Multiplication
                    | AssignmentOperator::Division
                    | AssignmentOperator::Remainder
                    | AssignmentOperator::Exponential
                    | AssignmentOperator::ShiftLeft
                    | AssignmentOperator::ShiftRight
                    | AssignmentOperator::BitwiseOR
                    | AssignmentOperator::BitwiseXOR
                    | AssignmentOperator::BitwiseAnd
            ) {
                return false;
            }

            if is_number_expression(&assign.right, ctx) {
                return true;
            }
            let Some(left) = assign.left.get_expression() else {
                return false;
            };
            is_number_expression(left, ctx)
        }
        Expression::BinaryExpression(binary) => {
            // `>>>` (zero-fill right shift) can't use on `BigInt`
            if binary.operator == BinaryOperator::ShiftRightZeroFill {
                return true;
            }

            if binary.operator == BinaryOperator::Addition {
                // `+` can be used for both string concatenation and numeric addition, so we need to check the operands
                return is_number_expression(&binary.left, ctx)
                    && is_number_expression(&binary.right, ctx);
            }

            // `a * b` can be `BigInt`, we need make sure at least one side is number
            matches!(
                binary.operator,
                BinaryOperator::Subtraction
                    | BinaryOperator::Multiplication
                    | BinaryOperator::Division
                    | BinaryOperator::Remainder
                    | BinaryOperator::Exponential
                    | BinaryOperator::ShiftLeft
                    | BinaryOperator::ShiftRight
                    | BinaryOperator::BitwiseOR
                    | BinaryOperator::BitwiseXOR
                    | BinaryOperator::BitwiseAnd
            ) && (is_number_expression(&binary.left, ctx)
                || is_number_expression(&binary.right, ctx))
        }
        Expression::ConditionalExpression(conditional) => {
            is_number_expression(&conditional.consequent, ctx)
                && is_number_expression(&conditional.alternate, ctx)
        }
        // the last expression of a sequence expression is the one that determines the value of the whole expression
        // example: `new Array((2, (window, 3, 4, 6)))` is evaluated to `Array.from({ length: 6})`
        Expression::SequenceExpression(sequence) => {
            if let Some(last_expr) = sequence.expressions.last() {
                is_number_expression(last_expr, ctx)
            } else {
                false
            }
        }
        Expression::Identifier(ident) => {
            if ident.name == "NaN" {
                return true;
            }
            let Some(reference_id) = ident.reference_id.get() else {
                return false;
            };
            let reference = ctx.scoping().get_reference(reference_id);
            let Some(symbol_id) = reference.symbol_id() else {
                return false;
            };
            let variable = ctx.scoping().symbol_declaration(symbol_id);
            let node = ctx.nodes().get_node(variable);

            match node.kind() {
                AstKind::VariableDeclarator(variable) => {
                    if let Some(init) = &variable.init {
                        is_number_expression(init, ctx)
                    } else {
                        false
                    }
                }
                _ => false,
            }
        }
        _ => false,
    }
}

fn is_length_property(expression: &Expression) -> bool {
    if let Expression::StaticMemberExpression(member) = expression {
        return member.property.name == "length";
    }
    false
}

fn is_known_number_return_value(expression: &Expression) -> bool {
    let Expression::CallExpression(call) = expression else {
        return false;
    };
    match call.callee.get_inner_expression() {
        Expression::Identifier(ident) => {
            ident.name == "Number" || ident.name == "parseInt" || ident.name == "parseFloat"
        }
        Expression::StaticMemberExpression(member) => {
            let Expression::Identifier(ident_obj) = &member.object else {
                return false;
            };

            match ident_obj.name.as_str() {
                "Number"
                    if member.property.name == "parseInt"
                        || member.property.name == "parseFloat" =>
                {
                    true
                }
                "Math" if MATH_METHODS.contains(&member.property.name.as_str()) => true,
                _ => false,
            }
        }
        _ => false,
    }
}

fn is_known_number_property(expression: &Expression) -> bool {
    let Expression::StaticMemberExpression(member) = expression else {
        return false;
    };
    let Expression::Identifier(ident_obj) = &member.object else {
        return false;
    };
    match ident_obj.name.as_str() {
        "Number" if NUMBER_PROPERTIES.contains(&member.property.name.as_str()) => true,
        "Math" if MATH_PROPERTIES.contains(&member.property.name.as_str()) => true,
        _ => false,
    }
}

@github-actions github-actions Bot added the A-linter Area - Linter label May 22, 2026

Sysix commented May 22, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codspeed-hq

codspeed-hq Bot commented May 22, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 5 untouched benchmarks
⏩ 110 skipped benchmarks1


Comparing 05-23-feat_linter_add_suggestion_for_unicorn_no-new-array_ (e7d65e3) with main (5774052)

Open in CodSpeed

Footnotes

  1. 110 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@Sysix Sysix force-pushed the 05-23-feat_linter_add_suggestion_for_unicorn_no-new-array_ branch 2 times, most recently from b7a405b to cae9543 Compare May 23, 2026 12:38
@camc314 camc314 self-assigned this May 24, 2026
@Sysix Sysix force-pushed the 05-23-feat_linter_add_suggestion_for_unicorn_no-new-array_ branch 5 times, most recently from 2567517 to e7d65e3 Compare May 25, 2026 16:05
@Sysix Sysix requested a review from Copilot May 25, 2026 16:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enhances the unicorn/no-new-array lint rule by adding dangerous suggestions that offer two alternative fixes for new Array(singleArgument), without attempting full static-value resolution (unlike the upstream ESLint implementation).

Changes:

  • Promote the rule from pending to dangerous_suggestion and emit diagnostics with multiple possible fixes.
  • Add two dangerous suggestions: replace with Array.from({ length: argument }) or with [argument] (including ASI-hazard handling for the [ form).
  • Expand fixer test coverage to validate the multiple-fix behavior across many argument shapes (including spread).

Comment thread crates/oxc_linter/src/rules/unicorn/no_new_array.rs
Comment thread crates/oxc_linter/src/rules/unicorn/no_new_array.rs
Comment thread crates/oxc_linter/src/rules/unicorn/no_new_array.rs
@Sysix Sysix marked this pull request as ready for review May 25, 2026 16:16
@Sysix Sysix requested a review from camc314 as a code owner May 25, 2026 16:16
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label May 26, 2026

camc314 commented May 26, 2026

Copy link
Copy Markdown
Contributor

Merge activity

The implementation of the upstream rule is difficult to implement.

At first, it looks for a numeric value, this part can be migrated to rust. But terminating if one or two suggestions should be disabled, is depending on the static (resolved) value.

https://github.com/eslint-community/eslint-utils/blob/main/src/get-static-value.mjs

Instead, we just provide both suggestions and mark both as dangerous.

<details>
<summary>`isNumber` migrated rust code, which is not used anymore</summary>

```rust

const MATH_METHODS: &[&str] = &[
    "abs",
    "acos",
    "acosh",
    "asin",
    "asinh",
    "atan",
    "atan2",
    "atanh",
    "cbrt",
    "ceil",
    "clz32",
    "cos",
    "cosh",
    "exp",
    "expm1",
    "floor",
    "f16round",
    "fround",
    "hypot",
    "imul",
    "log",
    "log10",
    "log1p",
    "log2",
    "max",
    "min",
    "pow",
    "random",
    "round",
    "sign",
    "sin",
    "sinh",
    "sqrt",
    "sumPrecise",
    "tan",
    "tanh",
    "trunc",
];

const NUMBER_PROPERTIES: &[&str] = &[
    "EPSILON",
    "MAX_SAFE_INTEGER",
    "MAX_VALUE",
    "MIN_SAFE_INTEGER",
    "MIN_VALUE",
    "NaN",
    "NEGATIVE_INFINITY",
    "POSITIVE_INFINITY",
];

const MATH_PROPERTIES: &[&str] = &["E", "LN10", "LN2", "LOG10E", "LOG2E", "PI", "SQRT1_2", "SQRT2"];

fn is_number_expression(expression: &Expression, ctx: &LintContext) -> bool {
    if is_known_number_property(expression) {
        return true;
    }
    if is_known_number_return_value(expression) {
        return true;
    }
    if is_length_property(expression) {
        return true;
    }
    match expression {
        Expression::NumericLiteral(_) | Expression::UpdateExpression(_) => true,
        // Expression::UpdateExpression(update) => is_number_expression(&update.argument),
        Expression::UnaryExpression(unary) => {
            // `+` can't use on `BigInt`
            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt#operators
            if matches!(unary.operator, UnaryOperator::UnaryPlus) {
                return true;
            }

            if !matches!(unary.operator, UnaryOperator::UnaryNegation | UnaryOperator::BitwiseNot) {
                return false;
            }

            is_number_expression(&unary.argument, ctx)
        }
        Expression::AssignmentExpression(assign) => {
            if assign.operator == AssignmentOperator::Assign {
                return is_number_expression(&assign.right, ctx);
            }

            // `>>>=` (zero-fill right shift) can't use on `BigInt`
            if assign.operator == AssignmentOperator::ShiftRightZeroFill {
                return true;
            }

            if assign.operator == AssignmentOperator::Addition {
                let Some(left) = assign.left.get_expression() else {
                    return false;
                };
                // `+=` can be used for both string concatenation and numeric addition, so we need
                return is_number_expression(&assign.right, ctx) && is_number_expression(left, ctx);
            }

            // `a * b` can be `BigInt`, we need make sure at least one side is number
            if !matches!(
                assign.operator,
                AssignmentOperator::Subtraction
                    | AssignmentOperator::Multiplication
                    | AssignmentOperator::Division
                    | AssignmentOperator::Remainder
                    | AssignmentOperator::Exponential
                    | AssignmentOperator::ShiftLeft
                    | AssignmentOperator::ShiftRight
                    | AssignmentOperator::BitwiseOR
                    | AssignmentOperator::BitwiseXOR
                    | AssignmentOperator::BitwiseAnd
            ) {
                return false;
            }

            if is_number_expression(&assign.right, ctx) {
                return true;
            }
            let Some(left) = assign.left.get_expression() else {
                return false;
            };
            is_number_expression(left, ctx)
        }
        Expression::BinaryExpression(binary) => {
            // `>>>` (zero-fill right shift) can't use on `BigInt`
            if binary.operator == BinaryOperator::ShiftRightZeroFill {
                return true;
            }

            if binary.operator == BinaryOperator::Addition {
                // `+` can be used for both string concatenation and numeric addition, so we need to check the operands
                return is_number_expression(&binary.left, ctx)
                    && is_number_expression(&binary.right, ctx);
            }

            // `a * b` can be `BigInt`, we need make sure at least one side is number
            matches!(
                binary.operator,
                BinaryOperator::Subtraction
                    | BinaryOperator::Multiplication
                    | BinaryOperator::Division
                    | BinaryOperator::Remainder
                    | BinaryOperator::Exponential
                    | BinaryOperator::ShiftLeft
                    | BinaryOperator::ShiftRight
                    | BinaryOperator::BitwiseOR
                    | BinaryOperator::BitwiseXOR
                    | BinaryOperator::BitwiseAnd
            ) && (is_number_expression(&binary.left, ctx)
                || is_number_expression(&binary.right, ctx))
        }
        Expression::ConditionalExpression(conditional) => {
            is_number_expression(&conditional.consequent, ctx)
                && is_number_expression(&conditional.alternate, ctx)
        }
        // the last expression of a sequence expression is the one that determines the value of the whole expression
        // example: `new Array((2, (window, 3, 4, 6)))` is evaluated to `Array.from({ length: 6})`
        Expression::SequenceExpression(sequence) => {
            if let Some(last_expr) = sequence.expressions.last() {
                is_number_expression(last_expr, ctx)
            } else {
                false
            }
        }
        Expression::Identifier(ident) => {
            if ident.name == "NaN" {
                return true;
            }
            let Some(reference_id) = ident.reference_id.get() else {
                return false;
            };
            let reference = ctx.scoping().get_reference(reference_id);
            let Some(symbol_id) = reference.symbol_id() else {
                return false;
            };
            let variable = ctx.scoping().symbol_declaration(symbol_id);
            let node = ctx.nodes().get_node(variable);

            match node.kind() {
                AstKind::VariableDeclarator(variable) => {
                    if let Some(init) = &variable.init {
                        is_number_expression(init, ctx)
                    } else {
                        false
                    }
                }
                _ => false,
            }
        }
        _ => false,
    }
}

fn is_length_property(expression: &Expression) -> bool {
    if let Expression::StaticMemberExpression(member) = expression {
        return member.property.name == "length";
    }
    false
}

fn is_known_number_return_value(expression: &Expression) -> bool {
    let Expression::CallExpression(call) = expression else {
        return false;
    };
    match call.callee.get_inner_expression() {
        Expression::Identifier(ident) => {
            ident.name == "Number" || ident.name == "parseInt" || ident.name == "parseFloat"
        }
        Expression::StaticMemberExpression(member) => {
            let Expression::Identifier(ident_obj) = &member.object else {
                return false;
            };

            match ident_obj.name.as_str() {
                "Number"
                    if member.property.name == "parseInt"
                        || member.property.name == "parseFloat" =>
                {
                    true
                }
                "Math" if MATH_METHODS.contains(&member.property.name.as_str()) => true,
                _ => false,
            }
        }
        _ => false,
    }
}

fn is_known_number_property(expression: &Expression) -> bool {
    let Expression::StaticMemberExpression(member) = expression else {
        return false;
    };
    let Expression::Identifier(ident_obj) = &member.object else {
        return false;
    };
    match ident_obj.name.as_str() {
        "Number" if NUMBER_PROPERTIES.contains(&member.property.name.as_str()) => true,
        "Math" if MATH_PROPERTIES.contains(&member.property.name.as_str()) => true,
        _ => false,
    }
}
```

</details>
@graphite-app graphite-app Bot force-pushed the 05-23-feat_linter_add_suggestion_for_unicorn_no-new-array_ branch from e7d65e3 to 1a7798b Compare May 26, 2026 08:25
@graphite-app graphite-app Bot merged commit 1a7798b into main May 26, 2026
29 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 26, 2026
@graphite-app graphite-app Bot deleted the 05-23-feat_linter_add_suggestion_for_unicorn_no-new-array_ branch May 26, 2026 08:30
camc314 pushed a commit that referenced this pull request Jun 1, 2026
# Oxlint
### 🚀 Features

- e4b1f46 linter/typescript: Implement `method-signature-style` rule
(#22679) (Mikhail Baev)
- bc462ca linter/vue: Implement no-reserved-component-names rule
(#22741) (bab)
- ef9e751 linter/vue: Implement component-definition-name-casing rule
(#22818) (bab)
- d67f51a linter/vue: Implement require-prop-type-constructor rule
(#22708) (bab)
- 1444f82 linter/promise/spec-only: Add `Promise.try` to `Promise`
static methods (#22812) (Ben Saufley)
- 8422e8b linter/jsdoc: Implement `require-yields-description` rule
(#22805) (Mikhail Baev)
- fe93f97 linter/eslint: Implement `prefer-named-capture-group` rule
(#22759) (Sebastian Poxhofer)
- 1a7798b linter: Add suggestion for `unicorn/no-new-array` (#22682)
(Sysix)

### 🐛 Bug Fixes

- 760a9f9 linter: Report errors when writing to the filesystem (#22881)
(camc314)
- e5a2748 linter: Avoid no-unreachable false positive after conditional
loop (#22869) (camc314)
- 39d92d6 linter/arrow-body-style: Preserve comments within function
(#22854) (Sysix)
- 3d13e29 parser: Reject `declare` in an already-ambient context
(TS1038) (#22850) (Boshen)
- 5152854 parser: Reject statements in ambient contexts (TS1036)
(#22849) (Boshen)
- 2eafea6 parser: Reject function implementations in ambient contexts
(TS1183) (#22845) (Boshen)
- c645615 parser: Reject incompatible class member modifiers (#22843)
(Boshen)
- 4a1ca4a linter/export: Detect duplicate explicit exports (#22798)
(camc314)
- 0a9a735 linter/no-loop-func: Allow safe let closures (#22811)
(camc314)
- 1599f11 linter: Align lsp extends default plugins (#22788) (camc314)
- db32ec9 linter/no-accumulating-spread: Use loop as primary span
(#22800) (camc314)
- 33ec6b4 linter/consistent-test-it: Avoid adjacent describe leakage
(#22796) (camc314)
- 2606069 linter/no-array-sort: Unwrap parenthesized sort args (#22794)
(camc314)
- 9f2f709 linter/no-array-sort: Skip non compare fn sort arguments
(#22752) (Gaurav Dubey)
- 27268a0 linter/no-else-return: Preserve statement boundary in fixer
(#22687) (camc314)
- d9cb6d8 linter/no-empty-function: Allow functions callbacks with
`allow: functions` (#22764) (camc314)
- a40a314 linter/no-shadow-restricted-names: Ignore enum members
(#22762) (camc314)
- 82366d9 linter/no-cond-assign: Align ternary handling (#22761)
(camc314)

### 📚 Documentation

- 5e113ba linter: Add license notices for ported ESLint plugins (#22768)
(Boshen)
# Oxfmt
### 🚀 Features

- d75cbbf oxfmt: Format `parser:json` files by `oxc_formatter_json`
(#22709) (leaysgur)
- 49db054 formatter_json: Implement `oxc_formatter_json` (json variant
only) (#22641) (leaysgur)
- 9c71f2e ast, codegen, formatter: Add `WithClauseKeyword::as_str`
helper and use it (#22791) (camc314)

### 🐛 Bug Fixes

- d3cdd62 oxfmt: Skip formatting for whitespace-only file (#22780)
(leaysgur)
- 23f0cc8 formatter: Don't move comments inside variable declaration in
for in loop (#22776) (leaysgur)
- f200c40 formatter: Don't move comments inside variable declaration in
for of loop (#22773) (Leonabcd123)

### 📚 Documentation

- 845f393 oxfmt,formatter,formatter_json,formatter_core: Add/update
AGENTS.md (#22873) (leaysgur)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants