[ty] Tighten up validation of subscripts and attributes in type expressions by AlexWaygood · Pull Request #24329 · astral-sh/ruff (original) (raw)
Summary
According to the type-expression grammar in the typing spec, only simple names and dotted names can be subscripted in type expressions. More complex expressions, such as list[T][int], constitute invalid type expressions. This PR adds the missing validation for this rule.
Test Plan
mdtests
Typing conformance results
The percentage of diagnostics emitted that were expected errors held steady at 86.76%. The percentage of expected errors that received a diagnostic held steady at 81.53%. The number of fully passing files held steady at 70/132.
Summary
How are test cases classified?
Each test case represents one expected error annotation or a group of annotations sharing a tag. Counts are per test case, not per diagnostic — multiple diagnostics on the same line count as one. Required annotations (E) are true positives when ty flags the expected location and false negatives when it does not. Optional annotations (E?) are true positives when flagged but true negatives (not false negatives) when not. Tagged annotations (E[tag]) require ty to flag exactly one of the tagged lines; tagged multi-annotations (E[tag+]) allow any number up to the tag count. Flagging unexpected locations counts as a false positive.
| Metric | Old | New | Diff | Outcome |
|---|---|---|---|---|
| True Positives | 865 | 865 | +0 | |
| False Positives | 132 | 132 | +0 | |
| False Negatives | 196 | 196 | +0 | |
| Total Diagnostics | 1050 | 1050 | +0 | |
| Precision | 86.76% | 86.76% | +0.00% | |
| Recall | 81.53% | 81.53% | +0.00% | |
| Passing Files | 70/132 | 70/132 | +0 |
True positives changed (5)
5 diagnostics
| Test case | Diff |
|---|---|
| aliases_type_statement.py:43 | -error[invalid-type-form] Invalid subscript of object of type `list[<class 'int'>]` in type expression +error[invalid-type-form] Only simple names and dotted names can be subscripted in type expressions |
| aliases_typealiastype.py:58 | -error[invalid-type-form] Invalid subscript of object of type `list[<class 'int'>]` in type expression +error[invalid-type-form] Only simple names and dotted names can be subscripted in type expressions |
| annotations_forward_refs.py:47 | -error[invalid-type-form] Invalid subscript of object of type `list[<class 'int'>]` in type expression +error[invalid-type-form] Only simple names and dotted names can be subscripted in type expressions |
| annotations_typeexpr.py:94 | -error[invalid-type-form] Invalid subscript of object of type `list[<class 'int'>]` in type expression +error[invalid-type-form] Only simple names and dotted names can be subscripted in type expressions |
| qualifiers_annotated.py:43 | -error[invalid-type-form] Invalid subscript of object of type `list[<class 'int'>]` in type expression +error[invalid-type-form] Only simple names and dotted names can be subscripted in type expressions |
Memory usage report
Memory usage unchanged ✅
ecosystem-analyzer results
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
| invalid-type-form | 0 | 0 | 5 |
| Total | 0 | 0 | 5 |
Raw diff:
egglog-python (https://github.com/egraphs-good/egglog-python)
- python/egglog/builtins.py:542:79 error[invalid-type-form] Invalid subscript of object of type
tuple[type, ...]in type expression
- python/egglog/builtins.py:542:79 error[invalid-type-form] Only simple names and dotted names can be subscripted in type expressions
- python/egglog/builtins.py:666:89 error[invalid-type-form] Invalid subscript of object of type
tuple[type, ...]in type expression
- python/egglog/builtins.py:666:89 error[invalid-type-form] Only simple names and dotted names can be subscripted in type expressions
- python/egglog/builtins.py:1060:83 error[invalid-type-form] Invalid subscript of object of type
tuple[type, ...]in type expression
- python/egglog/builtins.py:1060:83 error[invalid-type-form] Only simple names and dotted names can be subscripted in type expressions
- python/tests/test_convert.py:117:35 error[invalid-type-form] Invalid subscript of object of type
tuple[type, ...]in type expression
- python/tests/test_convert.py:117:35 error[invalid-type-form] Only simple names and dotted names can be subscripted in type expressions
- python/tests/test_convert.py:140:33 error[invalid-type-form] Invalid subscript of object of type
tuple[type, ...]in type expression
- python/tests/test_convert.py:140:33 error[invalid-type-form] Only simple names and dotted names can be subscripted in type expressions
Full report with detailed diff (timing results)
ruff-ecosystem results
Linter (stable)
✅ ecosystem check detected no linter changes.
Linter (preview)
✅ ecosystem check detected no linter changes.
Formatter (stable)
✅ ecosystem check detected no format changes.
Formatter (preview)
✅ ecosystem check detected no format changes.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this is what is specified (though it took me a few minutes to notice the extra clarification underneath the grammar table about name being allowed to be a qualified dotted name). It's perhaps worth noting that currently only mypy enforces this rule, so this PR puts us into the strictest tier with mypy (and zuban). Pyright and pyrefly and pycroscope are all fine with this, for example:
class C[T]: class Inner: pass
x: C[int].Inner = C.Inner()
Ecosystem results suggest it's not a problem in practice to be strict about this, though if someone proposed that the above should be allowed by the spec, I would not argue against allowing it. Which I guess suggests that my initial inclination is that it ought to be allowed.
| # error: [invalid-type-form] "Invalid subscript" |
|---|
| # error: [invalid-type-form] "Only simple names and dotted names can be subscripted in type expressions" |
| o: "[1, 2, 3][1:2]", |
| # error: [invalid-type-form] "Only simple names, dotted names and subscripts can be used in type expressions" |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wording of this message is a bit confusing, because the expression below could reasonably be said to contain only "simple names, dotted names, and subscripts". The subtlety is that "dotted names" can't include a subscript. Not sure how to make that subtlety clearer, though...
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well... the expression below might contain only "simple names, dotted names and subscripts", but it is nonetheless not a simple name, dotted name or subscript itself! (It's a non-dotted-name attribute expression where the value is a subscript.)
But, uh, yeah, possibly not my finest error message... I also don't really have any better ideas, though. We do link to the typing spec grammar in a subdiagnostic, so that's something at least
Pyright and pyrefly and pycroscope are all fine with this, for example:
class C[T]: class Inner: pass
x: C[int].Inner = C.Inner()
Sure, but is that actually useful? It "means" exactly the same thing as C.Inner -- the specialization of C here has no bearing on the meaning of Inner in a type expression. And it's impossible to construct an example where the specialization of C would have a bearing on that, because the typing spec mandates that type checkers must forbid type variables used in outer scopes from being shadowed in inner scopes.
The rules in the typing spec make sense to me here; I'm in favour of strictly enforcing them.
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
[ Show hidden characters]({{ revealButtonHref }})