Skip to content

Commit

Permalink
Add error code for mutable covariant override (#16399)
Browse files Browse the repository at this point in the history
Fixes #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.
  • Loading branch information
ilevkivskyi committed Nov 9, 2023
1 parent bc591c7 commit a1864d4
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 3 deletions.
31 changes: 31 additions & 0 deletions docs/source/error_code_list2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
21 changes: 18 additions & 3 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -3211,14 +3212,28 @@ 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,
message_registry.INCOMPATIBLE_TYPES_IN_ASSIGNMENT,
"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(
Expand Down
6 changes: 6 additions & 0 deletions mypy/errorcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions mypy/message_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
Expand Down
32 changes: 32 additions & 0 deletions test-data/unit/check-errorcodes.test
Original file line number Diff line number Diff line change
Expand Up @@ -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]

0 comments on commit a1864d4

Please sign in to comment.