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 8 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
173 changes: 144 additions & 29 deletions lib/rules/prefer-regex-literals.js
View file
Open in desktop
Expand Up @@ -146,6 +146,8 @@ 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 '{{ flags }}'.",
replaceWithIntendedLiteralAndFlags: "Replace with a regular expression literal with flags '{{ flags }}'.",
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 +260,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 +297,83 @@ 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 validator = new RegExpValidator({ ecmaVersion: regexppEcmaVersion });

try {
validator.validatePattern(pattern, 0, pattern.length, flags ? flags.includes("u") : false);
if (flags) {
validator.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 areFlagsEqual(flagsA, flagsB) {
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) {
const flagsSet = new Set([
...flagsA,
...flagsB
]);

return [...flagsSet].join("");
}

/**
* Checks whether a give node can be fixed to the given regex pattern and flags.
* @param {ASTNode} node The node to check.
* @param {string} pattern The regex pattern to check.
* @param {string} flags The regex flags
* @returns {boolean} True if a node can be fixed to the given regex pattern and flags.
*/
function canFixTo(node, pattern, flags) {
const tokenBefore = sourceCode.getTokenBefore(node);

return sourceCode.getCommentsInside(node).length === 0 &&
(!tokenBefore || validPrecedingTokens.has(tokenBefore.value)) &&
isValidRegexForEcmaVersion(pattern, flags);
}

/**
* Returns a safe output code considering the before and after tokens.
* @param {ASTNode} node The regex node.
* @param {string} newRegExpValue The new regex expression value.
* @returns {string} The output code.
*/
function getSafeOutput(node, newRegExpValue) {
const tokenBefore = sourceCode.getTokenBefore(node);
const tokenAfter = sourceCode.getTokenAfter(node);

return (tokenBefore && !canTokensBeAdjacent(tokenBefore, newRegExpValue) && tokenBefore.range[1] === node.range[0] ? " " : "") +
newRegExpValue +
(tokenAfter && !canTokensBeAdjacent(newRegExpValue, tokenAfter) && node.range[1] === tokenAfter.range[0] ? " " : "");

}

return {
Program() {
const scope = context.getScope();
Expand All @@ -306,10 +387,69 @@ 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 suggests = [];

const argFlags = getStringValue(node.arguments[1]) || "";

if (canFixTo(node, regexNode.regex.pattern, argFlags)) {
suggests.push({
messageId: "replaceWithLiteralAndFlags",
pattern: regexNode.regex.pattern,
flags: argFlags
});
}

const literalFlags = regexNode.regex.flags || "";
const mergedFlags = mergeRegexFlags(literalFlags, argFlags);

if (
!areFlagsEqual(mergedFlags, argFlags) &&
canFixTo(node, regexNode.regex.pattern, mergedFlags)
) {
suggests.push({
messageId: "replaceWithIntendedLiteralAndFlags",
pattern: regexNode.regex.pattern,
flags: mergedFlags
});
}

context.report({
node,
messageId: "unexpectedRedundantRegExpWithFlags",
suggest: suggests.map(({ flags, pattern, messageId }) => ({
messageId,
data: {
flags
},
fix(fixer) {
return fixer.replaceText(node, getSafeOutput(node, `/${pattern}/${flags}`));
}
}))
});
} else {
context.report({ node, messageId: "unexpectedRedundantRegExp" });
const outputs = [];

if (canFixTo(node, 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,
getSafeOutput(node, output)
);
}
}))
});
}
} else if (hasOnlyStaticStringArguments(node)) {
let regexContent = getStringValue(node.arguments[0]);
Expand All @@ -320,32 +460,14 @@ 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 {
noFix = true;
}

const tokenBefore = sourceCode.getTokenBefore(node);

if (tokenBefore && !validPrecedingTokens.has(tokenBefore.value)) {
if (!canFixTo(node, regexContent, flags)) {
noFix = true;
}

if (!/^[-a-zA-Z0-9\\[\](){} \t\r\n\v\f!@#$%^&*+^_=/~`.><?,'"|:;]*$/u.test(regexContent)) {
noFix = true;
}

if (sourceCode.getCommentsInside(node).length > 0) {
noFix = true;
}

if (regexContent && !noFix) {
let charIncrease = 0;

Expand Down Expand Up @@ -377,14 +499,7 @@ module.exports = {
suggest: noFix ? [] : [{
messageId: "replaceWithLiteral",
fix(fixer) {
const tokenAfter = sourceCode.getTokenAfter(node);

return fixer.replaceText(
node,
(tokenBefore && !canTokensBeAdjacent(tokenBefore, newRegExpValue) && tokenBefore.range[1] === node.range[0] ? " " : "") +
newRegExpValue +
(tokenAfter && !canTokensBeAdjacent(newRegExpValue, tokenAfter) && node.range[1] === tokenAfter.range[0] ? " " : "")
);
return fixer.replaceText(node, getSafeOutput(node, newRegExpValue));
}
}]
});
Expand Down