Skip to content

Commit

Permalink
feat(eslint-plugin): [pref-str-starts/ends-with] optional chain… (#1357)
Browse files Browse the repository at this point in the history 
* feat(eslint-plugin): [pref-str-starts/ends-with] optional chain support

* chore: switch already fixed rules to `:matches`
  • Loading branch information
@bradzacher
bradzacher committed Dec 21, 2019
1 parent 099225a commit fd37bc3
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 53 deletions.
2 changes: 1 addition & 1 deletion packages/eslint-plugin/src/rules/no-require-imports.ts
View file
Expand Up @@ -18,7 +18,7 @@ export default util.createRule({
defaultOptions: [],
create(context) {
return {
'CallExpression > Identifier[name="require"], OptionalCallExpression > Identifier[name="require"]'(
':matches(CallExpression, OptionalCallExpression) > Identifier[name="require"]'(
node: TSESTree.Identifier,
): void {
context.report({
Expand Down
14 changes: 2 additions & 12 deletions packages/eslint-plugin/src/rules/prefer-includes.ts
View file
Expand Up @@ -122,12 +122,7 @@ export default createRule({
}

return {
[[
"BinaryExpression > CallExpression.left > MemberExpression.callee[property.name='indexOf'][computed=false]",
"BinaryExpression > OptionalCallExpression.left > MemberExpression.callee[property.name='indexOf'][computed=false]",
"BinaryExpression > CallExpression.left > OptionalMemberExpression.callee[property.name='indexOf'][computed=false]",
"BinaryExpression > OptionalCallExpression.left > OptionalMemberExpression.callee[property.name='indexOf'][computed=false]",
].join(', ')](
"BinaryExpression > :matches(CallExpression, OptionalCallExpression).left > :matches(MemberExpression, OptionalMemberExpression).callee[property.name='indexOf'][computed=false]"(
node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression,
): void {
// Check if the comparison is equivalent to `includes()`.
Expand Down Expand Up @@ -181,12 +176,7 @@ export default createRule({
},

// /bar/.test(foo)
[[
'CallExpression > MemberExpression.callee[property.name="test"][computed=false]',
'OptionalCallExpression > MemberExpression.callee[property.name="test"][computed=false]',
'CallExpression > OptionalMemberExpression.callee[property.name="test"][computed=false]',
'OptionalCallExpression > OptionalMemberExpression.callee[property.name="test"][computed=false]',
].join(', ')](
':matches(CallExpression, OptionalCallExpression) > :matches(MemberExpression, OptionalMemberExpression).callee[property.name="test"][computed=false]'(
node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression,
): void {
const callNode = node.parent as
Expand Down
113 changes: 77 additions & 36 deletions packages/eslint-plugin/src/rules/prefer-string-starts-ends-with.ts
View file
Expand Up @@ -143,7 +143,10 @@ export default createRule({
node: TSESTree.Node,
expectedObjectNode: TSESTree.Node,
): boolean {
if (node.type === AST_NODE_TYPES.MemberExpression) {
if (
node.type === AST_NODE_TYPES.MemberExpression ||
node.type === AST_NODE_TYPES.OptionalMemberExpression
) {
return (
getPropertyName(node, globalScope) === 'length' &&
isSameTokens(node.object, expectedObjectNode)
Expand Down Expand Up @@ -191,7 +194,7 @@ export default createRule({
* @param node The member expression node to get.
*/
function getPropertyRange(
node: TSESTree.MemberExpression,
node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression,
): [number, number] {
const dotOrOpenBracket = sourceCode.getTokenAfter(
node.object,
Expand Down Expand Up @@ -269,26 +272,30 @@ export default createRule({
* @param fixer The rule fixer.
* @param node The node which was reported.
* @param kind The kind of the report.
* @param negative The flag to fix to negative condition.
* @param isNegative The flag to fix to negative condition.
*/
function* fixWithRightOperand(
fixer: TSESLint.RuleFixer,
node: TSESTree.BinaryExpression,
kind: 'start' | 'end',
negative: boolean,
isNegative: boolean,
isOptional: boolean,
): IterableIterator<TSESLint.RuleFix> {
// left is CallExpression or MemberExpression.
const leftNode = (node.left.type === AST_NODE_TYPES.CallExpression
const leftNode = (node.left.type === AST_NODE_TYPES.CallExpression ||
node.left.type === AST_NODE_TYPES.OptionalCallExpression
? node.left.callee
: node.left) as TSESTree.MemberExpression;
: node.left) as
| TSESTree.MemberExpression
| TSESTree.OptionalMemberExpression;
const propertyRange = getPropertyRange(leftNode);

if (negative) {
if (isNegative) {
yield fixer.insertTextBefore(node, '!');
}
yield fixer.replaceTextRange(
[propertyRange[0], node.right.range[0]],
`.${kind}sWith(`,
`${isOptional ? '?.' : '.'}${kind}sWith(`,
);
yield fixer.replaceTextRange([node.right.range[1], node.range[1]], ')');
}
Expand All @@ -306,16 +313,21 @@ export default createRule({
node: TSESTree.BinaryExpression,
kind: 'start' | 'end',
negative: boolean,
isOptional: boolean,
): IterableIterator<TSESLint.RuleFix> {
const callNode = node.left as TSESTree.CallExpression;
const calleeNode = callNode.callee as TSESTree.MemberExpression;
const callNode = node.left as
| TSESTree.CallExpression
| TSESTree.OptionalCallExpression;
const calleeNode = callNode.callee as
| TSESTree.MemberExpression
| TSESTree.OptionalMemberExpression;

if (negative) {
yield fixer.insertTextBefore(node, '!');
}
yield fixer.replaceTextRange(
getPropertyRange(calleeNode),
`.${kind}sWith`,
`${isOptional ? '?.' : '.'}${kind}sWith`,
);
yield fixer.removeRange([callNode.range[1], node.range[1]]);
}
Expand All @@ -325,13 +337,18 @@ export default createRule({
// foo.charAt(0) === "a"
// foo[foo.length - 1] === "a"
// foo.charAt(foo.length - 1) === "a"
[String([
'BinaryExpression > MemberExpression.left[computed=true]',
'BinaryExpression > CallExpression.left > MemberExpression.callee[property.name="charAt"][computed=false]',
])](node: TSESTree.MemberExpression): void {
[[
'BinaryExpression > :matches(MemberExpression, OptionalMemberExpression).left[computed=true]',
'BinaryExpression > :matches(CallExpression, OptionalCallExpression).left > :matches(MemberExpression, OptionalMemberExpression).callee[property.name="charAt"][computed=false]',
].join(', ')](
node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression,
): void {
let parentNode = node.parent!;
let indexNode: TSESTree.Node | null = null;
if (parentNode.type === AST_NODE_TYPES.CallExpression) {
if (
parentNode.type === AST_NODE_TYPES.CallExpression ||
parentNode.type === AST_NODE_TYPES.OptionalCallExpression
) {
if (parentNode.arguments.length === 1) {
indexNode = parentNode.arguments[0];
}
Expand Down Expand Up @@ -368,16 +385,19 @@ export default createRule({
eqNode,
isStartsWith ? 'start' : 'end',
eqNode.operator.startsWith('!'),
node.optional,
);
},
});
},

// foo.indexOf('bar') === 0
'BinaryExpression > CallExpression.left > MemberExpression.callee[property.name="indexOf"][computed=false]'(
node: TSESTree.MemberExpression,
'BinaryExpression > :matches(CallExpression, OptionalCallExpression).left > :matches(MemberExpression, OptionalMemberExpression).callee[property.name="indexOf"][computed=false]'(
node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression,
): void {
const callNode = node.parent! as TSESTree.CallExpression;
const callNode = node.parent as
| TSESTree.CallExpression
| TSESTree.OptionalCallExpression;
const parentNode = callNode.parent!;

if (
Expand All @@ -399,17 +419,20 @@ export default createRule({
parentNode,
'start',
parentNode.operator.startsWith('!'),
node.optional,
);
},
});
},

// foo.lastIndexOf('bar') === foo.length - 3
// foo.lastIndexOf(bar) === foo.length - bar.length
'BinaryExpression > CallExpression.left > MemberExpression.callee[property.name="lastIndexOf"][computed=false]'(
node: TSESTree.MemberExpression,
'BinaryExpression > :matches(CallExpression, OptionalCallExpression).left > :matches(MemberExpression, OptionalMemberExpression).callee[property.name="lastIndexOf"][computed=false]'(
node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression,
): void {
const callNode = node.parent! as TSESTree.CallExpression;
const callNode = node.parent! as
| TSESTree.CallExpression
| TSESTree.OptionalCallExpression;
const parentNode = callNode.parent!;

if (
Expand All @@ -434,17 +457,20 @@ export default createRule({
parentNode,
'end',
parentNode.operator.startsWith('!'),
node.optional,
);
},
});
},

// foo.match(/^bar/) === null
// foo.match(/bar$/) === null
'BinaryExpression > CallExpression.left > MemberExpression.callee[property.name="match"][computed=false]'(
node: TSESTree.MemberExpression,
'BinaryExpression > :matches(CallExpression, OptionalCallExpression).left > :matches(MemberExpression, OptionalMemberExpression).callee[property.name="match"][computed=false]'(
node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression,
): void {
const callNode = node.parent as TSESTree.CallExpression;
const callNode = node.parent as
| TSESTree.CallExpression
| TSESTree.OptionalCallExpression;
const parentNode = callNode.parent as TSESTree.BinaryExpression;
if (
!isEqualityComparison(parentNode) ||
Expand Down Expand Up @@ -472,7 +498,9 @@ export default createRule({
}
yield fixer.replaceTextRange(
getPropertyRange(node),
`.${isStartsWith ? 'start' : 'end'}sWith`,
`${node.optional ? '?.' : '.'}${
isStartsWith ? 'start' : 'end'
}sWith`,
);
yield fixer.replaceText(
callNode.arguments[0],
Expand All @@ -489,11 +517,15 @@ export default createRule({
// foo.substring(0, 3) === 'bar'
// foo.substring(foo.length - 3) === 'bar'
// foo.substring(foo.length - 3, foo.length) === 'bar'
[String([
'CallExpression > MemberExpression.callee[property.name=slice][computed=false]',
'CallExpression > MemberExpression.callee[property.name=substring][computed=false]',
])](node: TSESTree.MemberExpression): void {
const callNode = node.parent! as TSESTree.CallExpression;
[[
':matches(CallExpression, OptionalCallExpression) > :matches(MemberExpression, OptionalMemberExpression).callee[property.name="slice"][computed=false]',
':matches(CallExpression, OptionalCallExpression) > :matches(MemberExpression, OptionalMemberExpression).callee[property.name="substring"][computed=false]',
].join(', ')](
node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression,
): void {
const callNode = node.parent! as
| TSESTree.CallExpression
| TSESTree.OptionalCallExpression;
const parentNode = callNode.parent!;
if (
!isEqualityComparison(parentNode) ||
Expand Down Expand Up @@ -555,17 +587,20 @@ export default createRule({
parentNode,
isStartsWith ? 'start' : 'end',
parentNode.operator.startsWith('!'),
node.optional,
);
},
});
},

// /^bar/.test(foo)
// /bar$/.test(foo)
'CallExpression > MemberExpression.callee[property.name="test"][computed=false]'(
node: TSESTree.MemberExpression,
':matches(CallExpression, OptionalCallExpression) > :matches(MemberExpression, OptionalMemberExpression).callee[property.name="test"][computed=false]'(
node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression,
): void {
const callNode = node.parent as TSESTree.CallExpression;
const callNode = node.parent as
| TSESTree.CallExpression
| TSESTree.OptionalCallExpression;
const parsed =
callNode.arguments.length === 1 ? parseRegExp(node.object) : null;
if (parsed == null) {
Expand All @@ -585,7 +620,9 @@ export default createRule({
argNode.type !== AST_NODE_TYPES.TemplateLiteral &&
argNode.type !== AST_NODE_TYPES.Identifier &&
argNode.type !== AST_NODE_TYPES.MemberExpression &&
argNode.type !== AST_NODE_TYPES.CallExpression;
argNode.type !== AST_NODE_TYPES.OptionalMemberExpression &&
argNode.type !== AST_NODE_TYPES.CallExpression &&
argNode.type !== AST_NODE_TYPES.OptionalCallExpression;

yield fixer.removeRange([callNode.range[0], argNode.range[0]]);
if (needsParen) {
Expand All @@ -594,7 +631,11 @@ export default createRule({
}
yield fixer.insertTextAfter(
argNode,
`.${methodName}(${JSON.stringify(text)}`,
`${
callNode.type === AST_NODE_TYPES.OptionalCallExpression
? '?.'
: '.'
}${methodName}(${JSON.stringify(text)}`,
);
},
});
Expand Down
72 changes: 68 additions & 4 deletions packages/eslint-plugin/tests/rules/prefer-string-starts-ends-with.test.ts
View file
@@ -1,3 +1,4 @@
import { TSESLint } from '@typescript-eslint/experimental-utils';
import path from 'path';
import rule from '../../src/rules/prefer-string-starts-ends-with';
import { RuleTester } from '../RuleTester';
Expand All @@ -13,7 +14,7 @@ const ruleTester = new RuleTester({
});

ruleTester.run('prefer-string-starts-ends-with', rule, {
valid: [
valid: addOptional([
`
function f(s: string[]) {
s[0] === "a"
Expand Down Expand Up @@ -224,8 +225,8 @@ ruleTester.run('prefer-string-starts-ends-with', rule, {
x.test(s)
}
`,
],
invalid: [
]),
invalid: addOptional([
// String indexing.
{
code: `
Expand Down Expand Up @@ -1042,5 +1043,68 @@ ruleTester.run('prefer-string-starts-ends-with', rule, {
`,
errors: [{ messageId: 'preferStartsWith' }],
},
],
]),
});

type Case<TMessageIds extends string, TOptions extends Readonly<unknown[]>> =
| TSESLint.ValidTestCase<TOptions>
| TSESLint.InvalidTestCase<TMessageIds, TOptions>;
function addOptional<TOptions extends Readonly<unknown[]>>(
cases: (TSESLint.ValidTestCase<TOptions> | string)[],
): TSESLint.ValidTestCase<TOptions>[];
function addOptional<
TMessageIds extends string,
TOptions extends Readonly<unknown[]>
>(
cases: TSESLint.InvalidTestCase<TMessageIds, TOptions>[],
): TSESLint.InvalidTestCase<TMessageIds, TOptions>[];
function addOptional<
TMessageIds extends string,
TOptions extends Readonly<unknown[]>
>(
cases: (Case<TMessageIds, TOptions> | string)[],
): Case<TMessageIds, TOptions>[] {
function makeOptional(code: string): string;
function makeOptional(code: string | null | undefined): string | null;
function makeOptional(code: string | null | undefined): string | null {
if (code === null || code === undefined) {
return null;
}
return (
code
.replace(/([^.])\.([^.])/, '$1?.$2')
.replace(/([^.])(\[\d)/, '$1?.$2')
// fix up s[s.length - 1] === "a" which got broken by the first regex
.replace(/(\w+?)\[(\w+?)\?\.(length - 1)/, '$1?.[$2.$3')
);
}

return cases.reduce<Case<TMessageIds, TOptions>[]>((acc, c) => {
if (typeof c === 'string') {
acc.push({
code: c,
});
acc.push({
code: makeOptional(c),
});
} else {
acc.push(c);
const code = makeOptional(c.code);
let output: string | null | undefined = null;
if ('output' in c) {
if (code.indexOf('?.')) {
output = makeOptional(c.output);
} else {
output = c.output;
}
}
acc.push({
...c,
code,
output,
});
}

return acc;
}, []);
}
1 change: 1 addition & 0 deletions packages/eslint-plugin/typings/eslint-utils.d.ts
View file
Expand Up @@ -19,6 +19,7 @@ declare module 'eslint-utils' {
export function getPropertyName(
node:
| TSESTree.MemberExpression
| TSESTree.OptionalMemberExpression
| TSESTree.Property
| TSESTree.MethodDefinition,
initialScope?: TSESLint.Scope.Scope,
Expand Down

0 comments on commit fd37bc3

Please sign in to comment.