Skip to content

Commit

Permalink
fix: Fix suggestion message in no-useless-escape (#17339)
Browse files Browse the repository at this point in the history
* fix: Fix suggestion message in `no-useless-escape`

* Fixes

* Fix for nodes with null `directive`

* Empty commit

* Remove fallback for `isDirective`

* Remove duplicate tests
  • Loading branch information
fasttime committed Jul 11, 2023
1 parent 84d243b commit b79b6fb
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 44 deletions.
6 changes: 5 additions & 1 deletion lib/rules/no-useless-escape.js
Expand Up @@ -94,6 +94,7 @@ module.exports = {
messages: {
unnecessaryEscape: "Unnecessary escape character: \\{{character}}.",
removeEscape: "Remove the `\\`. This maintains the current functionality.",
removeEscapeDoNotKeepSemantics: "Remove the `\\` if it was inserted by mistake.",
escapeBackslash: "Replace the `\\` with `\\\\` to include the actual backslash character."
},

Expand Down Expand Up @@ -125,7 +126,10 @@ module.exports = {
data: { character },
suggest: [
{
messageId: "removeEscape",

// Removing unnecessary `\` characters in a directive is not guaranteed to maintain functionality.
messageId: astUtils.isDirective(node.parent)
? "removeEscapeDoNotKeepSemantics" : "removeEscape",
fix(fixer) {
return fixer.removeRange(range);
}
Expand Down
46 changes: 4 additions & 42 deletions lib/rules/padding-line-between-statements.js
Expand Up @@ -130,42 +130,6 @@ function isBlockLikeStatement(sourceCode, node) {
);
}

/**
* Check whether the given node is a directive or not.
* @param {ASTNode} node The node to check.
* @param {SourceCode} sourceCode The source code object to get tokens.
* @returns {boolean} `true` if the node is a directive.
*/
function isDirective(node, sourceCode) {
return (
astUtils.isTopLevelExpressionStatement(node) &&
node.expression.type === "Literal" &&
typeof node.expression.value === "string" &&
!astUtils.isParenthesised(sourceCode, node.expression)
);
}

/**
* Check whether the given node is a part of directive prologue or not.
* @param {ASTNode} node The node to check.
* @param {SourceCode} sourceCode The source code object to get tokens.
* @returns {boolean} `true` if the node is a part of directive prologue.
*/
function isDirectivePrologue(node, sourceCode) {
if (isDirective(node, sourceCode)) {
for (const sibling of node.parent.body) {
if (sibling === node) {
break;
}
if (!isDirective(sibling, sourceCode)) {
return false;
}
}
return true;
}
return false;
}

/**
* Gets the actual last token.
*
Expand Down Expand Up @@ -359,12 +323,10 @@ const StatementTypes = {
CJS_IMPORT.test(sourceCode.getText(node.declarations[0].init))
},
directive: {
test: isDirectivePrologue
test: astUtils.isDirective
},
expression: {
test: (node, sourceCode) =>
node.type === "ExpressionStatement" &&
!isDirectivePrologue(node, sourceCode)
test: node => node.type === "ExpressionStatement" && !astUtils.isDirective(node)
},
iife: {
test: isIIFEStatement
Expand All @@ -375,10 +337,10 @@ const StatementTypes = {
isBlockLikeStatement(sourceCode, node)
},
"multiline-expression": {
test: (node, sourceCode) =>
test: node =>
node.loc.start.line !== node.loc.end.line &&
node.type === "ExpressionStatement" &&
!isDirectivePrologue(node, sourceCode)
!astUtils.isDirective(node)
},

"multiline-const": newMultilineKeywordTester("const"),
Expand Down
12 changes: 11 additions & 1 deletion lib/rules/utils/ast-utils.js
Expand Up @@ -1006,6 +1006,15 @@ function isTopLevelExpressionStatement(node) {

}

/**
* Check whether the given node is a part of a directive prologue or not.
* @param {ASTNode} node The node to check.
* @returns {boolean} `true` if the node is a part of directive prologue.
*/
function isDirective(node) {
return node.type === "ExpressionStatement" && typeof node.directive === "string";
}

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -2158,5 +2167,6 @@ module.exports = {
getSwitchCaseColonToken,
getModuleExportName,
isConstant,
isTopLevelExpressionStatement
isTopLevelExpressionStatement,
isDirective
};
37 changes: 37 additions & 0 deletions tests/lib/rules/no-useless-escape.js
Expand Up @@ -1066,6 +1066,43 @@ ruleTester.run("no-useless-escape", rule, {
output: "`\\\\a```"
}]
}]
},

// https://github.com/eslint/eslint/issues/16988
{
code: String.raw`"use\ strict";`,
errors: [{
line: 1,
column: 5,
endColumn: 6,
message: "Unnecessary escape character: \\ .",
type: "Literal",
suggestions: [{
messageId: "removeEscapeDoNotKeepSemantics",
output: String.raw`"use strict";`
}, {
messageId: "escapeBackslash",
output: String.raw`"use\\ strict";`
}]
}]
},
{
code: String.raw`({ foo() { "foo"; "bar"; "ba\z" } })`,
parserOptions: { ecmaVersion: 6 },
errors: [{
line: 1,
column: 29,
endColumn: 30,
message: "Unnecessary escape character: \\z.",
type: "Literal",
suggestions: [{
messageId: "removeEscapeDoNotKeepSemantics",
output: String.raw`({ foo() { "foo"; "bar"; "baz" } })`
}, {
messageId: "escapeBackslash",
output: String.raw`({ foo() { "foo"; "bar"; "ba\\z" } })`
}]
}]
}
]
});
45 changes: 45 additions & 0 deletions tests/lib/rules/utils/ast-utils.js
Expand Up @@ -1875,4 +1875,49 @@ describe("ast-utils", () => {
});
});
});

describe("isDirective", () => {
const expectedResults = [
{ code: '"use strict";', expectedRetVal: true },
{ code: '"use strict"; "use asm";', nodeText: '"use asm";', expectedRetVal: true },
{ code: 'const a = () => { "foo"; }', nodeText: '"foo";', expectedRetVal: true },
{ code: '"";', expectedRetVal: true },
{ code: '{ "foo"; }', nodeText: '"foo";', expectedRetVal: false },
{ code: "foo();", expectedRetVal: false },
{ code: '"foo" + "bar";', expectedRetVal: false },
{ code: "12345;", expectedRetVal: false },
{ code: "`foo`;", expectedRetVal: false },
{ code: "('foo');", expectedRetVal: false },
{ code: 'foo(); "use strict";', nodeText: '"use strict";', expectedRetVal: false }
];

expectedResults.forEach(({ code, nodeText = code, expectedRetVal }) => {
it(`should return ${expectedRetVal} for \`${nodeText}\` in \`${code}\``, () => {
linter.defineRule("checker", {
create: mustCall(({ sourceCode }) => {
const assertForNode = mustCall(
node => assert.strictEqual(astUtils.isDirective(node), expectedRetVal)
);

return ({
ExpressionStatement(node) {
if (sourceCode.getText(node) === nodeText) {
assertForNode(node);

if (!expectedRetVal) {

// The flow parser sets `directive` to null on non-directive ExpressionStatement nodes.
node.directive = null;
assertForNode(node);
}
}
}
});
})
});

linter.verify(code, { rules: { checker: "error" }, parserOptions: { ecmaVersion: 2022 } });
});
});
});
});

0 comments on commit b79b6fb

Please sign in to comment.