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-unused-labels autofix before potential directives #17314

Merged
merged 1 commit into from
Jun 28, 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-unused-labels.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ Such labels take up space in the code and can lead to confusion by readers.

This rule is aimed at eliminating unused labels.

Problems reported by this rule can be fixed automatically, except when there are any comments between the label and the following statement, or when removing a label would cause the following statement to become a directive such as `"use strict"`.

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

::: incorrect
Expand Down
3 changes: 1 addition & 2 deletions lib/rules/dot-notation.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ module.exports = {
}
if (
node.computed &&
node.property.type === "TemplateLiteral" &&
node.property.expressions.length === 0
astUtils.isStaticTemplateLiteral(node.property)
) {
checkComputedProperty(node, node.property.quasis[0].value.cooked);
}
Expand Down
17 changes: 7 additions & 10 deletions lib/rules/no-restricted-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@
*/
"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -116,15 +122,6 @@ module.exports = {
return node && node.type === "Literal" && typeof node.value === "string";
}

/**
* Function to check if a node is a static string template literal.
* @param {ASTNode} node The node to check.
* @returns {boolean} If the node is a string template literal.
*/
function isStaticTemplateLiteral(node) {
return node && node.type === "TemplateLiteral" && node.expressions.length === 0;
}

/**
* Function to check if a node is a require call.
* @param {ASTNode} node The node to check.
Expand All @@ -144,7 +141,7 @@ module.exports = {
return node.value.trim();
}

if (isStaticTemplateLiteral(node)) {
if (astUtils.isStaticTemplateLiteral(node)) {
return node.quasis[0].value.cooked.trim();
}

Expand Down
59 changes: 46 additions & 13 deletions lib/rules/no-unused-labels.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -46,6 +52,45 @@ module.exports = {
};
}

/**
* Checks if a `LabeledStatement` node is fixable.
* For a node to be fixable, there must be no comments between the label and the body.
* Furthermore, is must be possible to remove the label without turning the body statement into a
* directive after other fixes are applied.
* @param {ASTNode} node The node to evaluate.
* @returns {boolean} Whether or not the node is fixable.
*/
function isFixable(node) {

/*
* Only perform a fix if there are no comments between the label and the body. This will be the case
* when there is exactly one token/comment (the ":") between the label and the body.
*/
if (sourceCode.getTokenAfter(node.label, { includeComments: true }) !==
sourceCode.getTokenBefore(node.body, { includeComments: true })) {
return false;
}

// Looking for the node's deepest ancestor which is not a `LabeledStatement`.
let ancestor = node.parent;

while (ancestor.type === "LabeledStatement") {
ancestor = ancestor.parent;
}

if (ancestor.type === "Program" ||
(ancestor.type === "BlockStatement" && astUtils.isFunction(ancestor.parent))) {
const { body } = node;

if (body.type === "ExpressionStatement" &&
((body.expression.type === "Literal" && typeof body.expression.value === "string") ||
astUtils.isStaticTemplateLiteral(body.expression))) {
return false; // potential directive
}
}
return true;
}

/**
* Removes the top of the stack.
* At the same time, this reports the label if it's never used.
Expand All @@ -58,19 +103,7 @@ module.exports = {
node: node.label,
messageId: "unused",
data: node.label,
fix(fixer) {

/*
* Only perform a fix if there are no comments between the label and the body. This will be the case
* when there is exactly one token/comment (the ":") between the label and the body.
*/
if (sourceCode.getTokenAfter(node.label, { includeComments: true }) ===
sourceCode.getTokenBefore(node.body, { includeComments: true })) {
return fixer.removeRange([node.range[0], node.body.range[0]]);
}

return null;
}
fix: isFixable(node) ? fixer => fixer.removeRange([node.range[0], node.body.range[0]]) : null
});
}

Expand Down
15 changes: 3 additions & 12 deletions lib/rules/prefer-regex-literals.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,6 @@ function isRegexLiteral(node) {
return node.type === "Literal" && Object.prototype.hasOwnProperty.call(node, "regex");
}

/**
* Determines whether the given node is a template literal without expressions.
* @param {ASTNode} node Node to check.
* @returns {boolean} True if the node is a template literal without expressions.
*/
function isStaticTemplateLiteral(node) {
return node.type === "TemplateLiteral" && node.expressions.length === 0;
}

const validPrecedingTokens = new Set([
"(",
";",
Expand Down Expand Up @@ -178,7 +169,7 @@ module.exports = {
return node.type === "TaggedTemplateExpression" &&
astUtils.isSpecificMemberAccess(node.tag, "String", "raw") &&
isGlobalReference(astUtils.skipChainExpression(node.tag).object) &&
isStaticTemplateLiteral(node.quasi);
astUtils.isStaticTemplateLiteral(node.quasi);
}

/**
Expand All @@ -191,7 +182,7 @@ module.exports = {
return node.value;
}

if (isStaticTemplateLiteral(node)) {
if (astUtils.isStaticTemplateLiteral(node)) {
return node.quasis[0].value.cooked;
}

Expand All @@ -209,7 +200,7 @@ module.exports = {
*/
function isStaticString(node) {
return isStringLiteral(node) ||
isStaticTemplateLiteral(node) ||
astUtils.isStaticTemplateLiteral(node) ||
isStringRawTaggedStaticTemplateLiteral(node);
}

Expand Down
9 changes: 9 additions & 0 deletions lib/rules/utils/ast-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -2123,6 +2123,15 @@ module.exports = {
return OCTAL_OR_NON_OCTAL_DECIMAL_ESCAPE_PATTERN.test(rawString);
},

/**
* Determines whether the given node is a template literal without expressions.
* @param {ASTNode} node Node to check.
* @returns {boolean} True if the node is a template literal without expressions.
*/
isStaticTemplateLiteral(node) {
return node.type === "TemplateLiteral" && node.expressions.length === 0;
},

isReferenceToGlobalVariable,
isLogicalExpression,
isCoalesceExpression,
Expand Down
8 changes: 7 additions & 1 deletion lib/rules/valid-typeof.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
*/
"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -88,7 +94,7 @@ module.exports = {
if (parent.type === "BinaryExpression" && OPERATORS.has(parent.operator)) {
const sibling = parent.left === node ? parent.right : parent.left;

if (sibling.type === "Literal" || sibling.type === "TemplateLiteral" && !sibling.expressions.length) {
if (sibling.type === "Literal" || astUtils.isStaticTemplateLiteral(sibling)) {
const value = sibling.type === "Literal" ? sibling.value : sibling.quasis[0].value.cooked;

if (!VALID_TYPES.has(value)) {
Expand Down
13 changes: 2 additions & 11 deletions lib/rules/yoda.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,13 @@ function isNegativeNumericLiteral(node) {
);
}

/**
* Determines whether a node is a Template Literal which can be determined statically.
* @param {ASTNode} node Node to test
* @returns {boolean} True if the node is a Template Literal without expression.
*/
function isStaticTemplateLiteral(node) {
return node.type === "TemplateLiteral" && node.expressions.length === 0;
}

/**
* Determines whether a non-Literal node should be treated as a single Literal node.
* @param {ASTNode} node Node to test
* @returns {boolean} True if the node should be treated as a single Literal node.
*/
function looksLikeLiteral(node) {
return isNegativeNumericLiteral(node) || isStaticTemplateLiteral(node);
return isNegativeNumericLiteral(node) || astUtils.isStaticTemplateLiteral(node);
}

/**
Expand All @@ -100,7 +91,7 @@ function getNormalizedLiteral(node) {
};
}

if (isStaticTemplateLiteral(node)) {
if (astUtils.isStaticTemplateLiteral(node)) {
return {
type: "Literal",
value: node.quasis[0].value.cooked,
Expand Down
63 changes: 63 additions & 0 deletions tests/lib/rules/no-unused-labels.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,69 @@ ruleTester.run("no-unused-labels", rule, {
code: "A /* comment */: foo",
output: null,
errors: [{ messageId: "unused" }]
},

// https://github.com/eslint/eslint/issues/16988
{
code: 'A: "use strict"',
output: null,
errors: [{ messageId: "unused" }]
},
{
code: '"use strict"; foo: "bar"',
output: null,
errors: [{ messageId: "unused" }]
},
{
code: 'A: ("use strict")', // Parentheses may be removed by another rule.
output: null,
errors: [{ messageId: "unused" }]
},
{
code: "A: `use strict`", // `use strict` may be changed to "use strict" by another rule.
output: null,
parserOptions: { ecmaVersion: 6 },
errors: [{ messageId: "unused" }]
},
{
code: "if (foo) { bar: 'baz' }",
output: "if (foo) { 'baz' }",
errors: [{ messageId: "unused" }]
},
{
code: "A: B: 'foo'",
output: "B: 'foo'",
errors: [{ messageId: "unused" }, { messageId: "unused" }]
},
{
code: "A: B: C: 'foo'",
output: "B: C: 'foo'", // Becomes "C: 'foo'" on the second pass.
errors: [{ messageId: "unused" }, { messageId: "unused" }, { messageId: "unused" }]
},
{
code: "A: B: C: D: 'foo'",
output: "B: D: 'foo'", // Becomes "D: 'foo'" on the second pass.
errors: [
{ messageId: "unused" },
{ messageId: "unused" },
{ messageId: "unused" },
{ messageId: "unused" }]
},
{
code: "A: B: C: D: E: 'foo'",
output: "B: D: E: 'foo'", // Becomes "E: 'foo'" on the third pass.
errors: [
{ messageId: "unused" },
{ messageId: "unused" },
{ messageId: "unused" },
{ messageId: "unused" },
{ messageId: "unused" }
]
},
{
code: "A: 42",
output: "42",
errors: [{ messageId: "unused" }]
}

/*
Expand Down
18 changes: 18 additions & 0 deletions tests/lib/rules/utils/ast-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1857,4 +1857,22 @@ describe("ast-utils", () => {
});
});
});

describe("isStaticTemplateLiteral", () => {
const expectedResults = {
"``": true,
"`foo`": true,
"`foo${bar}`": false,
"\"foo\"": false,
"foo`bar`": false
};

Object.entries(expectedResults).forEach(([code, expectedResult]) => {
it(`returns ${expectedResult} for ${code}`, () => {
const ast = espree.parse(code, { ecmaVersion: 6 });

assert.strictEqual(astUtils.isStaticTemplateLiteral(ast.body[0].expression), expectedResult);
});
});
});
});