Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add preceding semicolon in suggestions of no-object-constructor #17649

Merged
merged 4 commits into from Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
108 changes: 103 additions & 5 deletions lib/rules/no-object-constructor.js
Expand Up @@ -9,12 +9,51 @@
// Requirements
//------------------------------------------------------------------------------

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

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

const BREAK_OR_CONTINUE = new Set(["BreakStatement", "ContinueStatement"]);

// Declaration types that must contain a string Literal node at the end.
const DECLARATIONS = new Set(["ExportAllDeclaration", "ExportNamedDeclaration", "ImportDeclaration"]);

const IDENTIFIER_OR_KEYWORD = new Set(["Identifier", "Keyword"]);

// Keywords that can immediately precede an ExpressionStatement node, mapped to the their node types.
const NODE_TYPES_BY_KEYWORD = {
__proto__: null,
break: "BreakStatement",
continue: "ContinueStatement",
debugger: "DebuggerStatement",
do: "DoWhileStatement",
else: "IfStatement",
return: "ReturnStatement",
yield: "YieldExpression"
};

/*
* Before an opening parenthesis, `>` (for JSX), and postfix `++` and `--` always trigger ASI;
* the tokens `:`, `;`, `{` and `=>` don't expect a semicolon, as that would count as an empty statement.
*/
const PUNCTUATORS = new Set([":", ";", ">", "{", "=>", "++", "--"]);

/*
* Statements that can contain an `ExpressionStatement` after a closing parenthesis.
* DoWhileStatement is an exception in that it always triggers ASI after the closing parenthesis.
*/
const STATEMENTS = new Set([
"DoWhileStatement",
"ForInStatement",
"ForOfStatement",
"ForStatement",
"IfStatement",
"WhileStatement",
"WithStatement"
]);

/**
* Tests if a node appears at the beginning of an ancestor ExpressionStatement node.
* @param {ASTNode} node The node to check.
Expand Down Expand Up @@ -53,7 +92,8 @@ module.exports = {

messages: {
preferLiteral: "The object literal notation {} is preferable.",
useLiteral: "Replace with '{{replacement}}'."
useLiteral: "Replace with '{{replacement}}'.",
useLiteralAfterSemicolon: "Replace with '{{replacement}}', add preceding semicolon."
}
},

Expand All @@ -80,6 +120,50 @@ module.exports = {
return false;
}

/**
* Determines whether a parenthesized object literal that replaces a specified node needs to be preceded by a semicolon.
* @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`.
* @returns {boolean} Whether a semicolon is required before the parenthesized object literal.
*/
function needsSemicolon(node) {
const prevToken = sourceCode.getTokenBefore(node);

if (!prevToken || prevToken.type === "Punctuator" && PUNCTUATORS.has(prevToken.value)) {
return false;
}

const prevNode = sourceCode.getNodeByRangeIndex(prevToken.range[0]);

if (isClosingParenToken(prevToken)) {
return !STATEMENTS.has(prevNode.type);
}

if (isClosingBraceToken(prevToken)) {
return (
prevNode.type === "BlockStatement" && prevNode.parent.type === "FunctionExpression" ||
prevNode.type === "ClassBody" && prevNode.parent.type === "ClassExpression" ||
prevNode.type === "ObjectExpression"
);
}

if (IDENTIFIER_OR_KEYWORD.has(prevToken.type)) {
if (BREAK_OR_CONTINUE.has(prevNode.parent.type)) {
return false;
}

const keyword = prevToken.value;
const nodeType = NODE_TYPES_BY_KEYWORD[keyword];

return prevNode.type !== nodeType;
}

if (prevToken.type === "String") {
return !DECLARATIONS.has(prevNode.parent.type);
}

return true;
}

/**
* Reports on nodes where the `Object` constructor is called without arguments.
* @param {ASTNode} node The node to evaluate.
Expand All @@ -93,16 +177,30 @@ module.exports = {
const variable = getVariableByName(sourceCode.getScope(node), "Object");

if (variable && variable.identifiers.length === 0) {
const replacement = needsParentheses(node) ? "({})" : "{}";
let replacement;
let fixText;
let messageId = "useLiteral";

if (needsParentheses(node)) {
replacement = "({})";
if (needsSemicolon(node)) {
fixText = ";({})";
messageId = "useLiteralAfterSemicolon";
} else {
fixText = "({})";
}
} else {
replacement = fixText = "{}";
}

context.report({
node,
messageId: "preferLiteral",
suggest: [
{
messageId: "useLiteral",
messageId,
data: { replacement },
fix: fixer => fixer.replaceText(node, replacement)
fix: fixer => fixer.replaceText(node, fixText)
}
]
});
Expand Down