[ty] Refactor SpecialFormType to use nested enums for type qualifiers and aliases#23517
[ty] Refactor SpecialFormType to use nested enums for type qualifiers and aliases#23517AlexWaygood wants to merge 11 commits intomainfrom
Conversation
… use invalid PEP-604 unions for their second argument
…ialFormCategory` enum
… around type qualifiers
Merge the latest main branch into the alex/refactor-special-forms branch and resolve all resulting compile errors. Key changes: - Resolved merge conflicts across 8 files in ty_python_semantic - Updated MiscSpecialForm::in_type_expression to use Type::literal_string() instead of removed Type::LiteralString variant - Updated typing_self call to match new signature (ClassLiteral param, returns Option<BoundTypeVarInstance>) - Changed TypeAlias handling from DynamicType::TodoTypeAlias to proper InvalidTypeExpressionError - Re-added aliased_stdlib_class() method derived from SpecialFormCategory - Added add_qualifier() method and pub(crate) qualifiers field to TypeAndQualifiers for main's annotation_expression.rs changes - Integrated main's TypeGuard support, Callable refactoring, isinstance/issubclass improvements, and ClassVar/Final redundancy detection - Removed PartialSpecialization import (no longer exists) - Added Hash and GetSize derives to TypeQualifier enum https://claude.ai/code/session_01HXMzTUkfPVJCxdxTzZdePf
Refactor match arms on SpecialFormType variants to use exhaustive matches over SpecialFormCategory where possible: - In annotation_expression.rs, replace .as_type_qualifier().expect(...) with inner exhaustive matches on special_form.kind(), eliminating two .expect() calls and merging separate SpecialForm match arms. - Convert is_valid_in_type_expression and is_valid_isinstance_target from matches!/!matches! to exhaustive match expressions, so the compiler enforces updates when new SpecialFormCategory variants are added. - Remove the now-unused as_type_qualifier method. https://claude.ai/code/session_01HXMzTUkfPVJCxdxTzZdePf
Typing conformance resultsNo changes detected ✅ |
|
Memory usage reportMemory usage unchanged ✅ |
9f26118 to
c496a9a
Compare
Instead of having a flat SpecialFormType enum with a separate SpecialFormCategory enum that duplicates part of the structure via a kind() method, embed the subcategories directly into SpecialFormType itself as nested enum variants: - SpecialFormType::LegacyStdlibAlias(LegacyStdlibAlias) - SpecialFormType::TypeQualifier(TypeQualifier) This eliminates SpecialFormCategory, MiscSpecialForm, and the kind() mapping method, along with several From impls that existed only to convert between the parallel structures. Callers now match directly on SpecialFormType variants instead of calling .kind() first. This is an alternative to the SpecialFormCategory approach, as suggested in #23513 (comment) https://claude.ai/code/session_01AgLYCKoBuLEDsmkn1cnmvZ
2faa71a to
a1e48b1
Compare
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-await |
40 | 0 | 0 |
invalid-assignment |
0 | 1 | 0 |
invalid-return-type |
1 | 0 | 0 |
| Total | 41 | 1 | 0 |
- Reorder branches in is_valid_in_type_expression to match main's structure (false arm first, true arm second) - Reorder branches in is_valid_isinstance_target to match main's structure (two arms: true first, false second) - Use SpecialFormType:: instead of Self:: in name() and definition_modules() match patterns to match main's style https://claude.ai/code/session_01AgLYCKoBuLEDsmkn1cnmvZ
a1e48b1 to
252f872
Compare
| /// Parse a `SpecialFormType` from its runtime symbol name. | ||
| fn from_name(name: &str) -> Option<Self> { |
There was a problem hiding this comment.
I initially proposed #23513 (which added a SpecialFormType::kind() enum instead of refactoring SpecialFormType itself to contain nested enums), but @sharkdp suggested the alternative pattern that this PR uses. This PR is a much smaller diff than #23513 was overall -- the only significant disadvantage that this PR has over that one is that this function becomes a fair bit more verbose and more complicated. An alternative way of writing this function would be to have a manual match over string literals mapping each string to a SpecialFormType enum, but I'm really loath to do that -- we know from experience that it's really easy to forget to update matches like that when adding new variants to an enum.
| reveal_type(invalid_dict1) # revealed: dict[Unknown, str] | ||
| reveal_type(invalid_dict2) # revealed: dict[str, Unknown] | ||
| reveal_type(dict_too_few_args) # revealed: dict[str, Unknown] | ||
| reveal_type(dict_too_few_args) # revealed: dict[Unknown, Unknown] |
There was a problem hiding this comment.
Copying my comment from #23513 (comment):
I think this is the only change in behaviour from this PR, which comes as a result of unifying the parsing of stdlib aliases between infer/builder.rs and infer/builder/type_expression.rs so that they now share a common implementation. Either the answer we give on main or the new answer seem fine to me, since this is an invalid type expression, but I can look into it if anybody strongly prefers the answer we have on main
| // TODO -- seems incorrect? | ||
| SpecialFormType::Unpack => false, |
There was a problem hiding this comment.
As the comment says, this seems incorrect to me -- Unpack can definitely appear validly in type expressions -- but for now I'm just preserving the behaviour that we have on main, since this is meant to be a pure refactor...
Summary
Our
SpecialFormTypeenum is an enumeration of symbols (mostly in thetypingmodule) that are special enough that we consider each symbol to inhabit its own singleton type. That makes them "similar enough" that they all deserve to inhabit the sameType::SpecialFormtop-level variant, but it elides the fact that there are several distinct subcategories within this enumeration that need to be recognised at multiple distinct points throughout the codebase. The flat nature of theSpecialFormTypeenum often makes it hard to distinguish between these subcategories in a type-safe way; we often have to resort to.expect()orunreachable!()calls, e.g.ruff/crates/ty_python_semantic/src/types/infer/builder.rs
Lines 16143 to 16145 in fc1081a
ruff/crates/ty_python_semantic/src/types/infer/builder.rs
Lines 16200 to 16202 in 66defe9
ruff/crates/ty_python_semantic/src/types/infer/builder/annotation_expression.rs
Line 334 in fc1081a
It also leads to pretty repetitive code: we have to differentiate stdlib aliases from other kinds of special forms in
narrow.rs,infer/builder.rsandinfer/builder/type_expression.rs, in very similar ways.This PR refactors our type-qualifer special forms so that they all share a single variant in the
SpecialFormTypeenum, and does the same for our stdlib-alias special forms. This results in a small net increase of total lines of code, but it's more type-safe code (all three.expect()/unreachable!()calls highlighted above are gone), and much less repetitive code.Test Plan
Existing tests