Skip to content

Commit

Permalink
prefer-array-flat: Fix incorrect fix, check `Array.prototype.concat…
Browse files Browse the repository at this point in the history
….call` (#1317)
  • Loading branch information
fisker authored May 27, 2021
1 parent d37439f commit f6a939c
Show file tree
Hide file tree
Showing 6 changed files with 987 additions and 52 deletions.
34 changes: 23 additions & 11 deletions docs/rules/prefer-array-flat.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,49 +7,61 @@ This rule is fixable.
## Fail

```js
const foo = bar.flatMap(x => x);
const foo = array.flatMap(x => x);
```

```js
const foo = bar.reduce((a, b) => a.concat(b), []);
const foo = array.reduce((a, b) => a.concat(b), []);
```

```js
const foo = bar.reduce((a, b) => [...a, ...b], []);
const foo = array.reduce((a, b) => [...a, ...b], []);
```

```js
const foo = [].concat(bar);
const foo = [].concat(maybeArray);
```

```js
const foo = [].concat(...bar);
const foo = [].concat(...array);
```

```js
const foo = [].concat.apply([], bar);
const foo = [].concat.apply([], array);
```

```js
const foo = Array.prototype.concat.apply([], bar);
const foo = Array.prototype.concat.apply([], array);
```

```js
const foo = _.flatten(bar);
const foo = Array.prototype.concat.call([], maybeArray);
```

```js
const foo = lodash.flatten(bar);
const foo = Array.prototype.concat.call([], ...array);
```

```js
const foo = underscore.flatten(bar);
const foo = _.flatten(array);
```

```js
const foo = lodash.flatten(array);
```

```js
const foo = underscore.flatten(array);
```

## Pass

```js
const foo = bar.flat();
const foo = array.flat();
```

```js
const foo = [maybeArray].flat();
```

## Options
Expand Down
45 changes: 33 additions & 12 deletions rules/prefer-array-flat.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const {
const needsSemicolon = require('./utils/needs-semicolon');
const shouldAddParenthesesToMemberExpressionObject = require('./utils/should-add-parentheses-to-member-expression-object');
const {isNodeMatches, isNodeMatchesNameOrPath} = require('./utils/is-node-matches');
const {getParenthesizedText, isParenthesized} = require('./utils/parentheses');

const MESSAGE_ID = 'prefer-array-flat';
const messages = {
Expand Down Expand Up @@ -89,7 +90,7 @@ const arrayReduce2 = {
description: 'Array#reduce()'
};

// `[].concat(array)` and `[].concat(...array)`
// `[].concat(maybeArray)` and `[].concat(...array)`
const emptyArrayConcat = {
selector: [
methodCallSelector({
Expand All @@ -103,24 +104,33 @@ const emptyArrayConcat = {
const argumentNode = node.arguments[0];
return argumentNode.type === 'SpreadElement' ? argumentNode.argument : argumentNode;
},
description: '[].concat()'
description: '[].concat()',
shouldSwitchToArray: node => node.arguments[0].type !== 'SpreadElement'
};

// `[].concat.apply([], array)` and `Array.prototype.concat.apply([], array)`
// - `[].concat.apply([], array)` and `Array.prototype.concat.apply([], array)`
// - `[].concat.call([], maybeArray)` and `Array.prototype.concat.call([], maybeArray)`
// - `[].concat.call([], ...array)` and `Array.prototype.concat.call([], ...array)`
const arrayPrototypeConcat = {
selector: [
methodCallSelector({
name: 'apply',
length: 2
names: ['apply', 'call'],
length: 2,
allowSpreadElement: true
}),
emptyArraySelector('arguments.0'),
arrayPrototypeMethodSelector({
path: 'callee.object',
name: 'concat'
})
].join(''),
getArrayNode: node => node.arguments[1],
description: 'Array.prototype.concat()'
testFunction: node => node.arguments[1].type !== 'SpreadElement' || node.callee.property.name === 'call',
getArrayNode: node => {
const argumentNode = node.arguments[1];
return argumentNode.type === 'SpreadElement' ? argumentNode.argument : argumentNode;
},
description: 'Array.prototype.concat()',
shouldSwitchToArray: node => node.arguments[1].type !== 'SpreadElement' && node.callee.property.name === 'call'
};

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

function fix(node, array, sourceCode) {
function fix(node, array, sourceCode, shouldSwitchToArray) {
if (typeof shouldSwitchToArray === 'function') {
shouldSwitchToArray = shouldSwitchToArray(node);
}

return fixer => {
let fixed = sourceCode.getText(array);
if (shouldAddParenthesesToMemberExpressionObject(array, sourceCode)) {
let fixed = getParenthesizedText(array, sourceCode);
if (shouldSwitchToArray) {
// `array` is an argument, when it changes to `array[]`, we don't need add extra parentheses
fixed = `[${fixed}]`;
// And we don't need to add parentheses to the new array to call `.flat()`
} else if (
!isParenthesized(array, sourceCode) &&
shouldAddParenthesesToMemberExpressionObject(array, sourceCode)
) {
fixed = `(${fixed})`;
}

Expand Down Expand Up @@ -173,7 +194,7 @@ function create(context) {
}
];

for (const {selector, testFunction, description, getArrayNode} of cases) {
for (const {selector, testFunction, description, getArrayNode, shouldSwitchToArray} of cases) {
listeners[selector] = function (node) {
if (testFunction && !testFunction(node)) {
return;
Expand All @@ -196,7 +217,7 @@ function create(context) {
sourceCode.getCommentsInside(node).length ===
sourceCode.getCommentsInside(array).length
) {
problem.fix = fix(node, array, sourceCode);
problem.fix = fix(node, array, sourceCode, shouldSwitchToArray);
}

context.report(problem);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ function shouldAddParenthesesToMemberExpressionObject(node, sourceCode) {
case 'ChainExpression':
case 'TemplateLiteral':
case 'ThisExpression':
case 'ArrayExpression':
return false;
case 'NewExpression':
return !isNewExpressionWithParentheses(node, sourceCode);
Expand Down
99 changes: 93 additions & 6 deletions test/prefer-array-flat.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,11 @@ test.snapshot({
'[].concat?.(array)'
],
invalid: [
'[].concat(array)'
'[].concat(maybeArray)',
'[].concat( ((0, maybeArray)) )',
'[].concat( ((maybeArray)) )',
'[].concat( [foo] )',
'[].concat( [[foo]] )'
]
});

Expand All @@ -115,11 +119,16 @@ test.snapshot({
'[].concat?.(...array)'
],
invalid: [
'[].concat(...array)'
'[].concat(...array)',
'[].concat(...(( array )))',
'[].concat(...(( [foo] )))',
'[].concat(...(( [[foo]] )))'
]
});

// `[].concat.apply([], array)`
// - `[].concat.apply([], array)`
// - `[].concat.call([], maybeArray)`
// - `[].concat.call([], ...array)`
test.snapshot({
valid: [
'new [].concat.apply([], array)',
Expand All @@ -139,11 +148,29 @@ test.snapshot({
'[]?.concat.apply([], array)'
],
invalid: [
'[].concat.apply([], array)'
'[].concat.apply([], array)',
'[].concat.apply([], ((0, array)))',
'[].concat.apply([], ((array)))',
'[].concat.apply([], [foo])',
'[].concat.apply([], [[foo]])',

'[].concat.call([], maybeArray)',
'[].concat.call([], ((0, maybeArray)))',
'[].concat.call([], ((maybeArray)))',
'[].concat.call([], [foo])',
'[].concat.call([], [[foo]])',

'[].concat.call([], ...array)',
'[].concat.call([], ...((0, array)))',
'[].concat.call([], ...((array)))',
'[].concat.call([], ...[foo])',
'[].concat.call([], ...[[foo]])'
]
});

// `Array.prototype.concat.apply([], array)`
// - `Array.prototype.concat.apply([], array)`
// - `Array.prototype.concat.call([], maybeArray)`
// - `Array.prototype.concat.call([], ...array)`
test.snapshot({
valid: [
'new Array.prototype.concat.apply([], array)',
Expand All @@ -167,7 +194,23 @@ test.snapshot({
'object.Array.prototype.concat.apply([], array)'
],
invalid: [
'Array.prototype.concat.apply([], array)'
'Array.prototype.concat.apply([], array)',
'Array.prototype.concat.apply([], ((0, array)))',
'Array.prototype.concat.apply([], ((array)))',
'Array.prototype.concat.apply([], [foo])',
'Array.prototype.concat.apply([], [[foo]])',

'Array.prototype.concat.call([], maybeArray)',
'Array.prototype.concat.call([], ((0, maybeArray)))',
'Array.prototype.concat.call([], ((maybeArray)))',
'Array.prototype.concat.call([], [foo])',
'Array.prototype.concat.call([], [[foo]])',

'Array.prototype.concat.call([], ...array)',
'Array.prototype.concat.call([], ...((0, array)))',
'Array.prototype.concat.call([], ...((array)))',
'Array.prototype.concat.call([], ...[foo])',
'Array.prototype.concat.call([], ...[[foo]])'
]
});

Expand Down Expand Up @@ -294,9 +337,53 @@ test.snapshot({
before()
Array.prototype.concat.apply([], [array].concat(array))
`,
outdent`
before()
Array.prototype.concat.apply([], +1)
`,
outdent`
before()
Array.prototype.concat.call([], +1)
`,
// Parentheses
'Array.prototype.concat.apply([], (0, array))',
'Array.prototype.concat.call([], (0, array))',
'async function a() { return [].concat(await getArray()); }',
'_.flatten((0, array))',
'async function a() { return _.flatten(await getArray()); }',
'async function a() { return _.flatten((await getArray())); }',
outdent`
before()
Array.prototype.concat.apply([], 1)
`,
outdent`
before()
Array.prototype.concat.call([], 1)
`,
outdent`
before()
Array.prototype.concat.apply([], 1.)
`,
outdent`
before()
Array.prototype.concat.call([], 1.)
`,
outdent`
before()
Array.prototype.concat.apply([], .1)
`,
outdent`
before()
Array.prototype.concat.call([], .1)
`,
outdent`
before()
Array.prototype.concat.apply([], 1.0)
`,
outdent`
before()
Array.prototype.concat.call([], 1.0)
`,
// Comment
'[].concat(some./**/array)',
'[/**/].concat(some./**/array)',
Expand Down
Loading

0 comments on commit f6a939c

Please sign in to comment.