Skip to content

Commit

Permalink
Don't consider a branch unreachable if there is possible promotion (#…
Browse files Browse the repository at this point in the history
…14077)

Fixes #14030 

FWIW this looks like an acceptable compromise after discussions in the
issue. Also it is easy to implement. Let's see what `mypy_primer` will
show.
  • Loading branch information
ilevkivskyi authored Nov 14, 2022
1 parent cf59b82 commit dd0503e
Show file tree
Hide file tree
Showing 7 changed files with 180 additions and 20 deletions.
31 changes: 20 additions & 11 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -4824,7 +4824,7 @@ def make_fake_typeinfo(
return cdef, info

def intersect_instances(
self, instances: tuple[Instance, Instance], ctx: Context
self, instances: tuple[Instance, Instance], errors: list[tuple[str, str]]
) -> Instance | None:
"""Try creating an ad-hoc intersection of the given instances.
Expand All @@ -4851,6 +4851,17 @@ def intersect_instances(
curr_module = self.scope.stack[0]
assert isinstance(curr_module, MypyFile)

# First, retry narrowing while allowing promotions (they are disabled by default
# for isinstance() checks, etc). This way we will still type-check branches like
# x: complex = 1
# if isinstance(x, int):
# ...
left, right = instances
if is_proper_subtype(left, right, ignore_promotions=False):
return left
if is_proper_subtype(right, left, ignore_promotions=False):
return right

def _get_base_classes(instances_: tuple[Instance, Instance]) -> list[Instance]:
base_classes_ = []
for inst in instances_:
Expand Down Expand Up @@ -4891,17 +4902,10 @@ def _make_fake_typeinfo_and_full_name(
self.check_multiple_inheritance(info)
info.is_intersection = True
except MroError:
if self.should_report_unreachable_issues():
self.msg.impossible_intersection(
pretty_names_list, "inconsistent method resolution order", ctx
)
errors.append((pretty_names_list, "inconsistent method resolution order"))
return None

if local_errors.has_new_errors():
if self.should_report_unreachable_issues():
self.msg.impossible_intersection(
pretty_names_list, "incompatible method signatures", ctx
)
errors.append((pretty_names_list, "incompatible method signatures"))
return None

curr_module.names[full_name] = SymbolTableNode(GDEF, info)
Expand Down Expand Up @@ -6355,15 +6359,20 @@ def conditional_types_with_intersection(
possible_target_types.append(item)

out = []
errors: list[tuple[str, str]] = []
for v in possible_expr_types:
if not isinstance(v, Instance):
return yes_type, no_type
for t in possible_target_types:
intersection = self.intersect_instances((v, t), ctx)
intersection = self.intersect_instances((v, t), errors)
if intersection is None:
continue
out.append(intersection)
if len(out) == 0:
# Only report errors if no element in the union worked.
if self.should_report_unreachable_issues():
for types, reason in errors:
self.msg.impossible_intersection(types, reason, ctx)
return UninhabitedType(), expr_type
new_yes_type = make_simplified_union(out)
return new_yes_type, expr_type
Expand Down
14 changes: 10 additions & 4 deletions mypy/join.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,11 @@ def join_instances_via_supertype(self, t: Instance, s: Instance) -> ProperType:


def join_simple(declaration: Type | None, s: Type, t: Type) -> ProperType:
"""Return a simple least upper bound given the declared type."""
# TODO: check infinite recursion for aliases here?
"""Return a simple least upper bound given the declared type.
This function should be only used by binder, and should not recurse.
For all other uses, use `join_types()`.
"""
declaration = get_proper_type(declaration)
s = get_proper_type(s)
t = get_proper_type(t)
Expand All @@ -158,10 +161,10 @@ def join_simple(declaration: Type | None, s: Type, t: Type) -> ProperType:
if isinstance(s, ErasedType):
return t

if is_proper_subtype(s, t):
if is_proper_subtype(s, t, ignore_promotions=True):
return t

if is_proper_subtype(t, s):
if is_proper_subtype(t, s, ignore_promotions=True):
return s

if isinstance(declaration, UnionType):
Expand All @@ -176,6 +179,9 @@ def join_simple(declaration: Type | None, s: Type, t: Type) -> ProperType:
# Meets/joins require callable type normalization.
s, t = normalize_callables(s, t)

if isinstance(s, UnionType) and not isinstance(t, UnionType):
s, t = t, s

value = t.accept(TypeJoinVisitor(s))
if declaration is None or is_subtype(value, declaration):
return value
Expand Down
10 changes: 9 additions & 1 deletion mypy/meet.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,15 @@ def narrow_declared_type(declared: Type, narrowed: Type) -> Type:
[
narrow_declared_type(x, narrowed)
for x in declared.relevant_items()
if is_overlapping_types(x, narrowed, ignore_promotions=True)
# This (ugly) special-casing is needed to support checking
# branches like this:
# x: Union[float, complex]
# if isinstance(x, int):
# ...
if (
is_overlapping_types(x, narrowed, ignore_promotions=True)
or is_subtype(narrowed, x, ignore_promotions=False)
)
]
)
if is_enum_overlapping_union(declared, narrowed):
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -7209,7 +7209,7 @@ from typing import Callable
class C:
x: Callable[[C], int] = lambda x: x.y.g() # E: "C" has no attribute "y"

[case testOpWithInheritedFromAny]
[case testOpWithInheritedFromAny-xfail]
from typing import Any
C: Any
class D(C):
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-isinstance.test
Original file line number Diff line number Diff line change
Expand Up @@ -2392,7 +2392,7 @@ class B:
x1: Literal[1] = self.f()

def t2(self) -> None:
if isinstance(self, (A0, A1)): # E: Subclass of "B" and "A0" cannot exist: would have incompatible method signatures
if isinstance(self, (A0, A1)):
reveal_type(self) # N: Revealed type is "__main__.<subclass of "A1" and "B">1"
x0: Literal[0] = self.f() # E: Incompatible types in assignment (expression has type "Literal[1]", variable has type "Literal[0]")
x1: Literal[1] = self.f()
Expand Down
133 changes: 133 additions & 0 deletions test-data/unit/check-type-promotion.test
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,136 @@ def f(x: Union[SupportsFloat, T]) -> Union[SupportsFloat, T]: pass
f(0) # should not crash
[builtins fixtures/primitives.pyi]
[out]

[case testIntersectionUsingPromotion1]
# flags: --warn-unreachable
from typing import Union

x: complex = 1
reveal_type(x) # N: Revealed type is "builtins.complex"
if isinstance(x, int):
reveal_type(x) # N: Revealed type is "builtins.int"
else:
reveal_type(x) # N: Revealed type is "builtins.complex"
reveal_type(x) # N: Revealed type is "builtins.complex"

y: Union[int, float]
if isinstance(y, float):
reveal_type(y) # N: Revealed type is "builtins.float"
else:
reveal_type(y) # N: Revealed type is "builtins.int"

reveal_type(y) # N: Revealed type is "Union[builtins.int, builtins.float]"

if isinstance(y, int):
reveal_type(y) # N: Revealed type is "builtins.int"
else:
reveal_type(y) # N: Revealed type is "builtins.float"
[builtins fixtures/primitives.pyi]

[case testIntersectionUsingPromotion2]
# flags: --warn-unreachable
x: complex = 1
reveal_type(x) # N: Revealed type is "builtins.complex"
if isinstance(x, (int, float)):
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.float]"
else:
reveal_type(x) # N: Revealed type is "builtins.complex"

# Note we make type precise, since type promotions are involved
reveal_type(x) # N: Revealed type is "Union[builtins.complex, builtins.int, builtins.float]"
[builtins fixtures/primitives.pyi]

[case testIntersectionUsingPromotion3]
# flags: --warn-unreachable
x: object
if isinstance(x, int) and isinstance(x, complex):
reveal_type(x) # N: Revealed type is "builtins.int"
if isinstance(x, complex) and isinstance(x, int):
reveal_type(x) # N: Revealed type is "builtins.int"
[builtins fixtures/primitives.pyi]

[case testIntersectionUsingPromotion4]
# flags: --warn-unreachable
x: object
if isinstance(x, int):
if isinstance(x, complex):
reveal_type(x) # N: Revealed type is "builtins.int"
else:
reveal_type(x) # N: Revealed type is "builtins.int"
if isinstance(x, complex):
if isinstance(x, int):
reveal_type(x) # N: Revealed type is "builtins.int"
else:
reveal_type(x) # N: Revealed type is "builtins.complex"
[builtins fixtures/primitives.pyi]

[case testIntersectionUsingPromotion5]
# flags: --warn-unreachable
from typing import Union

x: Union[float, complex]
if isinstance(x, int):
reveal_type(x) # N: Revealed type is "builtins.int"
else:
reveal_type(x) # N: Revealed type is "Union[builtins.float, builtins.complex]"
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.float, builtins.complex]"
[builtins fixtures/primitives.pyi]

[case testIntersectionUsingPromotion6]
# flags: --warn-unreachable
from typing import Union

x: Union[str, complex]
if isinstance(x, int):
reveal_type(x) # N: Revealed type is "builtins.int"
else:
reveal_type(x) # N: Revealed type is "Union[builtins.str, builtins.complex]"
reveal_type(x) # N: Revealed type is "Union[builtins.str, builtins.int, builtins.complex]"
[builtins fixtures/primitives.pyi]

[case testIntersectionUsingPromotion7]
# flags: --warn-unreachable
from typing import Union

x: Union[int, float, complex]
if isinstance(x, int):
reveal_type(x) # N: Revealed type is "builtins.int"
else:
reveal_type(x) # N: Revealed type is "Union[builtins.float, builtins.complex]"

reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.float, builtins.complex]"

if isinstance(x, float):
reveal_type(x) # N: Revealed type is "builtins.float"
else:
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.complex]"

reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.float, builtins.complex]"

if isinstance(x, complex):
reveal_type(x) # N: Revealed type is "builtins.complex"
else:
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.float]"

reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.float, builtins.complex]"
[builtins fixtures/primitives.pyi]

[case testIntersectionUsingPromotion8]
# flags: --warn-unreachable
from typing import Union

x: Union[int, float, complex]
if isinstance(x, (int, float)):
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.float]"
else:
reveal_type(x) # N: Revealed type is "builtins.complex"
if isinstance(x, (int, complex)):
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.complex]"
else:
reveal_type(x) # N: Revealed type is "builtins.float"
if isinstance(x, (float, complex)):
reveal_type(x) # N: Revealed type is "Union[builtins.float, builtins.complex]"
else:
reveal_type(x) # N: Revealed type is "builtins.int"
[builtins fixtures/primitives.pyi]
8 changes: 6 additions & 2 deletions test-data/unit/fixtures/primitives.pyi
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# builtins stub with non-generic primitive types
from typing import Generic, TypeVar, Sequence, Iterator, Mapping, Iterable, overload
from typing import Generic, TypeVar, Sequence, Iterator, Mapping, Iterable, Tuple, Union

T = TypeVar('T')
V = TypeVar('V')
Expand All @@ -20,7 +20,9 @@ class int:
def __rmul__(self, x: int) -> int: pass
class float:
def __float__(self) -> float: pass
class complex: pass
def __add__(self, x: float) -> float: pass
class complex:
def __add__(self, x: complex) -> complex: pass
class bool(int): pass
class str(Sequence[str]):
def __add__(self, s: str) -> str: pass
Expand Down Expand Up @@ -63,3 +65,5 @@ class range(Sequence[int]):
def __getitem__(self, i: int) -> int: pass
def __iter__(self) -> Iterator[int]: pass
def __contains__(self, other: object) -> bool: pass

def isinstance(x: object, t: Union[type, Tuple]) -> bool: pass

0 comments on commit dd0503e

Please sign in to comment.