Repository: python/mypy
Pull request: meet.py should consider all Tuple[t1, t2, ...] as subtypes of plain tuple
Fixes #1691 
Fixes #2456 

This is a simple-minded fix. In the future we could add more rules for tuples in ``meet.py`` (for example for homogeneous tuples, a tuple and an iterable etc).

Tests are based on (extended) examples discussed in issues.

Problem statement:
Variable incorrectly typed as None, misleading error message
While debugging a more complicated issue, I reduced it to this:

``` python
from typing import Tuple, Union

Pair = Tuple[int, int]
Variant = Union[int, Pair]


def tuplify(v: Variant) -> None:
    if not isinstance(v, tuple):
        v = (v, v)
    print(v[0], v[1])

```

mypy running on this example detects an error on the `print` line saying:

```
foo.py:10: error: Value of type None is not indexable
```

For some reason it says that the value of `v` has type `None`. I understand why the checker may not be able to follow the negative isinstance check, but I expected the error to say something like `Value of type Union[...] is not indexable`.


isinstance checks don't work with tuple types
An invalid type is inferred for `x` after `isinstance` in this example:

```
from typing import Union, Tuple

def f(x: Union[str, Tuple[str, str]]) -> None:
    if isinstance(x, tuple):
        reveal_type(x)  # Revealed type is 'builtins.None'
```

This was originally reported by @davire at gitter.

Hints from issue discussion:
This looks like a bug to me.  If you change the check from `not isinstance(v, tuple)` to `isinstance(v, int)` (which in this case should be the same) then mypy passes without errors.  The logic around these isinstance checks is pretty complicated -- we probably have a slightly messed up case.

A minimal reproducer:

``` py
from typing import Tuple, Union

Pair = Tuple[int, int]
Variant = Union[int, Pair]

def tuplify(v: Variant) -> None:
    if isinstance(v, tuple):
        reveal_type(v)  # E: Revealed type is 'builtins.None'
```

One way to fix this would be to make `meet_types` of `Tuple[int, int]` and `tuple[Any]` return `Tuple[int, int]` (as it should because `Tuple[int, int]` is a subtype of `tuple[Any]`). I think the whole approach of using `meet` in `isinstance` checks is wrong though; instead we should have functions of types

``` py
def restrict_subtype_to(e: Type, t: TypeInfo) -> Type: ...
def restrict_subtype_away(e: Type, t: TypeInfo) -> Type: ...
```

since the second argument of `isinstance` has to be a type object (or tuple of type objects) and never anything complicated like a generic class with specific type arguments, or `Any`, etc.

I think it shouldn't be correct that «`Tuple[int, int]` is a subtype of `tuple[Any]`» (`Any` is actually something that should work as being subtype of everything, given that any operation on it is invalid). I don't know how mypy implements it, my statement is from an understanding of type systems outside of mypy.

I think the problem here should be that `isinstance(v, tuple)` should join the type with `Tuple[object...]` instead of joining with `Tuple[Any...]`, because checking `isinstance(v, tuple)` doesn't give you any information at all about the elements of the tuple and `object` is the type that means "I have a value with no information about it".

`Any` doesn't participate in normal subtyping relations, because it's meant to be used as an escape hatch.  It's actually both at the bottom and the top of the subtyping hierarchy (loosely speaking).

PEP 484 implies that `list` is supposed to be equivalent to `List[Any]` in types.  (From a quick skim, I'm not sure if it says this outright.)  I don't think that necessarily implies that we have to treat it that way in isinstance checks, though.  I think @dmoisset's suggestion is a good one -- we should consider treating containers as `object` filled rather than `Any` filled in isinstance checks.

That makes sense for covariant parameters, but might be a little weird for invariant and especially contravariant parameters. At any rate, if `v` has type `Union[int, List[int]]` and an `isinstance(v, list)` check is True, then the refined type of `v` must be `List[int]`, which is not the meet of `List[int]` and `List[object]` (since it's not a subtype of `List[object]`).

On the other hand, we can ask the question

``` py
def f(x: object) -> None:
    if isinstance(x, list):
        ... # what is the type of x here?
```

If the type is `List[object]`, then `f` could append any sort of object to a provided argument of type `List[int]`, which is unsound. Of course, the current choice `List[Any]` is even more unsound. Perhaps the theoretically correct thing to do would be to treat the type as `exists T. List[T]`; introduce a new type variable `T` (that is treated like a generic function type parameter) inside the block and give `x` type `List[T]`. This doesn't actually sound _that_ hard, but does seem rather low priority...

For future reference, here's a simpler repro of the problem:

``` py
from typing import Tuple, Union

x = (0, 0)  # type: Union[Tuple[int,int], int]
if isinstance(x, tuple):
    reveal_type(x)  # E: Revealed type is 'builtins.None'
else:
    reveal_type(x)  # E: Revealed type is 'builtins.int'
```

I also find it leaves the variable in a weird state:

```
def bad(blah):
    # type: (Union[Tuple[int, int], int]) -> None
    reveal_type(blah)  # blah is Union[Tuple[int, int], int]
    if isinstance(blah, tuple):
        reveal_type(blah)   # blah is now None?
    reveal_type(blah)  # blah is now int?
```

Note: if you reverse the instance check to check for the other side it works fine:

```
def good(blah):
    # type: (Union[Tuple[int, int], int]) -> None
    reveal_type(blah)  # blah is Union[Tuple[int, int], int]
    if isinstance(blah, int):
        reveal_type(blah)   # blah is now int
    else:
        reveal_type(blah)   # blah is Tuple[int, int]
    reveal_type(blah)  # blah is Union[Tuple[int, int], int]
```

Commit message: Fix isinstance(Tuple[int, str], tuple)

Patch to review:
diff --git a/mypy/meet.py b/mypy/meet.py
index 18796ae2491c..f078e345d37d 100644
--- a/mypy/meet.py
+++ b/mypy/meet.py
@@ -227,6 +227,8 @@ def visit_instance(self, t: Instance) -> Type:
                         return NoneTyp()
         elif isinstance(self.s, TypeType):
             return meet_types(t, self.s)
+        elif isinstance(self.s, TupleType):
+            return meet_types(t, self.s)
         else:
             return self.default(self.s)
 
@@ -243,6 +245,10 @@ def visit_tuple_type(self, t: TupleType) -> Type:
                 items.append(self.meet(t.items[i], self.s.items[i]))
             # TODO: What if the fallbacks are different?
             return TupleType(items, t.fallback)
+        # Special case: all Tuple[t1, t2, ...] are subtypes of plain tuple.
+        elif (isinstance(self.s, Instance) and self.s.type.fullname() == 'builtins.tuple'
+              and len(self.s.args) == 1 and isinstance(self.s.args[0], AnyType)):
+            return t
         else:
             return self.default(self.s)
 
diff --git a/test-data/unit/check-tuples.test b/test-data/unit/check-tuples.test
index bdd3805f6c24..e97ddcb90424 100644
--- a/test-data/unit/check-tuples.test
+++ b/test-data/unit/check-tuples.test
@@ -808,6 +808,73 @@ b = (1, 'x')
 a = (0, *b, '')
 [builtins fixtures/tuple.pyi]
 
+[case testTupleMeetTupleAny]
+from typing import Union, Tuple
+class A: pass
+class B: pass
+
+def f(x: Union[B, Tuple[A, A]]) -> None:
+    if isinstance(x, tuple):
+        reveal_type(x)
+    else:
+        reveal_type(x)
+
+def g(x: Union[str, Tuple[str, str]]) -> None:
+    if isinstance(x, tuple):
+        reveal_type(x) # E: Revealed type is 'Tuple[builtins.str, builtins.str]'
+    else:
+        reveal_type(x) # E: Revealed type is 'builtins.str'
+
+[builtins fixtures/tuple.pyi]
+[out]
+main: note: In function "f":
+main:7: error: Revealed type is 'Tuple[__main__.A, __main__.A]'
+main:9: error: Revealed type is '__main__.B'
+main: note: In function "g":
+
+[case testTupleMeetTUpleAnyComplex]
+from typing import Tuple, Union
+
+Pair = Tuple[int, int]
+Variant = Union[int, Pair]
+def tuplify(v: Variant) -> None:
+    reveal_type(v)
+    if not isinstance(v, tuple):
+        reveal_type(v)
+        v = (v, v)
+        reveal_type(v)
+    reveal_type(v)
+    reveal_type(v[0])
+
+Pair2 = Tuple[int, str]
+Variant2 = Union[int, Pair2]
+def tuplify2(v: Variant2) -> None:
+    if isinstance(v, tuple):
+        reveal_type(v) # E: Revealed type is 'Tuple[builtins.int, builtins.str]'
+    else:
+        reveal_type(v) # E: Revealed type is 'builtins.int'
+[builtins fixtures/tuple.pyi]
+[out]
+main: note: In function "tuplify":
+main:6: error: Revealed type is 'Union[builtins.int, Tuple[builtins.int, builtins.int]]'
+main:8: error: Revealed type is 'builtins.int'
+main:10: error: Revealed type is 'Tuple[builtins.int, builtins.int]'
+main:11: error: Revealed type is 'Tuple[builtins.int, builtins.int]'
+main:12: error: Revealed type is 'builtins.int'
+main: note: In function "tuplify2":
+
+[case testTupleMeetTupleAnyAfter]
+from typing import Tuple, Union
+
+def good(blah: Union[Tuple[int, int], int]) -> None:
+    reveal_type(blah) # E: Revealed type is 'Union[Tuple[builtins.int, builtins.int], builtins.int]'
+    if isinstance(blah, tuple):
+        reveal_type(blah) # E: Revealed type is 'Tuple[builtins.int, builtins.int]'
+    reveal_type(blah) # E: Revealed type is 'Union[Tuple[builtins.int, builtins.int], builtins.int]'
+[builtins fixtures/tuple.pyi]
+[out]
+main: note: In function "good":
+
 [case testTupleWithUndersizedContext]
 a = ([1], 'x')
 a = ([], 'x', 1)  # E: Incompatible types in assignment (expression has type "Tuple[List[int], str, int]", variable has type "Tuple[List[int], str]")
diff --git a/test-data/unit/fixtures/tuple.pyi b/test-data/unit/fixtures/tuple.pyi
index f6d36cc0d2df..6fe63b6ecd54 100644
--- a/test-data/unit/fixtures/tuple.pyi
+++ b/test-data/unit/fixtures/tuple.pyi
@@ -23,5 +23,6 @@ class str: pass # For convenience
 T = TypeVar('T')
 
 class list(Sequence[T], Generic[T]): pass
+def isinstance(x: object, t: type) -> bool: pass
 
 def sum(iterable: Iterable[T], start: T = None) -> T: pass


Code review comments:
diff_hunk: @@ -243,6 +245,10 @@ def visit_tuple_type(self, t: TupleType) -> Type:
                 items.append(self.meet(t.items[i], self.s.items[i]))
             # TODO: What if the fallbacks are different?
             return TupleType(items, t.fallback)
+        # Special case: all Tuple[t1, t2, ...] are subtypes of plain tuple.
+        elif (isinstance(self.s, Instance) and self.s.type.fullname() == 'builtins.tuple'
path: mypy/meet.py
text: @user1:
We could also handle cases like the meet of `Tuple[int, int]` and `Tuple[int, ...]`. However, as this is slightly trickier, it's okay to add a comment here about this and create a follow-up issue. Which would you prefer?

Implementing the general case shouldn't be hard, though it's not urgent really. The meet of arbitrary `Tuple[t1, <...>,  tn]` and `Tuple[s, ...]` (where `<...>` would not be a Python ellipsis but a mathematical one) would be `Tuple[meet(t1, s), ..., meet(tn, s)]`, I think. So the meet of types `Tuple[X, Y]` and `Tuple[Z, ...]` would be `Tuple[meet(X, Z), meet(Y, Z)]`, etc.

@author:
@user1 OK, I made this apply for all homogeneous tuples (instead of special-casing bare ``tuple``) as you propose.