Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -6812,13 +6812,20 @@ def narrow_type_by_identity_equality(
# We patch this here because it is desirable to widen to any for cases like
# isinstance(x, (y: Any))
continue

arg_type = self.lookup_type(expr_in_type_expr)
if_type, else_type = self.conditional_types_with_intersection(
arg_type, [current_type_range], expr_in_type_expr
)
if if_type is not None and (
not current_type_range.is_upper_bound
and not is_equivalent(if_type, current_type_range.item)
):
# type(x) and x.__class__ checks must exact match
if_type = UninhabitedType()

if_map, else_map = conditional_types_to_typemaps(
expr_in_type_expr,
*self.conditional_types_with_intersection(
self.lookup_type(expr_in_type_expr),
[current_type_range],
expr_in_type_expr,
),
expr_in_type_expr, if_type, else_type
)

is_final = (
Expand Down
42 changes: 41 additions & 1 deletion test-data/unit/check-narrowing.test
Original file line number Diff line number Diff line change
Expand Up @@ -3152,7 +3152,7 @@ else:
reveal_type(y) # N: Revealed type is "builtins.str"
[builtins fixtures/isinstance.pyi]

[case testTypeEqualsCheckUsingIsNonOverlappingChild-xfail]
[case testTypeEqualsCheckUsingIsNonOverlappingChild]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the test case I was targeting with this PR

# flags: --strict-equality --warn-unreachable
from typing import Union

Expand All @@ -3168,6 +3168,46 @@ def main(x: Union[B, C]):
reveal_type(x) # N: Revealed type is "__main__.B | __main__.C"
[builtins fixtures/isinstance.pyi]

[case testTypeEqualsCheckTypeVar]
# flags: --strict-equality --warn-unreachable
from typing import TypeVar

class A: ...
class B: ...

T = TypeVar("T")

def forgets_about_subclasses1(self, obj: T) -> T:
if isinstance(obj, A):
return A() # E: Incompatible return value type (got "A", expected "T")
elif isinstance(obj, B):
return B() # E: Incompatible return value type (got "B", expected "T")
raise

def correct1(self, obj: T) -> T:
if type(obj) == A:
return A() # E: Incompatible return value type (got "A", expected "T")
elif type(obj) == B:
return B() # E: Incompatible return value type (got "B", expected "T")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these returns be safe? I understand it is hard to support (since this will essentially require narrowing T, not obj), but then do we really need this test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, these returns are safe.

This PR doesn't change behaviour on def correct1 but it turns out it does fix behaviour on def correct2. correct2 was reported to mypy (I found it via https://github.com/sterliakov/mypy-issues/ ), so it makes sense to have a regression test for it. correct1 adds context for what we can and can't do.

In general we have a fair amount of tests where "we know the behaviour isn't ideal" and I've found these tests very helpful when doing my narrowing rewrite

raise

T_value = TypeVar("T_value", A, B)

def forgets_about_subclasses2(self, obj: T_value) -> T_value:
if isinstance(obj, A):
return A() # E: Incompatible return value type (got "A", expected "B")
elif isinstance(obj, B):
return B()
raise

def correct2(self, obj: T_value) -> T_value:
if type(obj) == A:
return A()
elif type(obj) == B:
return B()
raise
[builtins fixtures/primitives.pyi]

[case testTypeEqualsCheckUsingDifferentSpecializedTypes]
# flags: --warn-unreachable
from collections import defaultdict
Expand Down