Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
prefer-regexp-test: Use suggestions if not sure regexp without g
…flag (#1954)
  • Loading branch information
fisker committed Nov 8, 2022
1 parent aca21f2 commit 505a203
Show file tree
Hide file tree
Showing 6 changed files with 258 additions and 231 deletions.
2 changes: 1 addition & 1 deletion docs/rules/prefer-regexp-test.md
Expand Up @@ -2,7 +2,7 @@

✅ This rule is enabled in the `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs).

🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).
🔧💡 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) and manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).

<!-- end auto-generated rule header -->
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
Expand Down
2 changes: 1 addition & 1 deletion readme.md
Expand Up @@ -137,7 +137,7 @@ Use a [preset config](#preset-configs) or configure each rules in `package.json`
| [prefer-prototype-methods](docs/rules/prefer-prototype-methods.md) | Prefer borrowing methods from the prototype instead of the instance. || 🔧 | |
| [prefer-query-selector](docs/rules/prefer-query-selector.md) | Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()`. || 🔧 | |
| [prefer-reflect-apply](docs/rules/prefer-reflect-apply.md) | Prefer `Reflect.apply()` over `Function#apply()`. || 🔧 | |
| [prefer-regexp-test](docs/rules/prefer-regexp-test.md) | Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`. || 🔧 | |
| [prefer-regexp-test](docs/rules/prefer-regexp-test.md) | Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`. || 🔧 | 💡 |
| [prefer-set-has](docs/rules/prefer-set-has.md) | Prefer `Set#has()` over `Array#includes()` when checking for existence or non-existence. || 🔧 | 💡 |
| [prefer-spread](docs/rules/prefer-spread.md) | Prefer the spread operator over `Array.from(…)`, `Array#concat(…)`, `Array#slice()` and `String#split('')`. || 🔧 | 💡 |
| [prefer-string-replace-all](docs/rules/prefer-string-replace-all.md) | Prefer `String#replaceAll()` over regex searches with the global flag. | | 🔧 | |
Expand Down
44 changes: 32 additions & 12 deletions rules/prefer-regexp-test.js
Expand Up @@ -8,9 +8,11 @@ const shouldAddParenthesesToMemberExpressionObject = require('./utils/should-add

const REGEXP_EXEC = 'regexp-exec';
const STRING_MATCH = 'string-match';
const SUGGESTION = 'suggestion';
const messages = {
[REGEXP_EXEC]: 'Prefer `.test(…)` over `.exec(…)`.',
[STRING_MATCH]: 'Prefer `RegExp#test(…)` over `String#match(…)`.',
[SUGGESTION]: 'Switch to `RegExp#test(…)`.',
};

const cases = [
Expand Down Expand Up @@ -75,6 +77,21 @@ const isRegExpNode = node =>
&& node.callee.name === 'RegExp'
);

const isRegExpWithoutGFlag = (node, scope) => {
const staticResult = getStaticValue(node, scope);

// Don't know if there is `g` flag
if (!staticResult) {
return false;
}

const {value} = staticResult;
return (
Object.prototype.toString.call(value) === '[object RegExp]'
&& !value.flags.includes('g')
);
};

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => Object.fromEntries(
cases.map(checkCase => [
Expand All @@ -97,20 +114,22 @@ const create = context => Object.fromEntries(
messageId: type,
};

if (!isRegExpNode(regexpNode)) {
const staticResult = getStaticValue(regexpNode, context.getScope());
if (staticResult) {
const {value} = staticResult;
if (
Object.prototype.toString.call(value) !== '[object RegExp]'
|| value.flags.includes('g')
) {
return problem;
}
}
const fixFunction = fixer => fix(fixer, nodes, context.getSourceCode());

if (
isRegExpNode(regexpNode)
|| isRegExpWithoutGFlag(regexpNode, context.getScope())
) {
problem.fix = fixFunction;
} else {
problem.suggest = [
{
messageId: SUGGESTION,
fix: fixFunction,
},
];
}

problem.fix = fixer => fix(fixer, nodes, context.getSourceCode());
return problem;
},
]),
Expand All @@ -125,6 +144,7 @@ module.exports = {
description: 'Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.',
},
fixable: 'code',
hasSuggestions: true,
messages,
},
};
55 changes: 27 additions & 28 deletions test/prefer-regexp-test.mjs
Expand Up @@ -3,7 +3,7 @@ import {getTester} from './utils/test.mjs';

const {test} = getTester(import.meta);

test({
test.snapshot({
valid: [
'const bar = !re.test(foo)',
// Not `boolean`
Expand Down Expand Up @@ -41,38 +41,33 @@ test({
'if (foo.match(1n)) {}',
'if (foo.match(true)) {}',
],
invalid: [],
});

test.snapshot({
valid: [],
invalid: [
// `String#match()`
'const bar = !foo.match(re)',
'const bar = Boolean(foo.match(re))',
'if (foo.match(re)) {}',
'const bar = foo.match(re) ? 1 : 2',
'while (foo.match(re)) foo = foo.slice(1);',
'do {foo = foo.slice(1)} while (foo.match(re));',
'for (; foo.match(re); ) foo = foo.slice(1);',
'const re = /a/; const bar = !foo.match(re)',
'const re = /a/; const bar = Boolean(foo.match(re))',
'const re = /a/; if (foo.match(re)) {}',
'const re = /a/; const bar = foo.match(re) ? 1 : 2',
'const re = /a/; while (foo.match(re)) foo = foo.slice(1);',
'const re = /a/; do {foo = foo.slice(1)} while (foo.match(re));',
'const re = /a/; for (; foo.match(re); ) foo = foo.slice(1);',

// `RegExp#exec()`
'const bar = !re.exec(foo)',
'const bar = Boolean(re.exec(foo))',
'if (re.exec(foo)) {}',
'const bar = re.exec(foo) ? 1 : 2',
'while (re.exec(foo)) foo = foo.slice(1);',
'do {foo = foo.slice(1)} while (re.exec(foo));',
'for (; re.exec(foo); ) foo = foo.slice(1);',
'const re = /a/; const bar = !re.exec(foo)',
'const re = /a/; const bar = Boolean(re.exec(foo))',
'const re = /a/; if (re.exec(foo)) {}',
'const re = /a/; const bar = re.exec(foo) ? 1 : 2',
'const re = /a/; while (re.exec(foo)) foo = foo.slice(1);',
'const re = /a/; do {foo = foo.slice(1)} while (re.exec(foo));',
'const re = /a/; for (; re.exec(foo); ) foo = foo.slice(1);',

// Parentheses
'if ((0, foo).match(re)) {}',
'if ((0, foo).match((re))) {}',
'if ((foo).match(re)) {}',
'if ((foo).match((re))) {}',
'const re = /a/; if ((0, foo).match(re)) {}',
'const re = /a/; if ((0, foo).match((re))) {}',
'const re = /a/; if ((foo).match(re)) {}',
'const re = /a/; if ((foo).match((re))) {}',
'if (foo.match(/re/)) {}',
'if (foo.match(bar)) {}',
'if (foo.match(bar.baz)) {}',
'const re = /a/; if (foo.match(re)) {}',
'const bar = {bar: /a/}; if (foo.match(bar.baz)) {}',
'if (foo.match(bar.baz())) {}',
'if (foo.match(new RegExp("re", "g"))) {}',
'if (foo.match(new SomeRegExp())) {}',
Expand All @@ -89,14 +84,14 @@ test.snapshot({
'if ((foo).match(new SomeRegExp)) {}',
'if ((foo).match(bar?.baz)) {}',
'if ((foo).match(bar?.baz())) {}',
'if ((foo).match(bar || baz)) {}',
'const bar = false; const baz = /a/; if ((foo).match(bar || baz)) {}',
outdent`
async function a() {
if ((foo).match(await bar())) {}
}
`,
// Should not need handle ASI problem
'if (foo.match([re][0])) {}',
'const re = [/a/]; if (foo.match([re][0])) {}',

// Comments
outdent`
Expand Down Expand Up @@ -151,6 +146,10 @@ test.snapshot({
const regex = /weird/gyi;
if (regex.exec(foo));
`,
outdent`
let re = new RegExp('foo', 'g');
if(str.match(re));
`,
],
});

Expand Down

0 comments on commit 505a203

Please sign in to comment.