Skip to content

Commit 21ebf8a

Browse files
fasttimefasttime
authored
feat: update no-array-constructor rule (#17711)
* update rule `no-array-constructor` * add comments to conditional statements * add unit tests for edge cases * update JSDoc for `needsPrecedingSemicolon`
1 parent 05d6e99 commit 21ebf8a

7 files changed

Lines changed: 594 additions & 126 deletions

File tree

‎docs/src/_data/rules.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@
659659
"description": "Disallow `Array` constructors",
660660
"recommended": false,
661661
"fixable": false,
662-
"hasSuggestions": false
662+
"hasSuggestions": true
663663
},
664664
{
665665
"name": "no-bitwise",

‎docs/src/_data/rules_meta.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,8 @@
758758
"description": "Disallow `Array` constructors",
759759
"recommended": false,
760760
"url": "https://eslint.org/docs/latest/rules/no-array-constructor"
761-
}
761+
},
762+
"hasSuggestions": true
762763
},
763764
"no-async-promise-executor": {
764765
"type": "problem",

‎docs/src/rules/no-array-constructor.md‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,13 @@ Examples of **incorrect** code for this rule:
2424
```js
2525
/*eslint no-array-constructor: "error"*/
2626

27-
Array(0, 1, 2)
27+
Array();
2828

29-
new Array(0, 1, 2)
29+
Array(0, 1, 2);
30+
31+
new Array(0, 1, 2);
32+
33+
Array(...args);
3034
```
3135

3236
:::
@@ -38,11 +42,13 @@ Examples of **correct** code for this rule:
3842
```js
3943
/*eslint no-array-constructor: "error"*/
4044

41-
Array(500)
45+
Array(500);
46+
47+
new Array(someOtherArray.length);
4248

43-
new Array(someOtherArray.length)
49+
[0, 1, 2];
4450

45-
[0, 1, 2]
51+
const createArray = Array => new Array();
4652
```
4753

4854
:::

‎lib/rules/no-array-constructor.js‎

Lines changed: 85 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,18 @@
55

66
"use strict";
77

8+
//------------------------------------------------------------------------------
9+
// Requirements
10+
//------------------------------------------------------------------------------
11+
12+
const {
13+
getVariableByName,
14+
isClosingParenToken,
15+
isOpeningParenToken,
16+
isStartOfExpressionStatement,
17+
needsPrecedingSemicolon
18+
} = require("./utils/ast-utils");
19+
820
//------------------------------------------------------------------------------
921
// Rule Definition
1022
//------------------------------------------------------------------------------
@@ -20,15 +32,45 @@ module.exports = {
2032
url: "https://eslint.org/docs/latest/rules/no-array-constructor"
2133
},
2234

35+
hasSuggestions: true,
36+
2337
schema: [],
2438

2539
messages: {
26-
preferLiteral: "The array literal notation [] is preferable."
40+
preferLiteral: "The array literal notation [] is preferable.",
41+
useLiteral: "Replace with an array literal.",
42+
useLiteralAfterSemicolon: "Replace with an array literal, add preceding semicolon."
2743
}
2844
},
2945

3046
create(context) {
3147

48+
const sourceCode = context.sourceCode;
49+
50+
/**
51+
* Gets the text between the calling parentheses of a CallExpression or NewExpression.
52+
* @param {ASTNode} node A CallExpression or NewExpression node.
53+
* @returns {string} The text between the calling parentheses, or an empty string if there are none.
54+
*/
55+
function getArgumentsText(node) {
56+
const lastToken = sourceCode.getLastToken(node);
57+
58+
if (!isClosingParenToken(lastToken)) {
59+
return "";
60+
}
61+
62+
let firstToken = node.callee;
63+
64+
do {
65+
firstToken = sourceCode.getTokenAfter(firstToken);
66+
if (!firstToken || firstToken === lastToken) {
67+
return "";
68+
}
69+
} while (!isOpeningParenToken(firstToken));
70+
71+
return sourceCode.text.slice(firstToken.range[1], lastToken.range[0]);
72+
}
73+
3274
/**
3375
* Disallow construction of dense arrays using the Array constructor
3476
* @param {ASTNode} node node to evaluate
@@ -37,11 +79,48 @@ module.exports = {
3779
*/
3880
function check(node) {
3981
if (
40-
node.arguments.length !== 1 &&
41-
node.callee.type === "Identifier" &&
42-
node.callee.name === "Array"
43-
) {
44-
context.report({ node, messageId: "preferLiteral" });
82+
node.callee.type !== "Identifier" ||
83+
node.callee.name !== "Array" ||
84+
node.arguments.length === 1 &&
85+
node.arguments[0].type !== "SpreadElement") {
86+
return;
87+
}
88+
89+
const variable = getVariableByName(sourceCode.getScope(node), "Array");
90+
91+
/*
92+
* Check if `Array` is a predefined global variable: predefined globals have no declarations,
93+
* meaning that the `identifiers` list of the variable object is empty.
94+
*/
95+
if (variable && variable.identifiers.length === 0) {
96+
const argsText = getArgumentsText(node);
97+
let fixText;
98+
let messageId;
99+
100+
/*
101+
* Check if the suggested change should include a preceding semicolon or not.
102+
* Due to JavaScript's ASI rules, a missing semicolon may be inserted automatically
103+
* before an expression like `Array()` or `new Array()`, but not when the expression
104+
* is changed into an array literal like `[]`.
105+
*/
106+
if (isStartOfExpressionStatement(node) && needsPrecedingSemicolon(sourceCode, node)) {
107+
fixText = `;[${argsText}]`;
108+
messageId = "useLiteralAfterSemicolon";
109+
} else {
110+
fixText = `[${argsText}]`;
111+
messageId = "useLiteral";
112+
}
113+
114+
context.report({
115+
node,
116+
messageId: "preferLiteral",
117+
suggest: [
118+
{
119+
messageId,
120+
fix: fixer => fixer.replaceText(node, fixText)
121+
}
122+
]
123+
});
45124
}
46125
}
47126

‎lib/rules/no-object-constructor.js‎

Lines changed: 7 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -9,67 +9,12 @@
99
// Requirements
1010
//------------------------------------------------------------------------------
1111

12-
const { getVariableByName, isArrowToken, isClosingBraceToken, isClosingParenToken } = require("./utils/ast-utils");
13-
14-
//------------------------------------------------------------------------------
15-
// Helpers
16-
//------------------------------------------------------------------------------
17-
18-
const BREAK_OR_CONTINUE = new Set(["BreakStatement", "ContinueStatement"]);
19-
20-
// Declaration types that must contain a string Literal node at the end.
21-
const DECLARATIONS = new Set(["ExportAllDeclaration", "ExportNamedDeclaration", "ImportDeclaration"]);
22-
23-
const IDENTIFIER_OR_KEYWORD = new Set(["Identifier", "Keyword"]);
24-
25-
// Keywords that can immediately precede an ExpressionStatement node, mapped to the their node types.
26-
const NODE_TYPES_BY_KEYWORD = {
27-
__proto__: null,
28-
break: "BreakStatement",
29-
continue: "ContinueStatement",
30-
debugger: "DebuggerStatement",
31-
do: "DoWhileStatement",
32-
else: "IfStatement",
33-
return: "ReturnStatement",
34-
yield: "YieldExpression"
35-
};
36-
37-
/*
38-
* Before an opening parenthesis, postfix `++` and `--` always trigger ASI;
39-
* the tokens `:`, `;`, `{` and `=>` don't expect a semicolon, as that would count as an empty statement.
40-
*/
41-
const PUNCTUATORS = new Set([":", ";", "{", "=>", "++", "--"]);
42-
43-
/*
44-
* Statements that can contain an `ExpressionStatement` after a closing parenthesis.
45-
* DoWhileStatement is an exception in that it always triggers ASI after the closing parenthesis.
46-
*/
47-
const STATEMENTS = new Set([
48-
"DoWhileStatement",
49-
"ForInStatement",
50-
"ForOfStatement",
51-
"ForStatement",
52-
"IfStatement",
53-
"WhileStatement",
54-
"WithStatement"
55-
]);
56-
57-
/**
58-
* Tests if a node appears at the beginning of an ancestor ExpressionStatement node.
59-
* @param {ASTNode} node The node to check.
60-
* @returns {boolean} Whether the node appears at the beginning of an ancestor ExpressionStatement node.
61-
*/
62-
function isStartOfExpressionStatement(node) {
63-
const start = node.range[0];
64-
let ancestor = node;
65-
66-
while ((ancestor = ancestor.parent) && ancestor.range[0] === start) {
67-
if (ancestor.type === "ExpressionStatement") {
68-
return true;
69-
}
70-
}
71-
return false;
72-
}
12+
const {
13+
getVariableByName,
14+
isArrowToken,
15+
isStartOfExpressionStatement,
16+
needsPrecedingSemicolon
17+
} = require("./utils/ast-utils");
7318

7419
//------------------------------------------------------------------------------
7520
// Rule Definition
@@ -120,50 +65,6 @@ module.exports = {
12065
return false;
12166
}
12267

123-
/**
124-
* Determines whether a parenthesized object literal that replaces a specified node needs to be preceded by a semicolon.
125-
* @param {ASTNode} node The node to be replaced. This node should be at the start of an `ExpressionStatement` or at the start of the body of an `ArrowFunctionExpression`.
126-
* @returns {boolean} Whether a semicolon is required before the parenthesized object literal.
127-
*/
128-
function needsSemicolon(node) {
129-
const prevToken = sourceCode.getTokenBefore(node);
130-
131-
if (!prevToken || prevToken.type === "Punctuator" && PUNCTUATORS.has(prevToken.value)) {
132-
return false;
133-
}
134-
135-
const prevNode = sourceCode.getNodeByRangeIndex(prevToken.range[0]);
136-
137-
if (isClosingParenToken(prevToken)) {
138-
return !STATEMENTS.has(prevNode.type);
139-
}
140-
141-
if (isClosingBraceToken(prevToken)) {
142-
return (
143-
prevNode.type === "BlockStatement" && prevNode.parent.type === "FunctionExpression" ||
144-
prevNode.type === "ClassBody" && prevNode.parent.type === "ClassExpression" ||
145-
prevNode.type === "ObjectExpression"
146-
);
147-
}
148-
149-
if (IDENTIFIER_OR_KEYWORD.has(prevToken.type)) {
150-
if (BREAK_OR_CONTINUE.has(prevNode.parent.type)) {
151-
return false;
152-
}
153-
154-
const keyword = prevToken.value;
155-
const nodeType = NODE_TYPES_BY_KEYWORD[keyword];
156-
157-
return prevNode.type !== nodeType;
158-
}
159-
160-
if (prevToken.type === "String") {
161-
return !DECLARATIONS.has(prevNode.parent.type);
162-
}
163-
164-
return true;
165-
}
166-
16768
/**
16869
* Reports on nodes where the `Object` constructor is called without arguments.
16970
* @param {ASTNode} node The node to evaluate.
@@ -183,7 +84,7 @@ module.exports = {
18384

18485
if (needsParentheses(node)) {
18586
replacement = "({})";
186-
if (needsSemicolon(node)) {
87+
if (needsPrecedingSemicolon(sourceCode, node)) {
18788
fixText = ";({})";
18889
messageId = "useLiteralAfterSemicolon";
18990
} else {

0 commit comments

Comments
 (0)