[ty] Remove special casing for string-literal-in-tuple __contains__#19642
[ty] Remove special casing for string-literal-in-tuple __contains__#19642AlexWaygood merged 1 commit intomainfrom
__contains__#19642Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
This comment was marked as resolved.
This comment was marked as resolved.
|
We discussed this in person in our 1:1 today -- for completeness, here's an (admittedly slightly contrived) example where the current special casing would lead us to infer from typing import Literal
class Bar(tuple[Literal["foo"]]):
def __contains__(self, item) -> Literal[False]:
return False
def is_foo_in_tuple(x: tuple[Literal["foo"]]) -> Literal[True]:
return "foo" in x
reveal_type(is_foo_in_tuple(Bar(("foo",))))We would continue to let this example pass type checking without diagnostics even after astral-sh/ty#166 is implemented, because our current special casing does not synthesize a precise Even putting aside soundness issues, it feels very strange to me that you'd get a more precise result here by upcasting a tuple subclass to its supertype (and a less precise result after narrowing the type) -- you would usually expect the opposite: from typing import Literal
class Bar(tuple[Literal["foo"]]): ...
x = ("foo",)
reveal_type("foo" in x) # Literal[True]
if isinstance(x, Bar):
reveal_type("foo" in x) # now bool, even though the type of `x` is now more precise
def upcast(x: tuple[Literal["foo"]]) -> tuple[Literal["foo"]]:
return x
def _(x: Bar):
reveal_type("foo" in x) # revealed: bool
reveal_type("foo" in upcast(x)) # Literal[True], even though the `upcast()` function makes the type of `x` less precise! |
* main: (39 commits) [ty] Initial test suite for `TypedDict` (#19686) [ty] Improve debuggability of protocol types (#19662) [ty] Simplify lifetime requirements for `PySlice` trait (#19687) [ty] Improve `isinstance()` truthiness analysis for generic types (#19668) [`refurb`] Make example error out-of-the-box (`FURB164`) (#19673) Fix link: unused_import.rs (#19648) [ty] Remove `Specialization::display` (full) (#19682) [ty] Remove `KnownModule::is_enum` (#19681) [ty] Support `__setitem__` and improve `__getitem__` related diagnostics (#19578) [ty] Sync vendored typeshed stubs (#19676) [`flake8-use-pathlib`] Expand `PTH201` to check all `PurePath` subclasses (#19440) [`refurb`] Make example error out-of-the-box (`FURB180`) (#19672) [`pyupgrade`] Prevent infinite loop with `I002` (`UP010`, `UP035`) (#19413) [ty] Improve the `Display` for generic `type[]` types (#19667) [ty] Refactor `TypeInferenceBuilder::infer_subscript_expression_types` (#19658) Fix tests on 32-bit architectures (#19652) [ty] Move `pandas-stubs` to bad.txt (#19659) [ty] Remove special casing for string-literal-in-tuple `__contains__` (#19642) Update pre-commit's `ruff` id (#19654) Update salsa (#19449) ...
Summary
This PR rips out the special casing for
<string literal> in <tuple>that was added in #18251.We were unsure at the time whether it was worth it to add this special casing, since it added complexity to our type inference machinery but didn't appear to remove any false positives or false negatives when checking user code (the mypy_primer report on that PR is an empty diff). At the time, we thought that the benefits outweighed the costs, however, since the added complexity wasn't very large, and it allowed us to test our new
ide_support::all_membersAPI in an elegant way in mdtests.I think we should reconsider this, however. It's fundamentally inconsistent behaviour for us to infer the expression
"foo" in ("foo",)as evaluating toLiteral[True]if we do not also infer the expression"foo" in SingleElementTupleSubclass(("foo",))as evaluating toLiteral[True](whereSingleElementTupleSubclassis considered by ty to be a subtype oftuple[Literal["foo"]]). We therefore have three options:__contains__overloads, similar to what was done for__getitem__in [ty] Synthesize precise__getitem__overloads for tuple subclasses #19493TypeInferenceBuilder::infer_binary_type_comparisonso that it also applies to tuple subclasses(1) and (2) are both viable paths here, but they would both add significant complexity to the implementation of the special case, and I'm not sure it's worth doing that without evidence of patterns in user code that this enables us to understand better. Additionally, if we went with option (2) we would have to forbid users from overriding
__contains__on tuple subclasses in order for us to make tuple subclasses sound. If we went with option (1) we would not have to forbid overrides of__contains__on user subclasses oftuple; but in practice, we'd make it very difficult for users to do so after our implementation of the Liskov Substitution Principle has landed. Either forbidding__contains__overrides outright, or de-facto forbidding them by requiring the override to duplicate complicated overloads from the base class, feels overly pedantic to me. This PR therefore goes with option (3): ripping out the special casing.The remaining question was, therefore: what to do about our
all_members.mdtest? The solution this PR proposes is to adapt that test by adding a newty_extensions.has_member()function that returnsLiteral[True]if ouride_support_all_membersroutine considers an object to have a particular member, andLiteral[False]if not.Test Plan
Mdtests