From df5bcf66f99f62520538dad9bd19b0229fd422a3 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Mon, 12 Dec 2022 00:31:29 +0900 Subject: [PATCH 1/9] feat: add suggestions in prefer-regex-literals --- lib/rules/prefer-regex-literals.js | 112 ++++++++++++++-- tests/lib/rules/prefer-regex-literals.js | 157 +++++++++++++++++++++-- 2 files changed, 247 insertions(+), 22 deletions(-) diff --git a/lib/rules/prefer-regex-literals.js b/lib/rules/prefer-regex-literals.js index 02e2f5f44a0..38ff06de01b 100644 --- a/lib/rules/prefer-regex-literals.js +++ b/lib/rules/prefer-regex-literals.js @@ -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.", unexpectedRedundantRegExp: "Regular expression literal is unnecessarily wrapped within a 'RegExp' constructor.", unexpectedRedundantRegExpWithFlags: "Use regular expression literal with flags instead of the 'RegExp' constructor." } @@ -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. @@ -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 }); + + 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) { + 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)) + .join(""); + } + return { Program() { const scope = context.getScope(); @@ -306,10 +354,60 @@ 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 = []; + const fixable = sourceCode.getCommentsInside(node).length <= 0; + + if (fixable) { + const literalFlags = regexNode.regex.flags || ""; + const argFlags = getStringValue(node.arguments[1]) || ""; + + if (isValidRegexForEcmaVersion(regexNode.regex.pattern, argFlags)) { + outputs.push(`/${regexNode.regex.pattern}/${argFlags}`); + } + + const mergedFlags = mergeRegexFlags(literalFlags, argFlags); + + if ( + literalFlags && + argFlags && + !isFlagsEqual(mergedFlags, argFlags) + ) { + if (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 output = sourceCode.getText(regexNode); + const fixable = sourceCode.getCommentsInside(node).length <= 0 && + isValidRegexForEcmaVersion(regexNode.regex.pattern, regexNode.regex.flags); + + context.report({ + node, + messageId: "unexpectedRedundantRegExp", + suggest: fixable ? [ + { + messageId: "replaceWithLiteral", + fix(fixer) { + return fixer.replaceText(node, output); + } + } + ] : null + }); } } else if (hasOnlyStaticStringArguments(node)) { let regexContent = getStringValue(node.arguments[0]); @@ -320,15 +418,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; } diff --git a/tests/lib/rules/prefer-regex-literals.js b/tests/lib/rules/prefer-regex-literals.js index 429df4fd46d..69e13e89873 100644 --- a/tests/lib/rules/prefer-regex-literals.js +++ b/tests/lib/rules/prefer-regex-literals.js @@ -576,12 +576,18 @@ ruleTester.run("prefer-regex-literals", rule, { messageId: "unexpectedRedundantRegExp", type: "NewExpression", line: 1, - column: 1 + column: 1, + suggestions: [ + { + messageId: "replaceWithLiteral", + output: "/a/;" + } + ] } ] }, { - code: "new RegExp(/a/, 'u');", + code: "new RegExp(/a/, 'g');", options: [ { disallowRedundantWrapping: true @@ -592,12 +598,18 @@ ruleTester.run("prefer-regex-literals", rule, { messageId: "unexpectedRedundantRegExpWithFlags", type: "NewExpression", line: 1, - column: 1 + column: 1, + suggestions: [ + { + messageId: "replaceWithLiteralAndFlags", + output: "/a/g;" + } + ] } ] }, { - code: "new RegExp(/a/, `u`);", + code: "new RegExp(/a/g, 'g');", options: [ { disallowRedundantWrapping: true @@ -608,12 +620,18 @@ ruleTester.run("prefer-regex-literals", rule, { messageId: "unexpectedRedundantRegExpWithFlags", type: "NewExpression", line: 1, - column: 1 + column: 1, + suggestions: [ + { + messageId: "replaceWithLiteralAndFlags", + output: "/a/g;" + } + ] } ] }, { - code: "new RegExp(/a/, String.raw`u`);", + code: "new RegExp(/a/ig, 'g');", options: [ { disallowRedundantWrapping: true @@ -624,12 +642,22 @@ ruleTester.run("prefer-regex-literals", rule, { messageId: "unexpectedRedundantRegExpWithFlags", type: "NewExpression", line: 1, - column: 1 + column: 1, + suggestions: [ + { + messageId: "replaceWithLiteralAndFlags", + output: "/a/g;" + }, + { + messageId: "replaceWithLiteralAndFlags", + output: "/a/ig;" + } + ] } ] }, { - code: "new RegExp('a');", + code: "new RegExp(/a/g, 'ig');", options: [ { disallowRedundantWrapping: true @@ -637,19 +665,126 @@ ruleTester.run("prefer-regex-literals", rule, { ], errors: [ { - messageId: "unexpectedRegExp", + messageId: "unexpectedRedundantRegExpWithFlags", type: "NewExpression", line: 1, column: 1, suggestions: [ { - messageId: "replaceWithLiteral", - output: "/a/;" + messageId: "replaceWithLiteralAndFlags", + output: "/a/ig;" + } + ] + } + ] + }, + { + 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;" + } + ] + } + ] + }, + { + code: "new RegExp(/a/, `u`);", + options: [ + { + disallowRedundantWrapping: true + } + ], + errors: [ + { + messageId: "unexpectedRedundantRegExpWithFlags", + type: "NewExpression", + line: 1, + column: 1, + suggestions: [ + { + messageId: "replaceWithLiteralAndFlags", + output: "/a/u;" + } + ] + } + ] + }, + { + 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;" } ] } ] }, + { + 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: [ From 5eb9f15344998631370a5e391910ca09d307fc62 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 14 Dec 2022 01:36:49 +0900 Subject: [PATCH 2/9] add tests & refactor --- lib/rules/prefer-regex-literals.js | 39 ++++++++++++------------ tests/lib/rules/prefer-regex-literals.js | 22 +++++++++++++ 2 files changed, 42 insertions(+), 19 deletions(-) diff --git a/lib/rules/prefer-regex-literals.js b/lib/rules/prefer-regex-literals.js index 38ff06de01b..df872dc88b5 100644 --- a/lib/rules/prefer-regex-literals.js +++ b/lib/rules/prefer-regex-literals.js @@ -358,9 +358,8 @@ module.exports = { if (node.arguments.length === 2) { const outputs = []; - const fixable = sourceCode.getCommentsInside(node).length <= 0; - if (fixable) { + if (sourceCode.getCommentsInside(node).length <= 0) { const literalFlags = regexNode.regex.flags || ""; const argFlags = getStringValue(node.arguments[1]) || ""; @@ -368,14 +367,13 @@ module.exports = { outputs.push(`/${regexNode.regex.pattern}/${argFlags}`); } - const mergedFlags = mergeRegexFlags(literalFlags, argFlags); + if (literalFlags && argFlags) { + const mergedFlags = mergeRegexFlags(literalFlags, argFlags); - if ( - literalFlags && - argFlags && - !isFlagsEqual(mergedFlags, argFlags) - ) { - if (isValidRegexForEcmaVersion(regexNode.regex.pattern, mergedFlags)) { + if ( + !isFlagsEqual(mergedFlags, argFlags) && + isValidRegexForEcmaVersion(regexNode.regex.pattern, mergedFlags) + ) { outputs.push(`/${regexNode.regex.pattern}/${mergedFlags}`); } } @@ -392,21 +390,24 @@ module.exports = { })) }); } else { - const output = sourceCode.getText(regexNode); - const fixable = sourceCode.getCommentsInside(node).length <= 0 && - isValidRegexForEcmaVersion(regexNode.regex.pattern, regexNode.regex.flags); + const outputs = []; + + if ( + sourceCode.getCommentsInside(node).length <= 0 && + isValidRegexForEcmaVersion(regexNode.regex.pattern, regexNode.regex.flags) + ) { + outputs.push(sourceCode.getText(regexNode)); + } context.report({ node, messageId: "unexpectedRedundantRegExp", - suggest: fixable ? [ - { - messageId: "replaceWithLiteral", - fix(fixer) { - return fixer.replaceText(node, output); - } + suggest: outputs.map(output => ({ + messageId: "replaceWithLiteral", + fix(fixer) { + return fixer.replaceText(node, output); } - ] : null + })) }); } } else if (hasOnlyStaticStringArguments(node)) { diff --git a/tests/lib/rules/prefer-regex-literals.js b/tests/lib/rules/prefer-regex-literals.js index 69e13e89873..48f6d3eaf74 100644 --- a/tests/lib/rules/prefer-regex-literals.js +++ b/tests/lib/rules/prefer-regex-literals.js @@ -726,6 +726,28 @@ ruleTester.run("prefer-regex-literals", rule, { } ] }, + { + code: "new RegExp(/a/, `gi`);", + options: [ + { + disallowRedundantWrapping: true + } + ], + errors: [ + { + messageId: "unexpectedRedundantRegExpWithFlags", + type: "NewExpression", + line: 1, + column: 1, + suggestions: [ + { + messageId: "replaceWithLiteralAndFlags", + output: "/a/gi;" + } + ] + } + ] + }, { code: "new RegExp(/a/, String.raw`u`);", options: [ From c8cf3229a13e22b89bc0b7641b6e12990fe238b4 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 14 Dec 2022 01:41:28 +0900 Subject: [PATCH 3/9] change test case --- tests/lib/rules/prefer-regex-literals.js | 48 +++++++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/tests/lib/rules/prefer-regex-literals.js b/tests/lib/rules/prefer-regex-literals.js index 48f6d3eaf74..801a5ecbb62 100644 --- a/tests/lib/rules/prefer-regex-literals.js +++ b/tests/lib/rules/prefer-regex-literals.js @@ -587,7 +587,7 @@ ruleTester.run("prefer-regex-literals", rule, { ] }, { - code: "new RegExp(/a/, 'g');", + code: "new RegExp(/a/, 'u');", options: [ { disallowRedundantWrapping: true @@ -602,7 +602,29 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", - output: "/a/g;" + 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/;" } ] } @@ -748,6 +770,28 @@ ruleTester.run("prefer-regex-literals", rule, { } ] }, + { + code: "new RegExp('a');", + options: [ + { + disallowRedundantWrapping: true + } + ], + errors: [ + { + messageId: "unexpectedRegExp", + type: "NewExpression", + line: 1, + column: 1, + suggestions: [ + { + messageId: "replaceWithLiteral", + output: "/a/;" + } + ] + } + ] + }, { code: "new RegExp(/a/, String.raw`u`);", options: [ From 1a0074d557df30a90ba5e61c417967840ac6dbf1 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Thu, 15 Dec 2022 13:09:52 +0900 Subject: [PATCH 4/9] apply reviews --- lib/rules/prefer-regex-literals.js | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/rules/prefer-regex-literals.js b/lib/rules/prefer-regex-literals.js index df872dc88b5..142b2795b3f 100644 --- a/lib/rules/prefer-regex-literals.js +++ b/lib/rules/prefer-regex-literals.js @@ -303,12 +303,12 @@ module.exports = { * @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 }); + const validator = new RegExpValidator({ ecmaVersion: regexppEcmaVersion }); try { - RegExpValidatorInstance.validatePattern(pattern, 0, pattern.length, flags ? flags.includes("u") : false); + validator.validatePattern(pattern, 0, pattern.length, flags ? flags.includes("u") : false); if (flags) { - RegExpValidatorInstance.validateFlags(flags); + validator.validateFlags(flags); } return true; } catch { @@ -322,7 +322,7 @@ module.exports = { * @param {string} flagsB The regex flags. * @returns {boolean} True if two regex flags contain same flags. */ - function isFlagsEqual(flagsA, flagsB) { + function areFlagsEqual(flagsA, flagsB) { return [...flagsA].sort().join("") === [...flagsB].sort().join(""); } @@ -334,11 +334,12 @@ module.exports = { * @returns {string} The merged regex flags. */ function mergeRegexFlags(flagsA, flagsB) { - return [ + const flagsSet = new Set([ ...flagsA, ...flagsB - ].filter((flag, index, self) => index === self.indexOf(flag)) - .join(""); + ]); + + return [...flagsSet].join(""); } return { @@ -359,7 +360,7 @@ module.exports = { if (node.arguments.length === 2) { const outputs = []; - if (sourceCode.getCommentsInside(node).length <= 0) { + if (sourceCode.getCommentsInside(node).length === 0) { const literalFlags = regexNode.regex.flags || ""; const argFlags = getStringValue(node.arguments[1]) || ""; @@ -371,7 +372,7 @@ module.exports = { const mergedFlags = mergeRegexFlags(literalFlags, argFlags); if ( - !isFlagsEqual(mergedFlags, argFlags) && + !areFlagsEqual(mergedFlags, argFlags) && isValidRegexForEcmaVersion(regexNode.regex.pattern, mergedFlags) ) { outputs.push(`/${regexNode.regex.pattern}/${mergedFlags}`); @@ -393,7 +394,7 @@ module.exports = { const outputs = []; if ( - sourceCode.getCommentsInside(node).length <= 0 && + sourceCode.getCommentsInside(node).length === 0 && isValidRegexForEcmaVersion(regexNode.regex.pattern, regexNode.regex.flags) ) { outputs.push(sourceCode.getText(regexNode)); From 44514f9049bfe7da20a5980dd770fbcb94c91d8f Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sat, 17 Dec 2022 20:39:15 +0900 Subject: [PATCH 5/9] apply review and refactor --- lib/rules/prefer-regex-literals.js | 90 ++++++++++++++---------- tests/lib/rules/prefer-regex-literals.js | 70 ++++++++++++++++++ 2 files changed, 123 insertions(+), 37 deletions(-) diff --git a/lib/rules/prefer-regex-literals.js b/lib/rules/prefer-regex-literals.js index 142b2795b3f..5f76aad5cb4 100644 --- a/lib/rules/prefer-regex-literals.js +++ b/lib/rules/prefer-regex-literals.js @@ -342,6 +342,37 @@ module.exports = { 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 isFixableTo(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(); @@ -360,23 +391,22 @@ module.exports = { if (node.arguments.length === 2) { const outputs = []; - if (sourceCode.getCommentsInside(node).length === 0) { - const literalFlags = regexNode.regex.flags || ""; - const argFlags = getStringValue(node.arguments[1]) || ""; + const argFlags = getStringValue(node.arguments[1]) || ""; - if (isValidRegexForEcmaVersion(regexNode.regex.pattern, argFlags)) { - outputs.push(`/${regexNode.regex.pattern}/${argFlags}`); - } + if (isFixableTo(node, regexNode.regex.pattern, argFlags)) { + outputs.push(`/${regexNode.regex.pattern}/${argFlags}`); + } - if (literalFlags && argFlags) { - const mergedFlags = mergeRegexFlags(literalFlags, argFlags); + const literalFlags = regexNode.regex.flags || ""; - if ( - !areFlagsEqual(mergedFlags, argFlags) && - isValidRegexForEcmaVersion(regexNode.regex.pattern, mergedFlags) - ) { - outputs.push(`/${regexNode.regex.pattern}/${mergedFlags}`); - } + if (argFlags && literalFlags) { + const mergedFlags = mergeRegexFlags(literalFlags, argFlags); + + if ( + !areFlagsEqual(mergedFlags, argFlags) && + isFixableTo(node, regexNode.regex.pattern, mergedFlags) + ) { + outputs.push(`/${regexNode.regex.pattern}/${mergedFlags}`); } } @@ -386,7 +416,7 @@ module.exports = { suggest: outputs.map(output => ({ messageId: "replaceWithLiteralAndFlags", fix(fixer) { - return fixer.replaceText(node, output); + return fixer.replaceText(node, getSafeOutput(node, output)); } })) }); @@ -394,19 +424,22 @@ module.exports = { const outputs = []; if ( - sourceCode.getCommentsInside(node).length === 0 && - isValidRegexForEcmaVersion(regexNode.regex.pattern, regexNode.regex.flags) + isFixableTo(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, output); + return fixer.replaceText( + node, + getSafeOutput(node, output) + ); } })) }); @@ -420,13 +453,7 @@ module.exports = { flags = getStringValue(node.arguments[1]); } - if (!isValidRegexForEcmaVersion(regexContent, flags)) { - noFix = true; - } - - const tokenBefore = sourceCode.getTokenBefore(node); - - if (tokenBefore && !validPrecedingTokens.has(tokenBefore.value)) { + if (!isFixableTo(node, regexContent, flags)) { noFix = true; } @@ -434,10 +461,6 @@ module.exports = { noFix = true; } - if (sourceCode.getCommentsInside(node).length > 0) { - noFix = true; - } - if (regexContent && !noFix) { let charIncrease = 0; @@ -469,14 +492,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)); } }] }); diff --git a/tests/lib/rules/prefer-regex-literals.js b/tests/lib/rules/prefer-regex-literals.js index 801a5ecbb62..01446b9817e 100644 --- a/tests/lib/rules/prefer-regex-literals.js +++ b/tests/lib/rules/prefer-regex-literals.js @@ -851,6 +851,76 @@ ruleTester.run("prefer-regex-literals", rule, { } ] }, + { + code: "(a)\nnew RegExp(/b/);", + options: [{ + disallowRedundantWrapping: true + }], + errors: [ + { + messageId: "unexpectedRedundantRegExp", + type: "NewExpression", + line: 2, + column: 1, + suggestions: null + } + ] + }, + { + code: "(a)\nnew RegExp(/b/, 'g');", + options: [{ + disallowRedundantWrapping: true + }], + errors: [ + { + messageId: "unexpectedRedundantRegExpWithFlags", + type: "NewExpression", + line: 2, + column: 1, + suggestions: null + } + ] + }, + { + code: "a/RegExp(/foo/);", + options: [{ + disallowRedundantWrapping: true + }], + errors: [ + { + messageId: "unexpectedRedundantRegExp", + type: "CallExpression", + line: 1, + column: 3, + suggestions: [ + { + messageId: "replaceWithLiteral", + output: "a/ /foo/;" + } + ] + } + ] + }, + { + code: "RegExp(/foo/)in a;", + options: [{ + disallowRedundantWrapping: true + }], + errors: [ + { + messageId: "unexpectedRedundantRegExp", + type: "CallExpression", + line: 1, + column: 1, + suggestions: [ + { + messageId: "replaceWithLiteral", + output: "/foo/ in a;" + } + ] + } + ] + }, { code: "new RegExp((String?.raw)`a`);", errors: [ From b1b6a5705fce34b5454a5d0543c6b1e9c6a4aa17 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sat, 17 Dec 2022 20:49:12 +0900 Subject: [PATCH 6/9] refactor --- lib/rules/prefer-regex-literals.js | 35 ++++++++++++++---------- tests/lib/rules/prefer-regex-literals.js | 17 +++++++++++- 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/lib/rules/prefer-regex-literals.js b/lib/rules/prefer-regex-literals.js index 5f76aad5cb4..a5af331b44c 100644 --- a/lib/rules/prefer-regex-literals.js +++ b/lib/rules/prefer-regex-literals.js @@ -146,7 +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.", + replaceWithLiteralAndFlags: "Replace with an equivalent 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." } @@ -349,7 +349,7 @@ module.exports = { * @param {string} flags The regex flags * @returns {boolean} True if a node can be fixed to the given regex pattern and flags. */ - function isFixableTo(node, pattern, flags) { + function canFixTo(node, pattern, flags) { const tokenBefore = sourceCode.getTokenBefore(node); return sourceCode.getCommentsInside(node).length === 0 && @@ -389,12 +389,15 @@ module.exports = { const regexNode = node.arguments[0]; if (node.arguments.length === 2) { - const outputs = []; + const outputRegExps = []; const argFlags = getStringValue(node.arguments[1]) || ""; - if (isFixableTo(node, regexNode.regex.pattern, argFlags)) { - outputs.push(`/${regexNode.regex.pattern}/${argFlags}`); + if (canFixTo(node, regexNode.regex.pattern, argFlags)) { + outputRegExps.push({ + pattern: regexNode.regex.pattern, + flags: argFlags + }); } const literalFlags = regexNode.regex.flags || ""; @@ -404,28 +407,32 @@ module.exports = { if ( !areFlagsEqual(mergedFlags, argFlags) && - isFixableTo(node, regexNode.regex.pattern, mergedFlags) + canFixTo(node, regexNode.regex.pattern, mergedFlags) ) { - outputs.push(`/${regexNode.regex.pattern}/${mergedFlags}`); + outputRegExps.push({ + pattern: regexNode.regex.pattern, + flags: mergedFlags + }); } } context.report({ node, messageId: "unexpectedRedundantRegExpWithFlags", - suggest: outputs.map(output => ({ - messageId: "replaceWithLiteralAndFlags", + suggest: outputRegExps.map(({ flags, pattern }) => ({ + messageId: flags ? "replaceWithLiteralAndFlags" : "replaceWithLiteral", + data: { + flags + }, fix(fixer) { - return fixer.replaceText(node, getSafeOutput(node, output)); + return fixer.replaceText(node, getSafeOutput(node, `/${pattern}/${flags}`)); } })) }); } else { const outputs = []; - if ( - isFixableTo(node, regexNode.regex.pattern, regexNode.regex.flags) - ) { + if (canFixTo(node, regexNode.regex.pattern, regexNode.regex.flags)) { outputs.push(sourceCode.getText(regexNode)); } @@ -453,7 +460,7 @@ module.exports = { flags = getStringValue(node.arguments[1]); } - if (!isFixableTo(node, regexContent, flags)) { + if (!canFixTo(node, regexContent, flags)) { noFix = true; } diff --git a/tests/lib/rules/prefer-regex-literals.js b/tests/lib/rules/prefer-regex-literals.js index 01446b9817e..c09bd7e67c4 100644 --- a/tests/lib/rules/prefer-regex-literals.js +++ b/tests/lib/rules/prefer-regex-literals.js @@ -602,6 +602,9 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", + data: { + flags: "u" + }, output: "/a/u;" } ] @@ -623,7 +626,7 @@ ruleTester.run("prefer-regex-literals", rule, { column: 1, suggestions: [ { - messageId: "replaceWithLiteralAndFlags", + messageId: "replaceWithLiteral", output: "/a/;" } ] @@ -716,10 +719,16 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", + data: { + flags: "g" + }, output: "/a/g;" }, { messageId: "replaceWithLiteralAndFlags", + data: { + flags: "ig" + }, output: "/a/ig;" } ] @@ -742,6 +751,9 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", + data: { + flags: "u" + }, output: "/a/u;" } ] @@ -764,6 +776,9 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", + data: { + flags: "gi" + }, output: "/a/gi;" } ] From 9a673e3fe09561049084386cc99983ff201c4a23 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sat, 24 Dec 2022 02:03:34 +0900 Subject: [PATCH 7/9] apply review --- lib/rules/prefer-regex-literals.js | 37 ++++++++++++------------ tests/lib/rules/prefer-regex-literals.js | 33 +++++++++++++++++++-- 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/lib/rules/prefer-regex-literals.js b/lib/rules/prefer-regex-literals.js index a5af331b44c..3740dea86a7 100644 --- a/lib/rules/prefer-regex-literals.js +++ b/lib/rules/prefer-regex-literals.js @@ -146,7 +146,9 @@ 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 }}).", + replaceWithLiteralAndFlags: "Replace with an equivalent regular expression literal with flags '{{ flags }}'.", + replaceWithIntendedLiteral: "Replace with a regular expression literal.", + 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." } @@ -389,38 +391,37 @@ module.exports = { const regexNode = node.arguments[0]; if (node.arguments.length === 2) { - const outputRegExps = []; + const suggests = []; const argFlags = getStringValue(node.arguments[1]) || ""; if (canFixTo(node, regexNode.regex.pattern, argFlags)) { - outputRegExps.push({ + suggests.push({ + messageId: argFlags ? "replaceWithLiteralAndFlags" : "replaceWithLiteral", pattern: regexNode.regex.pattern, flags: argFlags }); } const literalFlags = regexNode.regex.flags || ""; - - if (argFlags && literalFlags) { - const mergedFlags = mergeRegexFlags(literalFlags, argFlags); - - if ( - !areFlagsEqual(mergedFlags, argFlags) && - canFixTo(node, regexNode.regex.pattern, mergedFlags) - ) { - outputRegExps.push({ - pattern: regexNode.regex.pattern, - flags: mergedFlags - }); - } + const mergedFlags = mergeRegexFlags(literalFlags, argFlags); + + if ( + !areFlagsEqual(mergedFlags, argFlags) && + canFixTo(node, regexNode.regex.pattern, mergedFlags) + ) { + suggests.push({ + messageId: mergedFlags ? "replaceWithIntendedLiteralAndFlags" : "replaceWithIntendedLiteral", + pattern: regexNode.regex.pattern, + flags: mergedFlags + }); } context.report({ node, messageId: "unexpectedRedundantRegExpWithFlags", - suggest: outputRegExps.map(({ flags, pattern }) => ({ - messageId: flags ? "replaceWithLiteralAndFlags" : "replaceWithLiteral", + suggest: suggests.map(({ flags, pattern, messageId }) => ({ + messageId, data: { flags }, diff --git a/tests/lib/rules/prefer-regex-literals.js b/tests/lib/rules/prefer-regex-literals.js index c09bd7e67c4..135bde60f84 100644 --- a/tests/lib/rules/prefer-regex-literals.js +++ b/tests/lib/rules/prefer-regex-literals.js @@ -628,6 +628,10 @@ ruleTester.run("prefer-regex-literals", rule, { { messageId: "replaceWithLiteral", output: "/a/;" + }, + { + messageId: "replaceWithIntendedLiteralAndFlags", + output: "/a/g;" } ] } @@ -674,7 +678,7 @@ ruleTester.run("prefer-regex-literals", rule, { output: "/a/g;" }, { - messageId: "replaceWithLiteralAndFlags", + messageId: "replaceWithIntendedLiteralAndFlags", output: "/a/ig;" } ] @@ -725,7 +729,7 @@ ruleTester.run("prefer-regex-literals", rule, { output: "/a/g;" }, { - messageId: "replaceWithLiteralAndFlags", + messageId: "replaceWithIntendedLiteralAndFlags", data: { flags: "ig" }, @@ -735,6 +739,31 @@ ruleTester.run("prefer-regex-literals", rule, { } ] }, + { + code: "new RegExp(/a/i, 'i');", + options: [ + { + disallowRedundantWrapping: true + } + ], + errors: [ + { + messageId: "unexpectedRedundantRegExpWithFlags", + type: "NewExpression", + line: 1, + column: 1, + suggestions: [ + { + messageId: "replaceWithLiteralAndFlags", + data: { + flags: "i" + }, + output: "/a/i;" + } + ] + } + ] + }, { code: "new RegExp(/a/, `u`);", options: [ From de98a290c4b6842460d2d0ce36d6a93b1865b87b Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 28 Dec 2022 01:03:46 +0900 Subject: [PATCH 8/9] Apply reviews --- lib/rules/prefer-regex-literals.js | 5 ++--- tests/lib/rules/prefer-regex-literals.js | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/rules/prefer-regex-literals.js b/lib/rules/prefer-regex-literals.js index 3740dea86a7..fdf18874e77 100644 --- a/lib/rules/prefer-regex-literals.js +++ b/lib/rules/prefer-regex-literals.js @@ -147,7 +147,6 @@ module.exports = { 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 }}'.", - replaceWithIntendedLiteral: "Replace with a regular expression literal.", 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." @@ -397,7 +396,7 @@ module.exports = { if (canFixTo(node, regexNode.regex.pattern, argFlags)) { suggests.push({ - messageId: argFlags ? "replaceWithLiteralAndFlags" : "replaceWithLiteral", + messageId: "replaceWithLiteralAndFlags", pattern: regexNode.regex.pattern, flags: argFlags }); @@ -411,7 +410,7 @@ module.exports = { canFixTo(node, regexNode.regex.pattern, mergedFlags) ) { suggests.push({ - messageId: mergedFlags ? "replaceWithIntendedLiteralAndFlags" : "replaceWithIntendedLiteral", + messageId: "replaceWithIntendedLiteralAndFlags", pattern: regexNode.regex.pattern, flags: mergedFlags }); diff --git a/tests/lib/rules/prefer-regex-literals.js b/tests/lib/rules/prefer-regex-literals.js index 135bde60f84..aa06b4d2e63 100644 --- a/tests/lib/rules/prefer-regex-literals.js +++ b/tests/lib/rules/prefer-regex-literals.js @@ -626,7 +626,7 @@ ruleTester.run("prefer-regex-literals", rule, { column: 1, suggestions: [ { - messageId: "replaceWithLiteral", + messageId: "replaceWithLiteralAndFlags", output: "/a/;" }, { From 3043c4c6d4f0a0b2b086661371f4484bc45d3637 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 28 Dec 2022 23:09:02 +0900 Subject: [PATCH 9/9] add data --- tests/lib/rules/prefer-regex-literals.js | 59 ++++++++++++++++-------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/tests/lib/rules/prefer-regex-literals.js b/tests/lib/rules/prefer-regex-literals.js index aa06b4d2e63..054d89be1d7 100644 --- a/tests/lib/rules/prefer-regex-literals.js +++ b/tests/lib/rules/prefer-regex-literals.js @@ -602,10 +602,10 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", + output: "/a/u;", data: { flags: "u" - }, - output: "/a/u;" + } } ] } @@ -627,11 +627,17 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", - output: "/a/;" + output: "/a/;", + data: { + flags: "" + } }, { messageId: "replaceWithIntendedLiteralAndFlags", - output: "/a/g;" + output: "/a/g;", + data: { + flags: "g" + } } ] } @@ -653,7 +659,10 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", - output: "/a/g;" + output: "/a/g;", + data: { + flags: "g" + } } ] } @@ -675,11 +684,17 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", - output: "/a/g;" + output: "/a/g;", + data: { + flags: "g" + } }, { messageId: "replaceWithIntendedLiteralAndFlags", - output: "/a/ig;" + output: "/a/ig;", + data: { + flags: "ig" + } } ] } @@ -701,7 +716,10 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", - output: "/a/ig;" + output: "/a/ig;", + data: { + flags: "ig" + } } ] } @@ -723,17 +741,17 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", + output: "/a/g;", data: { flags: "g" - }, - output: "/a/g;" + } }, { messageId: "replaceWithIntendedLiteralAndFlags", + output: "/a/ig;", data: { flags: "ig" - }, - output: "/a/ig;" + } } ] } @@ -755,10 +773,10 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", + output: "/a/i;", data: { flags: "i" - }, - output: "/a/i;" + } } ] } @@ -780,10 +798,10 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", + output: "/a/u;", data: { flags: "u" - }, - output: "/a/u;" + } } ] } @@ -805,10 +823,10 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", + output: "/a/gi;", data: { flags: "gi" - }, - output: "/a/gi;" + } } ] } @@ -852,7 +870,10 @@ ruleTester.run("prefer-regex-literals", rule, { suggestions: [ { messageId: "replaceWithLiteralAndFlags", - output: "/a/u;" + output: "/a/u;", + data: { + flags: "u" + } } ] }