[ty] Avoid double-analyzing tuple in Final subscript#21828
[ty] Avoid double-analyzing tuple in Final subscript#21828charliermarsh merged 3 commits intomainfrom
Final subscript#21828Conversation
| let num_arguments = arguments.len(); | ||
| let type_and_qualifiers = if num_arguments == 1 { | ||
| let mut type_and_qualifiers = self | ||
| .infer_annotation_expression_impl(slice, PEP613Policy::Disallowed); |
There was a problem hiding this comment.
We want to analyze the single element, rather than the slice. (They're the same thing if it's not a single-element tuple.)
Final subscriptFinal subscript
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
AlexWaygood
left a comment
There was a problem hiding this comment.
Great fix!!
I feel like some of the "Did you mean tuple[()]?" suggestions are misfiring a bit here, but this is a real edge case so it probably doesn't matter that much. The main thing is to make sure we don't panic
crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/type_qualifiers/classvar.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/type_qualifiers/initvar.md
Outdated
Show resolved
Hide resolved
|
This PR also fixes astral-sh/ty#1768, so you could add a test case that includes the MRE from that issue |
- Fix `ty#1793` links to be proper markdown hyperlinks - Add extended tests showing type qualifiers still work despite invalid type arguments: - ClassVar: show it's still a ClassVar (invalid-attribute-access on instance assignment) - Final: show it's still Final (invalid-assignment on reassignment) - InitVar: show it's still an InitVar (__init__ signature, unresolved-attribute on access) - Add test case for ty#1768 (ClassVar[int,] trailing comma syntax) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Yeah I had the same reaction. I did very that (at least) that code doesn't crash at runtime though... |
* origin/main: [ty] Add test case for fixed panic (#21832) [ty] Avoid double-analyzing tuple in `Final` subscript (#21828) [flake8-bandit] Fix false positive when using non-standard `CSafeLoader` path (S506). (#21830) Add minimal-size build profile (#21826) [ty] Allow `tuple[Any, ...]` to assign to `tuple[int, *tuple[int, ...]]` (#21803) [ty] Support renaming import aliases (#21792) [ty] Add redeclaration LSP tests (#21812) [ty] more detailed description of "Size limit on unions of literals" in mdtest (#21804) [ty] Complete support for `ParamSpec` (#21445) [ty] Update benchmark dependencies (#21815)
Summary
As-is, a single-element tuple gets destructured via:
But then, because it's a single element, we call
infer_annotation_expression_impl, passing in the tuple, rather than the first element.Closes astral-sh/ty#1793.
Closes astral-sh/ty#1768.