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.

Sign up for GitHub

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

Jump to bottom

feat: add suggestions for redundant wrapping in prefer-regex-literals #16658

Merged
merged 9 commits into from Jan 3, 2023
Merged

feat: add suggestions for redundant wrapping in prefer-regex-literals #16658

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
df5bcf6
feat: add suggestions in prefer-regex-literals
yeonjuan Dec 11, 2022
5eb9f15
add tests & refactor
yeonjuan Dec 13, 2022
c8cf322
change test case
yeonjuan Dec 13, 2022
1a0074d
apply reviews
yeonjuan Dec 15, 2022
44514f9
apply review and refactor
yeonjuan Dec 17, 2022
b1b6a57
refactor
yeonjuan Dec 17, 2022
9a673e3
apply review
yeonjuan Dec 23, 2022
de98a29
Apply reviews
yeonjuan Dec 27, 2022
3043c4c
add data
yeonjuan Dec 28, 2022
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
113 changes: 102 additions & 11 deletions lib/rules/prefer-regex-literals.js
View file
Open in desktop
Expand Up @@ -146,6 +146,7 @@ module.exports = {
messages: {
unexpectedRegExp: "Use a regular expression literal instead of the 'RegExp' constructor.",
replaceWithLiteral: "Replace with an equivalent regular expression literal.",
replaceWithLiteralAndFlags: "Replace with an equivalent regular expression literal with flags.",
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved
unexpectedRedundantRegExp: "Regular expression literal is unnecessarily wrapped within a 'RegExp' constructor.",
unexpectedRedundantRegExpWithFlags: "Use regular expression literal with flags instead of the 'RegExp' constructor."
}
Expand Down Expand Up @@ -258,6 +259,8 @@ module.exports = {
return Math.min(ecmaVersion, REGEXPP_LATEST_ECMA_VERSION);
}

const regexppEcmaVersion = getRegexppEcmaVersion(context.languageOptions.ecmaVersion);

/**
* Makes a character escaped or else returns null.
* @param {string} character The character to escape.
Expand Down Expand Up @@ -293,6 +296,51 @@ module.exports = {
}
}

/**
* Checks whether the given regex and flags are valid for the ecma version or not.
* @param {string} pattern The regex pattern to check.
* @param {string | undefined} flags The regex flags to check.
* @returns {boolean} True if the given regex pattern and flags are valid for the ecma version.
*/
function isValidRegexForEcmaVersion(pattern, flags) {
const RegExpValidatorInstance = new RegExpValidator({ ecmaVersion: regexppEcmaVersion });
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved

try {
RegExpValidatorInstance.validatePattern(pattern, 0, pattern.length, flags ? flags.includes("u") : false);
if (flags) {
RegExpValidatorInstance.validateFlags(flags);
}
return true;
} catch {
return false;
}
}

/**
* Checks whether two given regex flags contain the same flags or not.
* @param {string} flagsA The regex flags.
* @param {string} flagsB The regex flags.
* @returns {boolean} True if two regex flags contain same flags.
*/
function isFlagsEqual(flagsA, flagsB) {
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved
return [...flagsA].sort().join("") === [...flagsB].sort().join("");
}


/**
* Merges two regex flags.
* @param {string} flagsA The regex flags.
* @param {string} flagsB The regex flags.
* @returns {string} The merged regex flags.
*/
function mergeRegexFlags(flagsA, flagsB) {
return [
...flagsA,
...flagsB
].filter((flag, index, self) => index === self.indexOf(flag))
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved
.join("");
}

return {
Program() {
const scope = context.getScope();
Expand All @@ -306,10 +354,61 @@ module.exports = {

for (const { node } of tracker.iterateGlobalReferences(traceMap)) {
if (disallowRedundantWrapping && isUnnecessarilyWrappedRegexLiteral(node)) {
const regexNode = node.arguments[0];

if (node.arguments.length === 2) {
context.report({ node, messageId: "unexpectedRedundantRegExpWithFlags" });
const outputs = [];

if (sourceCode.getCommentsInside(node).length <= 0) {
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved
const literalFlags = regexNode.regex.flags || "";
const argFlags = getStringValue(node.arguments[1]) || "";

if (isValidRegexForEcmaVersion(regexNode.regex.pattern, argFlags)) {
outputs.push(`/${regexNode.regex.pattern}/${argFlags}`);
}

if (literalFlags && argFlags) {
const mergedFlags = mergeRegexFlags(literalFlags, argFlags);

if (
!isFlagsEqual(mergedFlags, argFlags) &&
isValidRegexForEcmaVersion(regexNode.regex.pattern, mergedFlags)
) {
outputs.push(`/${regexNode.regex.pattern}/${mergedFlags}`);
}
}
}

context.report({
node,
messageId: "unexpectedRedundantRegExpWithFlags",
suggest: outputs.map(output => ({
messageId: "replaceWithLiteralAndFlags",
fix(fixer) {
return fixer.replaceText(node, output);
}
}))
});
} else {
context.report({ node, messageId: "unexpectedRedundantRegExp" });
const outputs = [];

if (
sourceCode.getCommentsInside(node).length <= 0 &&
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved
isValidRegexForEcmaVersion(regexNode.regex.pattern, regexNode.regex.flags)
) {
outputs.push(sourceCode.getText(regexNode));
}

context.report({
node,
messageId: "unexpectedRedundantRegExp",
suggest: outputs.map(output => ({
messageId: "replaceWithLiteral",
fix(fixer) {
return fixer.replaceText(node, output);
}
}))
});
}
} else if (hasOnlyStaticStringArguments(node)) {
let regexContent = getStringValue(node.arguments[0]);
Expand All @@ -320,15 +419,7 @@ module.exports = {
flags = getStringValue(node.arguments[1]);
}

const regexppEcmaVersion = getRegexppEcmaVersion(context.languageOptions.ecmaVersion);
const RegExpValidatorInstance = new RegExpValidator({ ecmaVersion: regexppEcmaVersion });

try {
RegExpValidatorInstance.validatePattern(regexContent, 0, regexContent.length, flags ? flags.includes("u") : false);
if (flags) {
RegExpValidatorInstance.validateFlags(flags);
}
} catch {
if (!isValidRegexForEcmaVersion(regexContent, flags)) {
noFix = true;
}

Expand Down
211 changes: 206 additions & 5 deletions tests/lib/rules/prefer-regex-literals.js
View file
Open in desktop
Expand Up @@ -576,7 +576,13 @@ ruleTester.run("prefer-regex-literals", rule, {
messageId: "unexpectedRedundantRegExp",
type: "NewExpression",
line: 1,
column: 1
column: 1,
suggestions: [
{
messageId: "replaceWithLiteral",
output: "/a/;"
}
]
}
]
},
Expand All @@ -592,7 +598,131 @@ ruleTester.run("prefer-regex-literals", rule, {
messageId: "unexpectedRedundantRegExpWithFlags",
type: "NewExpression",
line: 1,
column: 1
column: 1,
suggestions: [
{
messageId: "replaceWithLiteralAndFlags",
output: "/a/u;"
}
]
}
]
},
{
code: "new RegExp(/a/g, '');",
options: [
{
disallowRedundantWrapping: true
}
],
errors: [
{
messageId: "unexpectedRedundantRegExpWithFlags",
type: "NewExpression",
line: 1,
column: 1,
suggestions: [
{
messageId: "replaceWithLiteralAndFlags",
output: "/a/;"
}
]
}
]
},
{
code: "new RegExp(/a/g, 'g');",
options: [
{
disallowRedundantWrapping: true
}
],
errors: [
{
messageId: "unexpectedRedundantRegExpWithFlags",
type: "NewExpression",
line: 1,
column: 1,
suggestions: [
{
messageId: "replaceWithLiteralAndFlags",
output: "/a/g;"
}
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
]
}
]
},
{
code: "new RegExp(/a/ig, 'g');",
options: [
{
disallowRedundantWrapping: true
}
],
errors: [
{
messageId: "unexpectedRedundantRegExpWithFlags",
type: "NewExpression",
line: 1,
column: 1,
suggestions: [
{
messageId: "replaceWithLiteralAndFlags",
output: "/a/g;"
},
{
messageId: "replaceWithLiteralAndFlags",
output: "/a/ig;"
}
]
}
]
},
{
code: "new RegExp(/a/g, 'ig');",
options: [
{
disallowRedundantWrapping: true
}
],
errors: [
{
messageId: "unexpectedRedundantRegExpWithFlags",
type: "NewExpression",
line: 1,
column: 1,
suggestions: [
{
messageId: "replaceWithLiteralAndFlags",
output: "/a/ig;"
}
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
]
}
]
},
{
code: "new RegExp(/a/i, 'g');",
options: [
{
disallowRedundantWrapping: true
}
],
errors: [
{
messageId: "unexpectedRedundantRegExpWithFlags",
type: "NewExpression",
line: 1,
column: 1,
suggestions: [
{
messageId: "replaceWithLiteralAndFlags",
output: "/a/g;"
},
{
messageId: "replaceWithLiteralAndFlags",
output: "/a/ig;"
}
]
}
]
},
Expand All @@ -608,12 +738,18 @@ ruleTester.run("prefer-regex-literals", rule, {
messageId: "unexpectedRedundantRegExpWithFlags",
type: "NewExpression",
line: 1,
column: 1
column: 1,
suggestions: [
{
messageId: "replaceWithLiteralAndFlags",
output: "/a/u;"
}
]
}
]
},
{
code: "new RegExp(/a/, String.raw`u`);",
code: "new RegExp(/a/, `gi`);",
options: [
{
disallowRedundantWrapping: true
Expand All @@ -624,7 +760,13 @@ ruleTester.run("prefer-regex-literals", rule, {
messageId: "unexpectedRedundantRegExpWithFlags",
type: "NewExpression",
line: 1,
column: 1
column: 1,
suggestions: [
{
messageId: "replaceWithLiteralAndFlags",
output: "/a/gi;"
}
]
}
]
},
Expand All @@ -650,6 +792,65 @@ ruleTester.run("prefer-regex-literals", rule, {
}
]
},
{
code: "new RegExp(/a/, String.raw`u`);",
options: [
{
disallowRedundantWrapping: true
}
],
errors: [
{
messageId: "unexpectedRedundantRegExpWithFlags",
type: "NewExpression",
line: 1,
column: 1,
suggestions: [
{
messageId: "replaceWithLiteralAndFlags",
output: "/a/u;"
}
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
]
}
]
},
{
code: "new RegExp(/a/ /* comment */);",
options: [
{
disallowRedundantWrapping: true
}
],
errors: [
{
messageId: "unexpectedRedundantRegExp",
type: "NewExpression",
line: 1,
column: 1,
suggestions: null
}
]
},
{
code: "new RegExp(/a/, 'd');",
options: [
{
disallowRedundantWrapping: true
}
],
parserOptions: {
ecmaVersion: 2021
},
errors: [
{
messageId: "unexpectedRedundantRegExpWithFlags",
type: "NewExpression",
line: 1,
column: 1,
suggestions: null
}
]
},
{
code: "new RegExp((String?.raw)`a`);",
errors: [
Expand Down