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

prefer-starts-ends-with: Add auto-fix #711

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
2 changes: 1 addition & 1 deletion docs/rules/prefer-starts-ends-with.md
Expand Up @@ -2,6 +2,7 @@

There are several ways of checking whether a string starts or ends with a certain string, such as `string.indexOf('foo') === 0` or using a regex with `/^foo/` or `/foo$/`. ES2015 introduced simpler alternatives named [`String#startsWith()`](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/startsWith) and [`String#endsWith()`](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/endsWith). This rule enforces the use of those whenever possible.

This rule is partly fixable.

## Fail

Expand All @@ -10,7 +11,6 @@ There are several ways of checking whether a string starts or ends with a certai
/bar$/.test(foo);
```


## Pass

```js
Expand Down
2 changes: 1 addition & 1 deletion readme.md
Expand Up @@ -135,7 +135,7 @@ Configure it in `package.json`.
- [prefer-replace-all](docs/rules/prefer-replace-all.md) - Prefer `String#replaceAll()` over regex searches with the global flag. *(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-starts-ends-with](docs/rules/prefer-starts-ends-with.md) - Prefer `String#startsWith()` & `String#endsWith()` over more complex alternatives.
- [prefer-starts-ends-with](docs/rules/prefer-starts-ends-with.md) - Prefer `String#startsWith()` & `String#endsWith()` over more complex alternatives. *(partly fixable)*
- [prefer-string-slice](docs/rules/prefer-string-slice.md) - Prefer `String#slice()` over `String#substr()` and `String#substring()`. *(partly fixable)*
- [prefer-text-content](docs/rules/prefer-text-content.md) - Prefer `.textContent` over `.innerText`. *(fixable)*
- [prefer-trim-start-end](docs/rules/prefer-trim-start-end.md) - Prefer `String#trimStart()` / `String#trimEnd()` over `String#trimLeft()` / `String#trimRight()`. *(fixable)*
Expand Down
130 changes: 93 additions & 37 deletions rules/prefer-starts-ends-with.js
@@ -1,5 +1,11 @@
'use strict';
const {isParenthesized} = require('eslint-utils');
const getDocumentationUrl = require('./utils/get-documentation-url');
const methodSelector = require('./utils/method-selector');
const quoteString = require('./utils/quote-string');

const MESSAGE_STARTS_WITH = 'prefer-starts-with';
const MESSAGE_ENDS_WITH = 'prefer-ends-with';

const doesNotContain = (string, characters) => characters.every(character => !string.includes(character));

Expand All @@ -8,51 +14,96 @@ const isSimpleString = string => doesNotContain(
['^', '$', '+', '[', '{', '(', '\\', '.', '?', '*']
);

const regexTestSelector = [
methodSelector({name: 'test', length: 1}),
'[callee.object.regex]'
].join('');

const stringMatchSelector = [
methodSelector({name: 'match', length: 1}),
'[arguments.0.regex]'
].join('');

const checkRegex = ({pattern, flags}) => {
if (flags.includes('i')) {
return;
}

if (pattern.startsWith('^')) {
const string = pattern.slice(1);

if (isSimpleString(string)) {
return {
messageId: MESSAGE_STARTS_WITH,
string
};
}
}

if (pattern.endsWith('$')) {
const string = pattern.slice(0, -1);

if (isSimpleString(string)) {
return {
messageId: MESSAGE_ENDS_WITH,
string
};
}
}
};

const create = context => {
return {
CallExpression(node) {
const {callee} = node;
const {property} = callee;
const sourceCode = context.getSourceCode();

if (!(property && callee.type === 'MemberExpression')) {
return {
[regexTestSelector](node) {
const regexNode = node.callee.object;
const {regex} = regexNode;
const result = checkRegex(regex);
if (!result) {
return;
}

const arguments_ = node.arguments;

let regex;
if (property.name === 'test' && callee.object.regex) {
({regex} = callee.object);
} else if (
property.name === 'match' &&
arguments_ &&
arguments_[0] &&
arguments_[0].regex
) {
({regex} = arguments_[0]);
} else {
return;
}
context.report({
node,
messageId: result.messageId,
fix: fixer => {
const method = result.messageId === MESSAGE_STARTS_WITH ? 'startsWith' : 'endsWith';
const [target] = node.arguments;
let targetString = sourceCode.getText(target);

if (
// If regex is parenthesized, we can use it, so we don't need add again
!isParenthesized(regexNode, sourceCode) &&
(isParenthesized(target, sourceCode) || target.type === 'AwaitExpression')
) {
targetString = `(${targetString})`;
}

// The regex literal always starts with `/` or `(`, so we don't need check ASI

if (regex.flags && regex.flags.includes('i')) {
return [
// Replace regex with string
fixer.replaceText(regexNode, targetString),
// `.test` => `.startsWith` / `.endsWith`
fixer.replaceText(node.callee.property, method),
// Replace argument with result.string
fixer.replaceText(target, quoteString(result.string))
];
}
});
},
[stringMatchSelector](node) {
const {regex} = node.arguments[0];
const result = checkRegex(regex);
if (!result) {
return;
}

const {pattern} = regex;
if (pattern.startsWith('^') && isSimpleString(pattern.slice(1))) {
context.report({
node,
message: 'Prefer `String#startsWith()` over a regex with `^`.'
});
} else if (
pattern.endsWith('$') &&
isSimpleString(pattern.slice(0, -1))
) {
context.report({
node,
message: 'Prefer `String#endsWith()` over a regex with `$`.'
});
}
context.report({
node,
messageId: result.messageId
});
}
};
};
Expand All @@ -63,6 +114,11 @@ module.exports = {
type: 'suggestion',
docs: {
url: getDocumentationUrl(__filename)
}
},
messages: {
[MESSAGE_STARTS_WITH]: 'Prefer `String#startsWith()` over a regex with `^`.',
[MESSAGE_ENDS_WITH]: 'Prefer `String#endsWith()` over a regex with `$`.'
},
fixable: 'code'
}
};
121 changes: 93 additions & 28 deletions test/prefer-starts-ends-with.js
@@ -1,27 +1,16 @@
import test from 'ava';
import {outdent} from 'outdent';
import avaRuleTester from 'eslint-ava-rule-tester';
import rule from '../rules/prefer-starts-ends-with';

const ruleTester = avaRuleTester(test, {
env: {
es6: true
parserOptions: {
ecmaVersion: 2020
}
});

const errors = {
startsWith: [
{
ruleId: 'prefer-starts-ends-with',
message: 'Prefer `String#startsWith()` over a regex with `^`.'
}
],
endsWith: [
{
ruleId: 'prefer-starts-ends-with',
message: 'Prefer `String#endsWith()` over a regex with `$`.'
}
]
};
const MESSAGE_STARTS_WITH = 'prefer-starts-with';
const MESSAGE_ENDS_WITH = 'prefer-ends-with';

const validRegex = [
/foo/,
Expand Down Expand Up @@ -58,17 +47,93 @@ ruleTester.run('prefer-starts-ends-with', rule, {
'test()',
'test.test()',
'startWith("bar")',
'foo()()'
'foo()()',

...validRegex.map(re => `${re}.test(bar)`),
...validRegex.map(re => `bar.match(${re})`)
],
invalid: [
...invalidRegex.map(re => {
let messageId = MESSAGE_STARTS_WITH;
let method = 'startsWith';
let string = re.source;

if (string.startsWith('^')) {
string = string.slice(1);
} else {
messageId = MESSAGE_ENDS_WITH;
method = 'endsWith';
string = string.slice(0, -1);
}

return {
code: `${re}.test(bar)`,
output: `bar.${method}('${string}')`,
errors: [{messageId}]
};
}),
// Parenthesized
{
code: '/^b/.test(("a"))',
output: '("a").startsWith((\'b\'))',
errors: [{messageId: MESSAGE_STARTS_WITH}]
},
{
code: '(/^b/).test(("a"))',
output: '("a").startsWith((\'b\'))',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

string 'b' is parenthesized because the original ("a") left the () when replacing text, I'm not going to remove them.

errors: [{messageId: MESSAGE_STARTS_WITH}]
},
{
code: 'const fn = async () => /^b/.test(await foo)',
output: 'const fn = async () => (await foo).startsWith(\'b\')',
errors: [{messageId: MESSAGE_STARTS_WITH}]
},
{
code: 'const fn = async () => (/^b/).test(await foo)',
output: 'const fn = async () => (await foo).startsWith(\'b\')',
errors: [{messageId: MESSAGE_STARTS_WITH}]
},
// Comments
{
code: outdent`
if (
/* comment 1 */
/^b/
/* comment 2 */
.test
/* comment 3 */
(
/* comment 4 */
foo
/* comment 5 */
)
) {}
`,
output: outdent`
if (
/* comment 1 */
foo
/* comment 2 */
.startsWith
/* comment 3 */
(
/* comment 4 */
'b'
/* comment 5 */
)
) {}
`,
errors: [{messageId: MESSAGE_STARTS_WITH}]
},

...invalidRegex.map(re => {
const code = `bar.match(${re})`;
const messageId = re.source.startsWith('^') ? MESSAGE_STARTS_WITH : MESSAGE_ENDS_WITH;
return {
code,
output: code,
errors: [{messageId}]
};
})
]
.concat(validRegex.map(re => `${re}.test(bar)`))
.concat(validRegex.map(re => `bar.match(${re})`)),
invalid: []
.concat(invalidRegex.map(re => ({
code: `${re}.test(bar)`,
errors: errors[`${re}`.startsWith('/^') ? 'startsWith' : 'endsWith']
})))
.concat(invalidRegex.map(re => ({
code: `bar.match(${re})`,
errors: errors[`${re}`.startsWith('/^') ? 'startsWith' : 'endsWith']
})))
});