From a1864d4fa498ccd8773c2247eb62282644174d26 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 9 Nov 2023 00:04:36 +0000 Subject: [PATCH] Add error code for mutable covariant override (#16399) Fixes https://github.com/python/mypy/issues/3208 Interestingly, we already prohibit this when the override is a mutable property (goes through `FuncDef`-related code), and in multiple inheritance. The logic there is not very principled, but I just left a TODO instead of extending the scope of this PR. --- 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 | 32 ++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 3 deletions(-) diff --git a/docs/source/error_code_list2.rst b/docs/source/error_code_list2.rst index cc5c9b0a1bc6..9e24f21909d5 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 # OK + z: Any # OK + + 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 f51ba746ea75..e4eb58d40715 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 c6e9de9f31c1..72ee63a6a897 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] = 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 93581d5aca90..8dc14e158d90 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 2282f21bcfa6..28487a456156 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -1148,3 +1148,35 @@ 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: ... + 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 + 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] + 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]