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

feat: Suggestions support for prefer-regex-literals #15077

Merged
merged 28 commits into from Dec 17, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
bac91cb
New: Autofix support to prefer-regex-literals
Yash-Singh1 Sep 16, 2021
dc34aa3
Merge branch 'master' into master
Yash-Singh1 Sep 17, 2021
52e6e00
Use canTokensBeAdjacent
Yash-Singh1 Sep 17, 2021
9ce5f47
Fix for NULL
Yash-Singh1 Sep 20, 2021
f2d582c
Switch to validatePattern and validateFlags
Yash-Singh1 Sep 20, 2021
ac05a36
Fix for unicode
Yash-Singh1 Sep 20, 2021
dbc3482
Apply a few suggestions from code review
Yash-Singh1 Sep 24, 2021
9e3b7e2
Fix: Double Escaping?
Yash-Singh1 Sep 25, 2021
4513700
Tests and fixes for no-unicode regexp
Yash-Singh1 Sep 25, 2021
64c556f
New: Drop usage of getStaticValue
Yash-Singh1 Sep 25, 2021
06109ba
Fix: Remove whitespace changes, fix jsdoc type, and convert to sugges…
Yash-Singh1 Sep 25, 2021
6cbcf4b
New: More test cases for .
Yash-Singh1 Sep 25, 2021
9ae6a20
Remove meta.docs.suggestion
Yash-Singh1 Oct 30, 2021
ea60c81
Fix linting
Yash-Singh1 Oct 30, 2021
5566402
Don't fix NULL
Yash-Singh1 Oct 30, 2021
852c5b6
Remove redundant wrapping suggestions for now
Yash-Singh1 Oct 31, 2021
ab9f17d
String.raw can have problematic chars
Yash-Singh1 Oct 31, 2021
4132ed9
Remove fixable
Yash-Singh1 Oct 31, 2021
9f42bdf
Fix messed up char increase
Yash-Singh1 Oct 31, 2021
8d96676
Apply suggestion from code review
Yash-Singh1 Dec 1, 2021
2ec405c
chore: use characterNode.raw instead of characterNode.value
Yash-Singh1 Dec 3, 2021
d4353a3
chore: do a bit of simplification of onCharacterEnter
Yash-Singh1 Dec 10, 2021
b4355d1
Apply suggestions from code review
Yash-Singh1 Dec 11, 2021
e292a77
chore: more changes following code review
Yash-Singh1 Dec 11, 2021
d6dcde5
chore: Use reliable way of testing if spacing needed
Yash-Singh1 Dec 11, 2021
300f349
diff msg for suggestion than main warning
Yash-Singh1 Dec 11, 2021
4fbf577
chore: stricter testing
Yash-Singh1 Dec 14, 2021
61bab7d
Apply suggestions from code review
Yash-Singh1 Dec 17, 2021
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
239 changes: 201 additions & 38 deletions lib/rules/prefer-regex-literals.js
Expand Up @@ -10,21 +10,14 @@
//------------------------------------------------------------------------------

const astUtils = require("./utils/ast-utils");
const { CALL, CONSTRUCT, ReferenceTracker, findVariable } = require("eslint-utils");
const { CALL, CONSTRUCT, ReferenceTracker, findVariable, getStaticValue } = require("eslint-utils");
const { RegExpValidator } = require("regexpp");
const { canTokensBeAdjacent } = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

/**
* Determines whether the given node is a string literal.
* @param {ASTNode} node Node to check.
* @returns {boolean} True if the node is a string literal.
*/
function isStringLiteral(node) {
return node.type === "Literal" && typeof node.value === "string";
}

/**
* Determines whether the given node is a regex literal.
* @param {ASTNode} node Node to check.
Expand All @@ -43,6 +36,75 @@ function isStaticTemplateLiteral(node) {
return node.type === "TemplateLiteral" && node.expressions.length === 0;
}

const validPrecedingTokens = [
"(",
";",
"[",
",",
"=",
"+",
"*",
"-",
"?",
"~",
"%",
"**",
"!",
"typeof",
"instanceof",
"&&",
"||",
"??",
"await",
"yield",
"return",
"...",
"delete",
"void",
"in",
"<",
">",
"<=",
">=",
"==",
"===",
"!=",
"!==",
"<<",
">>",
">>>",
"&",
"|",
"^",
":",
"{",
"=>",
"*=",
"<<=",
">>=",
">>>=",
"^=",
"|=",
"&=",
"??=",
"||=",
"&&=",
"**=",
"+=",
"-=",
"/=",
"%=",
"/",
"do",
"break",
"continue",
"debugger",
"case",
"throw",
"of",
Yash-Singh1 marked this conversation as resolved.
Show resolved Hide resolved
")"
Yash-Singh1 marked this conversation as resolved.
Show resolved Hide resolved
];


//------------------------------------------------------------------------------
// Rule Definition
Expand All @@ -58,6 +120,8 @@ module.exports = {
url: "https://eslint.org/docs/rules/prefer-regex-literals"
},

fixable: "code",

schema: [
{
type: "object",
Expand All @@ -80,6 +144,8 @@ module.exports = {

create(context) {
const [{ disallowRedundantWrapping = false } = {}] = context.options;
const sourceCode = context.getSourceCode();
const text = sourceCode.getText();

/**
* Determines whether the given identifier node is a reference to a global variable.
Expand Down Expand Up @@ -107,53 +173,81 @@ module.exports = {
}

/**
* Determines whether the given node is considered to be a static string by the logic of this rule.
* @param {ASTNode} node Node to check.
* @returns {boolean} True if the node is a static string.
*/
function isStaticString(node) {
return isStringLiteral(node) ||
isStaticTemplateLiteral(node) ||
isStringRawTaggedStaticTemplateLiteral(node);
}

/**
* Determines whether the relevant arguments of the given are all static string literals.
* @param {ASTNode} node Node to check.
* @returns {boolean} True if all arguments are static strings.
* Gets the value of a string
* @param {ASTNode} node The node to get the string of.
* @param {Scope} [scope] The scope
* @returns {string} The value of the node.
*/
function hasOnlyStaticStringArguments(node) {
const args = node.arguments;
function getStringValue(node, scope) {
const result = getStaticValue(node, scope);

if ((args.length === 1 || args.length === 2) && args.every(isStaticString)) {
return true;
if (result && typeof result.value === "string") {
return result.value;
}

return false;
return null;
}

/**
* Determines whether the arguments of the given node indicate that a regex literal is unnecessarily wrapped.
* @param {ASTNode} node Node to check.
* @param {Scope} scope The scope passed to getStringValue
* @returns {boolean} True if the node already contains a regex literal argument.
*/
function isUnnecessarilyWrappedRegexLiteral(node) {
function isUnnecessarilyWrappedRegexLiteral(node, scope) {
const args = node.arguments;

if (args.length === 1 && isRegexLiteral(args[0])) {
return true;
}

if (args.length === 2 && isRegexLiteral(args[0]) && isStaticString(args[1])) {
if (args.length === 2 && isRegexLiteral(args[0]) && getStringValue(args[1], scope)) {
return true;
}

return false;
}

/* eslint-disable jsdoc/valid-types -- eslint-plugin-jsdoc's type parser doesn't support square brackets */
/**
* Returns a ecmaVersion compatible for regexpp.
* @param {import("../linter/linter").ParserOptions["ecmaVersion"]} ecmaVersion The ecmaVersion to convert.
* @returns {import("regexpp/ecma-versions").EcmaVersion} The resulting ecmaVersion compatible for regexpp.
*/
function getRegexppEcmaVersion(ecmaVersion) {
/* eslint-enable jsdoc/valid-types -- JSDoc is over, enable jsdoc/valid-types again */
Yash-Singh1 marked this conversation as resolved.
Show resolved Hide resolved
if (ecmaVersion === 3 || ecmaVersion === 5) {
return 5;
}
return ecmaVersion + 2009;
}

/**
* Ensures that String is the only variable present in all child scopes
* @param {Scope} scope The scope to go within and remove variables from
* @param {boolean} [children] Whether to iterate over children or not and if false iterate through parents
* @returns {Scope} The newer scope with only String present
*/
function noStringScope(scope, children = true) {
scope.variables.filter(variable => variable.name !== "String").forEach(definedVariable => scope.set.delete(definedVariable.name));
if (children) {
for (const childScopeIndex in scope.childScopes) {
if (!isNaN(+childScopeIndex)) {
scope.childScopes[childScopeIndex] = noStringScope(scope.childScopes[childScopeIndex]);
}
}
if (scope.childScopes.length === 0 && scope.upper) {
scope.upper = noStringScope(scope.upper, false);
}
} else if (scope.upper) {
scope.upper = noStringScope(scope.upper, false);
}
return scope;
}
Yash-Singh1 marked this conversation as resolved.
Show resolved Hide resolved

return {
Program() {
const scope = context.getScope();
let scope = context.getScope();

const tracker = new ReferenceTracker(scope);
const traceMap = {
RegExp: {
Expand All @@ -163,14 +257,83 @@ module.exports = {
};

for (const { node } of tracker.iterateGlobalReferences(traceMap)) {
if (disallowRedundantWrapping && isUnnecessarilyWrappedRegexLiteral(node)) {
scope = noStringScope(scope);

if (disallowRedundantWrapping && isUnnecessarilyWrappedRegexLiteral(node, scope)) {
if (node.arguments.length === 2) {
context.report({ node, messageId: "unexpectedRedundantRegExpWithFlags" });
context.report({
node,
messageId: "unexpectedRedundantRegExpWithFlags",
// eslint-disable-next-line no-loop-func -- scope value won't change
fix(fixer) {
return fixer.replaceTextRange(node.range, node.arguments[0].raw + getStringValue(node.arguments[1], scope));
}
});
} else {
context.report({ node, messageId: "unexpectedRedundantRegExp" });
context.report({
node,
messageId: "unexpectedRedundantRegExp",
fix(fixer) {
return fixer.replaceTextRange(node.range, node.arguments[0].raw);
}
});
}
} else if (
(getStringValue(node.arguments[0], scope) !== null) &&
(!node.arguments[1] || getStringValue(node.arguments[1], scope) !== null) &&
(node.arguments.length === 1 || node.arguments.length === 2)
) {
let regexContent = getStringValue(node.arguments[0], scope);
let noFix = false;
let flags;

if (node.arguments[1]) {
flags = getStringValue(node.arguments[1], scope);
}
} else if (hasOnlyStaticStringArguments(node)) {
context.report({ node, messageId: "unexpectedRegExp" });

const RegExpValidatorInstance = new RegExpValidator({ ecmaVersion: getRegexppEcmaVersion(context.parserOptions.ecmaVersion) });
Yash-Singh1 marked this conversation as resolved.
Show resolved Hide resolved

try {
RegExpValidatorInstance.validatePattern(regexContent);
Yash-Singh1 marked this conversation as resolved.
Show resolved Hide resolved
if (flags) {
RegExpValidatorInstance.validateFlags(flags);
}
} catch {
noFix = true;
}

const tokenBefore = sourceCode.getTokenBefore(node);

if (tokenBefore && !validPrecedingTokens.includes(tokenBefore.value)) {
noFix = true;
}

if (!/^[-\na-zA-Z\0\r\v\f0-9\\[\](){}\s!@#$%^&*+^_=/~`.><?,'"|:;]*$/u.test(regexContent)) {
Yash-Singh1 marked this conversation as resolved.
Show resolved Hide resolved
noFix = true;
}

Yash-Singh1 marked this conversation as resolved.
Show resolved Hide resolved
if (regexContent && !isStringRawTaggedStaticTemplateLiteral(node.arguments[0])) {
regexContent = regexContent.replace(/\r/gu, "\\r").replace(/\n/gu, "\\n").replace(/\t/gu, "\\t").replace(/\f/gu, "\\f").replace(/\v/gu, "\\v").replace(/\0/gu, "\\0");
}

const newRegExpValue = `/${regexContent || "(?:)"}/${flags || ""}`;

context.report({
node,
messageId: "unexpectedRegExp",
...(noFix ? {} : {
fix(fixer) {
const tokenAfter = sourceCode.getTokenAfter(node);

return fixer.replaceTextRange(
node.range,
Yash-Singh1 marked this conversation as resolved.
Show resolved Hide resolved
(tokenBefore && !canTokensBeAdjacent(tokenBefore, newRegExpValue) && /\S/u.test(text[node.range[0] - 1]) ? " " : "") +
newRegExpValue +
(tokenAfter && !canTokensBeAdjacent(newRegExpValue, tokenAfter) && /\S/u.test(text[node.range[1]]) ? " " : "")
Yash-Singh1 marked this conversation as resolved.
Show resolved Hide resolved
);
}
})
});
}
}
}
Expand Down