From 3ae10f2de51f894449a7c6c703bfa19ec50badec Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 2 Nov 2023 23:50:10 +0000 Subject: [PATCH 1/4] Add error code for mutable covariant override --- docs/source/error_code_list2.rst | 31 ++++++++++++++++++++++++++++ mypy/checker.py | 21 ++++++++++++++++--- mypy/errorcodes.py | 6 ++++++ mypy/message_registry.py | 3 +++ test-data/unit/check-errorcodes.test | 24 +++++++++++++++++++++ 5 files changed, 82 insertions(+), 3 deletions(-) diff --git a/docs/source/error_code_list2.rst b/docs/source/error_code_list2.rst index cc5c9b0a1bc68..a6bd8d2917ad9 100644 --- a/docs/source/error_code_list2.rst +++ b/docs/source/error_code_list2.rst @@ -482,6 +482,37 @@ Example: def g(self, y: int) -> None: pass +.. _code-mutable-override: + +Check that overrides of mutable attributes are safe +--------------------------------------------------- + +This will enable the check for unsafe overrides of mutable attributes. For +historical reasons, and because this is a relatively common pattern in Python, +this check is not enabled by default. The example below is unsafe, and will be +flagged when this error code is enabled: + +.. code-block:: python + + from typing import Any + + class C: + x: float + y: float + z: float + + class D(C): + x: int # Error: Covariant override of a mutable attribute + # (base class "C" defined the type as "float", + # expression has type "int") [mutable-override] + y: float + z: Any + + def f(c: C) -> None: + c.x = 1.1 + d = D() + f(d) + d.x >> 1 # This will crash at runtime, because d.x is now float, not an int .. _code-unimported-reveal: diff --git a/mypy/checker.py b/mypy/checker.py index f51ba746ea758..e4eb58d40715d 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2041,7 +2041,6 @@ def check_method_override_for_base_with_name( pass elif isinstance(original_type, FunctionLike) and isinstance(typ, FunctionLike): # Check that the types are compatible. - # TODO overloaded signatures self.check_override( typ, original_type, @@ -2056,7 +2055,6 @@ def check_method_override_for_base_with_name( # Assume invariance for a non-callable attribute here. Note # that this doesn't affect read-only properties which can have # covariant overrides. - # pass elif ( original_node @@ -2636,6 +2634,9 @@ class C(B, A[int]): ... # this is unsafe because... first_type = get_proper_type(self.determine_type_of_member(first)) second_type = get_proper_type(self.determine_type_of_member(second)) + # TODO: use more principled logic to decide is_subtype() vs is_equivalent(). + # We should rely on mutability of superclass node, not on types being Callable. + # start with the special case that Instance can be a subtype of FunctionLike call = None if isinstance(first_type, Instance): @@ -3211,7 +3212,7 @@ def check_compatibility_super( if base_static and compare_static: lvalue_node.is_staticmethod = True - return self.check_subtype( + ok = self.check_subtype( compare_type, base_type, rvalue, @@ -3219,6 +3220,20 @@ def check_compatibility_super( "expression has type", f'base class "{base.name}" defined the type as', ) + if ( + ok + and codes.MUTABLE_OVERRIDE in self.options.enabled_error_codes + and self.is_writable_attribute(base_node) + ): + ok = self.check_subtype( + base_type, + compare_type, + rvalue, + message_registry.COVARIANT_OVERRIDE_OF_MUTABLE_ATTRIBUTE, + f'base class "{base.name}" defined the type as', + "expression has type", + ) + return ok return True def lvalue_type_from_base( diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index 98600679da532..89f7927f52650 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -255,6 +255,12 @@ def __hash__(self) -> int: "General", default_enabled=False, ) +MUTABLE_OVERRIDE: Final = ErrorCode( + "mutable-override", + "Reject covariant overrides for mutable attributes", + "General", + default_enabled=False, +) # Syntax errors are often blocking. diff --git a/mypy/message_registry.py b/mypy/message_registry.py index dc46eb5033909..1f61a923d98fc 100644 --- a/mypy/message_registry.py +++ b/mypy/message_registry.py @@ -63,6 +63,9 @@ def with_additional_msg(self, info: str) -> ErrorMessage: INCOMPATIBLE_TYPES_IN_ASSIGNMENT: Final = ErrorMessage( "Incompatible types in assignment", code=codes.ASSIGNMENT ) +COVARIANT_OVERRIDE_OF_MUTABLE_ATTRIBUTE: Final = ErrorMessage( + "Covariant override of a mutable attribute", code=codes.MUTABLE_OVERRIDE +) INCOMPATIBLE_TYPES_IN_AWAIT: Final = ErrorMessage('Incompatible types in "await"') INCOMPATIBLE_REDEFINITION: Final = ErrorMessage("Incompatible redefinition") INCOMPATIBLE_TYPES_IN_ASYNC_WITH_AENTER: Final = ( diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 2282f21bcfa6a..434e3de47c769 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -1148,3 +1148,27 @@ main:3: note: Revealed local types are: main:3: note: x: builtins.int main:3: error: Name "reveal_locals" is not defined [unimported-reveal] [builtins fixtures/isinstancelist.pyi] + +[case testCovariantMutableOverride] +# flags: --enable-error-code=mutable-override +from typing import Any + +class C: + x: float + y: float + z: float + w: Any + @property + def foo(self) -> float: ... + @property + def bar(self) -> float: ... + @bar.setter + def bar(self, val: float) -> None: ... +class D(C): + x: int # E: Covariant override of a mutable attribute (base class "C" defined the type as "float", expression has type "int") [mutable-override] + y: float + z: Any + w: float + foo: int + bar: int # E: Covariant override of a mutable attribute (base class "C" defined the type as "float", expression has type "int") [mutable-override] +[builtins fixtures/property.pyi] From a0a1071435903c572f3f04ac1c8d27bce69520bc Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 2 Nov 2023 23:57:25 +0000 Subject: [PATCH 2/4] Tweak docs --- docs/source/error_code_list2.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/error_code_list2.rst b/docs/source/error_code_list2.rst index a6bd8d2917ad9..9e24f21909d57 100644 --- a/docs/source/error_code_list2.rst +++ b/docs/source/error_code_list2.rst @@ -505,8 +505,8 @@ flagged when this error code is enabled: x: int # Error: Covariant override of a mutable attribute # (base class "C" defined the type as "float", # expression has type "int") [mutable-override] - y: float - z: Any + y: float # OK + z: Any # OK def f(c: C) -> None: c.x = 1.1 From d6d8bf97fdde45de9da6a76f66ff67372a92a857 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 3 Nov 2023 00:03:11 +0000 Subject: [PATCH 3/4] Sigh... --- mypy/errorcodes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index 89f7927f52650..2e591482d2ea7 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -255,7 +255,7 @@ def __hash__(self) -> int: "General", default_enabled=False, ) -MUTABLE_OVERRIDE: Final = ErrorCode( +MUTABLE_OVERRIDE: Final[ErrorCode] = ErrorCode( "mutable-override", "Reject covariant overrides for mutable attributes", "General", From 51da1dbe6ed835b0148e0aaf1ad812e132e47f7f Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 3 Nov 2023 23:58:28 +0000 Subject: [PATCH 4/4] Test more situations --- test-data/unit/check-errorcodes.test | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 434e3de47c769..28487a4561564 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -1164,6 +1164,9 @@ class C: def bar(self) -> float: ... @bar.setter def bar(self, val: float) -> None: ... + baz: float + bad1: float + bad2: float class D(C): x: int # E: Covariant override of a mutable attribute (base class "C" defined the type as "float", expression has type "int") [mutable-override] y: float @@ -1171,4 +1174,9 @@ class D(C): w: float foo: int bar: int # E: Covariant override of a mutable attribute (base class "C" defined the type as "float", expression has type "int") [mutable-override] + def one(self) -> None: + self.baz = 5 + bad1 = 5 # E: Covariant override of a mutable attribute (base class "C" defined the type as "float", expression has type "int") [mutable-override] + def other(self) -> None: + self.bad2: int = 5 # E: Covariant override of a mutable attribute (base class "C" defined the type as "float", expression has type "int") [mutable-override] [builtins fixtures/property.pyi]