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

Add prefer-regexp-test rule #970

Merged
merged 12 commits into from
Jan 8, 2021
21 changes: 21 additions & 0 deletions docs/rules/prefer-regexp-test.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`

When you want to know whether a pattern is found in a string, use [`RegExp#test()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/test) instead of [`String#match()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/match) and [`RegExp#exec()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/exec).

This rule is fixable.

## Fail

```js
if (string.match(/unicorn/)) {}
```

```js
if (/unicorn/.exec(string)) {}
```

## Pass

```js
if (/unicorn/.test(string)) {}
```
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ module.exports = {
'unicorn/prefer-optional-catch-binding': 'error',
'unicorn/prefer-query-selector': 'error',
'unicorn/prefer-reflect-apply': 'error',
'unicorn/prefer-regexp-test': 'error',
'unicorn/prefer-set-has': 'error',
'unicorn/prefer-spread': 'error',
// TODO: Enable this by default when targeting Node.js 16.
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ Configure it in `package.json`.
"unicorn/prefer-optional-catch-binding": "error",
"unicorn/prefer-query-selector": "error",
"unicorn/prefer-reflect-apply": "error",
"unicorn/prefer-regexp-test": "error",
"unicorn/prefer-set-has": "error",
"unicorn/prefer-spread": "error",
"unicorn/prefer-string-replace-all": "off",
Expand Down Expand Up @@ -160,6 +161,7 @@ Configure it in `package.json`.
- [prefer-optional-catch-binding](docs/rules/prefer-optional-catch-binding.md) - Prefer omitting the `catch` binding parameter. *(fixable)*
- [prefer-query-selector](docs/rules/prefer-query-selector.md) - Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()`. *(partly fixable)*
- [prefer-reflect-apply](docs/rules/prefer-reflect-apply.md) - Prefer `Reflect.apply()` over `Function#apply()`. *(fixable)*
- [prefer-regexp-test](docs/rules/prefer-regexp-test.md) - Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`. *(fixable)*
- [prefer-set-has](docs/rules/prefer-set-has.md) - Prefer `Set#has()` over `Array#includes()` when checking for existence or non-existence. *(fixable)*
- [prefer-spread](docs/rules/prefer-spread.md) - Prefer the spread operator over `Array.from()`. *(fixable)*
- [prefer-string-replace-all](docs/rules/prefer-string-replace-all.md) - Prefer `String#replaceAll()` over regex searches with the global flag. *(fixable)*
Expand Down
101 changes: 101 additions & 0 deletions rules/prefer-regexp-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
'use strict';
const {isParenthesized} = require('eslint-utils');
const getDocumentationUrl = require('./utils/get-documentation-url');
const methodSelector = require('./utils/method-selector');
const {isBooleanNode} = require('./utils/boolean');

const MESSAGE_ID_REGEXP_EXEC = 'regexp-exec';
const MESSAGE_ID_STRING_MATCH = 'string-match';
const messages = {
[MESSAGE_ID_REGEXP_EXEC]: 'Prefer `.test(…)` over `.exec(…)`.',
[MESSAGE_ID_STRING_MATCH]: 'Prefer `RegExp#test(…)` over `String#match(…)`.'
};

const regExpExecCallSelector = methodSelector({
name: 'exec',
length: 1
});

const stringMatchCallSelector = methodSelector({
name: 'match',
length: 1
});

const create = context => {
const sourceCode = context.getSourceCode();

return {
[regExpExecCallSelector](node) {
if (isBooleanNode(node)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would do an early-return here too.

node = node.callee.property;
context.report({
node,
messageId: MESSAGE_ID_REGEXP_EXEC,
fix: fixer => fixer.replaceText(node, 'test')
});
}
},
[stringMatchCallSelector](node) {
if (!isBooleanNode(node)) {
return;
}

const regexpNode = node.arguments[0];

if (regexpNode.type === 'Literal' && !regexpNode.regex) {
return;
}

const stringNode = node.callee.object;

context.report({
node,
messageId: MESSAGE_ID_STRING_MATCH,
* fix(fixer) {
yield fixer.replaceText(node.callee.property, 'test');

let stringText = sourceCode.getText(stringNode);
if (
!isParenthesized(regexpNode, sourceCode) &&
// Only `SequenceExpression` need add parentheses
stringNode.type === 'SequenceExpression'
) {
stringText = `(${stringText})`;
}

yield fixer.replaceText(regexpNode, stringText);

let regexpText = sourceCode.getText(regexpNode);
if (
!isParenthesized(stringNode, sourceCode) &&
!(
regexpNode.type === 'Literal' ||
regexpNode.type === 'Identifier' ||
regexpNode.type === 'MemberExpression' ||
regexpNode.type === 'CallExpression' ||
(regexpNode.type === 'NewExpression' && regexpText.endsWith(')'))
)
) {
regexpText = `(${regexpText})`;
}

// All nodes pass `isBooleanNode` can't have ASI problem
fisker marked this conversation as resolved.
Show resolved Hide resolved

yield fixer.replaceText(stringNode, regexpText);
}
});
}
};
};

module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
url: getDocumentationUrl(__filename)
},
fixable: 'code',
messages
}
};
110 changes: 110 additions & 0 deletions test/prefer-regexp-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import {outdent} from 'outdent';
import {test} from './utils/test';

test({
valid: [
'const bar = !re.test(foo)',
// Not `boolean`
'const matches = foo.match(re) || []',
'const matches = foo.match(re)',
'const matches = re.exec(foo)',
'while (foo = re.exec(bar)) {}',
'while ((foo = re.exec(bar))) {}',

// Method not match
'if (foo.notMatch(re)) {}',
'if (re.notExec(foo)) {}',
// Not `CallExpression`
'if (foo.match) {}',
'if (re.exec) {}',
// Computed
'if (foo[match](re)) {}',
'if (re[exec](foo)) {}',
'if (foo["match"](re)) {}',
'if (re["exec"](foo)) {}',
// Not `MemberExpression`
'if (match(re)) {}',
'if (exec(foo)) {}',
// More/Less arguments
'if (foo.match()) {}',
'if (re.exec()) {}',
'if (foo.match(re, another)) {}',
'if (re.exec(foo, another)) {}',
'if (foo.match(...[regexp])) {}',
'if (re.exec(...[string])) {}',
// Not regex
'if (foo.match(1)) {}',
'if (foo.match("1")) {}',
'if (foo.match(null)) {}',
'if (foo.match(1n)) {}',
'if (foo.match(true)) {}'
],
invalid: []
});

test.visualize([
// `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);',

// `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);',

// Parentheses
'if ((0, foo).match(re)) {}',
'if ((0, foo).match((re))) {}',
'if ((foo).match(re)) {}',
'if ((foo).match((re))) {}',
'if (foo.match(/re/)) {}',
'if (foo.match(bar)) {}',
'if (foo.match(bar.baz)) {}',
'if (foo.match(bar.baz())) {}',
'if (foo.match(new RegExp("re", "g"))) {}',
'if (foo.match(new SomeRegExp())) {}',
'if (foo.match(new SomeRegExp)) {}',
'if (foo.match(bar?.baz)) {}',
'if (foo.match(bar?.baz())) {}',
'if (foo.match(bar || baz)) {}',
outdent`
async function a() {
if (foo.match(await bar())) {}
}
`,
'if ((foo).match(/re/)) {}',
'if ((foo).match(new SomeRegExp)) {}',
'if ((foo).match(bar?.baz)) {}',
'if ((foo).match(bar?.baz())) {}',
'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])) {}',

// Comments
outdent`
async function a() {
if (
/* 1 */ foo() /* 2 */
./* 3 */ match /* 4 */ (
/* 5 */ await /* 6 */ bar() /* 7 */
,
/* 8 */
)
) {}
}
`
]);
Loading