Skip to content

Commit

Permalink
Use a dedicated error code for assignment to method (#14570)
Browse files Browse the repository at this point in the history
Fixes #2427

I also add some special logic, so that `# type: ignore[assignment]` will cover `[method-assign]`.
  • Loading branch information
ilevkivskyi authored Feb 5, 2023
1 parent 5614ffa commit 07f6721
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 4 deletions.
29 changes: 29 additions & 0 deletions docs/source/error_code_list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,35 @@ Example:
# variable has type "str") [assignment]
r.name = 5
Check that assignment target is not a method [method-assign]
------------------------------------------------------------

In general, assigning to a method on class object or instance (a.k.a.
monkey-patching) is ambiguous in terms of types, since Python's static type
system cannot express difference between bound and unbound callable types.
Consider this example:

.. code-block:: python
class A:
def f(self) -> None: pass
def g(self) -> None: pass
def h(self: A) -> None: pass
A.f = h # type of h is Callable[[A], None]
A().f() # this works
A.f = A().g # type of A().g is Callable[[], None]
A().f() # but this also works at runtime
To prevent the ambiguity, mypy will flag both assignments by default. If this
error code is disabled, mypy will treat all method assignments r.h.s. as unbound,
so the second assignment will still generate an error.

.. note::

This error code is a sub-error code of a wider ``[assignment]`` code.

Check type variable values [type-var]
-------------------------------------

Expand Down
11 changes: 11 additions & 0 deletions docs/source/error_codes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,14 @@ still keep the other two error codes enabled. The overall logic is following:
So one can e.g. enable some code globally, disable it for all tests in
the corresponding config section, and then re-enable it with an inline
comment in some specific test.

Sub-error codes of other error codes
------------------------------------

In rare cases (mostly for backwards compatibility reasons), some error
code may be covered by another, wider error code. For example, an error with
code ``[method-assign]`` can be ignored by ``# type: ignore[assignment]``.
Similar logic works for disabling error codes globally. If a given error code
is a sub code of another one, it must mentioned in the docs for the narrower
code. This hierarchy is not nested, there cannot be sub-error codes of other
sub-error codes.
19 changes: 18 additions & 1 deletion mypy/errorcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,30 @@

from __future__ import annotations

from collections import defaultdict
from typing_extensions import Final

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


class ErrorCode:
def __init__(
self, code: str, description: str, category: str, default_enabled: bool = True
self,
code: str,
description: str,
category: str,
default_enabled: bool = True,
sub_code_of: ErrorCode | None = None,
) -> None:
self.code = code
self.description = description
self.category = category
self.default_enabled = default_enabled
self.sub_code_of = sub_code_of
if sub_code_of is not None:
assert sub_code_of.sub_code_of is None, "Nested subcategories are not supported"
sub_code_map[sub_code_of.code].add(code)
error_codes[code] = self

def __str__(self) -> str:
Expand Down Expand Up @@ -51,6 +62,12 @@ def __str__(self) -> str:
ASSIGNMENT: Final[ErrorCode] = ErrorCode(
"assignment", "Check that assigned value is compatible with target", "General"
)
METHOD_ASSIGN: Final[ErrorCode] = ErrorCode(
"method-assign",
"Check that assignment target is not a method",
"General",
sub_code_of=ASSIGNMENT,
)
TYPE_ARG: Final = ErrorCode("type-arg", "Check that generic type arguments are present", "General")
TYPE_VAR: Final = ErrorCode("type-var", "Check that type variable values are valid", "General")
UNION_ATTR: Final = ErrorCode(
Expand Down
12 changes: 11 additions & 1 deletion mypy/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,11 @@ def is_ignored_error(self, line: int, info: ErrorInfo, ignores: dict[int, list[s
# Empty list means that we ignore all errors
return True
if info.code and self.is_error_code_enabled(info.code):
return info.code.code in ignores[line]
return (
info.code.code in ignores[line]
or info.code.sub_code_of is not None
and info.code.sub_code_of.code in ignores[line]
)
return False

def is_error_code_enabled(self, error_code: ErrorCode) -> bool:
Expand All @@ -601,6 +605,8 @@ def is_error_code_enabled(self, error_code: ErrorCode) -> bool:
return False
elif error_code in current_mod_enabled:
return True
elif error_code.sub_code_of is not None and error_code.sub_code_of in current_mod_disabled:
return False
else:
return error_code.default_enabled

Expand Down Expand Up @@ -641,6 +647,10 @@ def generate_unused_ignore_errors(self, file: str) -> None:
if len(ignored_codes) > 1 and len(unused_ignored_codes) > 0:
unused_codes_message = f"[{', '.join(sorted(unused_ignored_codes))}]"
message = f'Unused "type: ignore{unused_codes_message}" comment'
for unused in unused_ignored_codes:
narrower = set(used_ignored_codes) & codes.sub_code_map[unused]
if narrower:
message += f", use narrower [{', '.join(narrower)}] instead of [{unused}]"
# Don't use report since add_error_info will ignore the error!
info = ErrorInfo(
self.import_context(),
Expand Down
2 changes: 1 addition & 1 deletion mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -1402,7 +1402,7 @@ def base_class_definitions_incompatible(
)

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

def cant_assign_to_classvar(self, name: str, context: Context) -> None:
self.fail(f'Cannot assign to class variable "{name}" via instance', context)
Expand Down
34 changes: 33 additions & 1 deletion test-data/unit/check-errorcodes.test
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ class A:

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

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

[case testErrorCodeDefinedHereNoteIgnore]
import m
Expand Down Expand Up @@ -1006,3 +1006,35 @@ def f():
# flags: --disable-error-code=annotation-unchecked
def f():
x: int = "no" # No warning here

[case testMethodAssignmentSuppressed]
# flags: --disable-error-code=method-assign
class A:
def f(self) -> None: pass
def g(self) -> None: pass

def h(self: A) -> None: pass

A.f = h
# This actually works at runtime, but there is no way to express this in current type system
A.f = A().g # E: Incompatible types in assignment (expression has type "Callable[[], None]", variable has type "Callable[[A], None]") [assignment]

[case testMethodAssignCoveredByAssignmentIgnore]
class A:
def f(self) -> None: pass
def h(self: A) -> None: pass
A.f = h # type: ignore[assignment]

[case testMethodAssignCoveredByAssignmentFlag]
# flags: --disable-error-code=assignment
class A:
def f(self) -> None: pass
def h(self: A) -> None: pass
A.f = h # OK

[case testMethodAssignCoveredByAssignmentUnused]
# flags: --warn-unused-ignores
class A:
def f(self) -> None: pass
def h(self: A) -> None: pass
A.f = h # type: ignore[assignment] # E: Unused "type: ignore" comment, use narrower [method-assign] instead of [assignment]

0 comments on commit 07f6721

Please sign in to comment.