[pyflakes] Allow forward references in class bases in stub files (F821)
#10779
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlexWaygood
added
rule
|
Well, locally it seems to get rid of 74 F821 hits on typeshed :( Does the ecosystem check respect projects' |
|
To be more specific about why deferring all expressions in stub files seems trickier: here's a patch that does that: https://github.com/astral-sh/ruff/compare/main...AlexWaygood:ruff:f821-forward-refs-pyi?expand=1. And here's the impact it has on typeshed: Looking more closely at those new errors this morning, however, I think most fall into two buckets:
So, perhaps if those two issues were resolved, then the patch that defers resolution of all expressions might be a more principled basis for a fix for this issue? |
|
It is also worth noting that, while this PR doesn't get us to the generalised support for forward references that type checkers have for stubs, the approach here is actually very similar to the monkey-patching flake8-pyi does. Flake8-pyi monkey-patches flake8 to do three things:
|
charliermarsh
approved these changes
Apr 6, 2024
|
gah, why did I go make merge conflicts for myself? |
|
Although this doesn't directly match the behaviour of type checkers with regards to stub files, I think I will merge this for now:
|
Glyphack
pushed a commit
to Glyphack/ruff
that referenced
this pull request
Apr 12, 2024
…F821`) (astral-sh#10779) ## Summary Fixes astral-sh#3011. Type checkers currently allow forward references in all contexts in stub files, and stubs frequently make use of this capability (although it doesn't actually seem to be specc'd anywhere --neither in PEP 484, nor https://typing.readthedocs.io/en/latest/source/stubs.html#id6, nor the CPython typing docs). Implementing it so that Ruff allows forward references in _all contexts_ in stub files seems non-trivial, however (or at least, I couldn't figure out how to do it easily), so this PR does not do that. Perhaps it _should_; if we think this apporach isn't principled enough, I'm happy to close it and postpone changing anything here. However, this does reduce the number of F821 errors Ruff emits on typeshed down from 76 to 2, which would mean that we could enable the rule at typeshed. The remaining 2 F821 errors can be trivially fixed at typeshed by moving definitions around; forward references in class bases were really the only remaining places where there was a real _use case_ for forward references in stub files that Ruff wasn't yet allowing. ## Test plan `cargo test`. I also ran this PR branch on typeshed to check to see if there were any new false positives caused by the changes here; there were none.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3011.
Type checkers currently allow forward references in all contexts in stub files, and stubs frequently make use of this capability (although it doesn't actually seem to be specc'd anywhere --neither in PEP 484, nor https://typing.readthedocs.io/en/latest/source/stubs.html#id6, nor the CPython typing docs). Implementing it so that Ruff allows forward references in all contexts in stub files seems non-trivial, however (or at least, I couldn't figure out how to do it easily), so this PR does not do that. Perhaps it should; if we think this apporach isn't principled enough, I'm happy to close it and postpone changing anything here.
However, this does reduce the number of F821 errors Ruff emits on typeshed down from 76 to 2, which would mean that we could enable the rule at typeshed. The remaining 2 F821 errors can be trivially fixed at typeshed by moving definitions around; forward references in class bases were really the only remaining places where there was a real use case for forward references in stub files that Ruff wasn't yet allowing.
Test plan
cargo test. I also ran this PR branch on typeshed to check to see if there were any new false positives caused by the changes here; there were none.