Skip to content

Conversation

fasttime
Copy link
Member

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[X] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

Tell us about your environment (npx eslint --env-info):

Node version: v24.2.0
npm version: v11.3.0
Local ESLint version: v9.24.0 - v9.29.0
Global ESLint version:
Operating System: darwin 24.5.0

What parser are you using (place an "X" next to just one item)?

[X] Default (Espree)
[ ] @typescript-eslint/parser
[ ] @babel/eslint-parser
[ ] vue-eslint-parser
[ ] @angular-eslint/template-parser
[ ] Other

Please show your full configuration:

Configuration
export default [
    {
        "rules": {
            "no-array-constructor": [
                "error"
            ]
        }
    }
];

What did you do? Please include the actual source code causing the issue.

let foo
Array(1, 2, 3).forEach(doSomething)

Playground Link

What did you expect to happen?

The autofix for the above code should not introduce a semicolon between the let declaration and the following statement, i.e. the fixed code should look like this:

let foo
[1, 2, 3].forEach(doSomething)

What actually happened? Please include the actual, raw output from ESLint.

The autofixed code includes an additional semicolon:

let foo
;[1, 2, 3].forEach(doSomething)

What changes did you make? (Give an overview)

This pull requests affects autofixes and suggestions provided by no-array-constructor and no-object-constructor. In some cases, mostly related to TypeScript syntax, the rules would previously insert an unnecessary semicolon before the fixed code. The logic to determine if a semicolon is necessary resides in the needsPrecedingSemicolon function in lib/rules/utils/ast-utils.js. This function was updated to handle cases where an expression statement is predeced by:

  1. an uninitialized variable declaration
  2. TypeScript syntax that does not require a following semicolon

This change affects autofixes and suggestions provided by no-array-constructor and no-object-constructor.

These are some examples of code where the mentioned rules will no longer add a semicolon before an array or object literal that replaces new Array() or new Object():

/* eslint no-object-constructor: "error" */

var a
new Object()

/* eslint no-array-constructor: "error" */

let foo
new Array(1, 2)

// The next examples require the typescript-eslint parser

type xtring = string
Array()

declare function test(): void
new Array("A", "B")

const bar = Math.sign(Math.random() - .5) as -1 | 0 | 1
new Array(0, 1)

Playground Link

More examples for code that will no longer have a semicolon added before the next statement can be found in the unit tests for no-array-constructor:

"type T = Foo",
"type T = Foo<Bar>",
"type T = (A | B)",
"type T = -1",
"type T = 'foo'",
"const foo",
"declare const foo",
"function foo()",
"declare function foo()",
"function foo(): []",
"declare function foo(): []",
"function foo(): (Foo)",
"declare function foo(): (Foo)",
"let foo: bar",
"import Foo = require('foo')",
"import Foo = Bar",
"import Foo = Bar.Baz.Qux",

Is there anything you'd like reviewers to focus on?

@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Jun 16, 2025
@eslint-github-bot eslint-github-bot bot added the bug ESLint is working incorrectly label Jun 16, 2025
@github-actions github-actions bot added the rule Relates to ESLint's core rules label Jun 16, 2025
Copy link

netlify bot commented Jun 16, 2025

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit 0d3d4bd
🔍 Latest deploy log https://app.netlify.com/projects/docs-eslint/deploys/684fc1aa60e37700083e3310

@fasttime fasttime marked this pull request as ready for review June 16, 2025 07:14
@fasttime fasttime requested a review from a team as a code owner June 16, 2025 07:14
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Would like another review before merging.

@nzakas nzakas moved this from Needs Triage to Second Review Needed in Triage Jun 16, 2025
@lumirlumir lumirlumir added accepted There is consensus among the team that this change meets the criteria for inclusion labels Jun 17, 2025
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mdjermanovic mdjermanovic merged commit 7ef4cf7 into main Jun 17, 2025
30 checks passed
@mdjermanovic mdjermanovic deleted the fix-unncessary-semi branch June 17, 2025 19:58
@github-project-automation github-project-automation bot moved this from Second Review Needed to Complete in Triage Jun 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly rule Relates to ESLint's core rules
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

4 participants