Skip to content

Commit

Permalink
Fix: autofix shouldn't produce template literals with \8 or \9 (#…
Browse files Browse the repository at this point in the history 
…13737)

* Fix: autofix shouldn't produce template literals with `\8` or `\9`

* Rename functions
  • Loading branch information
@mdjermanovic
mdjermanovic committed Oct 24, 2020
1 parent b165aa5 commit 256f656
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 42 deletions.
36 changes: 14 additions & 22 deletions lib/rules/prefer-template.js
View file
Expand Up @@ -39,33 +39,25 @@ function getTopConcatBinaryExpression(node) {
}

/**
* Determines whether a given node is a octal escape sequence
* Checks whether or not a node contains a string literal with an octal or non-octal decimal escape sequence
* @param {ASTNode} node A node to check
* @returns {boolean} `true` if the node is an octal escape sequence
* @returns {boolean} `true` if at least one string literal within the node contains
* an octal or non-octal decimal escape sequence
*/
function isOctalEscapeSequence(node) {

// No need to check TemplateLiterals – would throw error with octal escape
const isStringLiteral = node.type === "Literal" && typeof node.value === "string";

if (!isStringLiteral) {
return false;
function hasOctalOrNonOctalDecimalEscapeSequence(node) {
if (isConcatenation(node)) {
return (
hasOctalOrNonOctalDecimalEscapeSequence(node.left) ||
hasOctalOrNonOctalDecimalEscapeSequence(node.right)
);
}

return astUtils.hasOctalEscapeSequence(node.raw);
}

/**
* Checks whether or not a node contains a octal escape sequence
* @param {ASTNode} node A node to check
* @returns {boolean} `true` if the node contains an octal escape sequence
*/
function hasOctalEscapeSequence(node) {
if (isConcatenation(node)) {
return hasOctalEscapeSequence(node.left) || hasOctalEscapeSequence(node.right);
// No need to check TemplateLiterals – would throw parsing error
if (node.type === "Literal" && typeof node.value === "string") {
return astUtils.hasOctalOrNonOctalDecimalEscapeSequence(node.raw);
}

return isOctalEscapeSequence(node);
return false;
}

/**
Expand Down Expand Up @@ -237,7 +229,7 @@ module.exports = {
function fixNonStringBinaryExpression(fixer, node) {
const topBinaryExpr = getTopConcatBinaryExpression(node.parent);

if (hasOctalEscapeSequence(topBinaryExpr)) {
if (hasOctalOrNonOctalDecimalEscapeSequence(topBinaryExpr)) {
return null;
}

Expand Down
7 changes: 5 additions & 2 deletions lib/rules/quotes.js
View file
Expand Up @@ -282,9 +282,12 @@ module.exports = {
description: settings.description
},
fix(fixer) {
if (quoteOption === "backtick" && astUtils.hasOctalEscapeSequence(rawVal)) {
if (quoteOption === "backtick" && astUtils.hasOctalOrNonOctalDecimalEscapeSequence(rawVal)) {

// An octal escape sequence in a template literal would produce syntax error, even in non-strict mode.
/*
* An octal or non-octal decimal escape sequence in a template literal would
* produce syntax error, even in non-strict mode.
*/
return null;
}

Expand Down
18 changes: 11 additions & 7 deletions lib/rules/utils/ast-utils.js
View file
Expand Up @@ -38,7 +38,9 @@ const LINEBREAKS = new Set(["\r\n", "\r", "\n", "\u2028", "\u2029"]);
const STATEMENT_LIST_PARENTS = new Set(["Program", "BlockStatement", "SwitchCase"]);

const DECIMAL_INTEGER_PATTERN = /^(?:0|0[0-7]*[89]\d*|[1-9](?:_?\d)*)$/u;
const OCTAL_ESCAPE_PATTERN = /^(?:[^\\]|\\[^0-7]|\\0(?![0-9]))*\\(?:[1-7]|0[0-9])/u;

// Tests the presence of at least one LegacyOctalEscapeSequence or NonOctalDecimalEscapeSequence in a raw string
const OCTAL_OR_NON_OCTAL_DECIMAL_ESCAPE_PATTERN = /^(?:[^\\]|\\.)*\\(?:[1-9]|0[0-9])/su;

const LOGICAL_ASSIGNMENT_OPERATORS = new Set(["&&=", "||=", "??="]);

Expand Down Expand Up @@ -1777,17 +1779,19 @@ module.exports = {
},

/**
* Determines whether the given raw string contains an octal escape sequence.
* Determines whether the given raw string contains an octal escape sequence
* or a non-octal decimal escape sequence ("\8", "\9").
*
* "\1", "\2" ... "\7"
* "\00", "\01" ... "\09"
* "\1", "\2" ... "\7", "\8", "\9"
* "\00", "\01" ... "\07", "\08", "\09"
*
* "\0", when not followed by a digit, is not an octal escape sequence.
* @param {string} rawString A string in its raw representation.
* @returns {boolean} `true` if the string contains at least one octal escape sequence.
* @returns {boolean} `true` if the string contains at least one octal escape sequence
* or at least one non-octal decimal escape sequence.
*/
hasOctalEscapeSequence(rawString) {
return OCTAL_ESCAPE_PATTERN.test(rawString);
hasOctalOrNonOctalDecimalEscapeSequence(rawString) {
return OCTAL_OR_NON_OCTAL_DECIMAL_ESCAPE_PATTERN.test(rawString);
},

isLogicalExpression,
Expand Down
5 changes: 5 additions & 0 deletions tests/lib/rules/prefer-template.js
View file
Expand Up @@ -194,6 +194,11 @@ ruleTester.run("prefer-template", rule, {
output: null,
errors
},
{
code: "foo + 'does not autofix non-octal decimal escape sequence' + '\\8'",
output: null,
errors
},
{
code: "foo + '\\n other text \\033'",
output: null,
Expand Down
13 changes: 13 additions & 0 deletions tests/lib/rules/quotes.js
View file
Expand Up @@ -614,6 +614,19 @@ ruleTester.run("quotes", rule, {
type: "Literal"
}
]
},
{
code: "var nonOctalDecimalEscape = '\\8'",
output: null,
options: ["backtick"],
parserOptions: { ecmaVersion: 6 },
errors: [
{
messageId: "wrongQuotes",
data: { description: "backtick" },
type: "Literal"
}
]
}
]
});
29 changes: 18 additions & 11 deletions tests/lib/rules/utils/ast-utils.js
View file
Expand Up @@ -1648,7 +1648,7 @@ describe("ast-utils", () => {
});
});

describe("hasOctalEscapeSequence", () => {
describe("hasOctalOrNonOctalDecimalEscapeSequence", () => {

/* eslint-disable quote-props */
const expectedResults = {
Expand Down Expand Up @@ -1683,27 +1683,31 @@ describe("ast-utils", () => {
"\\\\\\1": true,
"\\\\\\01": true,
"\\\\\\08": true,
"\\8": true,
"\\9": true,
"a\\8a": true,
"\\0\\8": true,
"\\8\\0": true,
"\\80": true,
"\\81": true,
"\\\\\\8": true,
"\\\n\\1": true,
"foo\\\nbar\\2baz": true,
"\\\n\\8": true,
"foo\\\nbar\\9baz": true,

"\\0": false,
"\\8": false,
"\\9": false,
" \\0": false,
"\\0 ": false,
"a\\0": false,
"\\0a": false,
"a\\8a": false,
"\\0\\8": false,
"\\8\\0": false,
"\\80": false,
"\\81": false,
"\\\\": false,
"\\\\0": false,
"\\\\01": false,
"\\\\08": false,
"\\\\1": false,
"\\\\12": false,
"\\\\\\0": false,
"\\\\\\8": false,
"\\0\\\\": false,
"0": false,
"1": false,
Expand All @@ -1713,15 +1717,18 @@ describe("ast-utils", () => {
"80": false,
"12": false,
"\\a": false,
"\\n": false
"\\n": false,
"\\\n": false,
"foo\\\nbar": false,
"128\\\n349": false
};
/* eslint-enable quote-props */

Object.keys(expectedResults).forEach(key => {
it(`should return ${expectedResults[key]} for ${key}`, () => {
const ast = espree.parse(`"${key}"`);

assert.strictEqual(astUtils.hasOctalEscapeSequence(ast.body[0].expression.raw), expectedResults[key]);
assert.strictEqual(astUtils.hasOctalOrNonOctalDecimalEscapeSequence(ast.body[0].expression.raw), expectedResults[key]);
});
});
});
Expand Down

0 comments on commit 256f656

Please sign in to comment.