From 0619809bb110b87b114b28ff0cc67bdba9f5b688 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Tue, 31 Jan 2023 23:19:49 +0000 Subject: [PATCH 1/4] Use a dedicated error code for assignment to method --- docs/source/error_code_list.rst | 27 ++++++++++++++++++++++++++- mypy/errorcodes.py | 3 +++ mypy/messages.py | 2 +- test-data/unit/check-errorcodes.test | 12 ++++++++++++ 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/docs/source/error_code_list.rst b/docs/source/error_code_list.rst index 674ad08c4d09..062def1e287f 100644 --- a/docs/source/error_code_list.rst +++ b/docs/source/error_code_list.rst @@ -322,6 +322,31 @@ 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. + Check type variable values [type-var] ------------------------------------- @@ -430,7 +455,7 @@ Example: # Error: Incompatible types (expression has type "float", # TypedDict item "x" has type "int") [typeddict-item] p: Point = {'x': 1.2, 'y': 4} - + Check TypedDict Keys [typeddict-unknown-key] -------------------------------------------- diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index ab49e70eaf20..9b028a3fc13e 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -51,6 +51,9 @@ 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" +) 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( diff --git a/mypy/messages.py b/mypy/messages.py index a5fd09493456..23b6f7c0e991 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -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) diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 8c6a446d101e..f540972cff91 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -1006,3 +1006,15 @@ 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] From df8da8f149fb2bc740f2379c09f795c0c0c68f99 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Wed, 1 Feb 2023 00:02:10 +0000 Subject: [PATCH 2/4] Update a test --- test-data/unit/check-errorcodes.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index f540972cff91..181d93ce3ffe 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -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 From e4c328988ee189ffe2e8e77d088a265c7f452908 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 3 Feb 2023 00:23:28 +0000 Subject: [PATCH 3/4] Add support for sub-error codes, and use it --- docs/source/error_code_list.rst | 4 ++++ docs/source/error_codes.rst | 11 +++++++++++ mypy/errorcodes.py | 15 +++++++++++++-- mypy/errors.py | 8 +++++++- test-data/unit/check-errorcodes.test | 13 +++++++++++++ 5 files changed, 48 insertions(+), 3 deletions(-) diff --git a/docs/source/error_code_list.rst b/docs/source/error_code_list.rst index 062def1e287f..817d3024e565 100644 --- a/docs/source/error_code_list.rst +++ b/docs/source/error_code_list.rst @@ -347,6 +347,10 @@ 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] ------------------------------------- diff --git a/docs/source/error_codes.rst b/docs/source/error_codes.rst index aabedf87f73a..34bb8ab6b5e1 100644 --- a/docs/source/error_codes.rst +++ b/docs/source/error_codes.rst @@ -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. diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index 9b028a3fc13e..ecbada2da3f7 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -12,12 +12,20 @@ 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" error_codes[code] = self def __str__(self) -> str: @@ -52,7 +60,10 @@ def __str__(self) -> str: "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" + "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") diff --git a/mypy/errors.py b/mypy/errors.py index d1e13ad701fc..c81e098b761b 100644 --- a/mypy/errors.py +++ b/mypy/errors.py @@ -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: @@ -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 diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 181d93ce3ffe..db8b41f9eebd 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -1018,3 +1018,16 @@ 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 From 67c580cda2f453ac50d2596a863a28236e3c6f23 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 3 Feb 2023 13:12:08 +0000 Subject: [PATCH 4/4] Custom error message for unused ignores --- mypy/errorcodes.py | 3 +++ mypy/errors.py | 4 ++++ test-data/unit/check-errorcodes.test | 7 +++++++ 3 files changed, 14 insertions(+) diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index ecbada2da3f7..8881a767d72e 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -5,9 +5,11 @@ 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: @@ -26,6 +28,7 @@ def __init__( 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: diff --git a/mypy/errors.py b/mypy/errors.py index c81e098b761b..7cc0c5764861 100644 --- a/mypy/errors.py +++ b/mypy/errors.py @@ -647,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(), diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index db8b41f9eebd..6e848e6a1e39 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -1031,3 +1031,10 @@ 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]