diff --git a/packages/eslint-plugin/docs/rules/prefer-includes.md b/packages/eslint-plugin/docs/rules/prefer-includes.md new file mode 100644 index 00000000000..1ea37102b1d --- /dev/null +++ b/packages/eslint-plugin/docs/rules/prefer-includes.md @@ -0,0 +1,71 @@ +# Enforce `includes` method over `indexOf` method (@typescript-eslint/prefer-includes) + +Until ES5, we were using `String#indexOf` method to check whether a string contains an arbitrary substring or not. +Until ES2015, we were using `Array#indexOf` method to check whether an array contains an arbitrary value or not. + +ES2015 has added `String#includes` and ES2016 has added `Array#includes`. +It makes code more understandable if we use those `includes` methods for the purpose. + +## Rule Details + +This rule is aimed at suggesting `includes` method if `indexOf` method was used to check whether an object contains an arbitrary value or not. + +If the receiver object of the `indexOf` method call has `includes` method and the two methods have the same parameters, this rule does suggestion. +There are such types: `String`, `Array`, `ReadonlyArray`, and typed arrays. + +Additionally, this rule reports the tests of simple regular expressions in favor of `String#includes`. + +Examples of **incorrect** code for this rule: + +```ts +let str: string; +let array: any[]; +let readonlyArray: ReadonlyArray; +let typedArray: UInt8Array; +let userDefined: { + indexOf(x: any): number; + includes(x: any): boolean; +}; + +str.indexOf(value) !== -1; +array.indexOf(value) !== -1; +readonlyArray.indexOf(value) === -1; +typedArray.indexOf(value) > -1; +userDefined.indexOf(value) >= 0; + +// simple RegExp test +/foo/.test(str); +``` + +Examples of **correct** code for this rule: + +```ts +let array: any[]; +let readonlyArray: ReadonlyArray; +let typedArray: UInt8Array; +let userDefined: { + indexOf(x: any): number; + includes(x: any): boolean; +}; +let mismatchExample: { + indexOf(x: any, fromIndex?: number): number; + includes(x: any): boolean; +}; + +str.includes(value); +array.includes(value); +readonlyArray.includes(value); +typedArray.includes(value); +userDefined.includes(value); + +// the two methods have different parameters. +mismatchExample.indexOf(value) >= 0; +``` + +## Options + +There are no options. + +## When Not To Use It + +If you don't want to suggest `includes`, you can safely turn this rule off. diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index e350e5ab9e6..6a292d723bd 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -37,6 +37,8 @@ "dependencies": { "@typescript-eslint/parser": "1.6.0", "@typescript-eslint/typescript-estree": "1.6.0", + "eslint-utils": "^1.3.1", + "regexpp": "^2.0.1", "requireindex": "^1.2.0", "tsutils": "^3.7.0" }, diff --git a/packages/eslint-plugin/src/rules/prefer-includes.ts b/packages/eslint-plugin/src/rules/prefer-includes.ts new file mode 100644 index 00000000000..f7b3eb153ee --- /dev/null +++ b/packages/eslint-plugin/src/rules/prefer-includes.ts @@ -0,0 +1,209 @@ +import { TSESTree } from '@typescript-eslint/typescript-estree'; +import { getStaticValue } from 'eslint-utils'; +import { AST as RegExpAST, parseRegExpLiteral } from 'regexpp'; +import ts from 'typescript'; +import { createRule, getParserServices } from '../util'; + +export default createRule({ + name: 'prefer-includes', + defaultOptions: [], + + meta: { + type: 'suggestion', + docs: { + description: 'Enforce `includes` method over `indexOf` method', + category: 'Best Practices', + recommended: false, + }, + fixable: 'code', + messages: { + preferIncludes: "Use 'includes()' method instead.", + preferStringIncludes: + 'Use `String#includes()` method with a string instead.', + }, + schema: [], + }, + + create(context) { + const globalScope = context.getScope(); + const services = getParserServices(context); + const types = services.program.getTypeChecker(); + + function isNumber(node: TSESTree.Node, value: number): boolean { + const evaluated = getStaticValue(node, globalScope); + return evaluated !== null && evaluated.value === value; + } + + function isPositiveCheck(node: TSESTree.BinaryExpression): boolean { + switch (node.operator) { + case '!==': + case '!=': + case '>': + return isNumber(node.right, -1); + case '>=': + return isNumber(node.right, 0); + default: + return false; + } + } + function isNegativeCheck(node: TSESTree.BinaryExpression): boolean { + switch (node.operator) { + case '===': + case '==': + case '<=': + return isNumber(node.right, -1); + case '<': + return isNumber(node.right, 0); + default: + return false; + } + } + + function hasSameParameters( + nodeA: ts.Declaration, + nodeB: ts.Declaration, + ): boolean { + if (!ts.isFunctionLike(nodeA) || !ts.isFunctionLike(nodeB)) { + return false; + } + + const paramsA = nodeA.parameters; + const paramsB = nodeB.parameters; + if (paramsA.length !== paramsB.length) { + return false; + } + + for (let i = 0; i < paramsA.length; ++i) { + const paramA = paramsA[i]; + const paramB = paramsB[i]; + + // Check name, type, and question token once. + if (paramA.getText() !== paramB.getText()) { + return false; + } + } + + return true; + } + + /** + * Parse a given node if it's a `RegExp` instance. + * @param node The node to parse. + */ + function parseRegExp(node: TSESTree.Node): string | null { + const evaluated = getStaticValue(node, globalScope); + if (evaluated == null || !(evaluated.value instanceof RegExp)) { + return null; + } + + const { pattern, flags } = parseRegExpLiteral(evaluated.value); + if ( + pattern.alternatives.length !== 1 || + flags.ignoreCase || + flags.global + ) { + return null; + } + + // Check if it can determine a unique string. + const chars = pattern.alternatives[0].elements; + if (!chars.every(c => c.type === 'Character')) { + return null; + } + + // To string. + return String.fromCodePoint( + ...chars.map(c => (c as RegExpAST.Character).value), + ); + } + + return { + "BinaryExpression > CallExpression.left > MemberExpression.callee[property.name='indexOf'][computed=false]"( + node: TSESTree.MemberExpression, + ): void { + // Check if the comparison is equivalent to `includes()`. + const callNode = node.parent as TSESTree.CallExpression; + const compareNode = callNode.parent as TSESTree.BinaryExpression; + const negative = isNegativeCheck(compareNode); + if (!negative && !isPositiveCheck(compareNode)) { + return; + } + + // Get the symbol of `indexOf` method. + const tsNode = services.esTreeNodeToTSNodeMap.get(node.property); + const indexofMethodSymbol = types.getSymbolAtLocation(tsNode); + if ( + indexofMethodSymbol == null || + indexofMethodSymbol.declarations.length === 0 + ) { + return; + } + + // Check if every declaration of `indexOf` method has `includes` method + // and the two methods have the same parameters. + for (const instanceofMethodDecl of indexofMethodSymbol.declarations) { + const typeDecl = instanceofMethodDecl.parent; + const type = types.getTypeAtLocation(typeDecl); + const includesMethodSymbol = type.getProperty('includes'); + if ( + includesMethodSymbol == null || + !includesMethodSymbol.declarations.some(includesMethodDecl => + hasSameParameters(includesMethodDecl, instanceofMethodDecl), + ) + ) { + return; + } + } + + // Report it. + context.report({ + node: compareNode, + messageId: 'preferIncludes', + *fix(fixer) { + if (negative) { + yield fixer.insertTextBefore(callNode, '!'); + } + yield fixer.replaceText(node.property, 'includes'); + yield fixer.removeRange([callNode.range[1], compareNode.range[1]]); + }, + }); + }, + + // /bar/.test(foo) + 'CallExpression > MemberExpression.callee[property.name="test"][computed=false]'( + node: TSESTree.MemberExpression, + ): void { + const callNode = node.parent as TSESTree.CallExpression; + const text = + callNode.arguments.length === 1 ? parseRegExp(node.object) : null; + if (text == null) { + return; + } + + context.report({ + node: callNode, + messageId: 'preferStringIncludes', + *fix(fixer) { + const argNode = callNode.arguments[0]; + const needsParen = + argNode.type !== 'Literal' && + argNode.type !== 'TemplateLiteral' && + argNode.type !== 'Identifier' && + argNode.type !== 'MemberExpression' && + argNode.type !== 'CallExpression'; + + yield fixer.removeRange([callNode.range[0], argNode.range[0]]); + if (needsParen) { + yield fixer.insertTextBefore(argNode, '('); + yield fixer.insertTextAfter(argNode, ')'); + } + yield fixer.insertTextAfter( + argNode, + `.includes(${JSON.stringify(text)}`, + ); + }, + }); + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/prefer-includes.test.ts b/packages/eslint-plugin/tests/rules/prefer-includes.test.ts new file mode 100644 index 00000000000..f754c487521 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/prefer-includes.test.ts @@ -0,0 +1,439 @@ +import path from 'path'; +import rule from '../../src/rules/prefer-includes'; +import { RuleTester } from '../RuleTester'; + +const rootPath = path.join(process.cwd(), 'tests/fixtures/'); + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, +}); + +ruleTester.run('prefer-includes', rule, { + valid: [ + ` + function f(a: string): void { + a.indexOf(b) + } + `, + ` + function f(a: string): void { + a.indexOf(b) + 0 + } + `, + ` + function f(a: string | {value: string}): void { + a.indexOf(b) !== -1 + } + `, + ` + type UserDefined = { + indexOf(x: any): number // don't have 'includes'. + } + function f(a: UserDefined): void { + a.indexOf(b) !== -1 + } + `, + ` + type UserDefined = { + indexOf(x: any, fromIndex?: number): number + includes(x: any): boolean // different parameters. + } + function f(a: UserDefined): void { + a.indexOf(b) !== -1 + } + `, + ` + type UserDefined = { + indexOf(x: any, fromIndex?: number): number + includes(x: any, fromIndex: number): boolean // different parameters. + } + function f(a: UserDefined): void { + a.indexOf(b) !== -1 + } + `, + ` + type UserDefined = { + indexOf(x: any, fromIndex?: number): number + includes: boolean // different type. + } + function f(a: UserDefined): void { + a.indexOf(b) !== -1 + } + `, + ` + function f(a: string): void { + /bar/i.test(a) + } + `, + ` + function f(a: string): void { + /ba[rz]/.test(a) + } + `, + ` + function f(a: string): void { + /foo|bar/.test(a) + } + `, + ` + function f(a: string): void { + /bar/.test() + } + `, + ` + function f(a: string): void { + something.test(a) + } + `, + ], + invalid: [ + // positive + { + code: ` + function f(a: string): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f(a: string): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: string): void { + a.indexOf(b) != -1 + } + `, + output: ` + function f(a: string): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: string): void { + a.indexOf(b) > -1 + } + `, + output: ` + function f(a: string): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: string): void { + a.indexOf(b) >= 0 + } + `, + output: ` + function f(a: string): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + + // negative + { + code: ` + function f(a: string): void { + a.indexOf(b) === -1 + } + `, + output: ` + function f(a: string): void { + !a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: string): void { + a.indexOf(b) == -1 + } + `, + output: ` + function f(a: string): void { + !a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: string): void { + a.indexOf(b) <= -1 + } + `, + output: ` + function f(a: string): void { + !a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: string): void { + a.indexOf(b) < 0 + } + `, + output: ` + function f(a: string): void { + !a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + + // RegExp#test + { + code: ` + function f(a: string): void { + /bar/.test(a) + } + `, + output: ` + function f(a: string): void { + a.includes("bar") + } + `, + errors: [{ messageId: 'preferStringIncludes' }], + }, + { + code: ` + const pattern = new RegExp("bar") + function f(a: string): void { + pattern.test(a) + } + `, + output: ` + const pattern = new RegExp("bar") + function f(a: string): void { + a.includes("bar") + } + `, + errors: [{ messageId: 'preferStringIncludes' }], + }, + { + code: ` + const pattern = /bar/ + function f(a: string, b: string): void { + pattern.test(a + b) + } + `, + output: ` + const pattern = /bar/ + function f(a: string, b: string): void { + (a + b).includes("bar") + } + `, + errors: [{ messageId: 'preferStringIncludes' }], + }, + + // type variation + { + code: ` + function f(a: any[]): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f(a: any[]): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: ReadonlyArray): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f(a: ReadonlyArray): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: Int8Array): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f(a: Int8Array): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: Int16Array): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f(a: Int16Array): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: Int32Array): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f(a: Int32Array): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: Uint8Array): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f(a: Uint8Array): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: Uint16Array): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f(a: Uint16Array): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: Uint32Array): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f(a: Uint32Array): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: Float32Array): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f(a: Float32Array): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: Float64Array): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f(a: Float64Array): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: T[] | ReadonlyArray): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f(a: T[] | ReadonlyArray): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f | Int8Array | Uint8Array | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array>(a: U): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f | Int8Array | Uint8Array | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array>(a: U): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + type UserDefined = { + indexOf(x: any): number + includes(x: any): boolean + } + function f(a: UserDefined): void { + a.indexOf(b) !== -1 + } + `, + output: ` + type UserDefined = { + indexOf(x: any): number + includes(x: any): boolean + } + function f(a: UserDefined): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + { + code: ` + function f(a: Readonly): void { + a.indexOf(b) !== -1 + } + `, + output: ` + function f(a: Readonly): void { + a.includes(b) + } + `, + errors: [{ messageId: 'preferIncludes' }], + }, + ], +}); diff --git a/packages/eslint-plugin/typings/eslint-utils.d.ts b/packages/eslint-plugin/typings/eslint-utils.d.ts new file mode 100644 index 00000000000..d926229b00e --- /dev/null +++ b/packages/eslint-plugin/typings/eslint-utils.d.ts @@ -0,0 +1,178 @@ +declare module 'eslint-utils' { + import { TSESTree } from '@typescript-eslint/typescript-estree'; + import { Scope, SourceCode } from 'ts-eslint'; + + export function getFunctionHeadLocation( + node: + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.ArrowFunctionExpression, + sourceCode: SourceCode, + ): TSESTree.SourceLocation; + + export function getFunctionNameWithKind( + node: + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.ArrowFunctionExpression, + ): string; + + export function getPropertyName( + node: + | TSESTree.MemberExpression + | TSESTree.Property + | TSESTree.MethodDefinition, + initialScope?: Scope.Scope, + ): string | null; + + export function getStaticValue( + node: TSESTree.Node, + initialScope?: Scope.Scope, + ): { value: any } | null; + + export function getStringIfConstant( + node: TSESTree.Node, + initialScope?: Scope.Scope, + ): string | null; + + export function hasSideEffect( + node: TSESTree.Node, + sourceCode: SourceCode, + options?: { + considerGetters?: boolean; + considerImplicitTypeConversion?: boolean; + }, + ): boolean; + + export function isParenthesized( + node: TSESTree.Node, + sourceCode: SourceCode, + ): boolean; + + export class PatternMatcher { + constructor(pattern: RegExp, options?: { escaped?: boolean }); + execAll(str: string): IterableIterator; + test(str: string): boolean; + } + + export function findVariable( + initialScope: Scope.Scope, + name: string, + ): Scope.Variable | null; + + export function getInnermostScope( + initialScope: Scope.Scope, + node: TSESTree.Node, + ): Scope.Scope; + + export class ReferenceTracker { + static readonly READ: unique symbol; + static readonly CALL: unique symbol; + static readonly CONSTRUCT: unique symbol; + + constructor( + globalScope: Scope.Scope, + options?: { + mode: 'strict' | 'legacy'; + globalObjectNames: ReadonlyArray; + }, + ); + + iterateGlobalReferences( + traceMap: ReferenceTracker.TraceMap, + ): IterableIterator>; + iterateCjsReferences( + traceMap: ReferenceTracker.TraceMap, + ): IterableIterator>; + iterateEsmReferences( + traceMap: ReferenceTracker.TraceMap, + ): IterableIterator>; + } + + export namespace ReferenceTracker { + export type READ = typeof ReferenceTracker.READ; + export type CALL = typeof ReferenceTracker.READ; + export type CONSTRUCT = typeof ReferenceTracker.READ; + export type ReferenceType = READ | CALL | CONSTRUCT; + export type TraceMap = Record>; + export interface TraceMapElement { + [ReferenceTracker.READ]?: T; + [ReferenceTracker.CALL]?: T; + [ReferenceTracker.CONSTRUCT]?: T; + [key: string]: TraceMapElement; + } + export interface FoundReference { + node: TSESTree.Node; + path: ReadonlyArray; + type: ReferenceType; + entry: T; + } + } + + export function isArrowToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isNotArrowToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isClosingBraceToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isNotClosingBraceToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isClosingBracketToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isNotClosingBracketToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isClosingParenToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isNotClosingParenToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isColonToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isNotColonToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isCommaToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isNotCommaToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isCommentToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isNotCommentToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isOpeningBraceToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isNotOpeningBraceToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isOpeningBracketToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isNotOpeningBracketToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isOpeningParenToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isNotOpeningParenToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isSemicolonToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; + export function isNotSemicolonToken( + token: TSESTree.Token | TSESTree.Comment, + ): boolean; +} diff --git a/packages/eslint-plugin/typings/ts-eslint.d.ts b/packages/eslint-plugin/typings/ts-eslint.d.ts index 33b667dab46..d6bd1eef8a7 100644 --- a/packages/eslint-plugin/typings/ts-eslint.d.ts +++ b/packages/eslint-plugin/typings/ts-eslint.d.ts @@ -297,7 +297,9 @@ declare module 'ts-eslint' { replaceTextRange(range: AST.Range, text: string): RuleFix; } - type ReportFixFunction = (fixer: RuleFixer) => null | RuleFix | RuleFix[]; + type ReportFixFunction = ( + fixer: RuleFixer, + ) => null | RuleFix | Iterable; interface ReportDescriptor { /**