8000 Use a dedicated error code for assignment to method (#14570) · python/mypy@07f6721 · GitHub
[go: up one dir, main page]
Skip to content

Commit 07f6721

Browse files
ilevkivskyiilevkivskyi
authored
Use a dedicated error code for assignment to method (#14570)
Fixes #2427 I also add some special logic, so that `# type: ignore[assignment]` will cover `[method-assign]`.
1 parent 5614ffa commit 07f6721

File tree

6 files changed

+103
-4
lines changed

6 files changed

+103
-4
lines changed

‎docs/source/error_code_list.rst

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,35 @@ Example:
339339
# variable has type "str") [assignment]
340340
r.name = 5
341341
342+
Check that assignment target is not a method [method-assign]
343+
------------------------------------------------------------
344+
345+
In general, assigning to a method on class object or instance (a.k.a.
346+
monkey-patching) is ambiguous in terms of types, since Python's static type
347+
system cannot express difference between bound and unbound callable types.
348+
Consider this example:
349+
350+
.. code-block:: python
351+
352+
class A:
353+
def f(self) -> None: pass
354+
def g(self) -> None: pass
355+
356+
def h(self: A) -> None: pass
357+
358+
A.f = h # type of h is Callable[[A], None]
359+
A().f() # this works
360+
A.f = A().g # type of A().g is Callable[[], None]
361+
A().f() # but this also works at runtime
362+
363+
To prevent the ambiguity, mypy will flag both assignments by default. If this
364+
error code is disabled, mypy will treat all method assignments r.h.s. as unbound,
365+
so the second assignment will still generate an error.
366+
367+
.. note::
368+
369+
This error code is a sub-error code of a wider ``[assignment]`` code.
370+
342371
Check type variable values [type-var]
343372
-------------------------------------
344373

‎docs/source/error_codes.rst

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,3 +113,14 @@ still keep the other two error codes enabled. The overall logic is following:
113113
So one can e.g. enable some code globally, disable it for all tests in
114114
the corresponding config section, and then re-enable it with an inline
115115
comment in some specific test.
116+
117+
Sub-error codes of other error codes
118+
------------------------------------
119+
120+
In rare cases (mostly for backwards compatibility reasons), some error
121+
code may be covered by another, wider error code. For example, an error with
122+
code ``[method-assign]`` can be ignored by ``# type: ignore[assignment]``.
123+
Similar logic works for disabling error codes globally. If a given error code
124+
is a sub code of another one, it must mentioned in the docs for the narrower
125+
code. This hierarchy is not nested, there cannot be sub-error codes of other
126+
sub-error codes.

‎mypy/errorcodes.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,30 @@
55

66
from __future__ import annotations
77

8+
from collections import defaultdict
89
from typing_extensions import Final
910

1011
error_codes: dict[str, ErrorCode] = {}
12+
sub_code_map: dict[str, set[str]] = defaultdict(set)
1113

1214

1315
class ErrorCode:
1416
def __init__(
< 6D40 div aria-hidden="true" class="position-absolute top-0 d-flex user-select-none DiffLineTableCellParts-module__comment-indicator--eI0hb">
15-
self, code: str, description: str, category: str, default_enabled: bool = True
17+
self,
18+
code: str,
19+
description: str,
20+
category: str,
21+
default_enabled: bool = True,
22+
sub_code_of: ErrorCode | None = None,
1623
) -> None:
1724
self.code = code
1825
self.description = description
1926
self.category = category
2027
self.default_enabled = default_enabled
28+
self.sub_code_of = sub_code_of
29+
if sub_code_of is not None:
30+
assert sub_code_of.sub_code_of is None, "Nested subcategories are not supported"
31+
sub_code_map[sub_code_of.code].add(code)
2132
error_codes[code] = self
2233

2334
def __str__(self) -> str:
@@ -51,6 +62,12 @@ def __str__(self) -> str:
5162
ASSIGNMENT: Final[ErrorCode] = ErrorCode(
5263
"assignment", "Check that assigned value is compatible with target", "General"
5364
)
65+
METHOD_ASSIGN: Final[ErrorCode] = ErrorCode(
66+
"method-assign",
67+
"Check that assignment target is not a method",
68+
"General",
69+
sub_code_of=ASSIGNMENT,
70+
)
5471
TYPE_ARG: Final = ErrorCode("type-arg", "Check that generic type arguments are present", "General")
5572
TYPE_VAR: Final = ErrorCode("type-var", "Check that type variable values are valid", "General")
5673
UNION_ATTR: Final = ErrorCode(

‎mypy/errors.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,11 @@ def is_ignored_error(self, line: int, info: ErrorInfo, ignores: dict[int, list[s
586586
# Empty list means that we ignore all errors
587587
return True
588588
if info.code and self.is_error_code_enabled(info.code):
589-
return info.code.code in ignores[line]
589+
return (
590+
info.code.code in ignores[line]
591+
or info.code.sub_code_of is not None
592+
and info.code.sub_code_of.code in ignores[line]
593+
)
590594
return False
591595

592596
def is_error_code_enabled(self, error_code: ErrorCode) -> bool:
@@ -601,6 +605,8 @@ def is_error_code_enabled(self, error_code: ErrorCode) -> bool:
601605
return False
602606
elif error_code in current_mod_enabled:
603607
return True
608+
elif error_code.sub_code_of is not None and error_code.sub_code_of in current_mod_disabled:
609+
return False
604610
else:
605611
return error_code.default_enabled
606612

@@ -641,6 +647,10 @@ def generate_unused_ignore_errors(self, file: str) -> None:
641647
if len(ignored_codes) > 1 and len(unused_ignored_codes) > 0:
642648
unused_codes_message = f"[{', '.join(sorted(unused_ignored_codes))}]"
643649
message = f'Unused "type: ignore{unused_codes_message}" comment'
650+
for unused in unused_ignored_codes:
651+
narrower = set(used_ignored_codes) & codes.sub_code_map[unused]
652+
if narrower:
653+
message += f", use narrower [{', '.join(narrower)}] instead of [{unused}]"
644654
# Don't use report since add_error_info will ignore the error!
645655
info = ErrorInfo(
646656
self.import_context(),

‎mypy/messages.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1402,7 +1402,7 @@ def base_class_definitions_incompatible(
14021402
)
14031403

14041404
def cant_assign_to_method(self, context: Context) -> None:
1405-
self.fail(message_registry.CANNOT_ASSIGN_TO_METHOD, context, code=codes.ASSIGNMENT)
1405+
self.fail(message_registry.CANNOT_ASSIGN_TO_METHOD, context, code=codes.METHOD_ASSIGN)
14061406

14071407
def cant_assign_to_classvar(self, name: str, context: Context) -> None:
14081408
self.fail(f'Cannot assign to class variable "{name}" via instance', context)

‎test-data/unit/check-errorcodes.test

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,7 @@ class A:
672672

673673
def g(self: A) -> None: pass
674674

675-
A.f = g # E: Cannot assign to a method [assignment]
675+
A.f = g # E: Cannot assign to a method [method-assign]
676676

677677
[case testErrorCodeDefinedHereNoteIgnore]
678678
import m
@@ -1006,3 +1006,35 @@ def f():
10061006
# flags: --disable-error-code=annotation-unchecked
10071007
def f():
10081008
x: int = "no" # No warning here
1009+
1010+
[case testMethodAssignmentSuppressed]
1011+
# flags: --disable-error-code=method-assign
1012+
class A:
1013+
def f(self) -> None: pass
1014+
def g(self) -> None: pass
1015+
1016+
def h(self: A) -> None: pass
1017+
1018+
A.f = h
1019+
# This actually works at runtime, but there is no way to express this in current type system
1020+
A.f = A().g # E: Incompatible types in assignment (expression has type "Callable[[], None]", variable has type "Callable[[A], None]") [assignment]
1021+
1022+
[case testMethodAssignCoveredByAssignmentIgnore]
1023+
class A:
1024+
def f(self) -> None: pass
1025+
def h(self: A) -> None: pass
1026+
A.f = h # type: ignore[assignment]
1027+
1028+
[case testMethodAssignCoveredByAssignmentFlag]
1029+
# flags: --disable-error-code=assignment
1030+
class A:
1031+
def f(self) -> None: pass
1032+
def h(self: A) -> None: pass
1033+
A.f = h # OK
1034+
1035+
[case testMethodAssignCoveredByAssignmentUnused]
1036+
# flags: --warn-unused-ignores
1037+
class A:
1038+
def f(self) -> None: pass
1039+
def h(self: A) -> None: pass
1040+
A.f = h # type: ignore[assignment] # E: Unused "type: ignore" comment, use narrower [method-assign] instead of [assignment]

0 commit comments

Comments
 (0)
0