Skip to content

Commit

Permalink
no-array-for-each: Stop mention "Array" (#1783)
Browse files Browse the repository at this point in the history
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
  • Loading branch information
fisker and sindresorhus committed Apr 2, 2022
1 parent 32ae436 commit 67bc49f
Show file tree
Hide file tree
Showing 5 changed files with 245 additions and 246 deletions.
4 changes: 2 additions & 2 deletions docs/rules/no-array-for-each.md
@@ -1,4 +1,4 @@
# Prefer `for…of` over `Array#forEach(…)`
# Prefer `for…of` over the `forEach` method

<!-- Do not manually modify RULE_NOTICE part. Run: `npm run generate-rule-notices` -->
<!-- RULE_NOTICE -->
Expand All @@ -7,7 +7,7 @@
🔧💡 *This rule is [auto-fixable](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) and provides [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).*
<!-- /RULE_NOTICE -->

Benefits of [`for…of` statement](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of) over [`Array#forEach(…)`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/forEach) can include:
Benefits of [`for…of` statement](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of) over the `forEach` method can include:

- Faster
- Better readability
Expand Down
2 changes: 1 addition & 1 deletion readme.md
Expand Up @@ -69,7 +69,7 @@ Each rule has emojis denoting:
| [new-for-builtins](docs/rules/new-for-builtins.md) | Enforce the use of `new` for all builtins, except `String`, `Number`, `Boolean`, `Symbol` and `BigInt`. || 🔧 | |
| [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) | Enforce specifying rules to disable in `eslint-disable` comments. || | |
| [no-array-callback-reference](docs/rules/no-array-callback-reference.md) | Prevent passing a function reference directly to iterator methods. || | 💡 |
| [no-array-for-each](docs/rules/no-array-for-each.md) | Prefer `for…of` over `Array#forEach(…)`. || 🔧 | 💡 |
| [no-array-for-each](docs/rules/no-array-for-each.md) | Prefer `for…of` over the `forEach` method. || 🔧 | 💡 |
| [no-array-method-this-argument](docs/rules/no-array-method-this-argument.md) | Disallow using the `this` argument in array methods. || 🔧 | 💡 |
| [no-array-push-push](docs/rules/no-array-push-push.md) | Enforce combining multiple `Array#push()` into one call. || 🔧 | 💡 |
| [no-array-reduce](docs/rules/no-array-reduce.md) | Disallow `Array#reduce()` and `Array#reduceRight()`. || | |
Expand Down
53 changes: 26 additions & 27 deletions rules/no-array-for-each.js
Expand Up @@ -22,11 +22,11 @@ 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';
const messages = {
[MESSAGE_ID_ERROR]: 'Use `for…of` instead of `Array#forEach(…)`.',
[MESSAGE_ID_ERROR]: 'Use `for…of` instead of `.forEach(…)`.',
[MESSAGE_ID_SUGGESTION]: 'Switch to `for…of`.',
};

const arrayForEachCallSelector = methodCallSelector({
const forEachMethodCallSelector = methodCallSelector({
method: 'forEach',
includeOptionalCall: true,
includeOptionalMember: true,
Expand Down Expand Up @@ -80,34 +80,34 @@ function getFixFunction(callExpression, functionInfo, context) {
const sourceCode = context.getSourceCode();
const [callback] = callExpression.arguments;
const parameters = callback.params;
const array = callExpression.callee.object;
const iterableObject = callExpression.callee.object;
const {returnStatements} = functionInfo.get(callback);
const isOptionalArray = callExpression.callee.optional;
const isOptionalObject = callExpression.callee.optional;
const expressionStatement = stripChainExpression(callExpression).parent;
const arrayText = sourceCode.getText(array);
const objectText = sourceCode.getText(iterableObject);

const getForOfLoopHeadText = () => {
const [elementText, indexText] = parameters.map(parameter => sourceCode.getText(parameter));
const useEntries = parameters.length === 2;
const shouldUseEntries = parameters.length === 2;

let text = 'for (';
text += isFunctionParameterVariableReassigned(callback, context) ? 'let' : 'const';
text += ' ';
text += useEntries ? `[${indexText}, ${elementText}]` : elementText;
text += shouldUseEntries ? `[${indexText}, ${elementText}]` : elementText;
text += ' of ';

const shouldAddParenthesesToArray
= isParenthesized(array, sourceCode)
const shouldAddParenthesesToObject
= isParenthesized(iterableObject, sourceCode)
|| (
// `1?.forEach()` -> `(1).entries()`
isOptionalArray
&& useEntries
&& shouldAddParenthesesToMemberExpressionObject(array, sourceCode)
isOptionalObject
&& shouldUseEntries
&& shouldAddParenthesesToMemberExpressionObject(iterableObject, sourceCode)
);

text += shouldAddParenthesesToArray ? `(${arrayText})` : arrayText;
text += shouldAddParenthesesToObject ? `(${objectText})` : objectText;

if (useEntries) {
if (shouldUseEntries) {
text += '.entries()';
}

Expand Down Expand Up @@ -257,8 +257,8 @@ function getFixFunction(callExpression, functionInfo, context) {

yield * fixSpaceAroundKeyword(fixer, callExpression.parent, sourceCode);

if (isOptionalArray) {
yield fixer.insertTextBefore(callExpression, `if (${arrayText}) `);
if (isOptionalObject) {
yield fixer.insertTextBefore(callExpression, `if (${objectText}) `);
}

// Prevent possible variable conflicts
Expand All @@ -276,7 +276,7 @@ const isChildScope = (child, parent) => {
return false;
};

function isFunctionParametersSafeToFix(callbackFunction, {context, scope, array, allIdentifiers}) {
function isFunctionParametersSafeToFix(callbackFunction, {context, scope, callExpression, allIdentifiers}) {
const variables = context.getDeclaredVariables(callbackFunction);

for (const variable of variables) {
Expand All @@ -290,13 +290,13 @@ function isFunctionParametersSafeToFix(callbackFunction, {context, scope, array,
}

const variableName = definition.name.name;
const [arrayStart, arrayEnd] = array.range;
const [callExpressionStart, callExpressionEnd] = callExpression.range;
for (const identifier of allIdentifiers) {
const {name, range: [start, end]} = identifier;
if (
name !== variableName
|| start < arrayStart
|| end > arrayEnd
|| start < callExpressionStart
|| end > callExpressionEnd
) {
continue;
}
Expand Down Expand Up @@ -352,7 +352,7 @@ function isFixable(callExpression, {scope, functionInfo, allIdentifiers, context
if (
!(parameters.length === 1 || parameters.length === 2)
|| parameters.some(({type, typeAnnotation}) => type === 'RestElement' || typeAnnotation)
|| !isFunctionParametersSafeToFix(callback, {scope, array: callExpression, allIdentifiers, context})
|| !isFunctionParametersSafeToFix(callback, {scope, callExpression, allIdentifiers, context})
) {
return false;
}
Expand Down Expand Up @@ -405,7 +405,7 @@ const create = context => {
const {returnStatements} = functionInfo.get(currentFunction);
returnStatements.push(node);
},
[arrayForEachCallSelector](node) {
[forEachMethodCallSelector](node) {
if (isNodeMatches(node.callee.object, ignoredObjects)) {
return;
}
Expand All @@ -417,11 +417,10 @@ const create = context => {
},
* 'Program:exit'() {
for (const {node, scope} of callExpressions) {
// TODO: Rename this to iteratable
const array = node.callee;
const iterable = node.callee;

const problem = {
node: array.property,
node: iterable.property,
messageId: MESSAGE_ID_ERROR,
};

Expand All @@ -430,7 +429,7 @@ const create = context => {
continue;
}

const shouldUseSuggestion = array.optional && hasSideEffect(array, sourceCode);
const shouldUseSuggestion = iterable.optional && hasSideEffect(iterable, sourceCode);
const fix = getFixFunction(node, functionInfo, context);

if (shouldUseSuggestion) {
Expand All @@ -456,7 +455,7 @@ module.exports = {
meta: {
type: 'suggestion',
docs: {
description: 'Prefer `for…of` over `Array#forEach(…)`.',
description: 'Prefer `for…of` over the `forEach` method.',
},
fixable: 'code',
hasSuggestions: true,
Expand Down

0 comments on commit 67bc49f

Please sign in to comment.