Skip to content

Commit 970f498

Browse files
fireairforcefireairforce
authored
feat(useGuardForIn): add rule (#4104)
1 parent 2e5b656 commit 970f498

File tree

13 files changed

+341
-11
lines changed

13 files changed

+341
-11
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
4646
#### New features
4747

4848
- Add [noDocumentCookie](https://biomejs.dev/linter/rules/no-document-cookie/). Contributed by @tunamaguro
49+
- Add [guardForIn](https://biomejs.dev/linter/rules/guard-for-in/). Contributed by @fireairforce
4950

5051
#### Bug Fixes
5152

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

Lines changed: 8 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: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3403,6 +3403,9 @@ pub struct Nursery {
34033403
#[serde(skip_serializing_if = "Option::is_none")]
34043404
pub use_explicit_function_return_type:
34053405
Option<RuleConfiguration<biome_js_analyze::options::UseExplicitFunctionReturnType>>,
3406+
#[doc = "Require for-in loops to include an if statement."]
3407+
#[serde(skip_serializing_if = "Option::is_none")]
3408+
pub use_guard_for_in: Option<RuleConfiguration<biome_js_analyze::options::UseGuardForIn>>,
34063409
#[doc = "Disallows package private imports."]
34073410
#[serde(skip_serializing_if = "Option::is_none")]
34083411
pub use_import_restrictions:
@@ -3476,6 +3479,7 @@ impl Nursery {
34763479
"useConsistentMemberAccessibility",
34773480
"useDeprecatedReason",
34783481
"useExplicitFunctionReturnType",
3482+
"useGuardForIn",
34793483
"useImportRestrictions",
34803484
"useSortedClasses",
34813485
"useStrictMode",
@@ -3510,7 +3514,7 @@ impl Nursery {
35103514
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29]),
35113515
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[33]),
35123516
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[34]),
3513-
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[38]),
3517+
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[39]),
35143518
];
35153519
const ALL_RULES_AS_FILTERS: &'static [RuleFilter<'static>] = &[
35163520
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[0]),
@@ -3554,6 +3558,7 @@ impl Nursery {
35543558
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[38]),
35553559
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[39]),
35563560
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[40]),
3561+
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[41]),
35573562
];
35583563
#[doc = r" Retrieves the recommended rules"]
35593564
pub(crate) fn is_recommended_true(&self) -> bool {
@@ -3750,31 +3755,36 @@ impl Nursery {
37503755
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[35]));
37513756
}
37523757
}
3753-
if let Some(rule) = self.use_import_restrictions.as_ref() {
3758+
if let Some(rule) = self.use_guard_for_in.as_ref() {
37543759
if rule.is_enabled() {
37553760
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[36]));
37563761
}
37573762
}
3758-
if let Some(rule) = self.use_sorted_classes.as_ref() {
3763+
if let Some(rule) = self.use_import_restrictions.as_ref() {
37593764
if rule.is_enabled() {
37603765
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[37]));
37613766
}
37623767
}
3763-
if let Some(rule) = self.use_strict_mode.as_ref() {
3768+
if let Some(rule) = self.use_sorted_classes.as_ref() {
37643769
if rule.is_enabled() {
37653770
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[38]));
37663771
}
37673772
}
3768-
if let Some(rule) = self.use_trim_start_end.as_ref() {
3773+
if let Some(rule) = self.use_strict_mode.as_ref() {
37693774
if rule.is_enabled() {
37703775
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[39]));
37713776
}
37723777
}
3773-
if let Some(rule) = self.use_valid_autocomplete.as_ref() {
3778+
if let Some(rule) = self.use_trim_start_end.as_ref() {
37743779
if rule.is_enabled() {
37753780
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[40]));
37763781
}
37773782
}
3783+
if let Some(rule) = self.use_valid_autocomplete.as_ref() {
3784+
if rule.is_enabled() {
3785+
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[41]));
3786+
}
3787+
}
37783788
index_set
37793789
}
37803790
pub(crate) fn get_disabled_rules(&self) -> FxHashSet<RuleFilter<'static>> {
@@ -3959,31 +3969,36 @@ impl Nursery {
39593969
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[35]));
39603970
}
39613971
}
3962-
if let Some(rule) = self.use_import_restrictions.as_ref() {
3972+
if let Some(rule) = self.use_guard_for_in.as_ref() {
39633973
if rule.is_disabled() {
39643974
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[36]));
39653975
}
39663976
}
3967-
if let Some(rule) = self.use_sorted_classes.as_ref() {
3977+
if let Some(rule) = self.use_import_restrictions.as_ref() {
39683978
if rule.is_disabled() {
39693979
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[37]));
39703980
}
39713981
}
3972-
if let Some(rule) = self.use_strict_mode.as_ref() {
3982+
if let Some(rule) = self.use_sorted_classes.as_ref() {
39733983
if rule.is_disabled() {
39743984
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[38]));
39753985
}
39763986
}
3977-
if let Some(rule) = self.use_trim_start_end.as_ref() {
3987+
if let Some(rule) = self.use_strict_mode.as_ref() {
39783988
if rule.is_disabled() {
39793989
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[39]));
39803990
}
39813991
}
3982-
if let Some(rule) = self.use_valid_autocomplete.as_ref() {
3992+
if let Some(rule) = self.use_trim_start_end.as_ref() {
39833993
if rule.is_disabled() {
39843994
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[40]));
39853995
}
39863996
}
3997+
if let Some(rule) = self.use_valid_autocomplete.as_ref() {
3998+
if rule.is_disabled() {
3999+
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[41]));
4000+
}
4001+
}
39874002
index_set
39884003
}
39894004
#[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"]
@@ -4164,6 +4179,10 @@ impl Nursery {
41644179
.use_explicit_function_return_type
41654180
.as_ref()
41664181
.map(|conf| (conf.level(), conf.get_options())),
4182+
"useGuardForIn" => self
4183+
.use_guard_for_in
4184+
.as_ref()
4185+
.map(|conf| (conf.level(), conf.get_options())),
41674186
"useImportRestrictions" => self
41684187
.use_import_restrictions
41694188
.as_ref()

‎crates/biome_diagnostics_categories/src/categories.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ define_categories! {
190190
"lint/nursery/useConsistentMemberAccessibility": "https://biomejs.dev/linter/rules/use-consistent-member-accessibility",
191191
"lint/nursery/useDeprecatedReason": "https://biomejs.dev/linter/rules/use-deprecated-reason",
192192
"lint/nursery/useExplicitFunctionReturnType": "https://biomejs.dev/linter/rules/use-explicit-function-return-type",
193+
"lint/nursery/useGuardForIn": "https://biomejs.dev/linter/rules/use-guard-for-in",
193194
"lint/nursery/useImportRestrictions": "https://biomejs.dev/linter/rules/use-import-restrictions",
194195
"lint/nursery/useJsxCurlyBraceConvention": "https://biomejs.dev/linter/rules/use-jsx-curly-brace-convention",
195196
"lint/nursery/useSortedClasses": "https://biomejs.dev/linter/rules/use-sorted-classes",

‎crates/biome_js_analyze/src/lint/nursery.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ pub mod use_component_export_only_modules;
2929
pub mod use_consistent_curly_braces;
3030
pub mod use_consistent_member_accessibility;
3131
pub mod use_explicit_function_return_type;
32+
pub mod use_guard_for_in;
3233
pub mod use_import_restrictions;
3334
pub mod use_sorted_classes;
3435
pub mod use_strict_mode;
@@ -66,6 +67,7 @@ declare_lint_group! {
6667
self :: use_consistent_curly_braces :: UseConsistentCurlyBraces ,
6768
self :: use_consistent_member_accessibility :: UseConsistentMemberAccessibility ,
6869
self :: use_explicit_function_return_type :: UseExplicitFunctionReturnType ,
70+
self :: use_guard_for_in :: UseGuardForIn ,
6971
self :: use_import_restrictions :: UseImportRestrictions ,
7072
self :: use_sorted_classes :: UseSortedClasses ,
7173
self :: use_strict_mode :: UseStrictMode ,

‎crates/biome_js_analyze/src/lint/nursery/use_guard_for_in.rs‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
use biome_analyze::{
2+
context::RuleContext, declare_lint_rule, Ast, Rule, RuleDiagnostic, RuleSource,
3+
};
4+
use biome_console::markup;
5+
use biome_js_syntax::{AnyJsStatement, JsForInStatement};
6+
use biome_rowan::{AstNode, AstNodeList};
7+
8+
declare_lint_rule! {
9+
/// Require `for-in` loops to include an `if` statement.
10+
///
11+
/// Looping over objects with a `for-in` loop will include properties inherited through the prototype chain.
12+
/// This behavior can lead to unexpected items in your for loop.
13+
///
14+
/// For codebases that do not support ES2022, `Object.prototype.hasOwnProperty.call(foo, key)` can be used as a check that the property is not inherited.
15+
///
16+
/// For codebases that do support ES2022, `Object.hasOwn(foo, key)` can be used as a shorter and more reliable alternative.
17+
///
18+
/// ## Examples
19+
///
20+
/// ### Invalid
21+
///
22+
/// ```js,expect_diagnostic
23+
/// for (key in foo) {
24+
/// doSomething(key);
25+
/// }
26+
/// ```
27+
///
28+
/// ### Valid
29+
///
30+
/// ```js
31+
/// for (key in foo) {
32+
/// if (Object.hasOwn(foo, key)) {
33+
/// doSomething(key);
34+
/// }
35+
/// }
36+
/// ```
37+
///
38+
/// ```js
39+
/// for (key in foo) {
40+
/// if (Object.prototype.hasOwnProperty.call(foo, key)) {
41+
/// doSomething(key);
42+
/// }
43+
/// }
44+
/// ```
45+
///
46+
/// ```js
47+
/// for (key in foo) {
48+
/// if ({}.hasOwnProperty.call(foo, key)) {
49+
/// doSomething(key);
50+
/// }
51+
/// }
52+
/// ```
53+
///
54+
pub UseGuardForIn {
55+
version: "next",
56+
name: "useGuardForIn",
57+
language: "js",
58+
sources: &[RuleSource::Eslint("guard-for-in")],
59+
recommended: false,
60+
}
61+
}
62+
63+
impl Rule for UseGuardForIn {
64+
type Query = Ast<JsForInStatement>;
65+
type State = ();
66+
type Signals = Option<Self::State>;
67+
type Options = ();
68+
69+
fn run(ctx: &RuleContext<Self>) -> Self::Signals {
70+
let node = ctx.query();
71+
let body = node.body().ok()?;
72+
73+
match body {
74+
AnyJsStatement::JsEmptyStatement(_) | AnyJsStatement::JsIfStatement(_) => None,
75+
AnyJsStatement::JsBlockStatement(block) => {
76+
let statements = block.statements();
77+
78+
match statements.len() {
79+
0 => None,
80+
1 => {
81+
let first_statement = statements.first()?;
82+
if first_statement.as_js_if_statement().is_none() {
83+
Some(())
84+
} else {
85+
None
86+
}
87+
}
88+
_ => {
89+
let first_statement = statements.first()?;
90+
if let Some(first_if_statement) = first_statement.as_js_if_statement() {
91+
match first_if_statement.consequent().ok()? {
92+
AnyJsStatement::JsBlockStatement(block)
93+
if block.statements().len() == 1 =>
94+
{
95+
if block
96+
.statements()
97+
.first()?
98+
.as_js_continue_statement()
99+
.is_none()
100+
{
101+
Some(())
102+
} else {
103+
None
104+
}
105+
}
106+
AnyJsStatement::JsContinueStatement(_) => None,
107+
_ => Some(()),
108+
}
109+
} else {
110+
Some(())
111+
}
112+
}
113+
}
114+
}
115+
_ => Some(()),
116+
}
117+
}
118+
119+
fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
120+
let node = ctx.query();
121+
Some(
122+
RuleDiagnostic::new(
123+
rule_category!(),
124+
node.range(),
125+
markup! {
126+
"The body of a for-in should be wrapped in an `if` statement."
127+
},
128+
)
129+
.note(markup! {
130+
"Looping over the object with for-in loop will include properties that are inherited through the prototype chain, the behaviour can lead to some unexpected items in your loop."
131+
}).note(markup! {
132+
"To resolve this issue, add an if statement like `if (Object.hasOwn(foo, key)) {...}` to filter out the extraneous properties. "
133+
}),
134+
)
135+
}
136+
}

‎crates/biome_js_analyze/src/options.rs‎

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

‎crates/biome_js_analyze/tests/specs/nursery/useGuardForIn/invalid.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
for (var x in o) { if (x) { f(); continue; } g(); }
2+
for (var x in o) { if (x) { continue; f(); } g(); }
3+
for (var x in o) { if (x) { f(); } g(); }
4+
for (var x in o) { if (x) f(); g(); }
5+
for (var x in o) { foo() }
6+
for (var x in o) foo();

0 commit comments

Comments
 (0)