Skip to content

Conversation

@chirizxc
Copy link
Contributor

Summary

Fixes #21162

Test Plan

cargo nextest run pylint

@github-actions
Copy link
Contributor

github-actions bot commented Oct 31, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@ntBre ntBre added bug Something isn't working preview Related to preview mode features rule Implementing or modifying a lint rule labels Nov 6, 2025
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 for working on this, but I think the fix here is too simplistic, and we actually need to be traversing scopes. As one example, this code triggers the runtime error in CPython but is no longer flagged by the rule:

>>> def h():
...     yield 1
...     class C:
...         raise StopIteration
...     yield C
...
... for i in h(): ...
...
Ellipsis
Traceback (most recent call last):
  File "<python-input-0>", line 3, in h
    class C:
        raise StopIteration
  File "<python-input-0>", line 4, in C
    raise StopIteration
StopIteration

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<python-input-0>", line 7, in <module>
    for i in h(): ...
             ~^^
RuntimeError: generator raised StopIteration

I'm wondering if we should just run the rule on StmtFunctionDefs instead of raise statements and then check for both yield and raise StopIteration while visiting the function body, but I haven't thought about it much more than that.

@chirizxc chirizxc requested a review from ntBre November 14, 2025 13:35
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 looks good to me now, I just had a couple more minor simplification suggestions.

@chirizxc chirizxc requested a review from ntBre November 21, 2025 14:19
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!

@ntBre ntBre changed the title [pylint] Fix PLR1708 false positives [pylint] Fix PLR1708 false positives on nested functions Nov 21, 2025
@ntBre ntBre merged commit 09d457a into astral-sh:main Nov 21, 2025
37 checks passed
@chirizxc chirizxc deleted the plr1708 branch November 21, 2025 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working 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.

PLR1708 false positives on nested functions

2 participants