[ty] improve typevar solving from constraint sets#22411
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
3 | 19 | 48 |
invalid-return-type |
2 | 16 | 14 |
unresolved-attribute |
0 | 23 | 2 |
invalid-assignment |
1 | 16 | 5 |
type-assertion-failure |
0 | 22 | 0 |
no-matching-overload |
18 | 0 | 0 |
possibly-missing-attribute |
3 | 3 | 1 |
invalid-await |
2 | 0 | 0 |
not-subscriptable |
1 | 0 | 0 |
redundant-cast |
1 | 0 | 0 |
unused-ignore-comment |
0 | 1 | 0 |
| Total | 31 | 100 | 70 |
|
Mostly removed diagnostics here, which is a positive sign. Still analyzing some of the added diagnostics, but I think this looks like a net positive regardless, so I'll put it up for review. |
|
The cki-lib diagnostics are weird. They look like true positives in some sense, because if Still analyzing the other new diagnostics... |
|
The redundant-cast in anyio is good news -- we now get the type right, so the cast is now redundant. |
|
The new diagnostic in jinja seems to be the same as the ones in cki-lib, related to |
AlexWaygood
left a comment
There was a problem hiding this comment.
This makes sense to me, thanks! I haven't gone through the ecosystem results in depth but I trust that you'll only merge if they look good to you 😄
|
This looks good! This issue came up for me also in #21902. I think it's worth landing this separately, especially since you have a nice regression test for it. I dug into the cki-libs/jinja failures, and they look unrelated. We are incorrectly relating That says that transitivity applies for tl;dr 👍 to merge this |
|
Summary
Fixes astral-sh/ty#2292
When solving a bounded typevar, we preferred the upper bound over the actual type seen in the call. This change fixes that.
Test Plan
Added mdtest, existing tests pass.