Skip to content

Commit dcfe573

Browse files
authored
fix: add preceding semicolon in suggestions of no-object-constructor (#17649)
* Add semicolon in suggestions of `no-object-constructor` * Apply suggestions from code review * `output` in test suggestions * Apply suggestions
1 parent 660ed3a commit dcfe573

2 files changed

Lines changed: 360 additions & 6 deletions

File tree

lib/rules/no-object-constructor.js

Lines changed: 103 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,51 @@
99
// Requirements
1010
//------------------------------------------------------------------------------
1111

12-
const { getVariableByName, isArrowToken } = require("./utils/ast-utils");
12+
const { getVariableByName, isArrowToken, isClosingBraceToken, isClosingParenToken } = require("./utils/ast-utils");
1313

1414
//------------------------------------------------------------------------------
1515
// Helpers
1616
//------------------------------------------------------------------------------
1717

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, `>` (for JSX), and 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+
1857
/**
1958
* Tests if a node appears at the beginning of an ancestor ExpressionStatement node.
2059
* @param {ASTNode} node The node to check.
@@ -53,7 +92,8 @@ module.exports = {
5392

5493
messages: {
5594
preferLiteral: "The object literal notation {} is preferable.",
56-
useLiteral: "Replace with '{{replacement}}'."
95+
useLiteral: "Replace with '{{replacement}}'.",
96+
useLiteralAfterSemicolon: "Replace with '{{replacement}}', add preceding semicolon."
5797
}
5898
},
5999

@@ -80,6 +120,50 @@ module.exports = {
80120
return false;
81121
}
82122

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+
83167
/**
84168
* Reports on nodes where the `Object` constructor is called without arguments.
85169
* @param {ASTNode} node The node to evaluate.
@@ -93,16 +177,30 @@ module.exports = {
93177
const variable = getVariableByName(sourceCode.getScope(node), "Object");
94178

95179
if (variable && variable.identifiers.length === 0) {
96-
const replacement = needsParentheses(node) ? "({})" : "{}";
180+
let replacement;
181+
let fixText;
182+
let messageId = "useLiteral";
183+
184+
if (needsParentheses(node)) {
185+
replacement = "({})";
186+
if (needsSemicolon(node)) {
187+
fixText = ";({})";
188+
messageId = "useLiteralAfterSemicolon";
189+
} else {
190+
fixText = "({})";
191+
}
192+
} else {
193+
replacement = fixText = "{}";
194+
}
97195

98196
context.report({
99197
node,
100198
messageId: "preferLiteral",
101199
suggest: [
102200
{
103-
messageId: "useLiteral",
201+
messageId,
104202
data: { replacement },
105-
fix: fixer => fixer.replaceText(node, replacement)
203+
fix: fixer => fixer.replaceText(node, fixText)
106204
}
107205
]
108206
});

0 commit comments

Comments
 (0)