-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Truncate type display for long unions in some situations #20730
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ty] Truncate type display for long unions in some situations #20730
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
|
Lint rule | Added | Removed | Changed |
---|---|---|---|
invalid-argument-type |
112 | 0 | 181 |
possibly-missing-attribute |
0 | 0 | 164 |
invalid-assignment |
0 | 0 | 22 |
unsupported-operator |
0 | 0 | 20 |
possibly-missing-implicit-call |
0 | 0 | 6 |
invalid-return-type |
0 | 0 | 4 |
invalid-type-form |
0 | 0 | 3 |
invalid-parameter-default |
0 | 0 | 1 |
no-matching-overload |
0 | 1 | 0 |
Total | 112 | 1 | 401 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks very much! I think there's some subtleties here that we need to think through a bit
crates/ty_python_semantic/resources/mdtest/exception/control_flow.md
Outdated
Show resolved
Hide resolved
...alling_a_union_of_f…_-_Try_to_cover_all_pos…_-_Cover_non-keyword_re…_(707b284610419a54).snap
Outdated
Show resolved
Hide resolved
028a676
to
e69d110
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks excellent, thank you!! Will wait until tomorrow before merging in case anybody else has thoughts
And this had exactly the effect on #20368 that I was hoping for. Some of the new diagnostics on that PR branch are so much better now -- thank you! ![]() |
Summary
Fixes astral-sh/ty#1307
Unions with length <= 5 are unaffected to minimize test churn
Unions with length > 5 will only display the first 3 elements + "... omitted x union elements"
Here "length" is defined as the number of elements after condensation to literals
Edit: we no longer truncate in revel case.
Before:
After:
The below comparisons are outdated, but left here as a reference.
Before:
reveal_type(x) # revealed: Literal[1, 2] | A | B | C | D | E | F | G
reveal_type(x) # revealed: Result1A | Result1B | Result2A | Result2B | Result3 | Result4
After:
reveal_type(x) # revealed: Literal[1, 2] | A | B | ... omitted 5 union elements
reveal_type(x) # revealed: Result1A | Result1B | Result2A | ... omitted 3 union elements
This formatting is consistent with
crates/ty_python_semantic/src/types/call/bind.rs
line 2992Test Plan
Cosmetic only, covered and verified by changes in mdtest