Skip to content

Commit 7983940

Browse files
feat(js_analyze): implement noForIn (#8085)
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent c2983f9 commit 7983940

File tree

14 files changed

+396
-92
lines changed

14 files changed

+396
-92
lines changed

.changeset/five-rats-train.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
Added the nursery rule [`noForIn`](https://biomejs.dev/linter/rules/no-for-in/). Disallow iterating using a for-in loop.
6+
7+
**Invalid:**
8+
9+
```js
10+
for (const i in array) {
11+
console.log(i, array[i]);
12+
}
13+
```

crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/biome_configuration/src/analyzer/linter/rules.rs

Lines changed: 94 additions & 73 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/biome_diagnostics_categories/src/categories.rs

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ define_categories! {
170170
"lint/nursery/noDuplicateDependencies": "https://biomejs.dev/linter/rules/no-duplicate-dependencies",
171171
"lint/nursery/noEmptySource": "https://biomejs.dev/linter/rules/no-empty-source",
172172
"lint/nursery/noFloatingPromises": "https://biomejs.dev/linter/rules/no-floating-promises",
173+
"lint/nursery/noForIn": "https://biomejs.dev/linter/rules/no-for-in",
173174
"lint/nursery/noImplicitCoercion": "https://biomejs.dev/linter/rules/no-implicit-coercion",
174175
"lint/nursery/noImportCycles": "https://biomejs.dev/linter/rules/no-import-cycles",
175176
"lint/nursery/noIncrementDecrement": "https://biomejs.dev/linter/rules/no-increment-decrement",
@@ -190,22 +191,14 @@ define_categories! {
190191
"lint/nursery/noUselessUndefined": "https://biomejs.dev/linter/rules/no-useless-undefined",
191192
"lint/nursery/noVueDataObjectDeclaration": "https://biomejs.dev/linter/rules/no-vue-data-object-declaration",
192193
"lint/nursery/noVueDuplicateKeys": "https://biomejs.dev/linter/rules/no-vue-duplicate-keys",
193-
"lint/nursery/useVueValidVBind": "https://biomejs.dev/linter/rules/use-vue-valid-v-bind",
194-
"lint/nursery/useVueValidVIf": "https://biomejs.dev/linter/rules/use-vue-valid-v-if",
195-
"lint/nursery/useVueValidVElse": "https://biomejs.dev/linter/rules/use-vue-valid-v-else",
196-
"lint/nursery/useVueValidVElseIf": "https://biomejs.dev/linter/rules/use-vue-valid-v-else-if",
197-
"lint/nursery/useVueValidVFor": "https://biomejs.dev/linter/rules/use-vue-valid-v-for",
198-
"lint/nursery/useVueValidVHtml": "https://biomejs.dev/linter/rules/use-vue-valid-v-html",
199-
"lint/nursery/useVueValidVModel": "https://biomejs.dev/linter/rules/use-vue-valid-v-model",
200-
"lint/nursery/useVueValidVOn": "https://biomejs.dev/linter/rules/use-vue-valid-v-on",
201194
"lint/nursery/noVueReservedKeys": "https://biomejs.dev/linter/rules/no-vue-reserved-keys",
202195
"lint/nursery/noVueReservedProps": "https://biomejs.dev/linter/rules/no-vue-reserved-props",
203196
"lint/nursery/useAnchorHref": "https://biomejs.dev/linter/rules/use-anchor-href",
204197
"lint/nursery/useArraySortCompare": "https://biomejs.dev/linter/rules/use-array-sort-compare",
205198
"lint/nursery/useBiomeSuppressionComment": "https://biomejs.dev/linter/rules/use-biome-suppression-comment",
206199
"lint/nursery/useConsistentArrowReturn": "https://biomejs.dev/linter/rules/use-consistent-arrow-return",
207-
"lint/nursery/useConsistentObjectDefinition": "https://biomejs.dev/linter/rules/use-consistent-object-definition",
208200
"lint/nursery/useConsistentGraphqlDescriptions": "https://biomejs.dev/linter/rules/use-consistent-graphql-descriptions",
201+
"lint/nursery/useConsistentObjectDefinition": "https://biomejs.dev/linter/rules/use-consistent-object-definition",
209202
"lint/nursery/useDeprecatedDate": "https://biomejs.dev/linter/rules/use-deprecated-date",
210203
"lint/nursery/useExhaustiveSwitchCases": "https://biomejs.dev/linter/rules/use-exhaustive-switch-cases",
211204
"lint/nursery/useExplicitFunctionReturnType": "https://biomejs.dev/linter/rules/use-explicit-type",
@@ -218,6 +211,14 @@ define_categories! {
218211
"lint/nursery/useSortedClasses": "https://biomejs.dev/linter/rules/use-sorted-classes",
219212
"lint/nursery/useVueDefineMacrosOrder": "https://biomejs.dev/linter/rules/use-vue-define-macros-order",
220213
"lint/nursery/useVueMultiWordComponentNames": "https://biomejs.dev/linter/rules/use-vue-multi-word-component-names",
214+
"lint/nursery/useVueValidVBind": "https://biomejs.dev/linter/rules/use-vue-valid-v-bind",
215+
"lint/nursery/useVueValidVElse": "https://biomejs.dev/linter/rules/use-vue-valid-v-else",
216+
"lint/nursery/useVueValidVElseIf": "https://biomejs.dev/linter/rules/use-vue-valid-v-else-if",
217+
"lint/nursery/useVueValidVFor": "https://biomejs.dev/linter/rules/use-vue-valid-v-for",
218+
"lint/nursery/useVueValidVHtml": "https://biomejs.dev/linter/rules/use-vue-valid-v-html",
219+
"lint/nursery/useVueValidVIf": "https://biomejs.dev/linter/rules/use-vue-valid-v-if",
220+
"lint/nursery/useVueValidVModel": "https://biomejs.dev/linter/rules/use-vue-valid-v-model",
221+
"lint/nursery/useVueValidVOn": "https://biomejs.dev/linter/rules/use-vue-valid-v-on",
221222
"lint/performance/noAccumulatingSpread": "https://biomejs.dev/linter/rules/no-accumulating-spread",
222223
"lint/performance/noAwaitInLoops": "https://biomejs.dev/linter/rules/no-await-in-loops",
223224
"lint/performance/noBarrelFile": "https://biomejs.dev/linter/rules/no-barrel-file",

crates/biome_js_analyze/src/lint/nursery.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ pub mod no_continue;
77
pub mod no_deprecated_imports;
88
pub mod no_empty_source;
99
pub mod no_floating_promises;
10+
pub mod no_for_in;
1011
pub mod no_import_cycles;
1112
pub mod no_increment_decrement;
1213
pub mod no_jsx_literals;
@@ -35,4 +36,4 @@ pub mod use_qwik_valid_lexical_scope;
3536
pub mod use_sorted_classes;
3637
pub mod use_vue_define_macros_order;
3738
pub mod use_vue_multi_word_component_names;
38-
declare_lint_group! { pub Nursery { name : "nursery" , rules : [self :: no_continue :: NoContinue , self :: no_deprecated_imports :: NoDeprecatedImports , self :: no_empty_source :: NoEmptySource , self :: no_floating_promises :: NoFloatingPromises , self :: no_import_cycles :: NoImportCycles , self :: no_increment_decrement :: NoIncrementDecrement , self :: no_jsx_literals :: NoJsxLiterals , self :: no_misused_promises :: NoMisusedPromises , self :: no_next_async_client_component :: NoNextAsyncClientComponent , self :: no_parameters_only_used_in_recursion :: NoParametersOnlyUsedInRecursion , self :: no_react_forward_ref :: NoReactForwardRef , self :: no_shadow :: NoShadow , self :: no_unknown_attribute :: NoUnknownAttribute , self :: no_unnecessary_conditions :: NoUnnecessaryConditions , self :: no_unresolved_imports :: NoUnresolvedImports , self :: no_unused_expressions :: NoUnusedExpressions , self :: no_useless_catch_binding :: NoUselessCatchBinding , self :: no_useless_undefined :: NoUselessUndefined , self :: no_vue_data_object_declaration :: NoVueDataObjectDeclaration , self :: no_vue_duplicate_keys :: NoVueDuplicateKeys , self :: no_vue_reserved_keys :: NoVueReservedKeys , self :: no_vue_reserved_props :: NoVueReservedProps , self :: use_array_sort_compare :: UseArraySortCompare , self :: use_consistent_arrow_return :: UseConsistentArrowReturn , self :: use_exhaustive_switch_cases :: UseExhaustiveSwitchCases , self :: use_explicit_type :: UseExplicitType , self :: use_max_params :: UseMaxParams , self :: use_qwik_method_usage :: UseQwikMethodUsage , self :: use_qwik_valid_lexical_scope :: UseQwikValidLexicalScope , self :: use_sorted_classes :: UseSortedClasses , self :: use_vue_define_macros_order :: UseVueDefineMacrosOrder , self :: use_vue_multi_word_component_names :: UseVueMultiWordComponentNames ,] } }
39+
declare_lint_group! { pub Nursery { name : "nursery" , rules : [self :: no_continue :: NoContinue , self :: no_deprecated_imports :: NoDeprecatedImports , self :: no_empty_source :: NoEmptySource , self :: no_floating_promises :: NoFloatingPromises , self :: no_for_in :: NoForIn , self :: no_import_cycles :: NoImportCycles , self :: no_increment_decrement :: NoIncrementDecrement , self :: no_jsx_literals :: NoJsxLiterals , self :: no_misused_promises :: NoMisusedPromises , self :: no_next_async_client_component :: NoNextAsyncClientComponent , self :: no_parameters_only_used_in_recursion :: NoParametersOnlyUsedInRecursion , self :: no_react_forward_ref :: NoReactForwardRef , self :: no_shadow :: NoShadow , self :: no_unknown_attribute :: NoUnknownAttribute , self :: no_unnecessary_conditions :: NoUnnecessaryConditions , self :: no_unresolved_imports :: NoUnresolvedImports , self :: no_unused_expressions :: NoUnusedExpressions , self :: no_useless_catch_binding :: NoUselessCatchBinding , self :: no_useless_undefined :: NoUselessUndefined , self :: no_vue_data_object_declaration :: NoVueDataObjectDeclaration , self :: no_vue_duplicate_keys :: NoVueDuplicateKeys , self :: no_vue_reserved_keys :: NoVueReservedKeys , self :: no_vue_reserved_props :: NoVueReservedProps , self :: use_array_sort_compare :: UseArraySortCompare , self :: use_consistent_arrow_return :: UseConsistentArrowReturn , self :: use_exhaustive_switch_cases :: UseExhaustiveSwitchCases , self :: use_explicit_type :: UseExplicitType , self :: use_max_params :: UseMaxParams , self :: use_qwik_method_usage :: UseQwikMethodUsage , self :: use_qwik_valid_lexical_scope :: UseQwikValidLexicalScope , self :: use_sorted_classes :: UseSortedClasses , self :: use_vue_define_macros_order :: UseVueDefineMacrosOrder , self :: use_vue_multi_word_component_names :: UseVueMultiWordComponentNames ,] } }
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
use biome_analyze::{
2+
Ast, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule,
3+
};
4+
use biome_console::markup;
5+
use biome_js_syntax::JsForInStatement;
6+
use biome_rowan::AstNode;
7+
use biome_rule_options::no_for_in::NoForInOptions;
8+
9+
declare_lint_rule! {
10+
/// Disallow iterating using a for-in loop.
11+
///
12+
/// A for-in loop (`for (const i in o)`) iterates over the properties of an Object. While it is legal to use for-in loops with array values, it is not common. There are several potential bugs with this:
13+
///
14+
/// 1. It iterates over all enumerable properties, including non-index ones and the entire prototype chain. For example, [`RegExp.prototype.exec`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/exec) returns an array with additional properties, and `for-in` will iterate over them. Some libraries or even your own code may add additional methods to `Array.prototype` (either as polyfill or as custom methods), and if not done properly, they may be iterated over as well.
15+
/// 2. It skips holes in the array. While sparse arrays are rare and advised against, they are still possible and your code should be able to handle them.
16+
/// 3. The "index" is returned as a string, not a number. This can be caught by TypeScript, but can still lead to subtle bugs.
17+
///
18+
/// You may have confused for-in with for-of, which iterates over the elements of the array. If you actually need the index, use a regular `for` loop or the `forEach` method.
19+
///
20+
/// ## Examples
21+
///
22+
/// ### Invalid
23+
///
24+
/// ```js,expect_diagnostic
25+
/// for (const i in array) {
26+
/// console.log(i, array[i]);
27+
/// }
28+
/// ```
29+
///
30+
/// ### Valid
31+
///
32+
/// ```js
33+
/// for (const value of array) {
34+
/// console.log(value);
35+
/// }
36+
/// ```
37+
/// ```js
38+
/// for (let i = 0; i < array.length; i += 1) {
39+
/// console.log(i, array[i]);
40+
/// }
41+
/// ```
42+
/// ```js
43+
/// array.forEach((value, i) => {
44+
/// console.log(i, value);
45+
/// });
46+
/// ```
47+
/// ```js
48+
/// for (const [i, value] of array.entries()) {
49+
/// console.log(i, value);
50+
/// }
51+
/// ```
52+
///
53+
pub NoForIn {
54+
version: "next",
55+
name: "noForIn",
56+
language: "js",
57+
recommended: false,
58+
sources: &[RuleSource::EslintTypeScript("no-for-in-array").inspired()],
59+
}
60+
}
61+
62+
impl Rule for NoForIn {
63+
type Query = Ast<JsForInStatement>;
64+
type State = ();
65+
type Signals = Option<Self::State>;
66+
type Options = NoForInOptions;
67+
68+
fn run(_ctx: &RuleContext<Self>) -> Self::Signals {
69+
Some(())
70+
}
71+
72+
fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
73+
let node = ctx.query();
74+
Some(
75+
RuleDiagnostic::new(
76+
rule_category!(),
77+
node.range(),
78+
markup! {
79+
"Unexpected for-in loop."
80+
},
81+
)
82+
.note(markup! {
83+
"For-in loops are confusing and easy to misuse. You likely want to use a regular loop, for-of loop or forEach instead."
84+
}),
85+
)
86+
}
87+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
const array = [];
2+
3+
for (const i in array) {
4+
console.log(array[i]);
5+
}
6+
7+
for (const i in array) {
8+
console.log(i, array[i]);
9+
}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: invalid.js
4+
---
5+
# Input
6+
```js
7+
const array = [];
8+
9+
for (const i in array) {
10+
console.log(array[i]);
11+
}
12+
13+
for (const i in array) {
14+
console.log(i, array[i]);
15+
}
16+
17+
```
18+
19+
# Diagnostics
20+
```
21+
invalid.js:3:1 lint/nursery/noForIn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
22+
23+
i Unexpected for-in loop.
24+
25+
1 │ const array = [];
26+
2 │
27+
> 3 │ for (const i in array) {
28+
^^^^^^^^^^^^^^^^^^^^^^^^
29+
> 4console.log(array[i]);
30+
> 5}
31+
│ ^
32+
6 │
33+
7 │ for (const i in array) {
34+
35+
i For-in loops are confusing and easy to misuse. You likely want to use a regular loop, for-of loop or forEach instead.
36+
37+
38+
```
39+
40+
```
41+
invalid.js:7:1 lint/nursery/noForIn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
42+
43+
i Unexpected for-in loop.
44+
45+
5}
46+
6 │
47+
> 7 │ for (const i in array) {
48+
^^^^^^^^^^^^^^^^^^^^^^^^
49+
> 8console.log(i, array[i]);
50+
> 9}
51+
│ ^
52+
10 │
53+
54+
i For-in loops are confusing and easy to misuse. You likely want to use a regular loop, for-of loop or forEach instead.
55+
56+
57+
```
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/* should not generate diagnostics */
2+
const array = [];
3+
4+
for (const value of array) {
5+
console.log(value);
6+
}
7+
8+
for (let i = 0; i < array.length; i += 1) {
9+
console.log(i, array[i]);
10+
}
11+
12+
array.forEach((value, i) => {
13+
console.log(i, value);
14+
});
15+
16+
for (const [i, value] of array.entries()) {
17+
console.log(i, value);
18+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: valid.js
4+
---
5+
# Input
6+
```js
7+
/* should not generate diagnostics */
8+
const array = [];
9+
10+
for (const value of array) {
11+
console.log(value);
12+
}
13+
14+
for (let i = 0; i < array.length; i += 1) {
15+
console.log(i, array[i]);
16+
}
17+
18+
array.forEach((value, i) => {
19+
console.log(i, value);
20+
});
21+
22+
for (const [i, value] of array.entries()) {
23+
console.log(i, value);
24+
}
25+
26+
```

0 commit comments

Comments
 (0)