Skip to content

feat(linter): implement eslint/no-loop-func rule#16830

Merged
camc314 merged 6 commits intooxc-project:mainfrom
tt-a1i:feat/linter-no-loop-func
Dec 14, 2025
Merged

feat(linter): implement eslint/no-loop-func rule#16830
camc314 merged 6 commits intooxc-project:mainfrom
tt-a1i:feat/linter-no-loop-func

Conversation

@tt-a1i
Copy link
Copy Markdown
Contributor

@tt-a1i tt-a1i commented Dec 14, 2025

Summary

Implements the ESLint no-loop-func rule which disallows function declarations and expressions inside loop statements when they reference variables that may change across iterations.

What it detects:

  • Functions in loops referencing var variables declared in/around the loop
  • Functions referencing let variables declared outside the loop that are modified anywhere

Safe patterns (not flagged):

  • let/const in for loop header (fresh binding per iteration)
  • let/const declared inside loop body (fresh binding per iteration)
  • Variables that are never modified
  • const and import bindings (immutable)

Examples

Incorrect:

for (var i = 0; i < 10; i++) {
    funcs[i] = function() { return i; };
}

Correct:

for (let i = 0; i < 10; i++) {
    funcs[i] = function() { return i; };
}

Test plan

  • Added 27 pass test cases covering various scenarios
  • Added 13 fail test cases
  • cargo test -p oxc_linter -- no_loop_func passes
  • cargo clippy passes with no warnings

@tt-a1i
feat(linter): implement eslint/no-loop-func rule
c935a73 
This rule disallows function declarations and expressions inside loop
statements when they reference variables that may change across iterations.

The rule detects:
- Functions in loops referencing `var` variables declared in/around the loop
- Functions referencing `let` variables declared outside the loop that are modified

Safe patterns (not flagged):
- `let`/`const` in for loop header (fresh binding per iteration)
- `let`/`const` declared inside loop body (fresh binding per iteration)
- Variables never modified
- `const` and import bindings (immutable)
@tt-a1i tt-a1i requested a review from camc314 as a code owner December 14, 2025 14:33
@github-actions github-actions bot added A-linter Area - Linter C-enhancement Category - New feature or request labels Dec 14, 2025
camc314
camc314 reviewed Dec 14, 2025
View reviewed changes
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Dec 14, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #16830 will not alter performance

Comparing tt-a1i:feat/linter-no-loop-func (f1f6362) with main (8bb67a6)

Summary

✅ 4 untouched
⏩ 41 skipped1

Footnotes

  1. 41 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. ↩

@tt-a1i
Copy link
Copy Markdown
Contributor Author

tt-a1i commented Dec 14, 2025

Thanks for the review! I've run cargo lintgen to regenerate the file. Apologies for the oversight.

@tt-a1i
Copy link
Copy Markdown
Contributor Author

tt-a1i commented Dec 14, 2025

I'm currently refining some boundaries, and this PR will be updated again later.

@tt-a1i
Copy link
Copy Markdown
Contributor Author

tt-a1i commented Dec 14, 2025

I've made significant improvements to better align with ESLint's semantics:

Fixes:

  • IIFE handling: Now correctly skips immediately invoked function expressions (they execute immediately and don't capture variables for later)
  • Loop containment: init (for ForStatement) and right (for ForIn/ForOf) are now correctly excluded; test, update, left are considered inside the loop
  • Nested loops: Uses outermost loop's border for write detection
  • let modification check: Only flags writes that occur after the loop border (writes before loop start are safe)
  • Diagnostic message: Now includes the variable name(s) that are unsafe

Test coverage expanded to include:

  • IIFE in various positions (body, test, update)
  • let modified before vs after loop
  • Non-IIFE functions in test/update expressions
  • let declared inside loop body (fresh binding per iteration)

@camc314
Copy link
Copy Markdown
Contributor

camc314 commented Dec 14, 2025

I'm currently refining some boundaries, and this PR will be updated again later.

All good, i pulled it down loclaly and found some issues, I can finish this off.

@camc314 camc314 force-pushed the feat/linter-no-loop-func branch from 7aa663a to 16e4e7f Compare December 14, 2025 15:10
@camc314 camc314 self-assigned this Dec 14, 2025
camc314
camc314 approved these changes Dec 14, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

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

Thank you!

@camc314 camc314 merged commit 55eb0dc into oxc-project:main Dec 14, 2025
21 checks passed
@tt-a1i
Copy link
Copy Markdown
Contributor Author

tt-a1i commented Dec 14, 2025

Thank you so much for reviewing and offering to help finish this! Really appreciate it 🙏

@Boshen Boshen mentioned this pull request Dec 15, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@camc314 camc314 camc314 approved these changes

Assignees

@camc314 camc314

Labels

A-linter Area - Linter C-enhancement Category - New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tt-a1i @camc314