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: Remove no-extra-semi autofix before potential directives #17297

Merged
merged 1 commit into from Jun 20, 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
2 changes: 2 additions & 0 deletions docs/src/rules/no-extra-semi.md
Expand Up @@ -16,6 +16,8 @@ Typing mistakes and misunderstandings about where semicolons are required can le

This rule disallows unnecessary semicolons.

Problems reported by this rule can be fixed automatically, except when removing a semicolon would cause a following statement to become a directive such as `"use strict"`.

Examples of **incorrect** code for this rule:

::: incorrect
Expand Down
40 changes: 29 additions & 11 deletions lib/rules/no-extra-semi.js
Expand Up @@ -38,6 +38,23 @@ module.exports = {
create(context) {
const sourceCode = context.sourceCode;

/**
* Checks if a node or token is fixable.
* A node is fixable if it can be removed without turning a subsequent statement into a directive after fixing other nodes.
* @param {Token} nodeOrToken The node or token to check.
* @returns {boolean} Whether or not the node is fixable.
*/
function isFixable(nodeOrToken) {
const nextToken = sourceCode.getTokenAfter(nodeOrToken);

if (!nextToken || nextToken.type !== "String") {
return true;
}
const stringNode = sourceCode.getNodeByRangeIndex(nextToken.range[0]);

return !astUtils.isTopLevelExpressionStatement(stringNode.parent);
}

/**
* Reports an unnecessary semicolon error.
* @param {Node|Token} nodeOrToken A node or a token to be reported.
Expand All @@ -47,17 +64,18 @@ module.exports = {
context.report({
node: nodeOrToken,
messageId: "unexpected",
fix(fixer) {

/*
* Expand the replacement range to include the surrounding
* tokens to avoid conflicting with semi.
* https://github.com/eslint/eslint/issues/7928
*/
return new FixTracker(fixer, context.sourceCode)
.retainSurroundingTokens(nodeOrToken)
.remove(nodeOrToken);
}
fix: isFixable(nodeOrToken)
? fixer =>

/*
* Expand the replacement range to include the surrounding
* tokens to avoid conflicting with semi.
* https://github.com/eslint/eslint/issues/7928
*/
new FixTracker(fixer, context.sourceCode)
.retainSurroundingTokens(nodeOrToken)
.remove(nodeOrToken)
: null
});
}

Expand Down
37 changes: 37 additions & 0 deletions tests/lib/rules/no-extra-semi.js
Expand Up @@ -190,6 +190,43 @@ ruleTester.run("no-extra-semi", rule, {
output: "class A { static { a; } foo(){} }",
parserOptions: { ecmaVersion: 2022 },
errors: [{ messageId: "unexpected", type: "Punctuator", column: 24 }]
},

// https://github.com/eslint/eslint/issues/16988
{
code: "; 'use strict'",
output: null,
errors: [{ messageId: "unexpected", type: "EmptyStatement" }]
},
{
code: "; ; 'use strict'",
output: " ; 'use strict'",
errors: [{ messageId: "unexpected", type: "EmptyStatement" }, { messageId: "unexpected", type: "EmptyStatement" }]
},
{
code: "debugger;\n;\n'use strict'",
output: null,
errors: [{ messageId: "unexpected", type: "EmptyStatement", line: 2 }]
},
{
code: "function foo() { ; 'bar'; }",
output: null,
errors: [{ messageId: "unexpected", type: "EmptyStatement" }]
},
{
code: "{ ; 'foo'; }",
output: "{ 'foo'; }",
errors: [{ messageId: "unexpected", type: "EmptyStatement" }]
},
{
code: "; ('use strict');",
output: " ('use strict');",
errors: [{ messageId: "unexpected", type: "EmptyStatement" }]
},
{
code: "; 1;",
output: " 1;",
errors: [{ messageId: "unexpected", type: "EmptyStatement" }]
}
]
});