[ty] lint on the global keyword if there's no explicit definition in the…
#19344
Conversation
This file contains hidden or 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
reviewed
Jul 15, 2025
There was a problem hiding this comment.
Nice! This looks pretty good to me. I haven't gone through the mypy_primer hits, but it doesn't look prohibitively large to me.
The main thing I think we need to do before landing this is to improve the diagnostic to make it clearer what the problem is, and why we object to this coding pattern
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
unresolved-reference |
39 | 0 | 0 |
| Total | 39 | 0 | 0 |
AlexWaygood
reviewed
Jul 15, 2025
AlexWaygood
reviewed
Jul 15, 2025
AlexWaygood
reviewed
Jul 15, 2025
AlexWaygood
approved these changes
Jul 15, 2025
|
Most of the new lints in the ecosystem are as expected, class CounterTests(unittest.TestCase):
def setUp(self):
global Counter
from Crypto.Util import CounterSo that's doing a global import, but not in the global scope. If the user is sure they want to do this sort of thing, is there an alternative way for them to do it that will type-check? (Maybe something better than |
oconnor663
force-pushed
the
jack/global_must_refer_to_definition
branch
from
7ae687d to
d6d4914
Compare
|
Ok, folks who want to do |
oconnor663
added a commit
that referenced
this pull request
Jul 16, 2025
…al` lints This is a follow-up to #19344 that improves the error formatting slightly. For example with this program: ```py def f(): global foo, bar ``` Before we printed: ``` 1 | def f(): 2 | global foo, bar | ^^^^^^^^^^^^^^^ `foo` has no declarations or bindings in the global scope ... 1 | def f(): 2 | global foo, bar | ^^^^^^^^^^^^^^^ `bar` has no declarations or bindings in the global scope ``` Now we print: ``` 1 | def f(): 2 | global foo, bar | ^^^ `foo` has no declarations or bindings in the global scope ... 1 | def f(): 2 | global foo, bar | ^^^ `bar` has no declarations or bindings in the global scope ```
oconnor663
added a commit
that referenced
this pull request
Jul 16, 2025
…`global` lints This is a follow-up to #19344 that improves the error formatting slightly. For example with this program: ```py def f(): global foo, bar ``` Before we printed: ``` 1 | def f(): 2 | global foo, bar | ^^^^^^^^^^^^^^^ `foo` has no declarations or bindings in the global scope ... 1 | def f(): 2 | global foo, bar | ^^^^^^^^^^^^^^^ `bar` has no declarations or bindings in the global scope ``` Now we print: ``` 1 | def f(): 2 | global foo, bar | ^^^ `foo` has no declarations or bindings in the global scope ... 1 | def f(): 2 | global foo, bar | ^^^ `bar` has no declarations or bindings in the global scope ```
oconnor663
added a commit
that referenced
this pull request
Jul 16, 2025
…`global` lints (#19379) This is a follow-up to #19344 that improves the error formatting slightly. For example with this program: ```py def f(): global foo, bar ``` Before we printed: ``` 1 | def f(): 2 | global foo, bar | ^^^^^^^^^^^^^^^ `foo` has no declarations or bindings in the global scope ... 1 | def f(): 2 | global foo, bar | ^^^^^^^^^^^^^^^ `bar` has no declarations or bindings in the global scope ``` Now we print: ``` 1 | def f(): 2 | global foo, bar | ^^^ `foo` has no declarations or bindings in the global scope ... 1 | def f(): 2 | global foo, bar | ^^^ `bar` has no declarations or bindings in the global scope ``` --------- Co-authored-by: Micha Reiser <[email protected]>
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.
We currently lint on examples like this (even though it's allowed at runtime, as long as you call
fbeforeg):However, linting only on the
printis kind of confusing. This PR adds a lint on theglobalstatement itself (edit: updated after review feedback):Because this is legal Python, I expect this is going to have a lot of new hits in the
ecosystem-analyzer(and I had to fix some of our mdtests). I don't know what would count as an "excessive" number of new failures, though, and I'll need advice from reviewers :)