Skip to content

Commit fff4a07

Browse files
gabrogabro
authored
Merge pull request #14 from buildo/prefer-chain
Add prefer-chain
2 parents ced60c9 + 3879efa commit fff4a07

File tree

6 files changed

+384
-80
lines changed

6 files changed

+384
-80
lines changed

‎docs/rules/prefer-chain.md

+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# Replace map + flatten with chain (fp-ts/prefer-chain)
2+
3+
Suggest replacing the combination of `map` (or `mapWithIndex`) followed by
4+
`flatten` with `chain` (or `chainWithIndex`).
5+
6+
**Fixable**: This rule is automatically fixable using the `--fix` flag on the
7+
command line.
8+
9+
## Rule Details
10+
11+
Examples of **incorrect** code for this rule:
12+
13+
```ts
14+
import { pipe } from "fp-ts/function";
15+
import { map, flatten } from "fp-ts/Array";
16+
17+
pipe(
18+
[1, 2, 3],
19+
map((n) => [n, n + 1]),
20+
flatten
21+
);
22+
```
23+
24+
```ts
25+
import { pipe } from "fp-ts/function";
26+
import { mapWithIndex, flatten } from "fp-ts/Array";
27+
28+
pipe(
29+
[1, 2, 3],
30+
mapWithIndex((i, n) => [i, n]),
31+
flatten
32+
);
33+
```
34+
35+
Examples of **correct** code for this rule:
36+
37+
```ts
38+
import { pipe } from "fp-ts/function";
39+
import { chain } from "fp-ts/Array";
40+
41+
pipe(
42+
[1, 2, 3],
43+
chain((n) => [n, n + 1])
44+
);
45+
```
46+
47+
```ts
48+
import { pipe } from "fp-ts/function";
49+
import { chainWithIndex } from "fp-ts/Array";
50+
51+
pipe(
52+
[1, 2, 3],
53+
chainWithIndex((i, n) => [i, n])
54+
);
55+
```

‎src/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@ export const rules = {
33
"no-pipeable": require("./rules/no-pipeable"),
44
"prefer-traverse": require("./rules/prefer-traverse"),
55
"no-redundant-flow": require("./rules/no-redundant-flow"),
6+
"prefer-chain": require("./rules/prefer-chain"),
67
};

‎src/rules/prefer-chain.ts

+66
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { TSESLint } from "@typescript-eslint/experimental-utils";
2+
import {
3+
calleeIdentifier,
4+
getAdjacentCombinators,
5+
isPipeOrFlowExpression,
6+
} from "../utils";
7+
8+
const messages = {
9+
mapFlattenIsChain: "map followed by flatten can be replaced by chain",
10+
replaceMapFlattenWithChain: "replace map and flatten with chain",
11+
} as const;
12+
type MessageIds = keyof typeof messages;
13+
14+
export const meta: TSESLint.RuleMetaData<MessageIds> = {
15+
type: "suggestion",
16+
fixable: "code",
17+
schema: [],
18+
messages,
19+
};
20+
21+
export function create(
22+
context: TSESLint.RuleContext<MessageIds, []>
23+
): TSESLint.RuleListener {
24+
return {
25+
CallExpression(node) {
26+
if (isPipeOrFlowExpression(node, context)) {
27+
const result = getAdjacentCombinators(
28+
node,
29+
[{ name: /map|mapWithIndex/ }, { name: "flatten" }],
30+
true
31+
);
32+
if (result) {
33+
const [mapNode, flattenNode] = result;
34+
35+
const chainIndentifier =
36+
calleeIdentifier(mapNode)?.name === "mapWithIndex"
37+
? "chainWithIndex"
38+
: "chain";
39+
40+
context.report({
41+
loc: {
42+
start: mapNode.loc.start,
43+
end: flattenNode.loc.end,
44+
},
45+
messageId: "mapFlattenIsChain",
46+
suggest: [
47+
{
48+
messageId: "replaceMapFlattenWithChain",
49+
fix(fixer) {
50+
return [
51+
fixer.remove(flattenNode),
52+
fixer.removeRange([mapNode.range[1], flattenNode.range[0]]),
53+
fixer.replaceText(
54+
calleeIdentifier(mapNode)!,
55+
chainIndentifier
56+
),
57+
];
58+
},
59+
},
60+
],
61+
});
62+
}
63+
}
64+
},
65+
};
66+
}

‎src/rules/prefer-traverse.ts

+36-66
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
import {
2-
AST_NODE_TYPES,
3-
TSESLint,
4-
} from "@typescript-eslint/experimental-utils";
1+
import { TSESLint, TSESTree } from "@typescript-eslint/experimental-utils";
52
import {
63
calleeIdentifier,
74
getAdjacentCombinators,
@@ -28,76 +25,49 @@ export function create(
2825
return {
2926
CallExpression(node) {
3027
if (isPipeOrFlowExpression(node, context)) {
31-
const result = getAdjacentCombinators(node, [
32-
{ name: /map|mapWithIndex/ },
33-
{ name: "sequence" },
34-
]);
28+
const result = getAdjacentCombinators(
29+
node,
30+
[{ name: /map|mapWithIndex/ }, { name: "sequence" }],
31+
true
32+
);
3533
if (result) {
36-
const [mapNode, sequenceNode] = result;
37-
38-
// NOTE(gabro): this is a naive way of checking whether map and sequence are from the same module
39-
// We assume the most commonly used syntax is something like:
40-
//
41-
// import { array } from 'fp-ts'
42-
// pipe(
43-
// [1, 2],
44-
// array.map(...),
45-
// array.sequence(...)
46-
// )
47-
//
48-
// So we check that array.map and array.sequence have the same prefix ("array." in this example)
49-
// by pretty-printing it and comparing the result.
50-
//
51-
// This works well enough in practice, but if needed this can be made more exact by using
52-
// TypeScript's compiler API and comparing the types.
53-
const mapPrefix =
54-
mapNode.callee.type === AST_NODE_TYPES.MemberExpression
55-
? prettyPrint(mapNode.callee.object)
56-
: "";
57-
58-
const sequencePrefix =
59-
sequenceNode.callee.type === AST_NODE_TYPES.MemberExpression
60-
? prettyPrint(sequenceNode.callee.object)
61-
: "";
62-
63-
const samePrefix = mapPrefix === sequencePrefix;
34+
const mapNode = result[0] as TSESTree.CallExpression;
35+
const sequenceNode = result[1] as TSESTree.CallExpression;
6436

6537
const traverseIdentifier =
6638
calleeIdentifier(mapNode)?.name === "mapWithIndex"
6739
? "traverseWithIndex"
6840
: "traverse";
6941

70-
if (mapNode && sequenceNode && mapNode && samePrefix) {
71-
context.report({
72-
loc: {
73-
start: mapNode.loc.start,
74-
end: sequenceNode.loc.end,
75-
},
76-
messageId: "mapSequenceIsTraverse",
77-
suggest: [
78-
{
79-
messageId: "replaceMapSequenceWithTraverse",
80-
fix(fixer) {
81-
return [
82-
fixer.remove(sequenceNode),
83-
fixer.removeRange([
84-
mapNode.range[1],
85-
sequenceNode.range[0],
86-
]),
87-
fixer.replaceText(
88-
calleeIdentifier(mapNode)!,
89-
traverseIdentifier
90-
),
91-
fixer.insertTextAfter(
92-
mapNode.callee,
93-
`(${prettyPrint(sequenceNode.arguments[0]!)})`
94-
),
95-
];
96-
},
42+
context.report({
43+
loc: {
44+
start: mapNode.loc.start,
45+
end: sequenceNode.loc.end,
46+
},
47+
messageId: "mapSequenceIsTraverse",
48+
suggest: [
49+
{
50+
messageId: "replaceMapSequenceWithTraverse",
51+
fix(fixer) {
52+
return [
53+
fixer.remove(sequenceNode),
54+
fixer.removeRange([
55+
mapNode.range[1],
56+
sequenceNode.range[0],
57+
]),
58+
fixer.replaceText(
59+
calleeIdentifier(mapNode)!,
60+
traverseIdentifier
61+
),
62+
fixer.insertTextAfter(
63+
mapNode.callee,
64+
`(${prettyPrint(sequenceNode.arguments[0]!)})`
65+
),
66+
];
9767
},
98-
],
99-
});
100-
}
68+
},
69+
],
70+
});
10171
}
10272
}
10373
},

‎src/utils.ts

+79-14
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,33 @@ export function isIdentifierImportedFrom<
2525
}
2626

2727
export function calleeIdentifier(
28-
node: TSESTree.CallExpression
28+
node:
29+
| TSESTree.CallExpression
30+
| TSESTree.MemberExpression
31+
| TSESTree.Identifier
2932
): TSESTree.Identifier | undefined {
30-
switch (node.callee.type) {
31-
case AST_NODE_TYPES.Identifier:
32-
return node.callee;
33+
switch (node.type) {
3334
case AST_NODE_TYPES.MemberExpression:
34-
if (node.callee.property.type === AST_NODE_TYPES.Identifier) {
35-
return node.callee.property;
35+
if (node.property.type === AST_NODE_TYPES.Identifier) {
36+
return node.property;
3637
} else {
3738
return undefined;
3839
}
40+
case AST_NODE_TYPES.CallExpression:
41+
switch (node.callee.type) {
42+
case AST_NODE_TYPES.Identifier:
43+
return node.callee;
44+
case AST_NODE_TYPES.MemberExpression:
45+
if (node.callee.property.type === AST_NODE_TYPES.Identifier) {
46+
return node.callee.property;
47+
} else {
48+
return undefined;
49+
}
50+
}
51+
return undefined;
52+
case AST_NODE_TYPES.Identifier:
53+
return node;
3954
}
40-
return undefined;
4155
}
4256

4357
export function isFlowExpression<
@@ -73,18 +87,29 @@ export function isPipeOrFlowExpression<
7387
type CombinatorQuery = {
7488
name: string | RegExp;
7589
};
90+
type CombinatorNode =
91+
| TSESTree.CallExpression
92+
| TSESTree.MemberExpression
93+
| TSESTree.Identifier;
7694
export function getAdjacentCombinators(
7795
pipeOrFlowExpression: TSESTree.CallExpression,
78-
combinatorQueries: [CombinatorQuery, CombinatorQuery]
79-
): [TSESTree.CallExpression, TSESTree.CallExpression] | undefined {
96+
combinatorQueries: [CombinatorQuery, CombinatorQuery],
97+
requireMatchingPrefix: boolean
98+
): [CombinatorNode, CombinatorNode] | undefined {
8099
const firstCombinatorIndex = pipeOrFlowExpression.arguments.findIndex(
81100
(a, index) => {
82101
if (
83-
a.type === AST_NODE_TYPES.CallExpression &&
102+
(a.type === AST_NODE_TYPES.CallExpression ||
103+
a.type === AST_NODE_TYPES.MemberExpression ||
104+
a.type === AST_NODE_TYPES.Identifier) &&
84105
index < pipeOrFlowExpression.arguments.length - 1
85106
) {
86107
const b = pipeOrFlowExpression.arguments[index + 1];
87-
if (b?.type === AST_NODE_TYPES.CallExpression) {
108+
if (
109+
b?.type === AST_NODE_TYPES.CallExpression ||
110+
b?.type === AST_NODE_TYPES.MemberExpression ||
111+
b?.type === AST_NODE_TYPES.Identifier
112+
) {
88113
return (
89114
calleeIdentifier(a)?.name.match(combinatorQueries[0].name) &&
90115
calleeIdentifier(b)?.name.match(combinatorQueries[1].name)
@@ -98,13 +123,53 @@ export function getAdjacentCombinators(
98123
if (firstCombinatorIndex >= 0) {
99124
const firstCombinator = pipeOrFlowExpression.arguments[
100125
firstCombinatorIndex
101-
] as TSESTree.CallExpression;
126+
] as CombinatorNode;
102127

103128
const secondCombinator = pipeOrFlowExpression.arguments[
104129
firstCombinatorIndex + 1
105-
] as TSESTree.CallExpression;
130+
] as CombinatorNode;
106131

107-
return [firstCombinator, secondCombinator];
132+
if (requireMatchingPrefix) {
133+
// NOTE(gabro): this is a naive way of checking whether two combinators are
134+
// from the same module.
135+
// We assume the most commonly used syntax is something like:
136+
//
137+
// import { array } from 'fp-ts'
138+
// pipe(
139+
// [1, 2],
140+
// array.map(...),
141+
// array.sequence(...)
142+
// )
143+
//
144+
// So we check that array.map and array.sequence have the same prefix ("array." in this example)
145+
// by pretty-printing it and comparing the result.
146+
//
147+
// This works well enough in practice, but if needed this can be made more exact by using
148+
// TypeScript's compiler API and comparing the types.
149+
const getPrefix = (
150+
node:
151+
| TSESTree.CallExpression
152+
| TSESTree.MemberExpression
153+
| TSESTree.Identifier
154+
): string => {
155+
switch (node.type) {
156+
case AST_NODE_TYPES.CallExpression:
157+
return node.callee.type === AST_NODE_TYPES.MemberExpression
158+
? prettyPrint(node.callee.object)
159+
: "";
160+
case AST_NODE_TYPES.MemberExpression:
161+
return prettyPrint(node.object);
162+
case AST_NODE_TYPES.Identifier:
163+
return "";
164+
}
165+
};
166+
167+
if (getPrefix(firstCombinator) === getPrefix(secondCombinator)) {
168+
return [firstCombinator, secondCombinator];
169+
}
170+
} else {
171+
return [firstCombinator, secondCombinator];
172+
}
108173
}
109174

110175
return undefined;

0 commit comments

Comments
 (0)