Skip to content

feat(lint/js): add noLoopFunc#9815

Merged
dyc3 merged 1 commit intomainfrom
dyc3/noLoopFunc
Apr 19, 2026
Merged

feat(lint/js): add noLoopFunc#9815
dyc3 merged 1 commit intomainfrom
dyc3/noLoopFunc

Conversation

@dyc3
Copy link
Copy Markdown
Contributor

@dyc3 dyc3 commented Apr 5, 2026

Summary

This adds noLoopFunc which is a port of https://eslint.org/docs/latest/rules/no-loop-func

Generated by gpt 5.4. I didn't really have to steer it too hard. Tweaked it a little manually by hand.

Test Plan

snapshots

Docs

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 5, 2026

🦋 Changeset detected

Latest commit: 0355e87

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@biomejs/biome Patch
@biomejs/cli-win32-x64 Patch
@biomejs/cli-win32-arm64 Patch
@biomejs/cli-darwin-x64 Patch
@biomejs/cli-darwin-arm64 Patch
@biomejs/cli-linux-x64 Patch
@biomejs/cli-linux-arm64 Patch
@biomejs/cli-linux-x64-musl Patch
@biomejs/cli-linux-arm64-musl Patch
@biomejs/wasm-web Patch
@biomejs/wasm-bundler Patch
@biomejs/wasm-nodejs Patch
@biomejs/backend-jsonrpc Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions Bot added A-CLI Area: CLI A-Project Area: project A-Linter Area: linter L-JavaScript Language: JavaScript and super languages A-Diagnostic Area: diagnostocis labels Apr 5, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 5, 2026

Merging this PR will not alter performance

✅ 59 untouched benchmarks
⏩ 195 skipped benchmarks1


Comparing dyc3/noLoopFunc (0355e87) with main (131019e)

Open in CodSpeed

Footnotes

  1. 195 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@dyc3 dyc3 marked this pull request as ready for review April 5, 2026 19:36
@dyc3 dyc3 requested review from a team April 5, 2026 19:36
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9de5997d-6f65-4c9b-9ff6-0535b9b46265

📥 Commits

Reviewing files that changed from the base of the PR and between e5650c9 and 0355e87.

⛔ Files ignored due to path filters (3)
  • crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs is excluded by !**/migrate/eslint_any_rule_to_biome.rs and included by **
  • crates/biome_configuration/src/analyzer/linter/rules.rs is excluded by !**/rules.rs and included by **
  • crates/biome_configuration/src/generated/linter_options_check.rs is excluded by !**/generated/**, !**/generated/** and included by **
📒 Files selected for processing (1)
  • .changeset/hot-foxes-refuse.md
✅ Files skipped from review due to trivial changes (1)
  • .changeset/hot-foxes-refuse.md

Walkthrough

Adds a new nursery lint rule noLoopFunc to the Biome JavaScript analyser that warns when functions declared inside syntactic loop constructs capture outer variables that may change across iterations. The rule finds the innermost enclosing loop (with special handling for for initialisers and for..in/of RHS), skips non-escaping plain IIFEs, computes closure captures (including nested closures within the same enclosing range), classifies captures as safe or unsafe, deduplicates by name, and emits a warning. Adds tests, rule options, and a changeset.

Suggested reviewers

  • ematipico
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarises the main change: adding a new noLoopFunc linter rule to the JavaScript analyzer.
Description check ✅ Passed The description clearly explains the purpose (porting the ESLint rule), acknowledges AI assistance as required, and outlines the test approach via snapshots.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dyc3/noLoopFunc

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs (1)

88-92: Keep UnsafeCapture private and below the rule impl.

It is only used as this module’s State, so pub widens the surface for no gain. Repo convention also keeps helper structs under impl Rule; this one is the odd duck out.

♻️ Tidy-up
-#[derive(Debug)]
-pub struct UnsafeCapture {
-    name: TokenText,
-    range: TextRange,
-}
-
 impl Rule for NoLoopFunc {
@@
 }
+
+#[derive(Debug)]
+struct UnsafeCapture {
+    name: TokenText,
+    range: TextRange,
+}

Based on learnings: In crates/biome_analyze/**/*.rs rule files, all helper functions, structs, and enums must be placed below the impl Rule block. The only exception is when declaring a node union to use in the rule's Query type, which can be kept above the rule block for better readability.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs` around lines 88 -
92, The UnsafeCapture struct is declared pub and placed above the rule impl;
make it private (remove pub) and move its definition below the impl Rule block
so it becomes the module State type's helper located with other helper types;
update any references to UnsafeCapture (e.g., State) to ensure visibility
remains correct within the module after moving it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.changeset/hot-foxes-refuse.md:
- Line 5: Update the release note line about the new nursery rule `noLoopFunc`
in .changeset/hot-foxes-refuse.md so it doesn't imply the rule is on by default;
change the phrasing to indicate it's inactive unless enabled (for example: "When
enabled, it warns when a function declared inside a loop captures outer
variables that can change across iterations.") and ensure you reference the rule
name `noLoopFunc` and that it is `recommended: false`.

In `@crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs`:
- Around line 250-267: get_top_loop_node currently sets the border to
excluded_node.text_range().end(), which causes traversal to stop at the inner
loop and misclassify outer-loop let captures; change the border to use
excluded_node.text_range().start() so the search can climb past the inner loop
to the true top loop (update the border calculation in get_top_loop_node
accordingly), ensure the same logic is applied in the analogous block around
lines 296-321, and add/update a snapshot/regression test that covers the nested
for (let i ...) { for (...) { handlers.push(() => i); } } case to lock the
behavior.

---

Nitpick comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs`:
- Around line 88-92: The UnsafeCapture struct is declared pub and placed above
the rule impl; make it private (remove pub) and move its definition below the
impl Rule block so it becomes the module State type's helper located with other
helper types; update any references to UnsafeCapture (e.g., State) to ensure
visibility remains correct within the module after moving it.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0841a91e-f499-4122-be24-cfa271ebca02

📥 Commits

Reviewing files that changed from the base of the PR and between 1d09f0f and 9b0a3b6.

⛔ Files ignored due to path filters (10)
  • crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs is excluded by !**/migrate/eslint_any_rule_to_biome.rs and included by **
  • crates/biome_configuration/src/analyzer/linter/rules.rs is excluded by !**/rules.rs and included by **
  • crates/biome_configuration/src/generated/linter_options_check.rs is excluded by !**/generated/**, !**/generated/** and included by **
  • crates/biome_diagnostics_categories/src/categories.rs is excluded by !**/categories.rs and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.js.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.ts.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.js.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.ts.snap is excluded by !**/*.snap and included by **
  • packages/@biomejs/backend-jsonrpc/src/workspace.ts is excluded by !**/backend-jsonrpc/src/workspace.ts and included by **
  • packages/@biomejs/biome/configuration_schema.json is excluded by !**/configuration_schema.json and included by **
📒 Files selected for processing (8)
  • .changeset/hot-foxes-refuse.md
  • crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.js
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.ts
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.js
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.ts
  • crates/biome_rule_options/src/lib.rs
  • crates/biome_rule_options/src/no_loop_func.rs

Comment thread .changeset/hot-foxes-refuse.md Outdated
Comment on lines +250 to +267
fn get_top_loop_node(
loop_node: &JsSyntaxNode,
excluded_node: Option<&JsSyntaxNode>,
model: &SemanticModel,
) -> JsSyntaxNode {
let border = excluded_node.map_or(TextSize::from(0), |node| node.text_range().end());
let mut top_loop = loop_node.clone();
let mut current_loop = loop_node.clone();

while current_loop.text_range().start() >= border {
top_loop = current_loop.clone();
let Some(containing_loop) = get_containing_loop_node(&current_loop, model) else {
break;
};
current_loop = containing_loop;
}

top_loop
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Nested inner-loop captures of an outer let still come out as unsafe.

For for (let i ...) { for (...) { handlers.push(() => i); } }, the decl.is_let() shortcut does not fire because i is declared outside the inner loop. get_top_loop_node() then stops at the inner loop, so the outer loop’s i++ write is treated as crossing the border and this becomes a false positive, even though i gets a fresh binding per outer iteration. Please adjust the border logic and lock it down with a valid snapshot.

🧪 Handy regression case
+for (let i = 0; i < 3; i++) {
+    for (let j = 0; j < 3; j++) {
+        handlers.push(() => i);
+    }
+}

Also applies to: 296-321

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs` around lines 250 -
267, get_top_loop_node currently sets the border to
excluded_node.text_range().end(), which causes traversal to stop at the inner
loop and misclassify outer-loop let captures; change the border to use
excluded_node.text_range().start() so the search can climb past the inner loop
to the true top loop (update the border calculation in get_top_loop_node
accordingly), ensure the same logic is applied in the analogous block around
lines 296-321, and add/update a snapshot/regression test that covers the nested
for (let i ...) { for (...) { handlers.push(() => i); } } case to lock the
behavior.

@dyc3 dyc3 force-pushed the dyc3/noLoopFunc branch from 9b0a3b6 to 6a11aaf Compare April 6, 2026 13:46
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs (1)

257-274: ⚠️ Potential issue | 🟠 Major

Nested outer-loop let captures still come out unsafe.

get_top_loop_node() still uses the excluded declaration’s end as the border, so the climb stops at the inner loop instead of reaching the loop that owns the fresh binding. for (let i ...) { for (...) { handlers.push(() => i); } } is still a false positive here, and the same shape bites outer-loop lets declared in the body before the inner loop. Please fix the border logic and add a valid snapshot. As per coding guidelines, "All code changes MUST include appropriate tests: lint rules require snapshot tests in tests/specs/{group}/{rule}/."

Also applies to: 313-320

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs` around lines 257 -
274, get_top_loop_node currently computes the border using
excluded_node.text_range().end(), which causes the climb to stop too early (at
the inner loop) and yields false positives; change the border to use
excluded_node.text_range().start() instead (i.e., use the excluded node's start
offset) so the loop climb can reach the outer loop that owns the fresh binding,
and make the same change in the other occurrence around lines 313-320; then add
a snapshot test under tests/specs/{group}/{rule}/ (e.g., a case like `for (let
i...) { for (...) { handlers.push(() => i); } }`) that reproduces the previous
false positive and confirms the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs`:
- Around line 257-274: get_top_loop_node currently computes the border using
excluded_node.text_range().end(), which causes the climb to stop too early (at
the inner loop) and yields false positives; change the border to use
excluded_node.text_range().start() instead (i.e., use the excluded node's start
offset) so the loop climb can reach the outer loop that owns the fresh binding,
and make the same change in the other occurrence around lines 313-320; then add
a snapshot test under tests/specs/{group}/{rule}/ (e.g., a case like `for (let
i...) { for (...) { handlers.push(() => i); } }`) that reproduces the previous
false positive and confirms the fix.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b609284a-3e12-4698-bc1b-8614fb48fde2

📥 Commits

Reviewing files that changed from the base of the PR and between 9b0a3b6 and 6a11aaf.

⛔ Files ignored due to path filters (10)
  • crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs is excluded by !**/migrate/eslint_any_rule_to_biome.rs and included by **
  • crates/biome_configuration/src/analyzer/linter/rules.rs is excluded by !**/rules.rs and included by **
  • crates/biome_configuration/src/generated/linter_options_check.rs is excluded by !**/generated/**, !**/generated/** and included by **
  • crates/biome_diagnostics_categories/src/categories.rs is excluded by !**/categories.rs and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.js.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.ts.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.js.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.ts.snap is excluded by !**/*.snap and included by **
  • packages/@biomejs/backend-jsonrpc/src/workspace.ts is excluded by !**/backend-jsonrpc/src/workspace.ts and included by **
  • packages/@biomejs/biome/configuration_schema.json is excluded by !**/configuration_schema.json and included by **
📒 Files selected for processing (8)
  • .changeset/hot-foxes-refuse.md
  • crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.js
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.ts
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.js
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.ts
  • crates/biome_rule_options/src/lib.rs
  • crates/biome_rule_options/src/no_loop_func.rs
✅ Files skipped from review due to trivial changes (7)
  • .changeset/hot-foxes-refuse.md
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.ts
  • crates/biome_rule_options/src/lib.rs
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.js
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.ts
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.js
  • crates/biome_rule_options/src/no_loop_func.rs

Comment on lines +20 to +22
declare_node_union! {
pub AnyLoopFunction = JsFunctionDeclaration | JsFunctionExpression | JsArrowFunctionExpression
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's check the file structure and the specific lines mentioned
wc -l crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs

Repository: biomejs/biome

Length of output: 117


🏁 Script executed:

# Read the union definition and surrounding context
sed -n '15,30p' crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs

Repository: biomejs/biome

Length of output: 743


🏁 Script executed:

# Read the is_function_boundary function
sed -n '525,555p' crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs

Repository: biomejs/biome

Length of output: 781


🏁 Script executed:

# Check for snapshot tests for this rule
find . -path '*/tests/specs/*' -name '*no_loop_func*' -o -path '*/tests/specs/*' -name '*no-loop-func*'

Repository: biomejs/biome

Length of output: 39


🏁 Script executed:

# Check the directory structure of tests to understand where tests should be
find . -path '*/tests/specs/*/no*' -type f | head -20

Repository: biomejs/biome

Length of output: 1992


🏁 Script executed:

# Search for any existing tests related to nursery rules
find . -path '*/tests/specs/*/nursery*' -type d

Repository: biomejs/biome

Length of output: 39


🏁 Script executed:

# Search for the test directory structure for js_analyze
find crates/biome_js_analyze/tests -type d -name 'specs' | head -5

Repository: biomejs/biome

Length of output: 92


🏁 Script executed:

# List the actual test structure
ls -la crates/biome_js_analyze/tests/specs/ 2>/dev/null | head -20

Repository: biomejs/biome

Length of output: 700


🏁 Script executed:

# Search for any existing no_loop_func tests
find crates/biome_js_analyze/tests -name '*no_loop*' -o -name '*no-loop*' 2>/dev/null

Repository: biomejs/biome

Length of output: 39


🏁 Script executed:

# Check what nursery tests exist
find crates/biome_js_analyze/tests/specs -type d -name 'nursery' 2>/dev/null

Repository: biomejs/biome

Length of output: 100


🏁 Script executed:

# List files in nursery directory if it exists
ls -la crates/biome_js_analyze/tests/specs/nursery/ 2>/dev/null | head -30

Repository: biomejs/biome

Length of output: 2045


🏁 Script executed:

# Check how the union and boundary check are actually used in the code
sed -n '100,150p' crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs

Repository: biomejs/biome

Length of output: 2026


🏁 Script executed:

# Check the run function which is the main rule logic
sed -n '170,250p' crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs

Repository: biomejs/biome

Length of output: 2732


🏁 Script executed:

# Check what's in the noLoopFunc test directory
ls -la crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/

Repository: biomejs/biome

Length of output: 670


🏁 Script executed:

# Look at the test files
cat crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.js

Repository: biomejs/biome

Length of output: 547


🏁 Script executed:

# Check if there's a snapshot file
cat crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.js.snap

Repository: biomejs/biome

Length of output: 6426


🏁 Script executed:

# Now let's trace through the logic more carefully - check how the rule is invoked
# Look at the query mechanism
sed -n '50,100p' crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs

Repository: biomejs/biome

Length of output: 1289


🏁 Script executed:

# Check the structure of how the rule is declared
sed -n '20,80p' crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs

Repository: biomejs/biome

Length of output: 1966


🏁 Script executed:

# Check if there are any method or getter/setter test cases in the test files
grep -n "run\|get \|set \|constructor" crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.js

Repository: biomejs/biome

Length of output: 39


🏁 Script executed:

# Check the TypeScript test cases
cat crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.ts

Repository: biomejs/biome

Length of output: 232


🏁 Script executed:

# Check valid cases too
cat crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.js

Repository: biomejs/biome

Length of output: 442


🏁 Script executed:

# Let's verify the query mechanism - how semantic queries work for the union
# Check if there are examples of other rules that query methods
grep -r "declare_node_union" crates/biome_js_analyze/src/lint/nursery/ | head -5

Repository: biomejs/biome

Length of output: 622


Methods and accessors aren't queried by the rule, creating coverage asymmetry.

AnyLoopFunction queries only JsFunctionDeclaration, JsFunctionExpression, and JsArrowFunctionExpression, but is_function_boundary() treats all function-like nodes (including JS_METHOD_*, JS_GETTER_*, JS_SETTER_*, and JS_CONSTRUCTOR_*) as boundaries. This means object and class methods created inside loops escape the rule entirely. Either expand the union to include method types and add snapshot tests covering them, or document this as intentional scoping. Per coding guidelines, all lint rule changes require snapshot tests in tests/specs/nursery/noLoopFunc/.

Comment thread crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs Outdated
@dyc3 dyc3 force-pushed the dyc3/noLoopFunc branch from 6a11aaf to e5650c9 Compare April 6, 2026 14:43
@dyc3 dyc3 requested a review from ematipico April 6, 2026 14:45
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs (1)

21-23: ⚠️ Potential issue | 🟠 Major

Methods, accessors, and constructors still dodge the rule.

AnyLoopFunction only queries declarations/expressions/arrows, but is_function_boundary() already treats method-like nodes as function boundaries. That leaves loop-created object/class methods, getters/setters, and constructors silently out of scope. Please expand the query or make the narrower scope explicit, and lock it down with snapshots in crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/.

As per coding guidelines, "All code changes MUST include appropriate tests: lint rules require snapshot tests in 'tests/specs/{group}/{rule}/'."

Also applies to: 534-547

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs` around lines 21 -
23, AnyLoopFunction currently only includes JsFunctionDeclaration |
JsFunctionExpression | JsArrowFunctionExpression which leaves method-like nodes
(methods, getters/setters) and constructors out of the lint though
is_function_boundary() treats them as boundaries; update the declare_node_union!
for AnyLoopFunction to also include the method/accessor/constructor node kinds
used by the AST in this crate (the same node kinds checked by
is_function_boundary()), and add snapshot tests under
crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/ covering loop-created
object/class methods, getters/setters, and constructors to lock behavior; ensure
the new tests fail when those node kinds are not treated as loop functions and
pass after expanding AnyLoopFunction.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs (1)

89-92: Please tuck UnsafeCapture below impl Rule.

Tiny housekeeping, but this repo keeps helper structs with the other rule helpers; the query node union is the only usual exception.

Based on learnings, "In crates/biome_analyze/**/*.rs rule files, all helper functions, structs, and enums must be placed below the impl Rule block. The only exception is when declaring a node union to use in the rule's Query type, which can be kept above the rule block for better readability."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs` around lines 89 -
92, The helper struct UnsafeCapture (#[derive(Debug)] pub struct UnsafeCapture {
name: TokenText, }) should be moved below the impl Rule block; locate the
UnsafeCapture declaration and cut it out, then paste it into the helper area
after the impl Rule { ... } block (with the other helper functions/structs),
keeping its attributes, visibility, and name unchanged so all references (e.g.,
code that constructs or matches UnsafeCapture) continue to compile; do not move
any Query node-union declarations—only relocate this helper struct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs`:
- Around line 307-311: The conditional that short-circuits loop-local `let`
declarations (the check using `declaration.as_ref().is_some_and(|decl|
decl.is_let() && decl.range().start() > loop_node.text_range().start() &&
decl.range().end() < loop_node.text_range().end())`) wrongly excludes fresh
`let` bindings from the write-analysis path; remove or alter this early-return
so loop-local `let` bindings are still analyzed for writes when computing
captured variables in the no_loop_func lint (affecting the same pattern around
the other occurrence at the block covering lines ~315-331), and add a snapshot
test under crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/ demonstrating
the regression (e.g., push a closure that reads a `let` then increment the `let`
in the same iteration) so the rule flags the case.

---

Duplicate comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs`:
- Around line 21-23: AnyLoopFunction currently only includes
JsFunctionDeclaration | JsFunctionExpression | JsArrowFunctionExpression which
leaves method-like nodes (methods, getters/setters) and constructors out of the
lint though is_function_boundary() treats them as boundaries; update the
declare_node_union! for AnyLoopFunction to also include the
method/accessor/constructor node kinds used by the AST in this crate (the same
node kinds checked by is_function_boundary()), and add snapshot tests under
crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/ covering loop-created
object/class methods, getters/setters, and constructors to lock behavior; ensure
the new tests fail when those node kinds are not treated as loop functions and
pass after expanding AnyLoopFunction.

---

Nitpick comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs`:
- Around line 89-92: The helper struct UnsafeCapture (#[derive(Debug)] pub
struct UnsafeCapture { name: TokenText, }) should be moved below the impl Rule
block; locate the UnsafeCapture declaration and cut it out, then paste it into
the helper area after the impl Rule { ... } block (with the other helper
functions/structs), keeping its attributes, visibility, and name unchanged so
all references (e.g., code that constructs or matches UnsafeCapture) continue to
compile; do not move any Query node-union declarations—only relocate this helper
struct.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cbc1206e-614f-4f3b-9d99-b364e8a3e68f

📥 Commits

Reviewing files that changed from the base of the PR and between 6a11aaf and e5650c9.

⛔ Files ignored due to path filters (10)
  • crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs is excluded by !**/migrate/eslint_any_rule_to_biome.rs and included by **
  • crates/biome_configuration/src/analyzer/linter/rules.rs is excluded by !**/rules.rs and included by **
  • crates/biome_configuration/src/generated/linter_options_check.rs is excluded by !**/generated/**, !**/generated/** and included by **
  • crates/biome_diagnostics_categories/src/categories.rs is excluded by !**/categories.rs and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.js.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.ts.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.js.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.ts.snap is excluded by !**/*.snap and included by **
  • packages/@biomejs/backend-jsonrpc/src/workspace.ts is excluded by !**/backend-jsonrpc/src/workspace.ts and included by **
  • packages/@biomejs/biome/configuration_schema.json is excluded by !**/configuration_schema.json and included by **
📒 Files selected for processing (8)
  • .changeset/hot-foxes-refuse.md
  • crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.js
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.ts
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.js
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.ts
  • crates/biome_rule_options/src/lib.rs
  • crates/biome_rule_options/src/no_loop_func.rs
✅ Files skipped from review due to trivial changes (6)
  • .changeset/hot-foxes-refuse.md
  • crates/biome_rule_options/src/lib.rs
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.ts
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.js
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/valid.js
  • crates/biome_rule_options/src/no_loop_func.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/biome_js_analyze/tests/specs/nursery/noLoopFunc/invalid.ts

Comment thread crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs
Copy link
Copy Markdown
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I paused the review midway because the docstrings felt more harmful than helpful. I'm not comfortable accepting such a large PR with these docs. As it is big, I would expect more human supervision.

Here's the incriminating patterns I found:

  • Sometimes they miss the very core of the business logic
  • Many functions have the same code snippets
  • Many examples require an explanation, since many of these examples are "the same"
  • Examples should clear things out if the logic is complex or needs some attention. Sometimes they might not be needed. E.g. is_async_or_generator is very straightforward.
  • Some docs say "here we match eslint"... duh! I mean, I expect that since the rule is the same.

Here are my suggestions:

  • Don't use LLM until you find a good way to create good docs. As far as I can tell, they are really bad.
  • If you do, double down and write as you should explain things for the next developer who arrives, and see your code.

Comment thread crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/no_loop_func.rs Outdated
@dyc3 dyc3 force-pushed the dyc3/noLoopFunc branch from e5650c9 to 0355e87 Compare April 17, 2026 15:29
Copy link
Copy Markdown
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look at the bot, it might be correct

@dyc3 dyc3 merged commit 5cc83b1 into main Apr 19, 2026
31 checks passed
@dyc3 dyc3 deleted the dyc3/noLoopFunc branch April 19, 2026 11:59
@github-actions github-actions Bot mentioned this pull request Apr 18, 2026
OIRNOIR pushed a commit to OIRNOIR/YouTube-Helper-Server that referenced this pull request Apr 24, 2026
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [@biomejs/biome](https://biomejs.dev) ([source](https://github.com/biomejs/biome/tree/HEAD/packages/@biomejs/biome)) | patch | `2.4.12` → `2.4.13` |

---

### Release Notes

<details>
<summary>biomejs/biome (@&#8203;biomejs/biome)</summary>

### [`v2.4.13`](https://github.com/biomejs/biome/blob/HEAD/packages/@&#8203;biomejs/biome/CHANGELOG.md#2413)

[Compare Source](https://github.com/biomejs/biome/compare/@biomejs/[email protected]...@biomejs/[email protected])

##### Patch Changes

- [#&#8203;9969](biomejs/biome#9969) [`c5eb92b`](biomejs/biome@c5eb92b) Thanks [@&#8203;officialasishkumar](https://github.com/officialasishkumar)! - Added the nursery rule [`noUnnecessaryTemplateExpression`](https://biomejs.dev/linter/rules/no-unnecessary-template-expression/), which disallows template literals that only contain string literal expressions. These can be replaced with a simpler string literal.

  For example, the following code triggers the rule:

  ```js
  const a = `${"hello"}`; // can be 'hello'
  const b = `${"prefix"}_suffix`; // can be 'prefix_suffix'
  const c = `${"a"}${"b"}`; // can be 'ab'
  ```

- [#&#8203;10037](biomejs/biome#10037) [`f785e8c`](biomejs/biome@f785e8c) Thanks [@&#8203;minseong0324](https://github.com/minseong0324)! - Fixed [#&#8203;9810](biomejs/biome#9810): [`noMisleadingReturnType`](https://biomejs.dev/linter/rules/no-misleading-return-type/) no longer reports false positives on a getter with a matching setter in the same namespace.

  ```ts
  class Store {
    get status(): string {
      if (Math.random() > 0.5) return "loading";
      return "idle";
    }
    set status(v: string) {}
  }
  ```

- [#&#8203;10084](biomejs/biome#10084) [`5e2f90c`](biomejs/biome@5e2f90c) Thanks [@&#8203;jiwon79](https://github.com/jiwon79)! - Fixed [#&#8203;10034](biomejs/biome#10034): [`noUselessEscapeInRegex`](https://biomejs.dev/linter/rules/no-useless-escape-in-regex/) no longer flags escapes of `ClassSetReservedPunctuator` characters (`&`, `!`, `#`, `%`, `,`, `:`, `;`, `<`, `=`, `>`, `@`, `` ` ``, `~`) inside `v`-flag character classes as useless. These characters are reserved as individual code points in `v`-mode, so the escape is required.

  The following pattern is now considered valid:

  ```js
  /[a-z\&]/v;
  ```

- [#&#8203;10063](biomejs/biome#10063) [`c9ffa16`](biomejs/biome@c9ffa16) Thanks [@&#8203;Netail](https://github.com/Netail)! - Added extra rule sources from ESLint CSS. `biome migrate eslint` should do a bit better detecting rules in your eslint configurations.

- [#&#8203;10035](biomejs/biome#10035) [`946b50e`](biomejs/biome@946b50e) Thanks [@&#8203;Netail](https://github.com/Netail)! - Fixed [#&#8203;10032](biomejs/biome#10032): [useIframeSandbox](https://biomejs.dev/linter/rules/use-iframe-sandbox/) now flags if there's no initializer value.

- [#&#8203;9865](biomejs/biome#9865) [`68fb8d4`](biomejs/biome@68fb8d4) Thanks [@&#8203;dyc3](https://github.com/dyc3)! - Added the new nursery rule [`useDomNodeTextContent`](https://biomejs.dev/linter/rules/use-dom-node-text-content/), which prefers `textContent` over `innerText` for DOM node text access and destructuring.

  For example, the following snippet triggers the rule:

  ```js
  const foo = node.innerText;
  ```

- [#&#8203;10023](biomejs/biome#10023) [`bd1e74f`](biomejs/biome@bd1e74f) Thanks [@&#8203;ematipico](https://github.com/ematipico)! - Added a new nursery rule [`noReactNativeDeepImports`](https://biomejs.dev/linter/rules/no-react-native-deep-imports/) that disallows deep imports from the `react-native` package. Internal paths like `react-native/Libraries/...` are not part of the public API and may change between versions.

  For example, the following code triggers the rule:

  ```js
  import View from "react-native/Libraries/Components/View/View";
  ```

- [#&#8203;9885](biomejs/biome#9885) [`3dce737`](biomejs/biome@3dce737) Thanks [@&#8203;dyc3](https://github.com/dyc3)! - Added a new nursery rule [`useDomQuerySelector`](https://biomejs.dev/linter/rules/use-dom-query-selector/) that prefers `querySelector()` and `querySelectorAll()` over older DOM query methods such as `getElementById()` and `getElementsByClassName()`.

- [#&#8203;9995](biomejs/biome#9995) [`4da9caf`](biomejs/biome@4da9caf) Thanks [@&#8203;siketyan](https://github.com/siketyan)! - Fixed [#&#8203;9994](biomejs/biome#9994): Biome now parses nested CSS rules correctly when declarations follow them inside embedded snippets.

- [#&#8203;10009](biomejs/biome#10009) [`b41cc5a`](biomejs/biome@b41cc5a) Thanks [@&#8203;Jayllyz](https://github.com/Jayllyz)! - Fixed [#&#8203;10004](biomejs/biome#10004): [`noComponentHookFactories`](https://biomejs.dev/linter/rules/no-component-hook-factories/) no longer reports false positives for object methods and class methods.

- [#&#8203;9988](biomejs/biome#9988) [`eabf54a`](biomejs/biome@eabf54a) Thanks [@&#8203;Netail](https://github.com/Netail)! - Tweaked the diagnostics range for [useAltText](https://biomejs.dev/linter/rules/use-alt-text), [useButtonType](https://biomejs.dev/linter/rules/use-button-type), [useHtmlLang](https://biomejs.dev/linter/rules/use-html-lang), [useIframeTitle](https://biomejs.dev/linter/rules/use-iframe-title), [useValidAriaRole](https://biomejs.dev/linter/rules/use-valid-aria-role) & [useIfameSandbox](https://biomejs.dev/linter/rules/use-iframe-sandbox) to report on the opening tag instead of the full tag.

- [#&#8203;10043](biomejs/biome#10043) [`fc65902`](biomejs/biome@fc65902) Thanks [@&#8203;mujpao](https://github.com/mujpao)! - Fixed [#&#8203;10003](biomejs/biome#10003): Biome no longer panics when parsing Svelte files containing `{#}`.

- [#&#8203;9815](biomejs/biome#9815) [`5cc83b1`](biomejs/biome@5cc83b1) Thanks [@&#8203;dyc3](https://github.com/dyc3)! - Added the new nursery rule [`noLoopFunc`](https://biomejs.dev/linter/rules/no-loop-func/). When enabled, it warns when a function declared inside a loop captures outer variables that can change across iterations.

- [#&#8203;9702](biomejs/biome#9702) [`ef470ba`](biomejs/biome@ef470ba) Thanks [@&#8203;ryan-m-walker](https://github.com/ryan-m-walker)! - Added the nursery rule [`useRegexpTest`](https://biomejs.dev/linter/rules/use-regexp-test/) that enforces `RegExp.prototype.test()` over `String.prototype.match()` and `RegExp.prototype.exec()` in boolean contexts. `test()` returns a boolean directly, avoiding unnecessary computation of match results.

  **Invalid**

  ```js
  if ("hello world".match(/hello/)) {
  }
  ```

  **Valid**

  ```js
  if (/hello/.test("hello world")) {
  }
  ```

- [#&#8203;9743](biomejs/biome#9743) [`245307d`](biomejs/biome@245307d) Thanks [@&#8203;leetdavid](https://github.com/leetdavid)! - Fixed [#&#8203;2245](biomejs/biome#2245): Svelte `<script>` tag language detection when the `generics` attribute contains `>` characters (e.g., `<script lang="ts" generics="T extends Record<string, unknown>">`). Biome now correctly recognizes TypeScript in such script blocks.

- [#&#8203;10046](biomejs/biome#10046) [`0707de7`](biomejs/biome@0707de7) Thanks [@&#8203;Conaclos](https://github.com/Conaclos)! - Fixed [#&#8203;10038](biomejs/biome#10038): [`organizeImports`](https://biomejs.dev/assist/actions/organize-imports/) now sorts imports in TypeScript modules and declaration files.

  ```diff
    declare module "mymodule" {
  -  	import type { B } from "b";
    	import type { A } from "a";
  +  	import type { B } from "b";
    }
  ```

- [#&#8203;10012](biomejs/biome#10012) [`94ccca9`](biomejs/biome@94ccca9) Thanks [@&#8203;ematipico](https://github.com/ematipico)! - Added the nursery rule [`noReactNativeLiteralColors`](https://biomejs.dev/linter/rules/no-react-native-literal-colors/), which disallows color literals inside React Native styles.

  The rule belongs to the `reactNative` domain. It reports properties whose name contains `color` and whose value is a string literal when they appear inside a `StyleSheet.create(...)` call or inside a JSX attribute whose name contains `style`.

  ```jsx
  // Invalid
  const Hello = () => <Text style={{ backgroundColor: "#FFFFFF" }}>hi</Text>;

  const styles = StyleSheet.create({
    text: { color: "red" },
  });
  ```

  ```jsx
  // Valid
  const red = "#f00";
  const styles = StyleSheet.create({
    text: { color: red },
  });
  ```

- [#&#8203;10005](biomejs/biome#10005) [`131019e`](biomejs/biome@131019e) Thanks [@&#8203;ematipico](https://github.com/ematipico)! - Added the nursery rule [`noReactNativeRawText`](https://biomejs.dev/linter/rules/no-react-native-raw-text/), which disallows raw text outside of `<Text>` components in React Native.

  The rule belongs to the new `reactNative` domain.

  ```jsx
  // Invalid
  <View>some text</View>
  <View>{'some text'}</View>
  ```

  ```jsx
  // Valid
  <View>
    <Text>some text</Text>
  </View>
  ```

  Additional components can be allowlisted through the `skip` option:

  ```json
  {
    "options": {
      "skip": ["Title"]
    }
  }
  ```

- [#&#8203;9911](biomejs/biome#9911) [`1603f78`](biomejs/biome@1603f78) Thanks [@&#8203;Netail](https://github.com/Netail)! - Added the nursery rule [`noJsxLeakedDollar`](https://biomejs.dev/linter/rules/no-jsx-leaked-dollar), which flags text nodes with a trailing `$` if the next sibling node is a JSX expression. This could be an unintentional mistake, resulting in a '$' being rendered as text in the output.

  **Invalid**:

  ```jsx
  function MyComponent({ user }) {
    return <div>Hello ${user.name}</div>;
  }
  ```

- [#&#8203;9999](biomejs/biome#9999) [`f42405f`](biomejs/biome@f42405f) Thanks [@&#8203;minseong0324](https://github.com/minseong0324)! - Fixed `noMisleadingReturnType` incorrectly flagging functions with reassigned `let` variables.

- [#&#8203;10075](biomejs/biome#10075) [`295f97f`](biomejs/biome@295f97f) Thanks [@&#8203;ematipico](https://github.com/ematipico)! - Fixed [`#9983`](biomejs/biome#9983): Biome now parses functions declared inside Svelte `#snippet` blocks without throwing errors.

- [#&#8203;10006](biomejs/biome#10006) [`cf4c1c9`](biomejs/biome@cf4c1c9) Thanks [@&#8203;minseong0324](https://github.com/minseong0324)! - Fixed [#&#8203;9810](biomejs/biome#9810): `noMisleadingReturnType` incorrectly flagging nested object literals with widened properties.

- [#&#8203;10033](biomejs/biome#10033) [`11ddc05`](biomejs/biome@11ddc05) Thanks [@&#8203;ematipico](https://github.com/ematipico)! - Added the nursery rule [`useReactNativePlatformComponents`](https://biomejs.dev/linter/rules/use-react-native-platform-components/) that ensures platform-specific React Native components (e.g. `ProgressBarAndroid`, `ActivityIndicatorIOS`) are only imported in files with a matching platform suffix. It also reports when Android and iOS components are mixed in the same file.

  The following code triggers the rule when the file does not have an `.android.js` suffix:

  ```js
  // file.js
  import { ProgressBarAndroid } from "react-native";
  ```

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMzkuNiIsInVwZGF0ZWRJblZlciI6IjQzLjEzOS42IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Reviewed-on: https://git.oirnoir.dev/OIRNOIR/YouTube-Helper-Server/pulls/1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-CLI Area: CLI A-Diagnostic Area: diagnostocis A-Linter Area: linter A-Project Area: project L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants