Skip to content

Commit f6a939c

Browse files
fiskerfisker
authored
prefer-array-flat: Fix incorrect fix, check Array.prototype.concat.call (#1317)
1 parent d37439f commit f6a939c

File tree

6 files changed

+987
-52
lines changed

6 files changed

+987
-52
lines changed

‎docs/rules/prefer-array-flat.md‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,49 +7,61 @@ This rule is fixable.
77
## Fail
88

99
```js
10-
const foo = bar.flatMap(x => x);
10+
const foo = array.flatMap(x => x);
1111
```
1212

1313
```js
14-
const foo = bar.reduce((a, b) => a.concat(b), []);
14+
const foo = array.reduce((a, b) => a.concat(b), []);
1515
```
1616

1717
```js
18-
const foo = bar.reduce((a, b) => [...a, ...b], []);
18+
const foo = array.reduce((a, b) => [...a, ...b], []);
1919
```
2020

2121
```js
22-
const foo = [].concat(bar);
22+
const foo = [].concat(maybeArray);
2323
```
2424

2525
```js
26-
const foo = [].concat(...bar);
26+
const foo = [].concat(...array);
2727
```
2828

2929
```js
30-
const foo = [].concat.apply([], bar);
30+
const foo = [].concat.apply([], array);
3131
```
3232

3333
```js
34-
const foo = Array.prototype.concat.apply([], bar);
34+
const foo = Array.prototype.concat.apply([], array);
3535
```
3636

3737
```js
38-
const foo = _.flatten(bar);
38+
const foo = Array.prototype.concat.call([], maybeArray);
3939
```
4040

4141
```js
42-
const foo = lodash.flatten(bar);
42+
const foo = Array.prototype.concat.call([], ...array);
4343
```
4444

4545
```js
46-
const foo = underscore.flatten(bar);
46+
const foo = _.flatten(array);
47+
```
48+
49+
```js
50+
const foo = lodash.flatten(array);
51+
```
52+
53+
```js
54+
const foo = underscore.flatten(array);
4755
```
4856

4957
## Pass
5058

5159
```js
52-
const foo = bar.flat();
60+
const foo = array.flat();
61+
```
62+
63+
```js
64+
const foo = [maybeArray].flat();
5365
```
5466

5567
## Options

‎rules/prefer-array-flat.js‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const {
99
const needsSemicolon = require('./utils/needs-semicolon');
1010
const shouldAddParenthesesToMemberExpressionObject = require('./utils/should-add-parentheses-to-member-expression-object');
1111
const {isNodeMatches, isNodeMatchesNameOrPath} = require('./utils/is-node-matches');
12+
const {getParenthesizedText, isParenthesized} = require('./utils/parentheses');
1213

1314
const MESSAGE_ID = 'prefer-array-flat';
1415
const messages = {
@@ -89,7 +90,7 @@ const arrayReduce2 = {
8990
description: 'Array#reduce()'
9091
};
9192

92-
// `[].concat(array)` and `[].concat(...array)`
93+
// `[].concat(maybeArray)` and `[].concat(...array)`
9394
const emptyArrayConcat = {
9495
selector: [
9596
methodCallSelector({
@@ -103,24 +104,33 @@ const emptyArrayConcat = {
103104
const argumentNode = node.arguments[0];
104105
return argumentNode.type === 'SpreadElement' ? argumentNode.argument : argumentNode;
105106
},
106-
description: '[].concat()'
107+
description: '[].concat()',
108+
shouldSwitchToArray: node => node.arguments[0].type !== 'SpreadElement'
107109
};
108110

109-
// `[].concat.apply([], array)` and `Array.prototype.concat.apply([], array)`
111+
// - `[].concat.apply([], array)` and `Array.prototype.concat.apply([], array)`
112+
// - `[].concat.call([], maybeArray)` and `Array.prototype.concat.call([], maybeArray)`
113+
// - `[].concat.call([], ...array)` and `Array.prototype.concat.call([], ...array)`
110114
const arrayPrototypeConcat = {
111115
selector: [
112116
methodCallSelector({
113-
name: 'apply',
114-
length: 2
117+
names: ['apply', 'call'],
118+
length: 2,
119+
allowSpreadElement: true
115120
}),
116121
emptyArraySelector('arguments.0'),
117122
arrayPrototypeMethodSelector({
118123
path: 'callee.object',
119124
name: 'concat'
120125
})
121126
].join(''),
122-
getArrayNode: node => node.arguments[1],
123-
description: 'Array.prototype.concat()'
127+
testFunction: node => node.arguments[1].type !== 'SpreadElement' || node.callee.property.name === 'call',
128+
getArrayNode: node => {
129+
const argumentNode = node.arguments[1];
130+
return argumentNode.type === 'SpreadElement' ? argumentNode.argument : argumentNode;
131+
},
132+
description: 'Array.prototype.concat()',
133+
shouldSwitchToArray: node => node.arguments[1].type !== 'SpreadElement' && node.callee.property.name === 'call'
124134
};
125135

126136
const lodashFlattenFunctions = [
@@ -133,10 +143,21 @@ const anyCall = {
133143
getArrayNode: node => node.arguments[0]
134144
};
135145

136-
function fix(node, array, sourceCode) {
146+
function fix(node, array, sourceCode, shouldSwitchToArray) {
147+
if (typeof shouldSwitchToArray === 'function') {
148+
shouldSwitchToArray = shouldSwitchToArray(node);
149+
}
150+
137151
return fixer => {
138-
let fixed = sourceCode.getText(array);
139-
if (shouldAddParenthesesToMemberExpressionObject(array, sourceCode)) {
152+
let fixed = getParenthesizedText(array, sourceCode);
153+
if (shouldSwitchToArray) {
154+
// `array` is an argument, when it changes to `array[]`, we don't need add extra parentheses
155+
fixed = `[${fixed}]`;
156+
// And we don't need to add parentheses to the new array to call `.flat()`
157+
} else if (
158+
!isParenthesized(array, sourceCode) &&
159+
shouldAddParenthesesToMemberExpressionObject(array, sourceCode)
160+
) {
140161
fixed = `(${fixed})`;
141162
}
142163

@@ -173,7 +194,7 @@ function create(context) {
173194
}
174195
];
175196

176-
for (const {selector, testFunction, description, getArrayNode} of cases) {
197+
for (const {selector, testFunction, description, getArrayNode, shouldSwitchToArray} of cases) {
177198
listeners[selector] = function (node) {
178199
if (testFunction && !testFunction(node)) {
179200
return;
@@ -196,7 +217,7 @@ function create(context) {
196217
sourceCode.getCommentsInside(node).length ===
197218
sourceCode.getCommentsInside(array).length
198219
) {
199-
problem.fix = fix(node, array, sourceCode);
220+
problem.fix = fix(node, array, sourceCode, shouldSwitchToArray);
200221
}
201222

202223
context.report(problem);

‎rules/utils/should-add-parentheses-to-member-expression-object.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ function shouldAddParenthesesToMemberExpressionObject(node, sourceCode) {
2020
case 'ChainExpression':
2121
case 'TemplateLiteral':
2222
case 'ThisExpression':
23+
case 'ArrayExpression':
2324
return false;
2425
case 'NewExpression':
2526
return !isNewExpressionWithParentheses(node, sourceCode);

‎test/prefer-array-flat.mjs‎

Lines changed: 93 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,11 @@ test.snapshot({
9797
'[].concat?.(array)'
9898
],
9999
invalid: [
100-
'[].concat(array)'
100+
'[].concat(maybeArray)',
101+
'[].concat( ((0, maybeArray)) )',
102+
'[].concat( ((maybeArray)) )',
103+
'[].concat( [foo] )',
104+
'[].concat( [[foo]] )'
101105
]
102106
});
103107

@@ -115,11 +119,16 @@ test.snapshot({
115119
'[].concat?.(...array)'
116120
],
117121
invalid: [
118-
'[].concat(...array)'
122+
'[].concat(...array)',
123+
'[].concat(...(( array )))',
124+
'[].concat(...(( [foo] )))',
125+
'[].concat(...(( [[foo]] )))'
119126
]
120127
});
121128

122-
// `[].concat.apply([], array)`
129+
// - `[].concat.apply([], array)`
130+
// - `[].concat.call([], maybeArray)`
131+
// - `[].concat.call([], ...array)`
123132
test.snapshot({
124133
valid: [
125134
'new [].concat.apply([], array)',
@@ -139,11 +148,29 @@ test.snapshot({
139148
'[]?.concat.apply([], array)'
140149
],
141150
invalid: [
142-
'[].concat.apply([], array)'
151+
'[].concat.apply([], array)',
152+
'[].concat.apply([], ((0, array)))',
153+
'[].concat.apply([], ((array)))',
154+
'[].concat.apply([], [foo])',
155+
'[].concat.apply([], [[foo]])',
156+
157+
'[].concat.call([], maybeArray)',
158+
'[].concat.call([], ((0, maybeArray)))',
159+
'[].concat.call([], ((maybeArray)))',
160+
'[].concat.call([], [foo])',
161+
'[].concat.call([], [[foo]])',
162+
163+
'[].concat.call([], ...array)',
164+
'[].concat.call([], ...((0, array)))',
165+
'[].concat.call([], ...((array)))',
166+
'[].concat.call([], ...[foo])',
167+
'[].concat.call([], ...[[foo]])'
143168
]
144169
});
145170

146-
// `Array.prototype.concat.apply([], array)`
171+
// - `Array.prototype.concat.apply([], array)`
172+
// - `Array.prototype.concat.call([], maybeArray)`
173+
// - `Array.prototype.concat.call([], ...array)`
147174
test.snapshot({
148175
valid: [
149176
'new Array.prototype.concat.apply([], array)',
@@ -167,7 +194,23 @@ test.snapshot({
167194
'object.Array.prototype.concat.apply([], array)'
168195
],
169196
invalid: [
170-
'Array.prototype.concat.apply([], array)'
197+
'Array.prototype.concat.apply([], array)',
198+
'Array.prototype.concat.apply([], ((0, array)))',
199+
'Array.prototype.concat.apply([], ((array)))',
200+
'Array.prototype.concat.apply([], [foo])',
201+
'Array.prototype.concat.apply([], [[foo]])',
202+
203+
'Array.prototype.concat.call([], maybeArray)',
204+
'Array.prototype.concat.call([], ((0, maybeArray)))',
205+
'Array.prototype.concat.call([], ((maybeArray)))',
206+
'Array.prototype.concat.call([], [foo])',
207+
'Array.prototype.concat.call([], [[foo]])',
208+
209+
'Array.prototype.concat.call([], ...array)',
210+
'Array.prototype.concat.call([], ...((0, array)))',
211+
'Array.prototype.concat.call([], ...((array)))',
212+
'Array.prototype.concat.call([], ...[foo])',
213+
'Array.prototype.concat.call([], ...[[foo]])'
171214
]
172215
});
173216

@@ -294,9 +337,53 @@ test.snapshot({
294337
before()
295338
Array.prototype.concat.apply([], [array].concat(array))
296339
`,
340+
outdent`
341+
before()
342+
Array.prototype.concat.apply([], +1)
343+
`,
344+
outdent`
345+
before()
346+
Array.prototype.concat.call([], +1)
347+
`,
297348
// Parentheses
298349
'Array.prototype.concat.apply([], (0, array))',
350+
'Array.prototype.concat.call([], (0, array))',
299351
'async function a() { return [].concat(await getArray()); }',
352+
'_.flatten((0, array))',
353+
'async function a() { return _.flatten(await getArray()); }',
354+
'async function a() { return _.flatten((await getArray())); }',
355+
outdent`
356+
before()
357+
Array.prototype.concat.apply([], 1)
358+
`,
359+
outdent`
360+
before()
361+
Array.prototype.concat.call([], 1)
362+
`,
363+
outdent`
364+
before()
365+
Array.prototype.concat.apply([], 1.)
366+
`,
367+
outdent`
368+
before()
369+
Array.prototype.concat.call([], 1.)
370+
`,
371+
outdent`
372+
before()
373+
Array.prototype.concat.apply([], .1)
374+
`,
375+
outdent`
376+
before()
377+
Array.prototype.concat.call([], .1)
378+
`,
379+
outdent`
380+
before()
381+
Array.prototype.concat.apply([], 1.0)
382+
`,
383+
outdent`
384+
before()
385+
Array.prototype.concat.call([], 1.0)
386+
`,
300387
// Comment
301388
'[].concat(some./**/array)',
302389
'[/**/].concat(some./**/array)',

0 commit comments

Comments
 (0)