From c928411cba8c7478d5be3d0f1fc4db00f63abee6 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 8 Mar 2020 16:18:27 +1300 Subject: [PATCH 01/12] feat(eslint-plugin): create `prefer-reduce-type-parameter` rule --- packages/eslint-plugin/README.md | 1 + .../rules/prefer-reduce-type-parameter.md | 54 ++++++ packages/eslint-plugin/src/configs/all.json | 1 + .../recommended-requiring-type-checking.json | 1 + packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/prefer-reduce-type-parameter.ts | 116 ++++++++++++ .../eslint-plugin/tests/fixtures/class.ts | 5 + .../prefer-reduce-type-parameter.test.ts | 167 ++++++++++++++++++ 8 files changed, 347 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/prefer-reduce-type-parameter.md create mode 100644 packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts create mode 100644 packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 809aac848a6..f5f221d5029 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -149,6 +149,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/prefer-optional-chain`](./docs/rules/prefer-optional-chain.md) | Prefer using concise optional chain expressions instead of chained logical ands | | :wrench: | | | [`@typescript-eslint/prefer-readonly`](./docs/rules/prefer-readonly.md) | Requires that private members are marked as `readonly` if they're never modified outside of the constructor | | :wrench: | :thought_balloon: | | [`@typescript-eslint/prefer-readonly-parameter-types`](./docs/rules/prefer-readonly-parameter-types.md) | Requires that function parameters are typed as readonly to prevent accidental mutation of inputs | | | :thought_balloon: | +| [`@typescript-eslint/prefer-reduce-type-parameter`](./docs/rules/prefer-reduce-type-parameter.md) | Prefer using type parameter when calling `Array#reduce` instead of casting | :heavy_check_mark: | :wrench: | :thought_balloon: | | [`@typescript-eslint/prefer-regexp-exec`](./docs/rules/prefer-regexp-exec.md) | Enforce that `RegExp#exec` is used instead of `String#match` if no global flag is provided | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/prefer-string-starts-ends-with`](./docs/rules/prefer-string-starts-ends-with.md) | Enforce the use of `String#startsWith` and `String#endsWith` instead of other equivalent methods of checking substrings | :heavy_check_mark: | :wrench: | :thought_balloon: | | [`@typescript-eslint/promise-function-async`](./docs/rules/promise-function-async.md) | Requires any function or method that returns a Promise to be marked async | | | :thought_balloon: | diff --git a/packages/eslint-plugin/docs/rules/prefer-reduce-type-parameter.md b/packages/eslint-plugin/docs/rules/prefer-reduce-type-parameter.md new file mode 100644 index 00000000000..d6f68021cc7 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/prefer-reduce-type-parameter.md @@ -0,0 +1,54 @@ +# Prefer using type parameter when calling `Array#reduce` instead of casting (`prefer-reduce-type-parameter`) + +It's common to call `Array#reduce` with a generic type, such as an array or object, as the initial value. +Since these values are empty, their types are not usable: + +- `[]` has type `never[]`, which can't have items pushed into it as nothing is type `never` +- `{}` has type `{}`, which doesn't have an index signature and so can't have properties added to it + +A common solution to this problem is to cast the initial value. While this will work, it's not the most optimal +solution as casting has subtle effects on the underlying types that can allow bugs to slip in. + +A better (and lesser known) solution is to pass the type in as a generic parameter to `Array#reduce` explicitly. +This means that TypeScript doesn't have to try to infer the type, and avoids the common pitfalls that come with casting. + +## Rule Details + +This rule looks for calls to `Array#reduce`, and warns if an initial value is being passed & casted, +suggesting instead to pass the cast type to `Array#reduce` as its generic parameter. + +Examples of **incorrect** code for this rule: + +```ts +[1, 2, 3].reduce((arr, num) => arr.concat(num * 2), [] as number[]); + +['a', 'b'].reduce( + (accum, name) => ({ + ...accum, + [name]: true, + }), + {} as Record, +); +``` + +Examples of **correct** code for this rule: + +```ts +[1, 2, 3].reduce((arr, num) => arr.concat(num * 2), []); + +['a', 'b'].reduce>( + (accum, name) => ({ + ...accum, + [name]: true, + }), + {}, +); +``` + +## Options + +There are no options. + +## When Not To Use It + +If you don't want to use typechecking in your linting, you can't use this rule. diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 85cf2e1ecc6..dc1ec0490ca 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -86,6 +86,7 @@ "@typescript-eslint/prefer-optional-chain": "error", "@typescript-eslint/prefer-readonly": "error", "@typescript-eslint/prefer-readonly-parameter-types": "error", + "@typescript-eslint/prefer-reduce-type-parameter": "error", "@typescript-eslint/prefer-regexp-exec": "error", "@typescript-eslint/prefer-string-starts-ends-with": "error", "@typescript-eslint/promise-function-async": "error", diff --git a/packages/eslint-plugin/src/configs/recommended-requiring-type-checking.json b/packages/eslint-plugin/src/configs/recommended-requiring-type-checking.json index 68867b53248..4f79e8a2d01 100644 --- a/packages/eslint-plugin/src/configs/recommended-requiring-type-checking.json +++ b/packages/eslint-plugin/src/configs/recommended-requiring-type-checking.json @@ -6,6 +6,7 @@ "@typescript-eslint/no-misused-promises": "error", "@typescript-eslint/no-unnecessary-type-assertion": "error", "@typescript-eslint/prefer-includes": "error", + "@typescript-eslint/prefer-reduce-type-parameter": "error", "@typescript-eslint/prefer-regexp-exec": "error", "@typescript-eslint/prefer-string-starts-ends-with": "error", "require-await": "off", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 15ce8dd1a82..e941f5148c3 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -75,6 +75,7 @@ import preferNullishCoalescing from './prefer-nullish-coalescing'; import preferOptionalChain from './prefer-optional-chain'; import preferReadonly from './prefer-readonly'; import preferReadonlyParameterTypes from './prefer-readonly-parameter-types'; +import preferReduceTypeParameter from './prefer-reduce-type-parameter'; import preferRegexpExec from './prefer-regexp-exec'; import preferStringStartsEndsWith from './prefer-string-starts-ends-with'; import promiseFunctionAsync from './promise-function-async'; @@ -172,6 +173,7 @@ export default { 'prefer-optional-chain': preferOptionalChain, 'prefer-readonly-parameter-types': preferReadonlyParameterTypes, 'prefer-readonly': preferReadonly, + 'prefer-reduce-type-parameter': preferReduceTypeParameter, 'prefer-regexp-exec': preferRegexpExec, 'prefer-string-starts-ends-with': preferStringStartsEndsWith, 'promise-function-async': promiseFunctionAsync, diff --git a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts new file mode 100644 index 00000000000..7077a4e8803 --- /dev/null +++ b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts @@ -0,0 +1,116 @@ +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import * as ts from 'typescript'; +import * as util from '../util'; + +const getMemberExpressionName = ( + member: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression, +): string | null => { + if (!member.computed) { + return member.property.name; + } + + if ( + member.property.type === AST_NODE_TYPES.Literal && + typeof member.property.value === 'string' + ) { + return member.property.value; + } + + return null; +}; + +export default util.createRule({ + name: 'prefer-reduce-type-parameter', + meta: { + type: 'problem', + docs: { + category: 'Best Practices', + recommended: 'error', + description: + 'Prefer using type parameter when calling `Array#reduce` instead of casting', + requiresTypeChecking: true, + }, + messages: { + preferTypeParameter: + 'Unnecessary cast: Array#reduce accepts a type parameter for the default value.', + }, + fixable: 'code', + schema: [], + }, + defaultOptions: [], + create(context) { + const service = util.getParserServices(context); + const checker = service.program.getTypeChecker(); + + const reportPossibleArrayReduceCasting = ( + node: TSESTree.CallExpression | TSESTree.OptionalCallExpression, + ): void => { + { + if ( + !util.isMemberOrOptionalMemberExpression(node.callee) || + getMemberExpressionName(node.callee) !== 'reduce' + ) { + return; + } + + const [, secondArg] = node.arguments; + + if ( + node.arguments.length < 2 || + (secondArg.type !== AST_NODE_TYPES.TSAsExpression && + secondArg.type !== AST_NODE_TYPES.TSTypeAssertion) + ) { + return; + } + + // Get the symbol of the `reduce` method. + const tsNode = service.esTreeNodeToTSNodeMap.get(node.callee); + const reduceSymbol = checker.getSymbolAtLocation(tsNode); + if (reduceSymbol == null) { + return; + } + + // Check the owner type of the `reduce` method. + for (const methodDecl of reduceSymbol.declarations) { + const typeDecl = methodDecl.parent; + if ( + ts.isInterfaceDeclaration(typeDecl) && + ts.isSourceFile(typeDecl.parent) && + typeDecl.name.escapedText === 'Array' + ) { + context.report({ + messageId: 'preferTypeParameter', + node: secondArg, + fix: fixer => [ + fixer.removeRange([ + secondArg.range[0], + secondArg.expression.range[0], + ]), + fixer.removeRange([ + secondArg.expression.range[1], + secondArg.range[1], + ]), + fixer.insertTextAfter( + node.callee, + `<${context + .getSourceCode() + .getText(secondArg.typeAnnotation)}>`, + ), + ], + }); + + return; + } + } + } + }; + + return { + OptionalCallExpression: reportPossibleArrayReduceCasting, + CallExpression: reportPossibleArrayReduceCasting, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/fixtures/class.ts b/packages/eslint-plugin/tests/fixtures/class.ts index 74f222338ff..044e44e44ae 100644 --- a/packages/eslint-plugin/tests/fixtures/class.ts +++ b/packages/eslint-plugin/tests/fixtures/class.ts @@ -3,3 +3,8 @@ export class Error {} // used by unbound-method test case to test imports export const console = { log() {} }; + +// used by prefer-reduce-type-parameter to test native vs userland check +export class Reducable { + reduce() {} +} diff --git a/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts b/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts new file mode 100644 index 00000000000..6a6c9a5863c --- /dev/null +++ b/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts @@ -0,0 +1,167 @@ +import rule from '../../src/rules/prefer-reduce-type-parameter'; +import { RuleTester, getFixturesRootDir } from '../RuleTester'; + +const rootPath = getFixturesRootDir(); + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + sourceType: 'module', + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, +}); + +ruleTester.run('prefer-reduce-type-parameter', rule, { + valid: [ + '(new class Mine { reduce() {} }).reduce(() => {}, 1 as any);', + ` + class Mine { reduce() {} } + + new Mine().reduce(() => {}, 1 as any); + `, + ` + import { Reducable } from './class'; + + new Reducable().reduce(() => {}, 1 as any); + `, + "[1, 2, 3]['reduce']((sum, num) => sum + num, {} as any)", + "[1, 2, 3]['reduce']((sum, num) => sum + num, 0)", + '[1, 2, 3][null]((sum, num) => sum + num, 0)', + '[1, 2, 3]?.[null]((sum, num) => sum + num, 0)', + '[1, 2, 3].reduce((sum, num) => sum + num, 0)', + '[1, 2, 3].reduce((a, s) => a.concat(s * 2), [])', + '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), [])', + ], + invalid: [ + { + code: '[1, 2, 3].reduce((a, s) => a.concat(s * 2), [] as number[],)', + output: '[1, 2, 3].reduce((a, s) => a.concat(s * 2), [],)', + errors: [ + { + messageId: 'preferTypeParameter', + column: 45, + line: 1, + }, + ], + }, + { + code: '[1, 2, 3].reduce((a, s) => a.concat(s * 2), [],)', + output: '[1, 2, 3].reduce((a, s) => a.concat(s * 2), [],)', + errors: [ + { + messageId: 'preferTypeParameter', + column: 45, + line: 1, + }, + ], + }, + { + code: '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), [] as number[])', + output: '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), [])', + errors: [ + { + messageId: 'preferTypeParameter', + column: 46, + line: 1, + }, + ], + }, + { + code: '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), [])', + output: '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), [])', + errors: [ + { + messageId: 'preferTypeParameter', + column: 46, + line: 1, + }, + ], + }, + { + code: ` +const names = ['a', 'b', 'c']; + +names.reduce( + (accum, name) => ({ + ...accum, + [name]: true, + }), + {} as Record +); + `, + output: ` +const names = ['a', 'b', 'c']; + +names.reduce>( + (accum, name) => ({ + ...accum, + [name]: true, + }), + {} +); + `, + errors: [ + { + messageId: 'preferTypeParameter', + column: 3, + line: 9, + }, + ], + }, + { + code: ` +['a', 'b'].reduce( + (accum, name) => ({ + ...accum, + [name]: true, + }), + >{} +); + `, + output: ` +['a', 'b'].reduce>( + (accum, name) => ({ + ...accum, + [name]: true, + }), + {} +); + `, + errors: [ + { + messageId: 'preferTypeParameter', + column: 3, + line: 7, + }, + ], + }, + // { + // code: ` + // ['a', 'b']['reduce']( + // (accum, name) => ({ + // ...accum, + // [name]: true + // }), + // {} as Record + // ); + // `, + // output: ` + // ['a', 'b']['reduce']>( + // (accum, name) => ({ + // ...accum, + // [name]: true + // }), + // {} + // ); + // `, + // errors: [ + // { + // messageId: 'preferTypeParameter', + // column: 3, + // line: 7, + // }, + // ], + // }, + ], +}); From 363059e8b0c1b637514956b6477f5247d78fc96b Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 8 Mar 2020 18:20:35 +1300 Subject: [PATCH 02/12] chore(eslint-plugin): remove commented out test --- .../prefer-reduce-type-parameter.test.ts | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts b/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts index 6a6c9a5863c..61a718240a1 100644 --- a/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts @@ -136,32 +136,5 @@ names.reduce>( }, ], }, - // { - // code: ` - // ['a', 'b']['reduce']( - // (accum, name) => ({ - // ...accum, - // [name]: true - // }), - // {} as Record - // ); - // `, - // output: ` - // ['a', 'b']['reduce']>( - // (accum, name) => ({ - // ...accum, - // [name]: true - // }), - // {} - // ); - // `, - // errors: [ - // { - // messageId: 'preferTypeParameter', - // column: 3, - // line: 7, - // }, - // ], - // }, ], }); From e83cf0ab540cc5a4403df085eca79603f08fc1f7 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Mon, 9 Mar 2020 08:37:15 +1300 Subject: [PATCH 03/12] chore(experimental-utils): use generic parameter for reduce --- packages/experimental-utils/src/eslint-utils/deepMerge.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/experimental-utils/src/eslint-utils/deepMerge.ts b/packages/experimental-utils/src/eslint-utils/deepMerge.ts index 5ac6509203f..9b7baee5ebb 100644 --- a/packages/experimental-utils/src/eslint-utils/deepMerge.ts +++ b/packages/experimental-utils/src/eslint-utils/deepMerge.ts @@ -25,7 +25,7 @@ export function deepMerge( // get the unique set of keys across both objects const keys = new Set(Object.keys(first).concat(Object.keys(second))); - return Array.from(keys).reduce((acc, key) => { + return Array.from(keys).reduce((acc, key) => { const firstHasKey = key in first; const secondHasKey = key in second; const firstValue = first[key]; @@ -46,7 +46,7 @@ export function deepMerge( } return acc; - }, {} as ObjectLike); + }, {}); } export { isObjectNotArray }; From 68b5b5f2119b66af0cd531e106ac1e891b4ad5c7 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Mon, 9 Mar 2020 08:40:22 +1300 Subject: [PATCH 04/12] chore(eslint-plugin): add eslint disable comment for rule --- packages/eslint-plugin/src/rules/naming-convention.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/src/rules/naming-convention.ts b/packages/eslint-plugin/src/rules/naming-convention.ts index f1963b17321..25187268eef 100644 --- a/packages/eslint-plugin/src/rules/naming-convention.ts +++ b/packages/eslint-plugin/src/rules/naming-convention.ts @@ -755,6 +755,7 @@ function parseOptions(context: Context): ParsedOptions { const parsedOptions = util.getEnumNames(Selectors).reduce((acc, k) => { acc[k] = createValidator(k, context, normalizedOptions); return acc; + // eslint-disable-next-line @typescript-eslint/prefer-reduce-type-parameter }, {} as ParsedOptions); return parsedOptions; From 1ef406f6784d18ad6753e2f8171d5dc25a52f8e7 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Tue, 10 Mar 2020 08:04:21 +1300 Subject: [PATCH 05/12] chore(eslint-plugin): unrecommend `prefer-reduce-type-parameter` --- packages/eslint-plugin/README.md | 2 +- .../src/configs/recommended-requiring-type-checking.json | 1 - .../eslint-plugin/src/rules/prefer-reduce-type-parameter.ts | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index f5f221d5029..d943ffdcec9 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -149,7 +149,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/prefer-optional-chain`](./docs/rules/prefer-optional-chain.md) | Prefer using concise optional chain expressions instead of chained logical ands | | :wrench: | | | [`@typescript-eslint/prefer-readonly`](./docs/rules/prefer-readonly.md) | Requires that private members are marked as `readonly` if they're never modified outside of the constructor | | :wrench: | :thought_balloon: | | [`@typescript-eslint/prefer-readonly-parameter-types`](./docs/rules/prefer-readonly-parameter-types.md) | Requires that function parameters are typed as readonly to prevent accidental mutation of inputs | | | :thought_balloon: | -| [`@typescript-eslint/prefer-reduce-type-parameter`](./docs/rules/prefer-reduce-type-parameter.md) | Prefer using type parameter when calling `Array#reduce` instead of casting | :heavy_check_mark: | :wrench: | :thought_balloon: | +| [`@typescript-eslint/prefer-reduce-type-parameter`](./docs/rules/prefer-reduce-type-parameter.md) | Prefer using type parameter when calling `Array#reduce` instead of casting | | :wrench: | :thought_balloon: | | [`@typescript-eslint/prefer-regexp-exec`](./docs/rules/prefer-regexp-exec.md) | Enforce that `RegExp#exec` is used instead of `String#match` if no global flag is provided | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/prefer-string-starts-ends-with`](./docs/rules/prefer-string-starts-ends-with.md) | Enforce the use of `String#startsWith` and `String#endsWith` instead of other equivalent methods of checking substrings | :heavy_check_mark: | :wrench: | :thought_balloon: | | [`@typescript-eslint/promise-function-async`](./docs/rules/promise-function-async.md) | Requires any function or method that returns a Promise to be marked async | | | :thought_balloon: | diff --git a/packages/eslint-plugin/src/configs/recommended-requiring-type-checking.json b/packages/eslint-plugin/src/configs/recommended-requiring-type-checking.json index 4f79e8a2d01..68867b53248 100644 --- a/packages/eslint-plugin/src/configs/recommended-requiring-type-checking.json +++ b/packages/eslint-plugin/src/configs/recommended-requiring-type-checking.json @@ -6,7 +6,6 @@ "@typescript-eslint/no-misused-promises": "error", "@typescript-eslint/no-unnecessary-type-assertion": "error", "@typescript-eslint/prefer-includes": "error", - "@typescript-eslint/prefer-reduce-type-parameter": "error", "@typescript-eslint/prefer-regexp-exec": "error", "@typescript-eslint/prefer-string-starts-ends-with": "error", "require-await": "off", diff --git a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts index 7077a4e8803..00fc96fd2a3 100644 --- a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts +++ b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts @@ -28,7 +28,7 @@ export default util.createRule({ type: 'problem', docs: { category: 'Best Practices', - recommended: 'error', + recommended: false, description: 'Prefer using type parameter when calling `Array#reduce` instead of casting', requiresTypeChecking: true, From 7357a1a65b1e3c27beb1a9e883852069fdbf251f Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Tue, 10 Mar 2020 08:29:43 +1300 Subject: [PATCH 06/12] chore(eslint-plugin): use `isArrayType` util --- .../src/rules/prefer-reduce-type-parameter.ts | 59 ++++++++----------- .../prefer-reduce-type-parameter.test.ts | 28 ++++++++- 2 files changed, 51 insertions(+), 36 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts index 00fc96fd2a3..f7d0d44b90e 100644 --- a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts +++ b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts @@ -2,7 +2,6 @@ import { AST_NODE_TYPES, TSESTree, } from '@typescript-eslint/experimental-utils'; -import * as ts from 'typescript'; import * as util from '../util'; const getMemberExpressionName = ( @@ -67,43 +66,33 @@ export default util.createRule({ } // Get the symbol of the `reduce` method. - const tsNode = service.esTreeNodeToTSNodeMap.get(node.callee); - const reduceSymbol = checker.getSymbolAtLocation(tsNode); - if (reduceSymbol == null) { - return; - } + const tsNode = service.esTreeNodeToTSNodeMap.get(node.callee.object); + const calleeObjType = checker.getTypeAtLocation(tsNode); // Check the owner type of the `reduce` method. - for (const methodDecl of reduceSymbol.declarations) { - const typeDecl = methodDecl.parent; - if ( - ts.isInterfaceDeclaration(typeDecl) && - ts.isSourceFile(typeDecl.parent) && - typeDecl.name.escapedText === 'Array' - ) { - context.report({ - messageId: 'preferTypeParameter', - node: secondArg, - fix: fixer => [ - fixer.removeRange([ - secondArg.range[0], - secondArg.expression.range[0], - ]), - fixer.removeRange([ - secondArg.expression.range[1], - secondArg.range[1], - ]), - fixer.insertTextAfter( - node.callee, - `<${context - .getSourceCode() - .getText(secondArg.typeAnnotation)}>`, - ), - ], - }); + if (checker.isArrayType(calleeObjType)) { + context.report({ + messageId: 'preferTypeParameter', + node: secondArg, + fix: fixer => [ + fixer.removeRange([ + secondArg.range[0], + secondArg.expression.range[0], + ]), + fixer.removeRange([ + secondArg.expression.range[1], + secondArg.range[1], + ]), + fixer.insertTextAfter( + node.callee, + `<${context + .getSourceCode() + .getText(secondArg.typeAnnotation)}>`, + ), + ], + }); - return; - } + return; } } }; diff --git a/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts b/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts index 61a718240a1..17ba3fedc9c 100644 --- a/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts @@ -25,7 +25,6 @@ ruleTester.run('prefer-reduce-type-parameter', rule, { new Reducable().reduce(() => {}, 1 as any); `, - "[1, 2, 3]['reduce']((sum, num) => sum + num, {} as any)", "[1, 2, 3]['reduce']((sum, num) => sum + num, 0)", '[1, 2, 3][null]((sum, num) => sum + num, 0)', '[1, 2, 3]?.[null]((sum, num) => sum + num, 0)', @@ -136,5 +135,32 @@ names.reduce>( }, ], }, + { + code: ` +['a', 'b']['reduce']( + (accum, name) => ({ + ...accum, + [name]: true + }), + {} as Record +); + `, + output: ` +['a', 'b']['reduce']>( + (accum, name) => ({ + ...accum, + [name]: true + }), + {} +); + `, + errors: [ + { + messageId: 'preferTypeParameter', + column: 3, + line: 7, + }, + ], + }, ], }); From 42fd67933bfce5941265fc5b7b5ef558af60110a Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Tue, 10 Mar 2020 08:30:38 +1300 Subject: [PATCH 07/12] chore(eslint-plugin): use `isTypeAssertion` util --- .../eslint-plugin/src/rules/prefer-reduce-type-parameter.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts index f7d0d44b90e..ad3994672ff 100644 --- a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts +++ b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts @@ -57,11 +57,7 @@ export default util.createRule({ const [, secondArg] = node.arguments; - if ( - node.arguments.length < 2 || - (secondArg.type !== AST_NODE_TYPES.TSAsExpression && - secondArg.type !== AST_NODE_TYPES.TSTypeAssertion) - ) { + if (node.arguments.length < 2 || !util.isTypeAssertion(secondArg)) { return; } From 243de8f30624bae64d726f3f4cf51d4d06cdd9ba Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Tue, 10 Mar 2020 08:50:25 +1300 Subject: [PATCH 08/12] chore(eslint-plugin): use selector to target nodes --- .../src/rules/prefer-reduce-type-parameter.ts | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts index ad3994672ff..08f829da12f 100644 --- a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts +++ b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts @@ -4,6 +4,11 @@ import { } from '@typescript-eslint/experimental-utils'; import * as util from '../util'; +type MemberExpressionWithCallExpressionParent = ( + | TSESTree.MemberExpression + | TSESTree.OptionalMemberExpression +) & { parent: TSESTree.CallExpression | TSESTree.OptionalCallExpression }; + const getMemberExpressionName = ( member: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression, ): string | null => { @@ -44,25 +49,25 @@ export default util.createRule({ const service = util.getParserServices(context); const checker = service.program.getTypeChecker(); - const reportPossibleArrayReduceCasting = ( - node: TSESTree.CallExpression | TSESTree.OptionalCallExpression, - ): void => { - { - if ( - !util.isMemberOrOptionalMemberExpression(node.callee) || - getMemberExpressionName(node.callee) !== 'reduce' - ) { + return { + ':matches(CallExpression, OptionalCallExpression) > :matches(MemberExpression, OptionalMemberExpression)'( + callee: MemberExpressionWithCallExpressionParent, + ) { + if (getMemberExpressionName(callee) !== 'reduce') { return; } - const [, secondArg] = node.arguments; + const [, secondArg] = callee.parent.arguments; - if (node.arguments.length < 2 || !util.isTypeAssertion(secondArg)) { + if ( + callee.parent.arguments.length < 2 || + !util.isTypeAssertion(secondArg) + ) { return; } // Get the symbol of the `reduce` method. - const tsNode = service.esTreeNodeToTSNodeMap.get(node.callee.object); + const tsNode = service.esTreeNodeToTSNodeMap.get(callee.object); const calleeObjType = checker.getTypeAtLocation(tsNode); // Check the owner type of the `reduce` method. @@ -80,7 +85,7 @@ export default util.createRule({ secondArg.range[1], ]), fixer.insertTextAfter( - node.callee, + callee, `<${context .getSourceCode() .getText(secondArg.typeAnnotation)}>`, @@ -90,12 +95,7 @@ export default util.createRule({ return; } - } - }; - - return { - OptionalCallExpression: reportPossibleArrayReduceCasting, - CallExpression: reportPossibleArrayReduceCasting, + }, }; }, }); From 2cd99693fd3676bd47a849e94650e8f703809c70 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 12 Mar 2020 08:54:35 +1300 Subject: [PATCH 09/12] fix(eslint-plugin): add missing return type --- packages/eslint-plugin/src/rules/naming-convention.ts | 1 - .../eslint-plugin/src/rules/prefer-reduce-type-parameter.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/naming-convention.ts b/packages/eslint-plugin/src/rules/naming-convention.ts index 25187268eef..f1963b17321 100644 --- a/packages/eslint-plugin/src/rules/naming-convention.ts +++ b/packages/eslint-plugin/src/rules/naming-convention.ts @@ -755,7 +755,6 @@ function parseOptions(context: Context): ParsedOptions { const parsedOptions = util.getEnumNames(Selectors).reduce((acc, k) => { acc[k] = createValidator(k, context, normalizedOptions); return acc; - // eslint-disable-next-line @typescript-eslint/prefer-reduce-type-parameter }, {} as ParsedOptions); return parsedOptions; diff --git a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts index 08f829da12f..0ee9e023b46 100644 --- a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts +++ b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts @@ -52,7 +52,7 @@ export default util.createRule({ return { ':matches(CallExpression, OptionalCallExpression) > :matches(MemberExpression, OptionalMemberExpression)'( callee: MemberExpressionWithCallExpressionParent, - ) { + ): void { if (getMemberExpressionName(callee) !== 'reduce') { return; } From d0637d559146c411e13cd4188ad562c980620281 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 12 Mar 2020 09:49:01 +1300 Subject: [PATCH 10/12] fix(eslint-plugin): use `getConstrainedTypeAtLocation` --- .../src/rules/prefer-reduce-type-parameter.ts | 5 ++++- .../prefer-reduce-type-parameter.test.ts | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts index 0ee9e023b46..002ca4ad6e3 100644 --- a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts +++ b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts @@ -68,7 +68,10 @@ export default util.createRule({ // Get the symbol of the `reduce` method. const tsNode = service.esTreeNodeToTSNodeMap.get(callee.object); - const calleeObjType = checker.getTypeAtLocation(tsNode); + const calleeObjType = util.getConstrainedTypeAtLocation( + checker, + tsNode, + ); // Check the owner type of the `reduce` method. if (checker.isArrayType(calleeObjType)) { diff --git a/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts b/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts index 17ba3fedc9c..dea0c534dbc 100644 --- a/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts @@ -162,5 +162,24 @@ names.reduce>( }, ], }, + { + code: ` +function f(a: U) { + return a.reduce(() => {}, {} as Record); +} +`, + output: ` +function f(a: U) { + return a.reduce>(() => {}, {}); +} +`, + errors: [ + { + messageId: 'preferTypeParameter', + column: 29, + line: 3, + }, + ], + }, ], }); From 2d9f2a923029eb56f72b6f6c846eeddec79ba658 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 12 Apr 2020 17:38:11 +1200 Subject: [PATCH 11/12] chore(eslint-plugin): fix linting errors --- .../prefer-reduce-type-parameter.test.ts | 68 ++++++++++--------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts b/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts index dea0c534dbc..a683a4ab0dc 100644 --- a/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-reduce-type-parameter.test.ts @@ -14,28 +14,34 @@ const ruleTester = new RuleTester({ ruleTester.run('prefer-reduce-type-parameter', rule, { valid: [ - '(new class Mine { reduce() {} }).reduce(() => {}, 1 as any);', ` - class Mine { reduce() {} } + new (class Mine { + reduce() {} + })().reduce(() => {}, 1 as any); + `, + ` + class Mine { + reduce() {} + } - new Mine().reduce(() => {}, 1 as any); + new Mine().reduce(() => {}, 1 as any); `, ` - import { Reducable } from './class'; + import { Reducable } from './class'; - new Reducable().reduce(() => {}, 1 as any); + new Reducable().reduce(() => {}, 1 as any); `, - "[1, 2, 3]['reduce']((sum, num) => sum + num, 0)", - '[1, 2, 3][null]((sum, num) => sum + num, 0)', - '[1, 2, 3]?.[null]((sum, num) => sum + num, 0)', - '[1, 2, 3].reduce((sum, num) => sum + num, 0)', - '[1, 2, 3].reduce((a, s) => a.concat(s * 2), [])', - '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), [])', + "[1, 2, 3]['reduce']((sum, num) => sum + num, 0);", + '[1, 2, 3][null]((sum, num) => sum + num, 0);', + '[1, 2, 3]?.[null]((sum, num) => sum + num, 0);', + '[1, 2, 3].reduce((sum, num) => sum + num, 0);', + '[1, 2, 3].reduce((a, s) => a.concat(s * 2), []);', + '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), []);', ], invalid: [ { - code: '[1, 2, 3].reduce((a, s) => a.concat(s * 2), [] as number[],)', - output: '[1, 2, 3].reduce((a, s) => a.concat(s * 2), [],)', + code: '[1, 2, 3].reduce((a, s) => a.concat(s * 2), [] as number[]);', + output: '[1, 2, 3].reduce((a, s) => a.concat(s * 2), []);', errors: [ { messageId: 'preferTypeParameter', @@ -45,8 +51,8 @@ ruleTester.run('prefer-reduce-type-parameter', rule, { ], }, { - code: '[1, 2, 3].reduce((a, s) => a.concat(s * 2), [],)', - output: '[1, 2, 3].reduce((a, s) => a.concat(s * 2), [],)', + code: '[1, 2, 3].reduce((a, s) => a.concat(s * 2), []);', + output: '[1, 2, 3].reduce((a, s) => a.concat(s * 2), []);', errors: [ { messageId: 'preferTypeParameter', @@ -56,8 +62,8 @@ ruleTester.run('prefer-reduce-type-parameter', rule, { ], }, { - code: '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), [] as number[])', - output: '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), [])', + code: '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), [] as number[]);', + output: '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), []);', errors: [ { messageId: 'preferTypeParameter', @@ -67,8 +73,8 @@ ruleTester.run('prefer-reduce-type-parameter', rule, { ], }, { - code: '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), [])', - output: '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), [])', + code: '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), []);', + output: '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), []);', errors: [ { messageId: 'preferTypeParameter', @@ -86,7 +92,7 @@ names.reduce( ...accum, [name]: true, }), - {} as Record + {} as Record, ); `, output: ` @@ -97,7 +103,7 @@ names.reduce>( ...accum, [name]: true, }), - {} + {}, ); `, errors: [ @@ -115,7 +121,7 @@ names.reduce>( ...accum, [name]: true, }), - >{} + >{}, ); `, output: ` @@ -124,7 +130,7 @@ names.reduce>( ...accum, [name]: true, }), - {} + {}, ); `, errors: [ @@ -140,20 +146,20 @@ names.reduce>( ['a', 'b']['reduce']( (accum, name) => ({ ...accum, - [name]: true + [name]: true, }), - {} as Record + {} as Record, ); - `, + `, output: ` ['a', 'b']['reduce']>( (accum, name) => ({ ...accum, - [name]: true + [name]: true, }), - {} + {}, ); - `, + `, errors: [ { messageId: 'preferTypeParameter', @@ -167,12 +173,12 @@ names.reduce>( function f(a: U) { return a.reduce(() => {}, {} as Record); } -`, + `, output: ` function f(a: U) { return a.reduce>(() => {}, {}); } -`, + `, errors: [ { messageId: 'preferTypeParameter', From d5f1a47caf033bb99241311df1d0527db74e9725 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 12 Apr 2020 18:02:55 +1200 Subject: [PATCH 12/12] fix(eslint-plugin): improve node selector --- .../eslint-plugin/src/rules/prefer-reduce-type-parameter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts index 002ca4ad6e3..7b63125e743 100644 --- a/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts +++ b/packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts @@ -50,7 +50,7 @@ export default util.createRule({ const checker = service.program.getTypeChecker(); return { - ':matches(CallExpression, OptionalCallExpression) > :matches(MemberExpression, OptionalMemberExpression)'( + ':matches(CallExpression, OptionalCallExpression) > :matches(MemberExpression, OptionalMemberExpression).callee'( callee: MemberExpressionWithCallExpressionParent, ): void { if (getMemberExpressionName(callee) !== 'reduce') {