Repository: python/mypy
Pull request: Don't consider a branch unreachable if there is possible promotion
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.

Problem statement:
0.990 regression with implicit complex/int promotion
Given this code:
```python
from typing import reveal_type
x: complex = 1
reveal_type(x)
if isinstance(x, int):
    reveal_type(x)
```
mypy 0.990:
```
% mypy --strict --warn-unreachable ~/py/tmp/compl.py
/Users/jelle/py/tmp/compl.py:3: note: Revealed type is "builtins.complex"
/Users/jelle/py/tmp/compl.py:4: error: Subclass of "complex" and "int" cannot exist: would have incompatible method signatures  [unreachable]
/Users/jelle/py/tmp/compl.py:5: error: Statement is unreachable  [unreachable]
Found 2 errors in 1 file (checked 1 source file)
```

mypy 0.982:
```
% mypy --strict --warn-unreachable ~/py/tmp/compl.py
/Users/jelle/py/tmp/compl.py:3: note: Revealed type is "builtins.complex"
/Users/jelle/py/tmp/compl.py:5: note: Revealed type is "builtins.int"
Success: no issues found in 1 source file
```

The 0.982 behavior is correct, since `int` is a subclass of `complex` in the static type system.

Hints from issue discussion:
This bug introduces 8 false positives when mypy checks the [`flake8-pyi` codebase](https://github.com/PyCQA/flake8-pyi/blob/master/pyi.py) (we have `--warn-unreachable` enabled in our `pyproject.toml` file):

<details>

```
pyi.py:1126: error: Subclass of "complex" and "int" cannot exist: would
have incompatible method signatures  [unreachable]
                    if type(literal.n) is int:
                            ^~~~~~~~~
pyi.py:1275: error: Subclass of "complex" and "int" cannot exist: would
have incompatible method signatures  [unreachable]
                    if isinstance(slc.n, int) and slc.n == 0:
                                  ^~~~~
pyi.py:1275: error: Right operand of "and" is never evaluated
[unreachable]
                    if isinstance(slc.n, int) and slc.n == 0:
                                                  ^~~~~~~~~~
pyi.py:1276: error: Statement is unreachable  [unreachable]
                        must_be_single = True
                        ^~~~~~~~~~~~~~~~~~~~~
pyi.py:1287: error: Subclass of "complex" and "int" cannot exist: would
have incompatible method signatures  [unreachable]
                        and isinstance(slc.upper.n, int)
                                       ^~~~~~~~~~~
pyi.py:1288: error: Right operand of "and" is never evaluated
[unreachable]
                        and slc.upper.n in (1, 2)
                            ^~~~~~~~~~~~~~~~~~~~~
pyi.py:1290: error: Statement is unreachable  [unreachable]
                        can_have_strict_equals = slc.upper.n
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pyi.py:1313: error: Subclass of "complex" and "int" cannot exist: would
have incompatible method signatures  [unreachable]
    ...nstance(comparator, ast.Num) or not isinstance(comparator.n, int):
                                                      ^~~~~~~~~~~~
Found 8 errors in 1 file (checked 1 source file)
```

</details>
cc @ilevkivskyi this is from https://github.com/python/mypy/issues/13781
FWIW this is intentional, `isinstance()` checks have always been determined by runtime behavior, not static subtyping. Previously promotions sometimes leaked, and caused various inconsistencies.

There is really no middle ground, if we enable promotions, there will be other issues, see, e.g. https://github.com/python/mypy/issues/6060. I would say current behavior is better because:
* It is more consistent with how `isinstance()` check behaves in other situations
* It may cause "false positives" with a strictness flag, while opposite causes false positives with default options.
Also fix should be trivial, use a more precise type, like `Union[int, complex]`.
Without strictness flags, the current behavior leads code to be silently skipped as unreachable, which is arguably worse. I agree that there are few good options here though.
Maybe there's some hack, so we keep the better narrowing behaviour, but it doesn't affect reachability
@hauntsaninja Actually, if you like hacks, I think there may be a (relatively) easy one: ignore the impossible intersection check in case if one type can be promoted to another one.
Well, we should probably still prohibit (consider unreachable) something like:
```python
if isinstance(x, int) and isinstance(x, float):
    ...
```
so it may require some work to implement.
Just making sure you all are aware of the idea in #11516

Specifically: in the if statement, split types up into this union and rely on already-existing narrowing.

Y'all are probably already aware, but just making sure this "solution" is considered. (I don't see any reasoning on that issue itself that would disqualify it)
@hauntsaninja Actually after thinking some more, it seems to me we shouldn't change this, otherwise we may not flag legitimately unreachable code like
```python
x = 1j
if isinstance(x, int):
    ...
```
that may potentially also hide bugs.

In general, most of the original motivation for type promotions was that practically all functions originally intended for `float` will also (accidentally) work for `int`. So to avoid forcing people to annotate all their argument types as e.g. `Union[int, float]`, this decision was made. On the opposite, if a function actually differentiates between `int` and `float`, it should be annotated as taking `Union[int, float]` to emphasize this:
```python
def weighted(a: float, b: float) -> float:
    return (a + 2 * b) / 3
weighted(1, 2)  # This is fine

def num_type(x: float) -> str:  # Bad, should be Union[int, float]
    if isinstance(x, int):
        return "Integer"
    return "Floating"
```
Anyway, it is still gray area and likely there will still be some people who likes (or used to) the old behavior. So we should clearly document this.

Commit message: Don't consider a branch unreachable if there is possible promotion

Patch to review:
diff --git a/mypy/checker.py b/mypy/checker.py
index e727c343aa14..ae127e744110 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -4850,6 +4850,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_:
diff --git a/test-data/unit/check-type-promotion.test b/test-data/unit/check-type-promotion.test
index f477a9f2b390..4dfcce244612 100644
--- a/test-data/unit/check-type-promotion.test
+++ b/test-data/unit/check-type-promotion.test
@@ -54,3 +54,59 @@ def f(x: Union[SupportsFloat, T]) -> Union[SupportsFloat, T]: pass
 f(0)  # should not crash
 [builtins fixtures/primitives.pyi]
 [out]
+
+[case testIntersectionUsingPromotion]
+# flags: --warn-unreachable
+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"
+[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"
+[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):
+    # Most likely this was an error, but we still type-check this branch
+    reveal_type(x)  # N: Revealed type is "<nothing>"
+else:
+    reveal_type(x)  # N: Revealed type is "Union[builtins.float, builtins.complex]"
+[builtins fixtures/primitives.pyi]
diff --git a/test-data/unit/fixtures/primitives.pyi b/test-data/unit/fixtures/primitives.pyi
index 9553df4b40c7..22860e8e2fde 100644
--- a/test-data/unit/fixtures/primitives.pyi
+++ b/test-data/unit/fixtures/primitives.pyi
@@ -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')
@@ -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: ...
+class complex:
+    def __add__(self, x: complex) -> complex: ...
 class bool(int): pass
 class str(Sequence[str]):
     def __add__(self, s: str) -> str: pass
@@ -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


Code review comments:
diff_hunk: @@ -54,3 +54,59 @@ def f(x: Union[SupportsFloat, T]) -> Union[SupportsFloat, T]: pass
 f(0)  # should not crash
 [builtins fixtures/primitives.pyi]
 [out]
+
+[case testIntersectionUsingPromotion]
+# flags: --warn-unreachable
+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"
+[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"
+[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):
+    # Most likely this was an error, but we still type-check this branch
+    reveal_type(x)  # N: Revealed type is "<nothing>"
path: test-data/unit/check-type-promotion.test
text: @user1:
hm, I'd expected this to also give `int`. What happens if `x` is a union with an unrelated type, like `complex | str`?

@author:
It will be also `<nothing>`. It is not easy to change. I however discovered couple more bugs while playing with this, so I will try to figure out something.