[red-knot] Add mdtests for global statement#17563
Merged
Conversation
Contributor
|
61f4ccc to
88af630
Compare
carljm
reviewed
Apr 23, 2025
Contributor
carljm
left a comment
There was a problem hiding this comment.
Awesome, thank you! A few comments, mostly just about adapting to our "house style" for TODO comments in mdtests.
crates/red_knot_python_semantic/resources/mdtest/scopes/global.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/scopes/global.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/scopes/global.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/scopes/global.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/scopes/global.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/scopes/global.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/scopes/global.md
Outdated
Show resolved
Hide resolved
Contributor
Author
|
Thanks for the review, it was very helpful! I think this should be in a much better state now. |
dcreager
added a commit
that referenced
this pull request
Apr 24, 2025
* main: [red-knot] fix collapsing literal and its negation to object (#17605) [red-knot] Add more tests for protocols (#17603) [red-knot] Ban direct instantiations of `Protocol` classes (#17597) [`pyupgrade`] Preserve parenthesis when fixing native literals containing newlines (`UP018`) (#17220) [`airflow`] fix typos (`AIR302`, `AIR312`) (#17574) [red-knot] Special case `@abstractmethod` for function type (#17591) [red-knot] Emit diagnostics for isinstance() and issubclass() calls where a non-runtime-checkable protocol is the second argument (#17561) [red-knot] Infer the members of a protocol class (#17556) [red-knot] Add `FunctionType::to_overloaded` (#17585) [red-knot] Add mdtests for `global` statement (#17563) [syntax-errors] Make duplicate parameter names a semantic error (#17131)
ntBre
added a commit
that referenced
this pull request
May 8, 2025
… syntax error (#17637) Summary -- This PR resolves both the typing-related and syntax error TODOs added in #17563 by tracking a set of `global` bindings for each scope. As discussed below, we avoid the additional AST traversal from ruff by collecting `Name`s from `global` statements while building the semantic index and emit a syntax error if the `Name` is already bound in the current scope at the point of the `global` statement. This has the downside of separating the error from the `SemanticSyntaxChecker`, but I plan to explore using this approach in the `SemanticSyntaxChecker` itself as a follow-up. It seems like this may be a better approach for ruff as well. Test Plan -- Updated all of the related mdtests to remove the TODOs (and add quotes I forgot on the messages). There is one remaining TODO, but it requires `nonlocal` support, which isn't even incorporated into the `SemanticSyntaxChecker` yet. --------- Co-authored-by: Alex Waygood <[email protected]> Co-authored-by: Carl Meyer <[email protected]>
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
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
This is a first step toward
globalsupport in red-knot (#15385). I went through all the matches forglobalin themypy/test-datadirectory, but I didn't find anything too interesting that wasn't already covered by @carljm's suggestions on Discord. I still pulled in a couple of cases for a little extra variety. I also included a section from the PLE0118 tests in ruff that will become syntax errors once #17463 is merged and we handleglobalstatements.I don't think I figured out how to use
@Todoproperly, so please let me know if I need to fix that. I hope this is a good start to the test suite otherwise.