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.

Sign up for GitHub

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

Jump to bottom

feat(eslint-plugin): [pref-str-starts/ends-with] optional chain support #1357

Merged
merged 3 commits into from Dec 21, 2019
Merged

feat(eslint-plugin): [pref-str-starts/ends-with] optional chain support #1357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3deead1
feat(eslint-plugin): [pref-str-starts/ends-with] optional chain support
bradzacher Dec 19, 2019
d145bb3
chore: switch already fixed rules to `:matches`
bradzacher Dec 19, 2019
f475fd1
Merge branch 'master' into optional-chain-support-2
bradzacher Dec 21, 2019
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 packages/eslint-plugin/src/rules/no-require-imports.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
@@ -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
Open in desktop
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