Skip to content

Conversation

@fatelei
Copy link
Contributor

@fatelei fatelei commented Oct 7, 2025

Summary

implement pylint rule stop-iteration-return / R1708

Test Plan

Copy link
Contributor

@ntBre ntBre 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! This looks good to me overall, I just had a few fairly small suggestions besides needing to add the test snapshots. Let me know if you need help with that. It might also be good for @amyreese to take a quick look.

I think this is a good rule to add, especially since this pattern will always (?) cause a runtime error on the versions of Python we support.

@ntBre ntBre added rule Implementing or modifying a lint rule preview Related to preview mode features labels Oct 15, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Thanks, this is looking good! I noticed that we have a related rule in return-in-generator (B901), and instead of triggering on return statements, it actually runs on function definition statements. Then its visitor looks for both yields and return statements. I think it might make sense to mirror that structure here so that we don't have to traverse multiple scopes looking for an enclosing function. B901 can even avoid visiting nested functions since those will be traversed later by the Checker:

Stmt::FunctionDef(_) => {
// Do not recurse into nested functions; they're evaluated separately.
}

@fatelei fatelei force-pushed the stop_iteration_return branch from 501b447 to 451807b Compare October 23, 2025 01:50
Copy link
Contributor

@ntBre ntBre 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! This looks good to me now if @amyreese is happy with it.

I pushed two commits fixing the merge conflicts from one of my PRs earlier and fixing a small nit about the function order.

@ntBre ntBre changed the title feat: implement pylint rule stop-iteration-return / R1708 [pylint] Implement stop-iteration-return (PLR1708) Oct 23, 2025
@amyreese amyreese merged commit 28aed61 into astral-sh:main Oct 23, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview Related to preview mode features rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants