[red-knot] Disallow more invalid type expressions #16427
Merged
+131
−40
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
requested review from
carljm,
MichaReiser and
sharkdp
as code owners
AlexWaygood
force-pushed
the
alex/reject-bad-type-exprs
branch
from
67d9d20 to
68c354c
Compare
carljm
approved these changes
Feb 28, 2025
crates/red_knot_python_semantic/resources/mdtest/annotations/unsupported_special_forms.md
Outdated
Show resolved
Hide resolved
AlexWaygood
force-pushed
the
alex/reject-bad-type-exprs
branch
2 times, most recently
from
1f5f98b to
2809aa4
Compare
AlexWaygood
force-pushed
the
alex/reject-bad-type-exprs
branch
from
2809aa4 to
e1bb7b1
Compare
dcreager
added a commit
that referenced
this pull request
Feb 28, 2025
* main: [red-knot] Switch to a handwritten parser for mdtest error assertions (#16422) [red-knot] Disallow more invalid type expressions (#16427) Bump version to Ruff 0.9.9 (#16434) Check `LinterSettings::preview` for version-related syntax errors (#16429) Avoid caching files with unsupported syntax errors (#16425) Prioritize "bug" label for changelog sections (#16433) [`flake8-copyright`] Add links to applicable options (`CPY001`) (#16421) Fix string-length limit in documentation for PYI054 (#16432) Show version-related syntax errors in the playground (#16419) Allow passing `ParseOptions` to inline tests (#16357) Bump version to 0.9.8 (#16414) [red-knot] Ignore surrounding whitespace when looking for `<!-- snapshot-diagnostics -->` directives in mdtests (#16380) Notify users for invalid client settings (#16361) Avoid indexing the project if `configurationPreference` is `editorOnly` (#16381)
dcreager
added a commit
that referenced
this pull request
Feb 28, 2025
* dcreager/alist-type: Remove unneeded shear override Update property test CI Move alist into red_knot_python_semantic [red-knot] Switch to a handwritten parser for mdtest error assertions (#16422) [red-knot] Disallow more invalid type expressions (#16427) Bump version to Ruff 0.9.9 (#16434) Check `LinterSettings::preview` for version-related syntax errors (#16429) Avoid caching files with unsupported syntax errors (#16425) Prioritize "bug" label for changelog sections (#16433) [`flake8-copyright`] Add links to applicable options (`CPY001`) (#16421) Fix string-length limit in documentation for PYI054 (#16432) Show version-related syntax errors in the playground (#16419) Allow passing `ParseOptions` to inline tests (#16357) Bump version to 0.9.8 (#16414) [red-knot] Ignore surrounding whitespace when looking for `<!-- snapshot-diagnostics -->` directives in mdtests (#16380) Notify users for invalid client settings (#16361) Avoid indexing the project if `configurationPreference` is `editorOnly` (#16381)
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
As part of working on #16302, I'm auditing all uses of
Type::to_instance(). I noticed that one use inType::in_type_expressionhas a pretty clear bug, in that it treatsType::SubclassOf()the same asType::ClassLiteral. That's incorrect: an object can only have atype[]type if it's a dynamic variable of some kind, and we should reject such variables in type expressions. The practical effect of this is that onmain, we have this incorrect behaviour, where we should be rejecty's type annotation altogether, but instead we accept it:This PR fixes the bug by making
Type::in_type_expression()exhaustive over allTypevariants and rejecting most of them when they appear in type expressions. (The method is not yet exhaustive over allKnownInstanceTypevariants; some of these would require special error messages, and it seemed out of the scope of this bugfix PR.)Test Plan
Added mdtests.