Skip to content

Conversation

mdjermanovic
Copy link
Member

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[x] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

Refs #19792, adds support for explicit resource management to the no-loop-func rule.

What changes did you make? (Give an overview)

Updates the no-loop-func rule to not report on references to using and await using variables, because they are constant.

Is there anything you'd like reviewers to focus on?

For using/await using variables declared inside the loop body, it might make sense to warn when they are referenced from a function because they might be used after the loop, which would be after the disposal. However, I don't think this potential error is specific to loops, but to any blocks. Thus, I think the mentioned is out of scope for this rule, which targets cases where the referenced variables are reassigned.

@mdjermanovic mdjermanovic requested a review from a team as a code owner June 29, 2025 19:55
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Jun 29, 2025
@eslint-github-bot eslint-github-bot bot added the feature This change adds a new feature to ESLint label Jun 29, 2025
@github-actions github-actions bot added the rule Relates to ESLint's core rules label Jun 29, 2025
Copy link

netlify bot commented Jun 29, 2025

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit 3941270
🔍 Latest deploy log https://app.netlify.com/projects/docs-eslint/deploys/68623f50a61fcd00088a98aa
😎 Deploy Preview https://deploy-preview-19895--docs-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@aladdin-add aladdin-add moved this from Needs Triage to Second Review Needed in Triage Jun 29, 2025
@aladdin-add aladdin-add added the accepted There is consensus among the team that this change meets the criteria for inclusion label Jun 29, 2025
@aladdin-add aladdin-add moved this from Second Review Needed to Implementing in Triage Jun 29, 2025
Comment on lines 134 to 144
for (var i=10; i; i--) {
using foo = getsomething(i);
var a = function() { return foo; }; // OK, all references are referring to block scoped variables in the loop.
a();
}

for (var i=10; i; i--) {
await using foo = getsomething(i);
var a = function() { return foo; }; // OK, all references are referring to block scoped variables in the loop.
a();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we will need to pass ECMA version 2026 in the code block to correctly open this in the playground.

Copy link
Member Author

Choose a reason for hiding this comment

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

Doesn't it already work the same with the default latest?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that would work too. Anyway I also added 2026 as an option in eslint/eslint.org#737

snitin315
snitin315 previously approved these changes Jun 30, 2025
Copy link
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Leaving it open for the 2nd review.

Copy link
Member

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

LGTM.

@aladdin-add aladdin-add moved this from Implementing to Merge Candidates in Triage Jun 30, 2025
@fasttime fasttime merged commit a6a6325 into main Jul 4, 2025
31 checks passed
@fasttime fasttime deleted the no-loop-func branch July 4, 2025 04:40
@github-project-automation github-project-automation bot moved this from Merge Candidates to Complete in Triage Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion feature This change adds a new feature to ESLint rule Relates to ESLint's core rules
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

4 participants