Give arguments a more reasonable location (#14562)

Modifies arguments parsed from the AST to use the AST's row and column
information. Modifies function definitions to not overwrite their
arguments' locations. Modifies incorrect override messages to use the
locations of arguments instead of the method itself. Modifies tests to
expect the new locations.

I'm not sure whether this handles bound arguments correctly; it passes
tests but I don't know whether there's some edge case I'm missing.

Fixes #8298.

mypy/checker.py

@@ -2067,15 +2067,22 @@ def erase_override(t: Type) -> Type:
                     if not is_subtype(
                         original.arg_types[i], erase_override(override.arg_types[i])
                     ):
+
                         arg_type_in_super = original.arg_types[i]
+
+                        if isinstance(node, FuncDef):
+                            context: Context = node.arguments[i + len(override.bound_args)]
+                        else:
+                            context = node
                         self.msg.argument_incompatible_with_supertype(
                             i + 1,
                             name,
                             type_name,
                             name_in_super,
                             arg_type_in_super,
                             supertype,
-                            node,
+                            context,
+                            secondary_context=node,
                         )
                         emitted_msg = True
 

mypy/errors.py

@@ -4,7 +4,7 @@
 import sys
 import traceback
 from collections import defaultdict
-from typing import Callable, NoReturn, Optional, TextIO, Tuple, TypeVar
+from typing import Callable, Iterable, NoReturn, Optional, TextIO, Tuple, TypeVar
 from typing_extensions import Final, Literal, TypeAlias as _TypeAlias
 
 from mypy import errorcodes as codes
@@ -78,7 +78,7 @@ class ErrorInfo:
 
     # Actual origin of the error message as tuple (path, line number, end line number)
     # If end line number is unknown, use line number.
-    origin: tuple[str, int, int]
+    origin: tuple[str, Iterable[int]]
 
     # Fine-grained incremental target where this was reported
     target: str | None = None
@@ -104,7 +104,7 @@ def __init__(
         blocker: bool,
         only_once: bool,
         allow_dups: bool,
-        origin: tuple[str, int, int] | None = None,
+        origin: tuple[str, Iterable[int]] | None = None,
         target: str | None = None,
     ) -> None:
         self.import_ctx = import_ctx
@@ -122,7 +122,7 @@ def __init__(
         self.blocker = blocker
         self.only_once = only_once
         self.allow_dups = allow_dups
-        self.origin = origin or (file, line, line)
+        self.origin = origin or (file, [line])
         self.target = target
 
 
@@ -367,7 +367,7 @@ def report(
         file: str | None = None,
         only_once: bool = False,
         allow_dups: bool = False,
-        origin_span: tuple[int, int] | None = None,
+        origin_span: Iterable[int] | None = None,
         offset: int = 0,
         end_line: int | None = None,
         end_column: int | None = None,
@@ -411,7 +411,7 @@ def report(
             message = " " * offset + message
 
         if origin_span is None:
-            origin_span = (line, line)
+            origin_span = [line]
 
         if end_line is None:
             end_line = line
@@ -434,7 +434,7 @@ def report(
             blocker,
             only_once,
             allow_dups,
-            origin=(self.file, *origin_span),
+            origin=(self.file, origin_span),
             target=self.current_target(),
         )
         self.add_error_info(info)
@@ -467,7 +467,7 @@ def _filter_error(self, file: str, info: ErrorInfo) -> bool:
         return False
 
     def add_error_info(self, info: ErrorInfo) -> None:
-        file, line, end_line = info.origin
+        file, lines = info.origin
         # process the stack of ErrorWatchers before modifying any internal state
         # in case we need to filter out the error entirely
         # NB: we need to do this both here and in _add_error_info, otherwise we
@@ -478,7 +478,7 @@ def add_error_info(self, info: ErrorInfo) -> None:
             if file in self.ignored_lines:
                 # Check each line in this context for "type: ignore" comments.
                 # line == end_line for most nodes, so we only loop once.
-                for scope_line in range(line, end_line + 1):
+                for scope_line in lines:
                     if self.is_ignored_error(scope_line, info, self.ignored_lines[file]):
                         # Annotation requests us to ignore all errors on this line.
                         self.used_ignored_lines[file][scope_line].append(

mypy/fastparse.py

@@ -1081,7 +1081,14 @@ def make_argument(
         if argument_elide_name(arg.arg):
             pos_only = True
 
-        return Argument(Var(arg.arg), arg_type, self.visit(default), kind, pos_only)
+        argument = Argument(Var(arg.arg), arg_type, self.visit(default), kind, pos_only)
+        argument.set_line(
+            arg.lineno,
+            arg.col_offset,
+            getattr(arg, "end_lineno", None),
+            getattr(arg, "end_col_offset", None),
+        )
+        return argument
 
     def fail_arg(self, msg: str, arg: ast3.arg) -> None:
         self.fail(msg, arg.lineno, arg.col_offset)

mypy/messages.py

@@ -12,6 +12,7 @@
 from __future__ import annotations
 
 import difflib
+import itertools
 import re
 from contextlib import contextmanager
 from textwrap import dedent
@@ -208,34 +209,40 @@ def report(
         origin: Context | None = None,
         offset: int = 0,
         allow_dups: bool = False,
+        secondary_context: Context | None = None,
     ) -> None:
         """Report an error or note (unless disabled).
 
         Note that context controls where error is reported, while origin controls
         where # type: ignore comments have effect.
         """
 
-        def span_from_context(ctx: Context) -> tuple[int, int]:
+        def span_from_context(ctx: Context) -> Iterable[int]:
             """This determines where a type: ignore for a given context has effect.
 
             Current logic is a bit tricky, to keep as much backwards compatibility as
             possible. We may reconsider this to always be a single line (or otherwise
             simplify it) when we drop Python 3.7.
             """
             if isinstance(ctx, (ClassDef, FuncDef)):
-                return ctx.deco_line or ctx.line, ctx.line
+                return range(ctx.deco_line or ctx.line, ctx.line + 1)
             elif not isinstance(ctx, Expression):
-                return ctx.line, ctx.line
+                return [ctx.line]
             else:
-                return ctx.line, ctx.end_line or ctx.line
+                return range(ctx.line, (ctx.end_line or ctx.line) + 1)
 
-        origin_span: tuple[int, int] | None
+        origin_span: Iterable[int] | None
         if origin is not None:
             origin_span = span_from_context(origin)
         elif context is not None:
             origin_span = span_from_context(context)
         else:
             origin_span = None
+
+        if secondary_context is not None:
+            assert origin_span is not None
+            origin_span = itertools.chain(origin_span, span_from_context(secondary_context))
+
         self.errors.report(
             context.line if context else -1,
             context.column if context else -1,
@@ -258,9 +265,18 @@ def fail(
         code: ErrorCode | None = None,
         file: str | None = None,
         allow_dups: bool = False,
+        secondary_context: Context | None = None,
     ) -> None:
         """Report an error message (unless disabled)."""
-        self.report(msg, context, "error", code=code, file=file, allow_dups=allow_dups)
+        self.report(
+            msg,
+            context,
+            "error",
+            code=code,
+            file=file,
+            allow_dups=allow_dups,
+            secondary_context=secondary_context,
+        )
 
     def note(
         self,
@@ -272,6 +288,7 @@ def note(
         allow_dups: bool = False,
         *,
         code: ErrorCode | None = None,
+        secondary_context: Context | None = None,
     ) -> None:
         """Report a note (unless disabled)."""
         self.report(
@@ -283,6 +300,7 @@ def note(
             offset=offset,
             allow_dups=allow_dups,
             code=code,
+            secondary_context=secondary_context,
         )
 
     def note_multiline(
@@ -293,11 +311,20 @@ def note_multiline(
         offset: int = 0,
         allow_dups: bool = False,
         code: ErrorCode | None = None,
+        *,
+        secondary_context: Context | None = None,
     ) -> None:
         """Report as many notes as lines in the message (unless disabled)."""
         for msg in messages.splitlines():
             self.report(
-                msg, context, "note", file=file, offset=offset, allow_dups=allow_dups, code=code
+                msg,
+                context,
+                "note",
+                file=file,
+                offset=offset,
+                allow_dups=allow_dups,
+                code=code,
+                secondary_context=secondary_context,
             )
 
     #
@@ -1151,6 +1178,7 @@ def argument_incompatible_with_supertype(
         arg_type_in_supertype: Type,
         supertype: str,
         context: Context,
+        secondary_context: Context,
     ) -> None:
         target = self.override_target(name, name_in_supertype, supertype)
         arg_type_in_supertype_f = format_type_bare(arg_type_in_supertype)
@@ -1161,17 +1189,26 @@ def argument_incompatible_with_supertype(
             ),
             context,
             code=codes.OVERRIDE,
+            secondary_context=secondary_context,
+        )
+        self.note(
+            "This violates the Liskov substitution principle",
+            context,
+            code=codes.OVERRIDE,
+            secondary_context=secondary_context,
         )
-        self.note("This violates the Liskov substitution principle", context, code=codes.OVERRIDE)
         self.note(
             "See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides",
             context,
             code=codes.OVERRIDE,
+            secondary_context=secondary_context,
         )
 
         if name == "__eq__" and type_name:
             multiline_msg = self.comparison_method_example_msg(class_name=type_name)
-            self.note_multiline(multiline_msg, context, code=codes.OVERRIDE)
+            self.note_multiline(
+                multiline_msg, context, code=codes.OVERRIDE, secondary_context=secondary_context
+            )
 
     def comparison_method_example_msg(self, class_name: str) -> str:
         return dedent(

mypy/nodes.py

@@ -701,18 +701,6 @@ def __init__(
     def max_fixed_argc(self) -> int:
         return self.max_pos
 
-    def set_line(
-        self,
-        target: Context | int,
-        column: int | None = None,
-        end_line: int | None = None,
-        end_column: int | None = None,
-    ) -> None:
-        super().set_line(target, column, end_line, end_column)
-        for arg in self.arguments:
-            # TODO: set arguments line/column to their precise locations.
-            arg.set_line(self.line, self.column, self.end_line, end_column)
-
     def is_dynamic(self) -> bool:
         return self.type is None
 

mypy/test/data.py

@@ -676,8 +676,8 @@ class DataFileCollector(pytest.Collector):
     parent: DataSuiteCollector
 
     @classmethod  # We have to fight with pytest here:
-    def from_parent(  # type: ignore[override]
-        cls, parent: DataSuiteCollector, *, name: str
+    def from_parent(
+        cls, parent: DataSuiteCollector, *, name: str  # type: ignore[override]
     ) -> DataFileCollector:
         collector = super().from_parent(parent, name=name)
         assert isinstance(collector, DataFileCollector)

test-data/unit/check-abstract.test

@@ -382,10 +382,10 @@ class A(I):
     def g(self, a: 'A') -> 'A':
         return A()
 [out]
+main:11: error: Return type "I" of "h" incompatible with return type "A" in supertype "I"
 main:11: error: Argument 1 of "h" is incompatible with supertype "I"; supertype defines the argument type as "I"
 main:11: note: This violates the Liskov substitution principle
 main:11: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
-main:11: error: Return type "I" of "h" incompatible with return type "A" in supertype "I"
 
 
 -- Accessing abstract members

test-data/unit/check-classes.test

@@ -331,6 +331,59 @@ main:7: note: This violates the Liskov substitution principle
 main:7: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
 main:9: error: Return type "object" of "h" incompatible with return type "A" in supertype "A"
 
+[case testMethodOverridingWithIncompatibleTypesOnMultipleLines]
+class A:
+    def f(self, x: int, y: str) -> None: pass
+class B(A):
+    def f(
+        self,
+        x: int,
+        y: bool,
+    ) -> None:
+        pass
+[out]
+main:7: error: Argument 2 of "f" is incompatible with supertype "A"; supertype defines the argument type as "str"
+main:7: note: This violates the Liskov substitution principle
+main:7: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
+
+[case testMultiLineMethodOverridingWithIncompatibleTypesIgnorableAtArgument]
+class A:
+    def f(self, x: int, y: str) -> None: pass
+
+class B(A):
+    def f(
+        self,
+        x: int,
+        y: bool,  # type: ignore[override]
+    ) -> None:
+        pass
+
+[case testMultiLineMethodOverridingWithIncompatibleTypesIgnorableAtDefinition]
+class A:
+    def f(self, x: int, y: str) -> None: pass
+class B(A):
+    def f(  # type: ignore[override]
+        self,
+        x: int,
+        y: bool,
+    ) -> None:
+        pass
+
+[case testMultiLineMethodOverridingWithIncompatibleTypesWrongIgnore]
+class A:
+    def f(self, x: int, y: str) -> None: pass
+class B(A):
+    def f(  # type: ignore[return-type]
+        self,
+        x: int,
+        y: bool,
+    ) -> None:
+        pass
+[out]
+main:7: error: Argument 2 of "f" is incompatible with supertype "A"; supertype defines the argument type as "str"
+main:7: note: This violates the Liskov substitution principle
+main:7: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
+
 [case testEqMethodsOverridingWithNonObjects]
 class A:
   def __eq__(self, other: A) -> bool: pass  # Fail
@@ -2626,10 +2679,10 @@ class D(A):
     def __iadd__(self, x: 'A') -> 'B': pass
 [out]
 main:6: error: Return type "A" of "__iadd__" incompatible with return type "B" in "__add__" of supertype "A"
+main:8: error: Signatures of "__iadd__" and "__add__" are incompatible
 main:8: error: Argument 1 of "__iadd__" is incompatible with "__add__" of supertype "A"; supertype defines the argument type as "A"
 main:8: note: This violates the Liskov substitution principle
 main:8: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
-main:8: error: Signatures of "__iadd__" and "__add__" are incompatible
 
 [case testGetattribute]
 

test-data/unit/check-columns.test

@@ -238,9 +238,9 @@ if int():
 class A:
     def f(self, x: int) -> None: pass
 class B(A):
-    def f(self, x: str) -> None: pass # E:5: Argument 1 of "f" is incompatible with supertype "A"; supertype defines the argument type as "int" \
-                                      # N:5: This violates the Liskov substitution principle \
-                                      # N:5: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
+    def f(self, x: str) -> None: pass # E:17: Argument 1 of "f" is incompatible with supertype "A"; supertype defines the argument type as "int" \
+                                      # N:17: This violates the Liskov substitution principle \
+                                      # N:17: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
 class C(A):
     def f(self, x: int) -> int: pass # E:5: Return type "int" of "f" incompatible with return type "None" in supertype "A"
 class D(A):