Skip to content

Commit

Permalink
Fix: false positive about ES2018 RegExp enhancements (fixes #9893) (#…
Browse files Browse the repository at this point in the history 
…10062)

* update package.json

* update no-control-regex

* update no-empty-character-class

* update no-invalid-regexp

* update no-unexpected-multiline

* update no-useless-escape

* update no-irregular-whitespace

* fix node 4 problem
  • Loading branch information
@mysticatea @not-an-aardvark
mysticatea authored and not-an-aardvark committed Mar 16, 2018
1 parent 935f4e4 commit 8d3814e
Show file tree
Hide file tree
Showing 12 changed files with 157 additions and 126 deletions.
123 changes: 51 additions & 72 deletions lib/rules/no-control-regex.js
View file
Expand Up @@ -5,6 +5,44 @@

"use strict";

const RegExpValidator = require("regexpp").RegExpValidator;
const collector = new class {
constructor() {
this.ecmaVersion = 2018;
this._source = "";
this._controlChars = [];
this._validator = new RegExpValidator(this);
}

onPatternEnter() {
this._controlChars = [];
}

onCharacter(start, end, cp) {
if (cp >= 0x00 &&
cp <= 0x1F &&
(
this._source.codePointAt(start) === cp ||
this._source.slice(start, end).startsWith("\\x") ||
this._source.slice(start, end).startsWith("\\u")
)
) {
this._controlChars.push(`\\x${`0${cp.toString(16)}`.slice(-2)}`);
}
}

collectControlChars(regexpStr) {
try {
this._source = regexpStr;
this._validator.validatePattern(regexpStr); // Call onCharacter hook
} catch (err) {

// Ignore syntax errors in RegExp.
}
return this._controlChars;
}
}();

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -33,87 +71,28 @@ module.exports = {
* @returns {RegExp|null} Regex if found else null
* @private
*/
function getRegExp(node) {
if (node.value instanceof RegExp) {
return node.value;
function getRegExpPattern(node) {
if (node.regex) {
return node.regex.pattern;
}
if (typeof node.value === "string") {

const parent = context.getAncestors().pop();

if ((parent.type === "NewExpression" || parent.type === "CallExpression") &&
parent.callee.type === "Identifier" && parent.callee.name === "RegExp"
) {

// there could be an invalid regular expression string
try {
return new RegExp(node.value);
} catch (ex) {
return null;
}
}
if (typeof node.value === "string" &&
(node.parent.type === "NewExpression" || node.parent.type === "CallExpression") &&
node.parent.callee.type === "Identifier" &&
node.parent.callee.name === "RegExp" &&
node.parent.arguments[0] === node
) {
return node.value;
}

return null;
}


const controlChar = /[\x00-\x1f]/g; // eslint-disable-line no-control-regex
const consecutiveSlashes = /\\+/g;
const consecutiveSlashesAtEnd = /\\+$/g;
const stringControlChar = /\\x[01][0-9a-f]/ig;
const stringControlCharWithoutSlash = /x[01][0-9a-f]/ig;

/**
* Return a list of the control characters in the given regex string
* @param {string} regexStr regex as string to check
* @returns {array} returns a list of found control characters on given string
* @private
*/
function getControlCharacters(regexStr) {

// check control characters, if RegExp object used
const controlChars = regexStr.match(controlChar) || [];

let stringControlChars = [];

// check substr, if regex literal used
const subStrIndex = regexStr.search(stringControlChar);

if (subStrIndex > -1) {

// is it escaped, check backslash count
const possibleEscapeCharacters = regexStr.slice(0, subStrIndex).match(consecutiveSlashesAtEnd);

const hasControlChars = possibleEscapeCharacters === null || !(possibleEscapeCharacters[0].length % 2);

if (hasControlChars) {
stringControlChars = regexStr.slice(subStrIndex, -1)
.split(consecutiveSlashes)
.filter(Boolean)
.map(x => {
const match = x.match(stringControlCharWithoutSlash) || [x];

return `\\${match[0]}`;
});
}
}

return controlChars.map(x => {
const hexCode = `0${x.charCodeAt(0).toString(16)}`.slice(-2);

return `\\x${hexCode}`;
}).concat(stringControlChars);
}

return {
Literal(node) {
const regex = getRegExp(node);

if (regex) {
const computedValue = regex.toString();
const pattern = getRegExpPattern(node);

const controlCharacters = getControlCharacters(computedValue);
if (pattern) {
const controlCharacters = collector.collectControlChars(pattern);

if (controlCharacters.length > 0) {
context.report({
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/no-empty-character-class.js
View file
Expand Up @@ -21,7 +21,7 @@
* 4. `[gimuy]*`: optional regexp flags
* 5. `$`: fix the match at the end of the string
*/
const regex = /^\/([^\\[]|\\.|\[([^\\\]]|\\.)+])*\/[gimuy]*$/;
const regex = /^\/([^\\[]|\\.|\[([^\\\]]|\\.)+])*\/[gimuys]*$/;

//------------------------------------------------------------------------------
// Rule Definition
Expand Down
96 changes: 58 additions & 38 deletions lib/rules/no-invalid-regexp.js
View file
Expand Up @@ -8,7 +8,10 @@
// Requirements
//------------------------------------------------------------------------------

const espree = require("espree");
const RegExpValidator = require("regexpp").RegExpValidator;
const validator = new RegExpValidator({ ecmaVersion: 2018 });
const validFlags = /[gimuys]/g;
const undefined1 = void 0;

//------------------------------------------------------------------------------
// Rule Definition
Expand Down Expand Up @@ -40,10 +43,14 @@ module.exports = {
create(context) {

const options = context.options[0];
let allowedFlags = "";
let allowedFlags = null;

if (options && options.allowConstructorFlags) {
allowedFlags = options.allowConstructorFlags.join("");
const temp = options.allowConstructorFlags.join("").replace(validFlags, "");

if (temp) {
allowedFlags = new RegExp(`[${temp}]`, "gi");
}
}

/**
Expand All @@ -57,51 +64,64 @@ module.exports = {
}

/**
* Validate strings passed to the RegExp constructor
* @param {ASTNode} node node to evaluate
* @returns {void}
* @private
* Check syntax error in a given pattern.
* @param {string} pattern The RegExp pattern to validate.
* @param {boolean} uFlag The Unicode flag.
* @returns {string|null} The syntax error.
*/
function validateRegExpPattern(pattern, uFlag) {
try {
validator.validatePattern(pattern, undefined1, undefined1, uFlag);
return null;
} catch (err) {
return err.message;
}
}

/**
* Check syntax error in a given flags.
* @param {string} flags The RegExp flags to validate.
* @returns {string|null} The syntax error.
*/
function check(node) {
if (node.callee.type === "Identifier" && node.callee.name === "RegExp" && isString(node.arguments[0])) {
let flags = isString(node.arguments[1]) ? node.arguments[1].value : "";
function validateRegExpFlags(flags) {
try {
validator.validateFlags(flags);
return null;
} catch (err) {
return `Invalid flags supplied to RegExp constructor '${flags}'`;
}
}

return {
"CallExpression, NewExpression"(node) {
if (node.callee.type !== "Identifier" || node.callee.name !== "RegExp" || !isString(node.arguments[0])) {
return;
}
const pattern = node.arguments[0].value;
let flags = "";

if (allowedFlags) {
flags = flags.replace(new RegExp(`[${allowedFlags}]`, "gi"), "");
if (node.arguments[1]) {
flags = isString(node.arguments[1]) ? node.arguments[1].value : null;
if (allowedFlags) {
flags = flags.replace(allowedFlags, "");
}
}

try {
void new RegExp(node.arguments[0].value);
} catch (e) {
// If flags are unknown, check both are errored or not.
const message = validateRegExpFlags(flags) || (
(flags === null)
? validateRegExpPattern(pattern, true) && validateRegExpPattern(pattern, false)
: validateRegExpPattern(pattern, flags.indexOf("u") !== -1)
);

if (message) {
context.report({
node,
message: "{{message}}.",
data: e
data: { message }
});
}

if (flags) {

try {
espree.parse(`/./${flags}`, context.parserOptions);
} catch (ex) {
context.report({
node,
message: "Invalid flags supplied to RegExp constructor '{{flags}}'.",
data: {
flags
}
});
}
}

}
}

return {
CallExpression: check,
NewExpression: check
};

}
};
2 changes: 1 addition & 1 deletion lib/rules/no-irregular-whitespace.js
View file
Expand Up @@ -101,7 +101,7 @@ module.exports = {
*/
function removeInvalidNodeErrorsInIdentifierOrLiteral(node) {
const shouldCheckStrings = skipStrings && (typeof node.value === "string");
const shouldCheckRegExps = skipRegExps && (node.value instanceof RegExp);
const shouldCheckRegExps = skipRegExps && Boolean(node.regex);

if (shouldCheckStrings || shouldCheckRegExps) {

Expand Down
2 changes: 1 addition & 1 deletion lib/rules/no-unexpected-multiline.js
View file
Expand Up @@ -33,7 +33,7 @@ module.exports = {
const TAGGED_TEMPLATE_MESSAGE = "Unexpected newline between template tag and template literal.";
const DIVISION_MESSAGE = "Unexpected newline between numerator and division operator.";

const REGEX_FLAG_MATCHER = /^[gimuy]+$/;
const REGEX_FLAG_MATCHER = /^[gimsuy]+$/;

const sourceCode = context.getSourceCode();

Expand Down
4 changes: 2 additions & 2 deletions lib/rules/no-useless-escape.js
View file
Expand Up @@ -25,8 +25,8 @@ function union(setA, setB) {
}

const VALID_STRING_ESCAPES = union(new Set("\\nrvtbfux"), astUtils.LINEBREAKS);
const REGEX_GENERAL_ESCAPES = new Set("\\bcdDfnrsStvwWxu0123456789]");
const REGEX_NON_CHARCLASS_ESCAPES = union(REGEX_GENERAL_ESCAPES, new Set("^/.$*+?[{}|()B"));
const REGEX_GENERAL_ESCAPES = new Set("\\bcdDfnpPrsStvwWxu0123456789]");
const REGEX_NON_CHARCLASS_ESCAPES = union(REGEX_GENERAL_ESCAPES, new Set("^/.$*+?[{}|()Bk"));

/**
* Parses a regular expression into a list of characters with character class info.
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Expand Up @@ -43,7 +43,7 @@
"doctrine": "^2.1.0",
"eslint-scope": "^3.7.1",
"eslint-visitor-keys": "^1.0.0",
"espree": "^3.5.2",
"espree": "^3.5.4",
"esquery": "^1.0.0",
"esutils": "^2.0.2",
"file-entry-cache": "^2.0.0",
Expand All @@ -65,6 +65,7 @@
"path-is-inside": "^1.0.2",
"pluralize": "^7.0.0",
"progress": "^2.0.0",
"regexpp": "^1.0.1",
"require-uncached": "^1.0.3",
"semver": "^5.3.0",
"strip-ansi": "^4.0.0",
Expand Down
13 changes: 9 additions & 4 deletions tests/lib/rules/no-control-regex.js
View file
Expand Up @@ -30,12 +30,17 @@ ruleTester.run("no-control-regex", rule, {
],
invalid: [
{ code: `var regex = ${/\x1f/}`, errors: [{ messageId: "unexpected", data: { controlChars: "\\x1f" }, type: "Literal" }] }, // eslint-disable-line no-control-regex
{ code: `var regex = ${/\\\x1f\\x1e/}`, errors: [{ messageId: "unexpected", data: { controlChars: "\\x1f, \\x1e" }, type: "Literal" }] }, // eslint-disable-line no-control-regex
{ code: `var regex = ${/\\\x1fFOO\\x00/}`, errors: [{ messageId: "unexpected", data: { controlChars: "\\x1f, \\x00" }, type: "Literal" }] }, // eslint-disable-line no-control-regex
{ code: `var regex = ${/FOO\\\x1fFOO\\x1f/}`, errors: [{ messageId: "unexpected", data: { controlChars: "\\x1f, \\x1f" }, type: "Literal" }] }, // eslint-disable-line no-control-regex
{ code: `var regex = ${/\\\x1f\\x1e/}`, errors: [{ messageId: "unexpected", data: { controlChars: "\\x1f" }, type: "Literal" }] }, // eslint-disable-line no-control-regex
{ code: `var regex = ${/\\\x1fFOO\\x00/}`, errors: [{ messageId: "unexpected", data: { controlChars: "\\x1f" }, type: "Literal" }] }, // eslint-disable-line no-control-regex
{ code: `var regex = ${/FOO\\\x1fFOO\\x1f/}`, errors: [{ messageId: "unexpected", data: { controlChars: "\\x1f" }, type: "Literal" }] }, // eslint-disable-line no-control-regex
{ code: "var regex = new RegExp('\\x1f\\x1e')", errors: [{ messageId: "unexpected", data: { controlChars: "\\x1f, \\x1e" }, type: "Literal" }] },
{ code: "var regex = new RegExp('\\x1fFOO\\x00')", errors: [{ messageId: "unexpected", data: { controlChars: "\\x1f, \\x00" }, type: "Literal" }] },
{ code: "var regex = new RegExp('FOO\\x1fFOO\\x1f')", errors: [{ messageId: "unexpected", data: { controlChars: "\\x1f, \\x1f" }, type: "Literal" }] },
{ code: "var regex = RegExp('\\x1f')", errors: [{ messageId: "unexpected", data: { controlChars: "\\x1f" }, type: "Literal" }] }
{ code: "var regex = RegExp('\\x1f')", errors: [{ messageId: "unexpected", data: { controlChars: "\\x1f" }, type: "Literal" }] },
{
code: "var regex = /(?<a>\\x1f)/",
parserOptions: { ecmaVersion: 2018 },
errors: [{ messageId: "unexpected", data: { controlChars: "\\x1f" }, type: "Literal" }]
}
]
});
4 changes: 3 additions & 1 deletion tests/lib/rules/no-empty-character-class.js
View file
Expand Up @@ -30,7 +30,9 @@ ruleTester.run("no-empty-character-class", rule, {
"var foo = /[\\[a-z[]]/;",
"var foo = /[\\-\\[\\]\\/\\{\\}\\(\\)\\*\\+\\?\\.\\\\^\\$\\|]/g;",
"var foo = /\\s*:\\s*/gim;",
{ code: "var foo = /[\\]]/uy;", parserOptions: { ecmaVersion: 6 } }
{ code: "var foo = /[\\]]/uy;", parserOptions: { ecmaVersion: 6 } },
{ code: "var foo = /[\\]]/s;", parserOptions: { ecmaVersion: 2018 } },
"var foo = /\\[]/"
],
invalid: [
{ code: "var foo = /^abc[]/;", errors: [{ messageId: "unexpected", type: "Literal" }] },
Expand Down
13 changes: 9 additions & 4 deletions tests/lib/rules/no-invalid-regexp.js
View file
Expand Up @@ -32,13 +32,18 @@ ruleTester.run("no-invalid-regexp", rule, {
{ code: "new RegExp('.', 'u')", parserOptions: { ecmaVersion: 6 } },
{ code: "new RegExp('.', 'yu')", parserOptions: { ecmaVersion: 6 } },
{ code: "new RegExp('/', 'yu')", parserOptions: { ecmaVersion: 6 } },
{ code: "new RegExp('\\/', 'yu')", parserOptions: { ecmaVersion: 6 } }
{ code: "new RegExp('\\/', 'yu')", parserOptions: { ecmaVersion: 6 } },
{ code: "new RegExp('\\\\u{65}', 'u')", parserOptions: { ecmaVersion: 2015 } },
{ code: "new RegExp('[\\\\u{0}-\\\\u{1F}]', 'u')", parserOptions: { ecmaVersion: 2015 } },
{ code: "new RegExp('.', 's')", parserOptions: { ecmaVersion: 2018 } },
{ code: "new RegExp('(?<=a)b')", parserOptions: { ecmaVersion: 2018 } },
{ code: "new RegExp('(?<!a)b')", parserOptions: { ecmaVersion: 2018 } },
{ code: "new RegExp('(?<a>b)\\k<a>')", parserOptions: { ecmaVersion: 2018 } },
{ code: "new RegExp('(?<a>b)\\k<a>', 'u')", parserOptions: { ecmaVersion: 2018 } },
{ code: "new RegExp('\\\\p{Letter}', 'u')", parserOptions: { ecmaVersion: 2018 } }
],
invalid: [
{ code: "RegExp('[');", errors: [{ message: "Invalid regular expression: /[/: Unterminated character class.", type: "CallExpression" }] },
{ code: "RegExp('.', 'y');", errors: [{ message: "Invalid flags supplied to RegExp constructor 'y'.", type: "CallExpression" }] },
{ code: "RegExp('.', 'u');", errors: [{ message: "Invalid flags supplied to RegExp constructor 'u'.", type: "CallExpression" }] },
{ code: "RegExp('.', 'yu');", errors: [{ message: "Invalid flags supplied to RegExp constructor 'yu'.", type: "CallExpression" }] },
{ code: "RegExp('.', 'z');", errors: [{ message: "Invalid flags supplied to RegExp constructor 'z'.", type: "CallExpression" }] },
{ code: "new RegExp(')');", errors: [{ message: "Invalid regular expression: /)/: Unmatched ')'.", type: "NewExpression" }] }
]
Expand Down

0 comments on commit 8d3814e

Please sign in to comment.