Repository: python/mypy
Pull request: Meaningful message with long tuple initializer
resolves #7977 

Current format of error message(especially for item-by-item case) need discussion

request for review if you are free @JukkaL @ilevkivskyi 

Problem statement:
Unhelpful message with a very long tuple initializer
This program genenerates an unhelpful error message:
```py
from typing import Tuple
t: Tuple[int, ...] = (
    1, 2, 3, 4, 5, 6, 7, 8, 9., 10, 11, 12, 13, 14, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29,
    30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57,
    3, 31, 32, 33, 34, 35, 36, 37, 3, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57,
    30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 4, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57,
    30, 31, 32, 33, 34, 234234, 36, 37, 38, 39, 40, 1, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56,
    57, 58, 59, 60, 61, 62, 63
)
```

This is the message:

```
t.py:3: error: Incompatible types in assignment (expression has type <tuple: 148 items>, variable has type "Tuple[int, ...]")
```

This is derived from a real example, where the type was `Tuple[Type[x], ...]` and the initializer contained a long tuple of type objects. Figuring out which type object is incompatible was tricky.

Some ideas about how to improve this:
* Point to individual tuple items that are incompatible. For example: `Tuple item 8 has type "float"; "int" expected` (also point the error message to the relevant line/column).
* If there are many incompatibilities, only show a fixed number of them and hide the remaining ones.

Hints from issue discussion:
Looks like not hard to support, at least in an ad-hoc way.

How would the desired error message looks like if every pair's types matches, but the number of pairs does not, for example:
```python
from typing import Tuple
t1: Tuple[int, int, int, int, int] = (5, 5, 5, 6, 7, 10, 10, 10)
t2: Tuple[int, int, int, int, int] = (5, 5, 5)
```

Another question is how do we determine the tuple initializer is **very long** (how many elems?) since the current error message is clear enough when the tuple is short and therefore may not need to change

@JukkaL 
If the number of items doesn't match but all items match and the tuple type is very long, we could say something like "expressions has type Tuple[int, int, ... <20 more items>]; variable has type Tuple[int, int, ... <19 more items>]". This form should probably only be used if the lengths of the tuples are different, as otherwise it would look confusing.

Maybe 10+ items would be a reasonable cutoff for considering a tuple as very long?

Commit message: add docstring, fix test on python3.8

Patch to review:
diff --git a/mypy/checker.py b/mypy/checker.py
index 0387ffce8549..4bb8a5e1fdc6 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -4027,7 +4027,9 @@ def check_subtype(self,
 
         subtype = get_proper_type(subtype)
         supertype = get_proper_type(supertype)
-
+        if self.check_long_tuple_assignment(subtype, supertype, context, msg,
+                                       subtype_label, supertype_label, code=code):
+            return False
         if self.should_suppress_optional_error([subtype]):
             return False
         extra_info = []  # type: List[str]
@@ -4378,6 +4380,61 @@ def infer_issubclass_maps(self, node: CallExpr,
         yes_map, no_map = map(convert_to_typetype, (yes_map, no_map))
         return yes_map, no_map
 
+    def check_long_tuple_assignment(self,
+                               subtype: ProperType,
+                               supertype: ProperType,
+                               context: Context,
+                               msg: str = message_registry.INCOMPATIBLE_TYPES,
+                               subtype_label: Optional[str] = None,
+                               supertype_label: Optional[str] = None,
+                               code: Optional[ErrorCode] = None) -> bool:
+        """Check if assignment error using long tuple initializers."""
+
+        def format_error_msg(msg: str, lhs_types: List[Type], rhs_types: List[Type]) -> str:
+            error_msg = ""
+            error_cnt = 0
+            for i, (lhs_t, rhs_t) in enumerate(zip(lhs_types, rhs_types)):
+                if not is_subtype(lhs_t, rhs_t):
+                    if error_cnt < 3:
+                        error_msg += 'expression Tuple item {} has type "{}"; "{}" expected; '\
+                            .format(str(i), format_type_bare(rhs_t), format_type_bare(lhs_t))
+                    error_cnt += 1
+            error_msg += 'Total {} items are incompatible; {} items are omitted'.format(
+                        str(error_cnt), str(max(0, error_cnt - 3)))
+            return msg + ' (' + error_msg + ')'
+
+        def format_long_tuple_type(typ: TupleType) -> str:
+            item_cnt = len(typ.items)
+            if item_cnt > 10:
+                return 'Tuple[{}, {}, ... <{} more items>]'\
+                        .format(format_type_bare(typ.items[0]),
+                            format_type_bare(typ.items[1]), str(item_cnt - 2))
+            else:
+                return format_type_bare(typ)
+
+        if isinstance(subtype, TupleType):
+            if (len(subtype.items) > 10 and
+                isinstance(supertype, Instance) and
+                    supertype.type.fullname == 'builtins.tuple'):
+                lhs_type = supertype.args[0]
+                lhs_types = [lhs_type] * len(subtype.items)
+                error_msg = format_error_msg(msg, lhs_types, subtype.items)
+                self.fail(error_msg, context, code=code)
+                return True
+            elif (isinstance(supertype, TupleType) and
+                    (len(subtype.items) > 10 or len(supertype.items) > 10)):
+                if len(subtype.items) != len(supertype.items):
+                    if supertype_label is not None and subtype_label is not None:
+                        error_msg = "{} ({} {}, {} {})".format(msg, subtype_label,
+                                        format_long_tuple_type(subtype), supertype_label,
+                                        format_long_tuple_type(supertype))
+                        self.fail(error_msg, context, code=code)
+                        return True
+                error_msg = format_error_msg(msg, supertype.items, subtype.items)
+                self.fail(error_msg, context, code=code)
+                return True
+        return False
+
 
 def conditional_type_map(expr: Expression,
                          current_type: Optional[Type],
diff --git a/test-data/unit/check-tuples.test b/test-data/unit/check-tuples.test
index 73c1c9ac6236..d3298d443c6d 100644
--- a/test-data/unit/check-tuples.test
+++ b/test-data/unit/check-tuples.test
@@ -1241,3 +1241,43 @@ b = ("bar", 7)
 reveal_type(a + b)  # N: Revealed type is 'Tuple[builtins.int, builtins.str, builtins.int, builtins.str, builtins.int]'
 
 [builtins fixtures/tuple.pyi]
+
+[case testAssigningWithLongTupleInitializer]
+from typing import Tuple
+
+# long initializer assignment with few mismatches
+t: Tuple[int, ...] = (
+    1, 2, 3, 4, 5, 6, 7, 8, "str", "str", "str", 11, 12, 14, 15,  # E: Incompatible types in assignment (expression Tuple item 8 has type "str"; "int" expected; expression Tuple item 9 has type "str"; "int" expected; expression Tuple item 10 has type "str"; "int" expected; Total 3 items are incompatible; 0 items are omitted)
+    16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30,
+    31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45,
+)
+
+# long initializer assignment with more mismatches
+t1: Tuple[int, ...] = (
+    1, 2, 3, 4, 5, 6, 7, 8, "str", "str", "str", "str", "str", "str", "str",  # E: Incompatible types in assignment (expression Tuple item 8 has type "str"; "int" expected; expression Tuple item 9 has type "str"; "int" expected; expression Tuple item 10 has type "str"; "int" expected; Total 7 items are incompatible; 4 items are omitted)
+    16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30,
+    31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45,
+)
+
+# short tuple initializer assignment
+t2: Tuple[int, ...] = (1, 2, "s", 4)  # E: Incompatible types in assignment (expression has type "Tuple[int, int, str, int]", variable has type "Tuple[int, ...]")
+
+# long initializer assignment with few mismatches, no ellipsis
+t3: Tuple[int, int, int, int, int, int, int, int, int, int, int, int] = (
+    1, 2, 3, 4, 5, 6, 7, 8, 9, 10, "str", "str",  # E: Incompatible types in assignment (expression Tuple item 10 has type "str"; "int" expected; expression Tuple item 11 has type "str"; "int" expected; Total 2 items are incompatible; 0 items are omitted)
+)
+
+# long initializer assignment with more mismatches, no ellipsis
+t4: Tuple[int, int, int, int, int, int, int, int, int, int, int, int] = (
+    1, 2, 3, 4, 5, 6, 7, 8, "str", "str", "str", "str",  # E: Incompatible types in assignment (expression Tuple item 8 has type "str"; "int" expected; expression Tuple item 9 has type "str"; "int" expected; expression Tuple item 10 has type "str"; "int" expected; Total 4 items are incompatible; 1 items are omitted)
+)
+
+# short tuple initializer assignment, no ellipsis
+t5: Tuple[int, int] = (1, 2, "s", 4)  # E: Incompatible types in assignment (expression has type "Tuple[int, int, str, int]", variable has type "Tuple[int, int]")
+
+# long initializer assignment with mismatched pairs
+t6: Tuple[int, int, int, int, int, int, int, int, int, int, int, int] = (
+    1, 2, 3, 4, 5, 6, 7, 8, "str", "str", "str", "str", 1, 1, 1, 1, 1  # E: Incompatible types in assignment (expression has type Tuple[int, int, ... <15 more items>], variable has type Tuple[int, int, ... <10 more items>]) 
+)
+
+[builtins fixtures/tuple.pyi]


Code review comments:
diff_hunk: @@ -1241,3 +1241,43 @@ b = ("bar", 7)
 reveal_type(a + b)  # N: Revealed type is 'Tuple[builtins.int, builtins.str, builtins.int, builtins.str, builtins.int]'
 
 [builtins fixtures/tuple.pyi]
+
+[case testAssigningWithLongTupleInitializer]
+from typing import Tuple
+
+# long initializer assignment with few mismatches
+t: Tuple[int, ...] = (
+    1, 2, 3, 4, 5, 6, 7, 8, "str", "str", "str", 11, 12, 14, 15,  # E: Incompatible types in assignment (expression Tuple item 8 has type "str"; "int" expected; expression Tuple item 9 has type "str"; "int" expected; expression Tuple item 10 has type "str"; "int" expected; Total 3 items are incompatible; 0 items are omitted)
path: test-data/unit/check-tuples.test
text: @user1:
I think that this is too much information in a single error message. It makes the error message hard to read. We generally split long messages into an error followed by one or more notes. In this case, something like could make sense (it doesn't have to be exactly like this):

```
...: error: Incompatible types in assignment (3 tuple items are incompatible)
...: note: Expression tuple item 8 has type "str"; "int" expected
...: note: Expression tuple item 9 has type "str"; "int" expected
...: note: Expression tuple item 10 has type "str"; "int" expected
```

In a test case you can write something like this to represent this:
```
<code> \
    # E: <message> \
    # N: <note>
...
```

The part "0 items are omitted" seems redundant. It can be left out if no items are omitted.

Even better, the notes could use the line/columns with the tuple item expression as context (when possible; clearly this is not always feasible). This would be particularly helpful when the tuple initializer spans several lines.

@author:
I tried to directly use tuple item from e.g. `subtype.items` as context and no row/column info is shown. I guess I may use it in an incorrect way.  see  1d05e8d

diff_hunk: @@ -4378,6 +4380,61 @@ def infer_issubclass_maps(self, node: CallExpr,
         yes_map, no_map = map(convert_to_typetype, (yes_map, no_map))
         return yes_map, no_map
 
+    def check_long_tuple_assignment(self,
+                               subtype: ProperType,
+                               supertype: ProperType,
+                               context: Context,
+                               msg: str = message_registry.INCOMPATIBLE_TYPES,
+                               subtype_label: Optional[str] = None,
+                               supertype_label: Optional[str] = None,
+                               code: Optional[ErrorCode] = None) -> bool:
+        """Check if assignment error using long tuple initializers."""
+
+        def format_error_msg(msg: str, lhs_types: List[Type], rhs_types: List[Type]) -> str:
path: mypy/checker.py
text: @user1:
Style nit: We generally avoid nested functions (unless they are very short). It would be more consistent if this function was a method (or maybe a module-level function, but this seems so tightly coupled to the surrounding method that it may not be a good idea).

@author:
The current implementation uses two nested functions: `format_long_tuple_type`, `format_error_msg`, should I turn all two of them into class methods(maybe a more clear name for latter one)?

@user1:
Our style is to generally use instance methods even if `self` is not used unless the method is being called from outside the class as `ClsName.method(...)`.