Skip to content

Commit

Permalink
no-array-for-each: Add fix for parenthesized call (#1784)
Browse files Browse the repository at this point in the history
  • Loading branch information
fisker authored Apr 1, 2022
1 parent 51166f4 commit 5f39c37
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 15 deletions.
13 changes: 5 additions & 8 deletions rules/no-array-for-each.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const {getParentheses} = require('./utils/parentheses.js');
const isFunctionSelfUsedInside = require('./utils/is-function-self-used-inside.js');
const {isNodeMatches} = require('./utils/is-node-matches.js');
const assertToken = require('./utils/assert-token.js');
const {fixSpaceAroundKeyword} = require('./fix/index.js');
const {fixSpaceAroundKeyword, removeParentheses} = require('./fix/index.js');

const MESSAGE_ID_ERROR = 'no-array-for-each/error';
const MESSAGE_ID_SUGGESTION = 'no-array-for-each/suggestion';
Expand Down Expand Up @@ -209,6 +209,9 @@ function getFixFunction(callExpression, functionInfo, context) {
}

return function * (fixer) {
// `(( foo.forEach(bar => bar) ))`
yield * removeParentheses(callExpression, fixer, sourceCode);

// Replace these with `for (const … of …) `
// foo.forEach(bar => bar)
// ^^^^^^^^^^^^^^^^^^ (space after `=>` didn't included)
Expand Down Expand Up @@ -323,14 +326,8 @@ function isFunctionParameterVariableReassigned(callbackFunction, context) {
}

function isFixable(callExpression, {scope, functionInfo, allIdentifiers, context}) {
const sourceCode = context.getSourceCode();
// Check `CallExpression`
if (
callExpression.optional
// TODO: Parenthesized should also be fixable.
|| isParenthesized(callExpression, sourceCode)
|| callExpression.arguments.length !== 1
) {
if (callExpression.optional || callExpression.arguments.length !== 1) {
return false;
}

Expand Down
3 changes: 2 additions & 1 deletion test/no-array-for-each.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ test.snapshot({
invalid: [
'foo.forEach?.(element => bar(element))',
...[
'(( foo.forEach(element => bar(element)) ))',

// Not fixable
'(foo.forEach(element => bar(element)))',
'foo.forEach(element => bar(element), thisArgument)',
'foo.forEach()',
'const baz = foo.forEach(element => bar(element))',
Expand Down
24 changes: 18 additions & 6 deletions test/snapshots/no-array-for-each.mjs.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,35 @@ Generated by [AVA](https://avajs.dev).
`

## Invalid #2
1 | (foo.forEach(element => bar(element)))
1 | (( foo.forEach(element => bar(element)) ))

> Output
`␊
1 | for (const element of foo) bar(element) ␊
`

> Error 1/1
`␊
> 1 | (foo.forEach(element => bar(element)))␊
| ^^^^^^^ Use \`for…of\` instead of \`Array#forEach(…)\`.␊
> 1 | (( foo.forEach(element => bar(element)) ))␊
| ^^^^^^^ Use \`for…of\` instead of \`Array#forEach(…)\`.␊
`

## Invalid #3
1 | (foo?.forEach(element => bar(element)))
1 | (( foo?.forEach(element => bar(element)) ))

> Output
`␊
1 | if (foo) for (const element of foo) bar(element) ␊
`

> Error 1/1
`␊
> 1 | (foo?.forEach(element => bar(element)))␊
| ^^^^^^^ Use \`for…of\` instead of \`Array#forEach(…)\`.␊
> 1 | (( foo?.forEach(element => bar(element)) ))␊
| ^^^^^^^ Use \`for…of\` instead of \`Array#forEach(…)\`.␊
`

## Invalid #4
Expand Down
Binary file modified test/snapshots/no-array-for-each.mjs.snap
Binary file not shown.

0 comments on commit 5f39c37

Please sign in to comment.