Skip to content

Commit

Permalink
feat(eslint-plugin): add prefer-for-of rule (#338)
Browse files Browse the repository at this point in the history
  • Loading branch information
edsrzf authored and bradzacher committed Apr 7, 2019
1 parent d476f15 commit 3e26ab6
Show file tree
Hide file tree
Showing 5 changed files with 578 additions and 1 deletion.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -146,6 +146,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e
| [`@typescript-eslint/no-use-before-define`](./docs/rules/no-use-before-define.md) | Disallow the use of variables before they are defined | :heavy_check_mark: | | |
| [`@typescript-eslint/no-useless-constructor`](./docs/rules/no-useless-constructor.md) | Disallow unnecessary constructors | | | |
| [`@typescript-eslint/no-var-requires`](./docs/rules/no-var-requires.md) | Disallows the use of require statements except in import statements (`no-var-requires` from TSLint) | :heavy_check_mark: | | |
| [`@typescript-eslint/prefer-for-of`](./docs/rules/prefer-for-of.md) | Prefer a ‘for-of’ loop over a standard ‘for’ loop if the index is only used to access the array being iterated. | | | |
| [`@typescript-eslint/prefer-function-type`](./docs/rules/prefer-function-type.md) | Use function types instead of interfaces with call signatures (`callable-types` from TSLint) | | :wrench: | |
| [`@typescript-eslint/prefer-interface`](./docs/rules/prefer-interface.md) | Prefer an interface declaration over a type literal (type T = { ... }) (`interface-over-type-literal` from TSLint) | :heavy_check_mark: | :wrench: | |
| [`@typescript-eslint/prefer-namespace-keyword`](./docs/rules/prefer-namespace-keyword.md) | Require the use of the `namespace` keyword instead of the `module` keyword to declare custom TypeScript modules. (`no-internal-module` from TSLint) | :heavy_check_mark: | :wrench: | |
Expand Down
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/)
217 changes: 217 additions & 0 deletions packages/eslint-plugin/src/rules/prefer-for-of.ts
@@ -0,0 +1,217 @@
import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/typescript-estree';
import * as util from '../util';
import { Scope } 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 {
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')
) {
return node.right.object;
}
return null;
}

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;
}

// a[i] = 1, a[i] += 1, etc.
if (
parent.type === AST_NODE_TYPES.AssignmentExpression &&
parent.left === node
) {
return true;
}

// delete a[i]
if (
parent.type === AST_NODE_TYPES.UnaryExpression &&
parent.operator === 'delete' &&
parent.argument === node
) {
return true;
}

// a[i]++, --a[i], etc.
if (
parent.type === AST_NODE_TYPES.UpdateExpression &&
parent.argument === node
) {
return true;
}

// [a[i]] = [0]
if (parent.type === AST_NODE_TYPES.ArrayPattern) {
return true;
}

// [...a[i]] = [0]
if (parent.type === AST_NODE_TYPES.RestElement) {
return true;
}

// ({ foo: a[i] }) = { foo: 0 }
if (
parent.type === AST_NODE_TYPES.Property &&
parent.parent !== undefined &&
parent.parent.type === AST_NODE_TYPES.ObjectExpression &&
parent.value === node &&
isAssignee(parent.parent)
) {
return true;
}

return false;
}

function isIndexOnlyUsedWithArray(
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(node.body, indexVar, arrayExpression)
) {
context.report({
node,
messageId: 'preferForOf',
});
}
},
};
},
});

0 comments on commit 3e26ab6

Please sign in to comment.