Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): add prefer-for-of rule #338

Merged
merged 6 commits into from Apr 7, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/eslint-plugin/ROADMAP.md
Expand Up @@ -30,7 +30,7 @@
| [`no-unnecessary-type-assertion`] | ✅ | [`@typescript-eslint/no-unnecessary-type-assertion`] |
| [`no-var-requires`] | ✅ | [`@typescript-eslint/no-var-requires`] |
| [`only-arrow-functions`] | 🔌 | [`prefer-arrow/prefer-arrow-functions`] |
| [`prefer-for-of`] | 🛑 | N/A |
| [`prefer-for-of`] | | [`@typescript-eslint/prefer-for-of`] |
| [`promise-function-async`] | ✅ | [`@typescript-eslint/promise-function-async`] |
| [`typedef`] | 🛑 | N/A |
| [`typedef-whitespace`] | ✅ | [`@typescript-eslint/type-annotation-spacing`] |
Expand Down Expand Up @@ -606,6 +606,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
[`@typescript-eslint/no-angle-bracket-type-assertion`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-angle-bracket-type-assertion.md
[`@typescript-eslint/no-parameter-properties`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-parameter-properties.md
[`@typescript-eslint/member-delimiter-style`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/member-delimiter-style.md
[`@typescript-eslint/prefer-for-of`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/prefer-for-of.md
[`@typescript-eslint/prefer-interface`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/prefer-interface.md
[`@typescript-eslint/no-array-constructor`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-array-constructor.md
[`@typescript-eslint/prefer-function-type`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/prefer-function-type.md
Expand Down
41 changes: 41 additions & 0 deletions packages/eslint-plugin/docs/rules/prefer-for-of.md
@@ -0,0 +1,41 @@
# Use for-of loops instead of standard for loops over arrays (prefer-for-of)

This rule recommends a for-of loop when the loop index is only used to read from an array that is being iterated.

## Rule Details

For cases where the index is only used to read from the array being iterated, a for-of loop is easier to read and write.

Examples of **incorrect** code for this rule:

```js
for (let i = 0; i < arr.length; i++) {
console.log(arr[i]);
}
```

Examples of **correct** code for this rule:

```js
for (const x of arr) {
console.log(x);
}

for (let i = 0; i < arr.length; i++) {
// i is used to write to arr, so for-of could not be used.
arr[i] = 0;
}

for (let i = 0; i < arr.length; i++) {
// i is used independent of arr, so for-of could not be used.
console.log(i, arr[i]);
}
```

## When Not To Use It

If you transpile for browsers that do not support for-of loops, you may wish to use traditional for loops that produce more compact code.

## Related to

- TSLint: ['prefer-for-of'](https://palantir.github.io/tslint/rules/prefer-for-of/)
198 changes: 198 additions & 0 deletions packages/eslint-plugin/src/rules/prefer-for-of.ts
@@ -0,0 +1,198 @@
import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/typescript-estree';
import * as util from '../util';
import { Scope, RuleContext } from 'ts-eslint';

export default util.createRule({
name: 'prefer-for-of',
meta: {
type: 'suggestion',
docs: {
description:
'Prefer a ‘for-of’ loop over a standard ‘for’ loop if the index is only used to access the array being iterated.',
category: 'Stylistic Issues',
recommended: false,
tslintName: 'prefer-for-of',
},
messages: {
preferForOf:
'Expected a `for-of` loop instead of a `for` loop with this simple iteration',
},
schema: [],
},
defaultOptions: [],
create(context) {
function isSingleVariableDeclaration(
node: TSESTree.Node | null,
): node is TSESTree.VariableDeclaration {
return (
node !== null &&
node.type === AST_NODE_TYPES.VariableDeclaration &&
node.kind !== 'const' &&
node.declarations.length === 1
);
}

function isLiteral(node: TSESTree.Expression, value: number): boolean {
return node.type === AST_NODE_TYPES.Literal && node.value === value;
}

function isZeroInitialized(node: TSESTree.VariableDeclarator): boolean {
return node.init !== null && isLiteral(node.init, 0);
}

function isMatchingIdentifier(
node: TSESTree.Expression,
name: string,
): boolean {
return node.type === AST_NODE_TYPES.Identifier && node.name === name;
}

function isLessThanLengthExpression(
node: TSESTree.Node | null,
name: string,
): TSESTree.Expression | null {
let arrayExpression: TSESTree.Expression | null = null;
edsrzf marked this conversation as resolved.
Show resolved Hide resolved
if (
node !== null &&
node.type === AST_NODE_TYPES.BinaryExpression &&
node.operator === '<' &&
isMatchingIdentifier(node.left, name) &&
node.right.type === AST_NODE_TYPES.MemberExpression &&
isMatchingIdentifier(node.right.property, 'length')
) {
arrayExpression = node.right.object;
}
return arrayExpression;
}

function isIncrement(node: TSESTree.Node | null, name: string): boolean {
if (!node) {
return false;
}
switch (node.type) {
case AST_NODE_TYPES.UpdateExpression:
// x++ or ++x
return (
node.operator === '++' && isMatchingIdentifier(node.argument, name)
);
case AST_NODE_TYPES.AssignmentExpression:
if (isMatchingIdentifier(node.left, name)) {
if (node.operator === '+=') {
// x += 1
return isLiteral(node.right, 1);
} else if (node.operator === '=') {
// x = x + 1 or x = 1 + x
const expr = node.right;
return (
expr.type === AST_NODE_TYPES.BinaryExpression &&
expr.operator === '+' &&
((isMatchingIdentifier(expr.left, name) &&
isLiteral(expr.right, 1)) ||
(isLiteral(expr.left, 1) &&
isMatchingIdentifier(expr.right, name)))
);
}
}
}
return false;
}

function contains(outer: TSESTree.Node, inner: TSESTree.Node): boolean {
return (
outer.range[0] <= inner.range[0] && outer.range[1] >= inner.range[1]
);
}

function isAssignee(node: TSESTree.Node): boolean {
const parent = node.parent;
if (!parent) {
return false;
}
return (
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
// a[i] = 1, a[i] += 1, etc.
(parent.type === AST_NODE_TYPES.AssignmentExpression &&
parent.left === node) ||
// delete a[i]
(parent.type === AST_NODE_TYPES.UnaryExpression &&
parent.operator === 'delete' &&
parent.argument === node) ||
// a[i]++, --a[i], etc.
(parent.type === AST_NODE_TYPES.UpdateExpression &&
parent.argument === node) ||
// [a[i]] = [0]
parent.type === AST_NODE_TYPES.ArrayPattern ||
// [...a[i]] = [0]
parent.type === AST_NODE_TYPES.RestElement ||
// ({ foo: a[i] }) = { foo: 0 }
(parent.type === AST_NODE_TYPES.Property &&
parent.parent !== undefined &&
parent.parent.type === AST_NODE_TYPES.ObjectExpression &&
parent.value === node &&
isAssignee(parent.parent))
);
}

function isIndexOnlyUsedWithArray(
context: RuleContext<'preferForOf', never[]>,
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
body: TSESTree.Statement,
indexVar: Scope.Variable,
arrayExpression: TSESTree.Expression,
): boolean {
const sourceCode = context.getSourceCode();
const arrayText = sourceCode.getText(arrayExpression);
return indexVar.references.every(reference => {
const id = reference.identifier;
const node = id.parent;
return (
!contains(body, id) ||
(node !== undefined &&
node.type === AST_NODE_TYPES.MemberExpression &&
node.property === id &&
sourceCode.getText(node.object) === arrayText &&
!isAssignee(node))
);
});
}

return {
'ForStatement:exit'(node: TSESTree.ForStatement) {
if (!isSingleVariableDeclaration(node.init)) {
return;
}

const [declarator] = node.init.declarations;
if (
!isZeroInitialized(declarator) ||
declarator.id.type !== AST_NODE_TYPES.Identifier
) {
return;
}

const indexName = declarator.id.name;
const arrayExpression = isLessThanLengthExpression(
node.test,
indexName,
);
if (!arrayExpression) {
return;
}

const [indexVar] = context.getDeclaredVariables(node.init);
if (
isIncrement(node.update, indexName) &&
isIndexOnlyUsedWithArray(
context,
node.body,
indexVar,
arrayExpression,
)
) {
context.report({
node,
messageId: 'preferForOf',
});
}
},
};
},
});