[ty] Use Top materializations for TypeIs special form#20591
[ty] Use Top materializations for TypeIs special form#20591AlexWaygood merged 3 commits intomainfrom
Top materializations for TypeIs special form#20591Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
3cb9437 to
2afd6a9
Compare
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
7 | 9 | 7 |
unresolved-attribute |
1 | 0 | 12 |
possibly-missing-attribute |
0 | 2 | 9 |
invalid-assignment |
2 | 0 | 7 |
invalid-return-type |
2 | 2 | 5 |
invalid-type-form |
0 | 0 | 3 |
not-iterable |
0 | 0 | 2 |
too-many-positional-arguments |
2 | 0 | 0 |
unsupported-operator |
0 | 0 | 2 |
no-matching-overload |
0 | 1 | 0 |
unused-ignore-comment |
1 | 0 | 0 |
| Total | 15 | 14 | 47 |
Ecosystem analysisOverall this PR adds about as many diagnostics as it removes, but I think it makes our narrowing behaviour for There are several hits that look like true positives: we're now inferring less dynamic types which is allowing us to highlight places where the code arguably is unsound ( The new diagnostic on We have some new false positives in ruff/crates/ty_python_semantic/src/types/signatures.rs Lines 1286 to 1291 in 57e1ff8 We have a new false positive on |
2afd6a9 to
ad032c5
Compare
No, it should be |
Right, yes, sorry |
Top materializations for TypeIs special formTop materializations for TypeIs special form
There was a problem hiding this comment.
I don't love implicitly transforming a type explicitly written by the user :/ but I do feel like this practically improves the results, given the way TypeIs is currently used in typeshed. Narrowing with a gradual type is just not really very useful, if it's handled precisely (as we do).
I feel like the user intent expressed by -> TypeIs[Foo[Any]] is "the type is some kind of Foo, I don't know which kind" (this is accurately modeled by taking the top materialization), but also then "treat the contained type of Foo as dynamic if I end up using it" -- and this latter part is what we don't do.
I wonder what it would look like if, instead of doing this, we simplified Foo[Any] & Foo[Bar] to Foo[Bar & Any]? I think that's a correct simplification? And I think that would give the same benefits as this PR, but with fewer false positives -- it would better respect the user's intent in writing TypeIs[Foo[Any]]. WDYT?
Edit: I guess then we'd have false negatives compared to other type checkers, who are happy to simplify Foo[Any] & Foo[Bar] to just Foo[Bar]. Which is not correct, but is maybe practically what users expect.
carljm
left a comment
There was a problem hiding this comment.
Going ahead and approving this -- I don't love it, but it seems like a net improvement for now, and it's easy to reverse if we come up with a better idea.
That's only a correct simplification if |
* main: (21 commits) [ty] Literal promotion refactor (#20646) [ty] Add tests for nested generic functions (#20631) [`cli`] Add conflict between `--add-noqa` and `--diff` options (#20642) [ty] Ensure first-party search paths always appear in a sensible order (#20629) [ty] Use `typing.Self` for the first parameter of instance methods (#20517) [ty] Remove unnecessary `parsed_module()` calls (#20630) Remove `TextEmitter` (#20595) [ty] Use fully qualified names to distinguish ambiguous protocols in diagnostics (#20627) [ty] Ecosystem analyzer: relax timeout thresholds (#20626) [ty] Apply type mappings to functions eagerly (#20596) [ty] Improve disambiguation of class names in diagnostics (#20603) Add the *The Basics* title back to CONTRIBUTING.md (#20624) [`playground`] Fix quick fixes for empty ranges in playground (#20599) Update dependency ruff to v0.13.2 (#20622) [`ruff`] Fix minor typos in doc comments (#20623) Update dependency PyYAML to v6.0.3 (#20621) Update cargo-bins/cargo-binstall action to v1.15.6 (#20620) Fixed documentation for try_consider_else (#20587) [ty] Use `Top` materializations for `TypeIs` special form (#20591) [ty] Simplify `Any | (Any & T)` to `Any` (#20593) ...
Fixes astral-sh#1703 Fixes incomplete type narrowing when using TypeIs with generic union types like: Result[T, E] = Ok[T] | Err[E]. Previously, type parameter E was materialized to 'object' before specialization could substitute it with concrete types like Exception, causing narrowing to produce Err[Unknown] instead of Err[Exception]. Changes: - Added is_materialized flag to TypeIsType to defer materialization - Implemented class-based union element matching for multi-typevar unions in generics.rs - Modified TypeIs creation to delay materialization until after specialization - Added test case for generic TypeIs narrowing to type_guards.md - Removed the comments which had a discussion about: astral-sh#20591
Summary
Typeshed has many functions that return
TypeIs[SomeCovariantGeneric[Any]]. This is somewhat unfortunate for ty, because for our purposes it's almost always better to intersect withCovariantGeneric[object]for the purposes of type narrowing, but it's somewhat hard to change these in typeshed without creating false-positive diagnostics for users of other type checkers. This PR therefore introduces a workaround for the typeshed issue: under the hood, we convert the type argument passed toTypeIsto its top materialization before intersecting with that type.Test Plan
I added an mdtest that fails on
mainwithout this change. It's similar to the false-positive diagnostics relating to the use ofasyncio.iscoroutinethat we see going away onhomeassistant/corein the mypy_primer report.Some more ecosystem analysis is found in #20591 (comment).