Use standard formatting for note on unexpected keyword (#20808)

Closes #4773
Closes #10480
Closes #20640

Currently this is the only message that uses this non-standard
formatting, while:
* Some users find it confusing
* We need ~400 lines of code to support this special case
* It still has bugs that will require even more special-casing in
various places to fix them.

So here I implement a minimal change I propose in
#4773, namely use this (standard)
format:
```
main.py:5: error: Unexpected keyword argument "location" for "method" of "Class"
main.py:5: note: "method" defined in "pkg.mod"
```
if the function is defined in a different module.

If we want to, we can add extra info (like available argument names with
same type), as well as start using this note in more places in separate
PRs.

Author: ilevkivskyi

mypy/build.py

@@ -90,14 +90,7 @@
     WORKER_START_TIMEOUT,
 )
 from mypy.error_formatter import OUTPUT_CHOICES, ErrorFormatter
-from mypy.errors import (
-    CompileError,
-    ErrorInfo,
-    Errors,
-    ErrorTuple,
-    ErrorTupleRaw,
-    report_internal_error,
-)
+from mypy.errors import CompileError, ErrorInfo, Errors, ErrorTuple, report_internal_error
 from mypy.graph_utils import prepare_sccs, strongly_connected_components, topsort
 from mypy.indirection import TypeIndirectionVisitor
 from mypy.ipc import (
@@ -111,21 +104,11 @@
     send,
 )
 from mypy.messages import MessageBuilder
-from mypy.nodes import (
-    ClassDef,
-    Context,
-    Import,
-    ImportAll,
-    ImportBase,
-    ImportFrom,
-    MypyFile,
-    SymbolTable,
-)
+from mypy.nodes import Import, ImportAll, ImportBase, ImportFrom, MypyFile, SymbolTable
 from mypy.options import OPTIONS_AFFECTING_CACHE_NO_PLATFORM
 from mypy.partially_defined import PossiblyUndefinedVariableVisitor
 from mypy.semanal import SemanticAnalyzer
 from mypy.semanal_pass1 import SemanticAnalyzerPreAnalysis
-from mypy.traverser import find_definitions
 from mypy.util import (
     DecodeError,
     decode_python_encoding,
@@ -908,10 +891,6 @@ def __init__(
         self.queue_order: int = 0
         # Is this an instance used by a parallel worker?
         self.parallel_worker = parallel_worker
-        # Cache for ASTs created during error message generation. Note these are
-        # raw parsed trees not analyzed with mypy. We use these to find absolute
-        # location of a symbol used as a location for an error message.
-        self.extra_trees: dict[str, MypyFile] = {}
         # Snapshot of import-related options per module. We record these even for
         # suppressed imports, since they can affect errors in the callers. Bytes
         # value is opaque but can be compared to detect changes in options.
@@ -1080,60 +1059,6 @@ def report_file(
         if self.reports is not None and self.source_set.is_source(file):
             self.reports.file(file, self.modules, type_map, options)
 
-    def resolve_location(self, graph: dict[str, State], fullname: str) -> Context | None:
-        """Resolve an absolute location of a symbol with given fullname."""
-        rest = []
-        head = fullname
-        while True:
-            # TODO: this mimics the logic in lookup.py but it is actually wrong.
-            # This is because we don't distinguish between submodule and a local symbol
-            # with the same name.
-            head, tail = head.rsplit(".", maxsplit=1)
-            rest.append(tail)
-            if head in graph:
-                state = graph[head]
-                break
-            if "." not in head:
-                return None
-        *prefix, name = reversed(rest)
-        # If this happens something is wrong, but it is better to give slightly
-        # less helpful error message than crash.
-        if state.path is None:
-            return None
-        if state.tree is not None and state.tree.defs:
-            # We usually free ASTs after processing, but reuse an existing AST if
-            # it is still available.
-            tree = state.tree
-        elif state.id in self.extra_trees:
-            tree = self.extra_trees[state.id]
-        else:
-            if state.source is not None:
-                # Sources are usually discarded after processing as well, check
-                # if we still have one just in case.
-                source = state.source
-            else:
-                path = state.manager.maybe_swap_for_shadow_path(state.path)
-                source = decode_python_encoding(state.manager.fscache.read(path))
-            tree = parse(source, state.path, state.id, state.manager.errors, state.options)
-            # TODO: run first pass of semantic analysis on freshly parsed trees,
-            # we need this to get correct reachability information.
-            self.extra_trees[state.id] = tree
-        statements = tree.defs
-        while prefix:
-            part = prefix.pop(0)
-            for statement in statements:
-                defs = find_definitions(statement, part)
-                if not defs or not isinstance((defn := defs[0]), ClassDef):
-                    continue
-                statements = defn.defs.body
-                break
-            else:
-                return None
-        for statement in statements:
-            if defs := find_definitions(statement, name):
-                return defs[0]
-        return None
-
     def verbosity(self) -> int:
         return self.options.verbosity
 
@@ -3999,9 +3924,7 @@ def find_stale_sccs(
                     path = manager.errors.simplify_path(graph[id].xpath)
                     formatted = manager.errors.format_messages(
                         path,
-                        transform_error_tuples(
-                            manager, graph, deserialize_codes(graph[id].error_lines)
-                        ),
+                        deserialize_codes(graph[id].error_lines),
                         formatter=manager.error_formatter,
                     )
                     manager.flush_errors(path, formatted, False)
@@ -4271,9 +4194,7 @@ def process_stale_scc(
         if graph[id].xpath not in manager.errors.ignored_files:
             errors = manager.errors.file_messages(graph[id].xpath)
             formatted = manager.errors.format_messages(
-                graph[id].xpath,
-                transform_error_tuples(manager, graph, errors),
-                formatter=manager.error_formatter,
+                graph[id].xpath, errors, formatter=manager.error_formatter
             )
             manager.flush_errors(manager.errors.simplify_path(graph[id].xpath), formatted, False)
             errors_by_id[id] = errors
@@ -4455,37 +4376,14 @@ def write_undocumented_ref_info(
     metastore.write(ref_info_file, json_dumps(deps_json))
 
 
-def transform_error_tuples(
-    manager: BuildManager, graph: dict[str, State], error_tuples_rel: list[ErrorTupleRaw]
-) -> list[ErrorTuple]:
-    """Transform raw error tuples by resolving relative error locations."""
-    error_tuples = []
-    for e in error_tuples_rel:
-        file, line_rel, column, end_line, end_column, severity, message, code = e
-        if isinstance(line_rel, int):
-            line = line_rel
-        else:
-            assert file is not None
-            loc = manager.resolve_location(graph, line_rel)
-            if loc is not None:
-                line = loc.line
-                column = loc.column
-                end_line = loc.end_line or -1
-                end_column = loc.end_column or -1
-            else:
-                line = -1
-        error_tuples.append((file, line, column, end_line, end_column, severity, message, code))
-    return error_tuples
-
-
-def serialize_codes(errs: list[ErrorTupleRaw]) -> list[SerializedError]:
+def serialize_codes(errs: list[ErrorTuple]) -> list[SerializedError]:
     return [
         (path, line, column, end_line, end_column, severity, message, code.code if code else None)
         for path, line, column, end_line, end_column, severity, message, code in errs
     ]
 
 
-def deserialize_codes(errs: list[SerializedError]) -> list[ErrorTupleRaw]:
+def deserialize_codes(errs: list[SerializedError]) -> list[ErrorTuple]:
     return [
         (
             path,

mypy/cache.py

@@ -69,9 +69,9 @@
 from mypy_extensions import u8
 
 # High-level cache layout format
-CACHE_VERSION: Final = 4
+CACHE_VERSION: Final = 5
 
-SerializedError: _TypeAlias = tuple[str | None, int | str, int, int, int, str, str, str | None]
+SerializedError: _TypeAlias = tuple[str | None, int, int, int, int, str, str, str | None]
 
 
 class CacheMeta:
@@ -504,10 +504,7 @@ def write_errors(data: WriteBuffer, errs: list[SerializedError]) -> None:
     for path, line, column, end_line, end_column, severity, message, code in errs:
         write_tag(data, TUPLE_GEN)
         write_str_opt(data, path)
-        if isinstance(line, str):
-            write_str(data, line)
-        else:
-            write_int(data, line)
+        write_int(data, line)
         write_int(data, column)
         write_int(data, end_line)
         write_int(data, end_column)
@@ -521,17 +518,10 @@ def read_errors(data: ReadBuffer) -> list[SerializedError]:
     result = []
     for _ in range(read_int_bare(data)):
         assert read_tag(data) == TUPLE_GEN
-        path = read_str_opt(data)
-        tag = read_tag(data)
-        if tag == LITERAL_STR:
-            line: str | int = read_str_bare(data)
-        else:
-            assert tag == LITERAL_INT
-            line = read_int_bare(data)
         result.append(
             (
-                path,
-                line,
+                read_str_opt(data),
+                read_int(data),
                 read_int(data),
                 read_int(data),
                 read_int(data),

mypy/errors.py

@@ -130,7 +130,6 @@ def __init__(
         target: str | None = None,
         priority: int = 0,
         parent_error: ErrorInfo | None = None,
-        location_ref: str | None = None,
     ) -> None:
         self.import_ctx = import_ctx
         self.file = file
@@ -152,18 +151,12 @@ def __init__(
         if parent_error is not None:
             assert severity == "note", "Only notes can specify parent errors"
         self.parent_error = parent_error
-        self.location_ref = location_ref
 
 
 # Type used internally to represent errors:
 #   (path, line, column, end_line, end_column, severity, message, code)
 ErrorTuple: _TypeAlias = tuple[str | None, int, int, int, int, str, str, ErrorCode | None]
 
-# A raw version of the above that can refer to either absolute or relative location.
-# If the location is relative, the first item (line) is a string with a symbol fullname,
-# and three other values (column, end_line, end_column) are set to -1.
-ErrorTupleRaw: _TypeAlias = tuple[str | None, int | str, int, int, int, str, str, ErrorCode | None]
-
 
 class ErrorWatcher:
     """Context manager that can be used to keep track of new errors recorded
@@ -569,14 +562,12 @@ def report(
         *,
         blocker: bool = False,
         severity: str = "error",
-        file: str | None = None,
         only_once: bool = False,
         origin_span: Iterable[int] | None = None,
         offset: int = 0,
         end_line: int | None = None,
         end_column: int | None = None,
         parent_error: ErrorInfo | None = None,
-        location_ref: str | None = None,
     ) -> ErrorInfo:
         """Report message at the given line using the current error context.
 
@@ -587,7 +578,6 @@ def report(
             code: error code (defaults to 'misc'; not shown for notes)
             blocker: if True, don't continue analysis after this error
             severity: 'error' or 'note'
-            file: if non-None, override current file as context
             only_once: if True, only report this exact message once per build
             origin_span: if non-None, override current context as origin
                          (type: ignores have effect here)
@@ -611,8 +601,6 @@ def report(
             else:
                 end_column = column + 1
 
-        if file is None:
-            file = self.file
         if offset:
             message = " " * offset + message
 
@@ -627,7 +615,7 @@ def report(
 
         info = ErrorInfo(
             import_ctx=self.import_context(),
-            file=file,
+            file=self.file,
             module=self.current_module(),
             typ=type,
             function_or_member=function,
@@ -643,7 +631,6 @@ def report(
             origin=(self.file, origin_span),
             target=self.current_target(),
             parent_error=parent_error,
-            location_ref=location_ref,
         )
         self.add_error_info(info)
         return info
@@ -1019,8 +1006,6 @@ def raise_error(self, use_stdout: bool = True) -> NoReturn:
         """
         # self.new_messages() will format all messages that haven't already
         # been returned from a file_messages() call.
-        # TODO: pass resolve_location callback here.
-        # This will be needed if we are going to use relative locations in blocker errors.
         raise CompileError(
             self.new_messages(), use_stdout=use_stdout, module_with_blocker=self.blocker_module()
         )
@@ -1084,7 +1069,7 @@ def format_messages_default(
                     a.append(" " * (DEFAULT_SOURCE_OFFSET + column) + marker)
         return a
 
-    def file_messages(self, path: str) -> list[ErrorTupleRaw]:
+    def file_messages(self, path: str) -> list[ErrorTuple]:
         """Return an error tuple list of new error messages from a given file."""
         if path not in self.error_info_map:
             return []
@@ -1127,9 +1112,7 @@ def find_shadow_file_mapping(self, path: str) -> str | None:
                 return i[1]
         return None
 
-    def new_messages(
-        self, resolve_location: Callable[[str], Context | None] | None = None
-    ) -> list[str]:
+    def new_messages(self) -> list[str]:
         """Return a string list of new error messages.
 
         Use a form suitable for displaying to the user.
@@ -1139,29 +1122,7 @@ def new_messages(
         msgs = []
         for path in self.error_info_map.keys():
             if path not in self.flushed_files:
-                error_tuples_rel = self.file_messages(path)
-                error_tuples = []
-                for e in error_tuples_rel:
-                    # This has a bit of code duplication with build.py, but it is hard
-                    # to avoid without either an import cycle or a performance penalty.
-                    file, line_rel, column, end_line, end_column, severity, message, code = e
-                    if isinstance(line_rel, int):
-                        line = line_rel
-                    elif resolve_location is not None:
-                        assert file is not None
-                        loc = resolve_location(line_rel)
-                        if loc is not None:
-                            line = loc.line
-                            column = loc.column
-                            end_line = loc.end_line or -1
-                            end_column = loc.end_column or -1
-                        else:
-                            line = -1
-                    else:
-                        line = -1
-                    error_tuples.append(
-                        (file, line, column, end_line, end_column, severity, message, code)
-                    )
+                error_tuples = self.file_messages(path)
                 msgs.extend(self.format_messages(path, error_tuples))
         return msgs
 
@@ -1173,15 +1134,15 @@ def targets(self) -> set[str]:
             info.target for errs in self.error_info_map.values() for info in errs if info.target
         }
 
-    def render_messages(self, errors: list[ErrorInfo]) -> list[ErrorTupleRaw]:
+    def render_messages(self, errors: list[ErrorInfo]) -> list[ErrorTuple]:
         """Translate the messages into a sequence of tuples.
 
         Each tuple is of form (path, line, col, severity, message, code).
         The rendered sequence includes information about error contexts.
         The path item may be None. If the line item is negative, the
         line number is not defined for the tuple.
         """
-        result: list[ErrorTupleRaw] = []
+        result: list[ErrorTuple] = []
         prev_import_context: list[tuple[str, int]] = []
         prev_function_or_member: str | None = None
         prev_type: str | None = None
@@ -1256,21 +1217,9 @@ def render_messages(self, errors: list[ErrorInfo]) -> list[ErrorTupleRaw]:
                 else:
                     result.append((file, -1, -1, -1, -1, "note", f'In class "{e.type}":', None))
 
-            if e.location_ref is not None:
-                result.append((file, e.location_ref, -1, -1, -1, e.severity, e.message, e.code))
-            else:
-                result.append(
-                    (
-                        file,
-                        e.line,
-                        e.column,
-                        e.end_line,
-                        e.end_column,
-                        e.severity,
-                        e.message,
-                        e.code,
-                    )
-                )
+            result.append(
+                (file, e.line, e.column, e.end_line, e.end_column, e.severity, e.message, e.code)
+            )
 
             prev_import_context = e.import_ctx
             prev_function_or_member = e.function_or_member