Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give arguments a more reasonable location #14562

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
18 changes: 9 additions & 9 deletions mypy/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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


Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand Down
9 changes: 8 additions & 1 deletion mypy/fastparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
55 changes: 46 additions & 9 deletions mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from __future__ import annotations

import difflib
import itertools
import re
from contextlib import contextmanager
from textwrap import dedent
Expand Down Expand Up @@ -210,34 +211,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,
Expand All @@ -260,9 +267,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,
Expand All @@ -274,6 +290,7 @@ def note(
allow_dups: bool = False,
*,
code: ErrorCode | None = None,
secondary_context: Context | None = None,
) -> None:
"""Report a note (unless disabled)."""
self.report(
Expand All @@ -285,6 +302,7 @@ def note(
offset=offset,
allow_dups=allow_dups,
code=code,
secondary_context=secondary_context,
)

def note_multiline(
Expand All @@ -295,11 +313,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,
)

#
Expand Down Expand Up @@ -1153,6 +1180,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)
Expand All @@ -1163,17 +1191,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(
Expand Down
12 changes: 0 additions & 12 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -704,18 +704,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

Expand Down
4 changes: 2 additions & 2 deletions mypy/test/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -677,8 +677,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)
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-abstract.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
55 changes: 54 additions & 1 deletion test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]

Expand Down
6 changes: 3 additions & 3 deletions test-data/unit/check-columns.test
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Loading