diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 8057f15843d..22754a977c4 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -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: | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index c0cdcbf0b3e..6df8aa7fe31 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -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`] | @@ -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 diff --git a/packages/eslint-plugin/docs/rules/prefer-for-of.md b/packages/eslint-plugin/docs/rules/prefer-for-of.md new file mode 100644 index 00000000000..a937709f7eb --- /dev/null +++ b/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/) diff --git a/packages/eslint-plugin/src/rules/prefer-for-of.ts b/packages/eslint-plugin/src/rules/prefer-for-of.ts new file mode 100644 index 00000000000..4b542cd9109 --- /dev/null +++ b/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', + }); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/prefer-for-of.test.ts b/packages/eslint-plugin/tests/rules/prefer-for-of.test.ts new file mode 100644 index 00000000000..db4dcd3bbf9 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/prefer-for-of.test.ts @@ -0,0 +1,317 @@ +import rule from '../../src/rules/prefer-for-of'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('prefer-for-of', rule, { + valid: [ + ` +for (let i = 0; i < arr1.length; i++) { + const x = arr1[i] === arr2[i]; +} + `, + ` +for (let i = 0; i < arr.length; i++) { + arr[i] = 0; +} + `, + ` +for (var c = 0; c < arr.length; c++) { + doMath(c); +} + `, + ` +for (var d = 0; d < arr.length; d++) doMath(d); +`, + ` +for (var e = 0; e < arr.length; e++) { + if(e > 5) { + doMath(e); + } + console.log(arr[e]); +} +`, + ` +for (var f = 0; f <= 40; f++) { + doMath(f); +} +`, + ` +for (var g = 0; g <= 40; g++) doMath(g); +`, + ` +for(var h=0, len=arr.length; h < len; h++) {} +`, + ` +for(var i=0, len=arr.length; i < len; i++) arr[i]; +`, + ` +var m = 0; +for (;;) { + if (m > 3) break; + console.log(m); + m++; +} +`, + ` +var n = 0; +for (; n < 9; n++) { + console.log(n); +} +`, + ` +var o = 0; +for (; o < arr.length; o++) { + console.log(arr[o]); +} +`, + ` +for(; x < arr.length; x++) {} +`, + ` +for(let x = 0;; x++) {} +`, + ` +for(let x = 0; x < arr.length;) {} +`, + ` +for(let x = 0; NOTX < arr.length; x++) {} +`, + ` +for(let x = 0; x < arr.length; NOTX++) {} +`, + ` +for(let NOTX = 0; x < arr.length; x++) {} +`, + ` +for(let x = 0; x < arr.length; x--) {} +`, + ` +for(let x = 0; x <= arr.length; x++) {} +`, + ` +for(let x = 1; x < arr.length; x++) {} + +`, + ` +for(let x = 0; x < arr.length(); x++) {} +`, + ` +for(let x = 0; x < arr.length; x+=11) {} +`, + ` +for(let x = 0; x < arr.length; x=x+11) {} +`, + ` +for(let x = 0; x < arr.length; x++) { + x++; +} +`, + ` +for(let x = 0; true; x++) {} +`, + ` +for (var q in obj) { + if (obj.hasOwnProperty(q)) { + console.log(q); + } +} +`, + ` +for (var r of arr) { + console.log(r); +} +`, + ` +for (let x = 0; x < arr.length; x++) { + let y = arr[x + 1]; +} +`, + ` +for (let i = 0; i < arr.length; i++) { + delete arr[i]; +} +`, + ` +for (let i = 0; i < arr.length; i++) { + [arr[i]] = [1]; +} +`, + ` +for (let i = 0; i < arr.length; i++) { + [...arr[i]] = [1]; +} +`, + ` +for (let i = 0; i < arr.length; i++) { + ({ foo: arr[i] }) = { foo: 0 }; +} +`, + ], + invalid: [ + { + code: ` +for (var a = 0; a < obj.arr.length; a++) { + console.log(obj.arr[a]); +} + `, + errors: [ + { + messageId: 'preferForOf', + }, + ], + }, + { + code: ` +for (var b = 0; b < arr.length; b++) console.log(arr[b]); + `, + errors: [ + { + messageId: 'preferForOf', + }, + ], + }, + { + code: ` +for (let a = 0; a < arr.length; a++) { + console.log(arr[a]); +} + `, + errors: [ + { + messageId: 'preferForOf', + }, + ], + }, + { + code: ` +for (let a = 0; a < arr.length; ++a) { + arr[a].whatever(); +} + `, + errors: [ + { + messageId: 'preferForOf', + }, + ], + }, + { + code: ` +for(let x = 0; x < arr.length; x++) {} + `, + errors: [ + { + messageId: 'preferForOf', + }, + ], + }, + { + code: ` +for(let x = 0; x < arr.length; x+=1) {} + `, + errors: [ + { + messageId: 'preferForOf', + }, + ], + }, + { + code: ` +for(let x = 0; x < arr.length; x=x+1) {} + `, + errors: [ + { + messageId: 'preferForOf', + }, + ], + }, + { + code: ` +for(let x = 0; x < arr.length; x=1+x) {} + `, + errors: [ + { + messageId: 'preferForOf', + }, + ], + }, + { + code: ` +for (let shadow = 0; shadow < arr.length; shadow++) { + for (let shadow = 0; shadow < arr.length; shadow++) { + } +} + `, + errors: [ + { + messageId: 'preferForOf', + }, + { + messageId: 'preferForOf', + }, + ], + }, + { + code: ` +for (let i = 0; i < arr.length; i++) { + obj[arr[i]] = 1; +} + `, + errors: [ + { + messageId: 'preferForOf', + }, + ], + }, + { + code: ` +for (let i = 0; i < arr.length; i++) { + delete obj[arr[i]]; +} + `, + errors: [ + { + messageId: 'preferForOf', + }, + ], + }, + { + code: ` +for (let i = 0; i < arr.length; i++) { + [obj[arr[i]]] = [1]; +} + `, + errors: [ + { + messageId: 'preferForOf', + }, + ], + }, + { + code: ` +for (let i = 0; i < arr.length; i++) { + [...obj[arr[i]]] = [1]; +} + `, + errors: [ + { + messageId: 'preferForOf', + }, + ], + }, + { + code: ` +for (let i = 0; i < arr.length; i++) { + ({ foo: obj[arr[i]] }) = { foo: 1 }; +} + `, + errors: [ + { + messageId: 'preferForOf', + }, + ], + }, + ], +});