feat(rome_js_analyze): noForEach (#4364)

feat(rome_js_analyze): noForEach

CHANGELOG.md

@@ -22,6 +22,7 @@ the code action is not formatted.
 - [`noNoninteractiveTabindex`](https://docs.rome.tools/lint/rules/noNoninteractiveTabindex/)
 - [`noAriaUnsupportedElements`](https://docs.rome.tools/lint/rules/noAriaUnsupportedElements/)
 - [`noConsoleLog`](https://docs.rome.tools/lint/rules/noConsoleLog/)
+- [`noForEach`](https://docs.rome.tools/lint/rules/noForEach/)
 
 ### Parser
 

crates/rome_diagnostics_categories/src/categories.rs

@@ -103,6 +103,7 @@ define_categories! {
     "lint/nursery/noNoninteractiveTabindex": "https://docs.rome.tools/lint/rules/noNoninteractiveTabindex",
     "lint/nursery/noAriaUnsupportedElements": "https://docs.rome.tools/lint/rules/noAriaUnsupportedElements",
     "lint/nursery/noConsoleLog": "https://docs.rome.tools/lint/rules/noConsoleLog",
+"lint/nursery/noForEach": "https://docs.rome.tools/lint/rules/noForEach",
     // Insert new nursery rule here
     "lint/nursery/noRedeclare": "https://docs.rome.tools/lint/rules/noRedeclare",
     "lint/nursery/useNamespaceKeyword": "https://docs.rome.tools/lint/rules/useNamespaceKeyword",

crates/rome_js_analyze/src/analyzers/nursery/no_for_each.rs

@@ -0,0 +1,98 @@
+use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic};
+use rome_console::markup;
+use rome_js_syntax::{AnyJsExpression, JsCallExpression};
+use rome_rowan::AstNode;
+
+declare_rule! {
+    /// Prefer `for...of` statement instead of `Array.forEach`.
+    ///
+    /// Here's a summary of why `forEach` may be disallowed, and why `for...of` is preferred for almost any use-case of `forEach`:
+    /// - Performance: Using `forEach` can lead to performance issues, especially when working with large arrays.
+    /// When more requirements are added on, `forEach` typically gets chained with other methods like `filter` or `map`, causing multiple iterations over the same Array.
+    /// Encouraging for loops discourages chaining and encourages single-iteration logic (e.g. using a continue instead of `filter`).
+    ///
+    /// - Readability: While `forEach` is a simple and concise way to iterate over an array, it can make the code less readable, especially when the callback function is complex.
+    /// In contrast, using a for loop or a `for...of` loop can make the code more explicit and easier to read.
+    ///
+    /// - Debugging: `forEach` can make debugging more difficult, because it hides the iteration process.
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```js,expect_diagnostic
+    /// els.forEach(el => {
+    ///   el
+    /// })
+    /// ```
+    ///
+    /// ```js,expect_diagnostic
+    /// els['forEach'](el => {
+    ///   el
+    /// })
+    /// ```
+    ///
+    /// ## Valid
+    ///
+    /// ```js
+    /// for (const el of els) {
+    ///   el
+    /// }
+    /// ```
+    ///
+    pub(crate) NoForEach {
+        version: "next",
+        name: "noForEach",
+        recommended: false,
+    }
+}
+
+impl Rule for NoForEach {
+    type Query = Ast<JsCallExpression>;
+    type State = ();
+    type Signals = Option<Self::State>;
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        let node = ctx.query();
+
+        let is_for_each = match node.callee().ok()?.omit_parentheses() {
+            AnyJsExpression::JsStaticMemberExpression(expression) => {
+                expression
+                    .member()
+                    .ok()?
+                    .as_js_name()?
+                    .value_token()
+                    .ok()?
+                    .text_trimmed()
+                    == "forEach"
+            }
+            AnyJsExpression::JsComputedMemberExpression(expression) => {
+                expression
+                    .member()
+                    .ok()?
+                    .as_any_js_literal_expression()?
+                    .as_js_string_literal_expression()?
+                    .inner_string_text()
+                    .ok()?
+                    .text()
+                    == "forEach"
+            }
+            _ => return None,
+        };
+
+        is_for_each.then_some(())
+    }
+
+    fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
+        let node = ctx.query();
+
+        Some(RuleDiagnostic::new(
+            rule_category!(),
+            node.syntax().text_trimmed_range(),
+            markup! {
+                "Prefer for...of instead of Array.forEach"
+            },
+        ))
+    }
+}

crates/rome_js_analyze/tests/specs/nursery/noForEach/invalid.js

@@ -0,0 +1,15 @@
+els.forEach((el) => {
+	el;
+});
+
+(els.forEach)((el) => {
+	el;
+});
+
+els['forEach']((el) => {
+	el;
+});
+
+(els['forEach'])((el) => {
+	el;
+});

crates/rome_js_analyze/tests/specs/nursery/noForEach/invalid.js.snap

@@ -0,0 +1,95 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+expression: invalid.js
+---
+# Input
+```js
+els.forEach((el) => {
+	el;
+});
+
+(els.forEach)((el) => {
+	el;
+});
+
+els['forEach']((el) => {
+	el;
+});
+
+(els['forEach'])((el) => {
+	el;
+});
+
+```
+
+# Diagnostics
+```
+invalid.js:1:1 lint/nursery/noForEach ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Prefer for...of instead of Array.forEach
+  
+  > 1 │ els.forEach((el) => {
+      │ ^^^^^^^^^^^^^^^^^^^^^
+  > 2 │ 	el;
+  > 3 │ });
+      │ ^^
+    4 │ 
+    5 │ (els.forEach)((el) => {
+  
+
+```
+
+```
+invalid.js:5:1 lint/nursery/noForEach ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Prefer for...of instead of Array.forEach
+  
+    3 │ });
+    4 │ 
+  > 5 │ (els.forEach)((el) => {
+      │ ^^^^^^^^^^^^^^^^^^^^^^^
+  > 6 │ 	el;
+  > 7 │ });
+      │ ^^
+    8 │ 
+    9 │ els['forEach']((el) => {
+  
+
+```
+
+```
+invalid.js:9:1 lint/nursery/noForEach ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Prefer for...of instead of Array.forEach
+  
+     7 │ });
+     8 │ 
+   > 9 │ els['forEach']((el) => {
+       │ ^^^^^^^^^^^^^^^^^^^^^^^^
+  > 10 │ 	el;
+  > 11 │ });
+       │ ^^
+    12 │ 
+    13 │ (els['forEach'])((el) => {
+  
+
+```
+
+```
+invalid.js:13:1 lint/nursery/noForEach ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Prefer for...of instead of Array.forEach
+  
+    11 │ });
+    12 │ 
+  > 13 │ (els['forEach'])((el) => {
+       │ ^^^^^^^^^^^^^^^^^^^^^^^^^^
+  > 14 │ 	el;
+  > 15 │ });
+       │ ^^
+    16 │ 
+  
+
+```
+
+

crates/rome_js_analyze/tests/specs/nursery/noForEach/valid.js

@@ -0,0 +1,3 @@
+for (const el of els) {
+   el
+ }

crates/rome_js_analyze/tests/specs/nursery/noForEach/valid.js.snap

@@ -0,0 +1,13 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+expression: valid.js
+---
+# Input
+```js
+for (const el of els) {
+   el
+ }
+
+```
+
+