Skip to content

Commit

Permalink
feat(eslint-plugin): add new rule no-for-in-array (#155)
Browse files Browse the repository at this point in the history
  • Loading branch information
uniqueiniquity authored and JamesHenry committed Feb 7, 2019
1 parent 07e950e commit 84162ba
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 4 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e
| [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces (`no-empty-interface` from TSLint) | :heavy_check_mark: | |
| [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type (`no-any` from TSLint) | :heavy_check_mark: | |
| [`@typescript-eslint/no-extraneous-class`](./docs/rules/no-extraneous-class.md) | Forbids the use of classes as namespaces (`no-unnecessary-class` from TSLint) | | |
| [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md) | Disallow iterating over an array with a for-in loop (`no-for-in-array` from TSLint) | | |
| [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md) | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean. (`no-inferrable-types` from TSLint) | :heavy_check_mark: | :wrench: |
| [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md) | Enforce valid definition of `new` and `constructor`. (`no-misused-new` from TSLint) | :heavy_check_mark: | |
| [`@typescript-eslint/no-namespace`](./docs/rules/no-namespace.md) | Disallow the use of custom TypeScript modules and namespaces (`no-namespace` from TSLint) | :heavy_check_mark: | |
Expand Down
12 changes: 9 additions & 3 deletions packages/eslint-plugin/ROADMAP.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
# Roadmap

✅ (29) = done<br>
<!--
When updating these totals, please make sure to subtract 1 from the total
count retrived via Ctrl/Cmd+F since this key shouldn’t count in the results.
-->

✅ (30) = done<br>
🌟 (79) = in ESLint core<br>
🔌 (33) = in another plugin<br>
🌓 (16) = implementations differ or ESLint version is missing functionality<br>
🛑 (68) = unimplemented
🛑 (68) = unimplemented<br>

## TSLint rules

Expand Down Expand Up @@ -59,7 +64,7 @@
| [`no-empty`] | 🌟 | [`no-empty`][no-empty] |
| [`no-eval`] | 🌟 | [`no-eval`][no-eval] |
| [`no-floating-promises`] | 🛑 | N/A ([relevant plugin][plugin:promise]) |
| [`no-for-in-array`] | 🛑 | N/A |
| [`no-for-in-array`] | | [`@typescript-eslint/no-for-in-array`] |
| [`no-implicit-dependencies`] | 🔌 | [`import/no-extraneous-dependencies`] |
| [`no-inferred-empty-object-type`] | 🛑 | N/A |
| [`no-invalid-template-strings`] | 🌟 | [`no-template-curly-in-string`][no-template-curly-in-string] |
Expand Down Expand Up @@ -586,6 +591,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
[`@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-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/no-for-in-array`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-for-in-array.md

<!-- eslint-plugin-import -->

Expand Down
44 changes: 44 additions & 0 deletions packages/eslint-plugin/docs/rules/no-for-in-array.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Disallow iterating over an array with a for-in loop (no-for-in-array)

This rule prohibits iterating over an array with a for-in loop.

## Rule Details

Rationale from TSLint:

A for-in loop (`for (var k in o)`) iterates over the properties of an Object.
While it is legal to use for-in loops with array types, it is not common.
for-in will iterate over the indices of the array as strings, omitting any "holes" in
the array.
More common is to use for-of, which iterates over the values of an array.
If you want to iterate over the indices, alternatives include:

```js
array.forEach((value, index) => { ... });
for (const [index, value] of array.entries()) { ... }
for (let i = 0; i < array.length; i++) { ... }
```

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

```js
for (const x in [3, 4, 5]) {
console.log(x);
}
```

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

```js
for (const x in { a: 3, b: 4, c: 5 }) {
console.log(x);
}
```

## When Not To Use It

If you want to iterate through a loop using the indices in an array as strings, you can turn off this rule.

## Related to

- TSLint: ['no-for-in-array'](https://palantir.github.io/tslint/rules/no-for-in-array/)
56 changes: 56 additions & 0 deletions packages/eslint-plugin/lib/rules/no-for-in-array.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/**
* @fileoverview Disallow iterating over an array with a for-in loop
* @author Benjamin Lichtman
*/
'use strict';
const ts = require('typescript');
const util = require('../util');

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

/**
* @type {import("eslint").Rule.RuleModule}
*/
module.exports = {
meta: {
docs: {
description: 'Disallow iterating over an array with a for-in loop',
category: 'Functionality',
recommended: false,
extraDescription: [util.tslintRule('no-for-in-array')],
url: util.metaDocsUrl('no-for-in-array')
},
fixable: null,
messages: {
forInViolation:
'For-in loops over arrays are forbidden. Use for-of or array.forEach instead.'
},
schema: [],
type: 'problem'
},

create(context) {
return {
ForInStatement(node) {
const parserServices = util.getParserServices(context);
const checker = parserServices.program.getTypeChecker();
const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node);

const type = checker.getTypeAtLocation(originalNode.expression);

if (
(typeof type.symbol !== 'undefined' &&
type.symbol.name === 'Array') ||
(type.flags & ts.TypeFlags.StringLike) !== 0
) {
context.report({
node,
messageId: 'forInViolation'
});
}
}
};
}
};
2 changes: 1 addition & 1 deletion packages/eslint-plugin/lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ exports.upperCaseFirst = str => str[0].toUpperCase() + str.slice(1);
/**
* Try to retrieve typescript parser service from context
* @param {RuleContext} context Rule context
* @returns {{esTreeNodeToTSNodeMap}|{program}|Object|*} parserServices
* @returns {{program: Program, esTreeNodeToTSNodeMap: NodeMap}} parserServices
*/
exports.getParserServices = context => {
if (
Expand Down
69 changes: 69 additions & 0 deletions packages/eslint-plugin/tests/lib/rules/no-for-in-array.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/**
* @fileoverview Disallow iterating over an array with a for-in loop
* @author Benjamin Lichtman
*/
'use strict';

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const rule = require('../../../lib/rules/no-for-in-array'),
RuleTester = require('eslint').RuleTester,
path = require('path');

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const rootDir = path.join(process.cwd(), 'tests/fixtures/');
const parserOptions = {
ecmaVersion: 2015,
tsconfigRootDir: rootDir,
project: './tsconfig.json'
};
const ruleTester = new RuleTester({
parserOptions,
parser: '@typescript-eslint/parser'
});

ruleTester.run('no-for-in-array', rule, {
valid: [
`
for (const x of [3, 4, 5]) {
console.log(x);
}`,
`
for (const x in { a: 1, b: 2, c: 3 }) {
console.log(x);
}`
],

invalid: [
{
code: `
for (const x in [3, 4, 5]) {
console.log(x);
}`,
errors: [
{
messageId: 'forInViolation',
type: 'ForInStatement'
}
]
},
{
code: `
const z = [3, 4, 5];
for (const x in z) {
console.log(x);
}`,
errors: [
{
messageId: 'forInViolation',
type: 'ForInStatement'
}
]
}
]
});

0 comments on commit 84162ba

Please sign in to comment.