Skip to content

Commit

Permalink
feat(eslint-plugin): add rule prefer-reduce-type-parameter (#1707)
Browse files Browse the repository at this point in the history
  • Loading branch information
G-Rath authored Apr 13, 2020
1 parent 2e9c202 commit c92d240
Show file tree
Hide file tree
Showing 8 changed files with 360 additions and 2 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | | :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: |
Expand Down
54 changes: 54 additions & 0 deletions packages/eslint-plugin/docs/rules/prefer-reduce-type-parameter.md
Original file line number Diff line number Diff line change
@@ -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<string, boolean>,
);
```

Examples of **correct** code for this rule:

```ts
[1, 2, 3].reduce<number[]>((arr, num) => arr.concat(num * 2), []);

['a', 'b'].reduce<Record<string, boolean>>(
(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.
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
104 changes: 104 additions & 0 deletions packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import {
AST_NODE_TYPES,
TSESTree,
} 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 => {
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: false,
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();

return {
':matches(CallExpression, OptionalCallExpression) > :matches(MemberExpression, OptionalMemberExpression).callee'(
callee: MemberExpressionWithCallExpressionParent,
): void {
if (getMemberExpressionName(callee) !== 'reduce') {
return;
}

const [, secondArg] = callee.parent.arguments;

if (
callee.parent.arguments.length < 2 ||
!util.isTypeAssertion(secondArg)
) {
return;
}

// Get the symbol of the `reduce` method.
const tsNode = service.esTreeNodeToTSNodeMap.get(callee.object);
const calleeObjType = util.getConstrainedTypeAtLocation(
checker,
tsNode,
);

// Check the owner type of the `reduce` method.
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(
callee,
`<${context
.getSourceCode()
.getText(secondArg.typeAnnotation)}>`,
),
],
});

return;
}
},
};
},
});
5 changes: 5 additions & 0 deletions packages/eslint-plugin/tests/fixtures/class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
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, 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<number[]>((a, s) => a.concat(s * 2), []);',
'[1, 2, 3]?.reduce<number[]>((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<number[]>((a, s) => a.concat(s * 2), []);',
errors: [
{
messageId: 'preferTypeParameter',
column: 45,
line: 1,
},
],
},
{
code: '[1, 2, 3].reduce((a, s) => a.concat(s * 2), <number[]>[]);',
output: '[1, 2, 3].reduce<number[]>((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<number[]>((a, s) => a.concat(s * 2), []);',
errors: [
{
messageId: 'preferTypeParameter',
column: 46,
line: 1,
},
],
},
{
code: '[1, 2, 3]?.reduce((a, s) => a.concat(s * 2), <number[]>[]);',
output: '[1, 2, 3]?.reduce<number[]>((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<string, boolean>,
);
`,
output: `
const names = ['a', 'b', 'c'];
names.reduce<Record<string, boolean>>(
(accum, name) => ({
...accum,
[name]: true,
}),
{},
);
`,
errors: [
{
messageId: 'preferTypeParameter',
column: 3,
line: 9,
},
],
},
{
code: `
['a', 'b'].reduce(
(accum, name) => ({
...accum,
[name]: true,
}),
<Record<string, boolean>>{},
);
`,
output: `
['a', 'b'].reduce<Record<string, boolean>>(
(accum, name) => ({
...accum,
[name]: true,
}),
{},
);
`,
errors: [
{
messageId: 'preferTypeParameter',
column: 3,
line: 7,
},
],
},
{
code: `
['a', 'b']['reduce'](
(accum, name) => ({
...accum,
[name]: true,
}),
{} as Record<string, boolean>,
);
`,
output: `
['a', 'b']['reduce']<Record<string, boolean>>(
(accum, name) => ({
...accum,
[name]: true,
}),
{},
);
`,
errors: [
{
messageId: 'preferTypeParameter',
column: 3,
line: 7,
},
],
},
{
code: `
function f<T, U extends T[]>(a: U) {
return a.reduce(() => {}, {} as Record<string, boolean>);
}
`,
output: `
function f<T, U extends T[]>(a: U) {
return a.reduce<Record<string, boolean>>(() => {}, {});
}
`,
errors: [
{
messageId: 'preferTypeParameter',
column: 29,
line: 3,
},
],
},
],
});
Loading

0 comments on commit c92d240

Please sign in to comment.