Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow overriding attribute with a settable property #13475

Merged
merged 6 commits into from
Aug 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 49 additions & 5 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
ReturnStmt,
StarExpr,
Statement,
SymbolNode,
SymbolTable,
SymbolTableNode,
TempNode,
Expand Down Expand Up @@ -1720,6 +1721,7 @@ def check_method_override_for_base_with_name(
context = defn.func

# Construct the type of the overriding method.
# TODO: this logic is much less complete than similar one in checkmember.py
if isinstance(defn, (FuncDef, OverloadedFuncDef)):
typ: Type = self.function_type(defn)
override_class_or_static = defn.is_class or defn.is_static
Expand Down Expand Up @@ -1769,15 +1771,37 @@ def check_method_override_for_base_with_name(
original_class_or_static = fdef.is_class or fdef.is_static
else:
original_class_or_static = False # a variable can't be class or static

if isinstance(original_type, FunctionLike):
original_type = self.bind_and_map_method(base_attr, original_type, defn.info, base)
if original_node and is_property(original_node):
original_type = get_property_type(original_type)

if isinstance(typ, FunctionLike) and is_property(defn):
typ = get_property_type(typ)
if (
isinstance(original_node, Var)
and not original_node.is_final
and (not original_node.is_property or original_node.is_settable_property)
and isinstance(defn, Decorator)
):
# We only give an error where no other similar errors will be given.
if not isinstance(original_type, AnyType):
self.msg.fail(
"Cannot override writeable attribute with read-only property",
# Give an error on function line to match old behaviour.
defn.func,
code=codes.OVERRIDE,
)

if isinstance(original_type, AnyType) or isinstance(typ, AnyType):
pass
elif isinstance(original_type, FunctionLike) and isinstance(typ, FunctionLike):
original = self.bind_and_map_method(base_attr, original_type, defn.info, base)
# Check that the types are compatible.
# TODO overloaded signatures
self.check_override(
typ,
original,
original_type,
defn.name,
name,
base.name,
Expand All @@ -1792,8 +1816,8 @@ def check_method_override_for_base_with_name(
#
pass
elif (
base_attr.node
and not self.is_writable_attribute(base_attr.node)
original_node
and not self.is_writable_attribute(original_node)
and is_subtype(typ, original_type)
):
# If the attribute is read-only, allow covariance
Expand Down Expand Up @@ -4311,7 +4335,8 @@ def visit_decorator(self, e: Decorator) -> None:
if len([k for k in sig.arg_kinds if k.is_required()]) > 1:
self.msg.fail("Too many arguments for property", e)
self.check_incompatible_property_override(e)
if e.func.info and not e.func.is_dynamic():
# For overloaded functions we already checked override for overload as a whole.
if e.func.info and not e.func.is_dynamic() and not e.is_overload:
self.check_method_override(e)

if e.func.info and e.func.name in ("__init__", "__new__"):
Expand Down Expand Up @@ -6066,6 +6091,8 @@ def conditional_types_with_intersection(
def is_writable_attribute(self, node: Node) -> bool:
"""Check if an attribute is writable"""
if isinstance(node, Var):
if node.is_property and not node.is_settable_property:
return False
return True
elif isinstance(node, OverloadedFuncDef) and node.is_property:
first_item = cast(Decorator, node.items[0])
Expand Down Expand Up @@ -6973,6 +7000,23 @@ def is_static(func: FuncBase | Decorator) -> bool:
assert False, f"Unexpected func type: {type(func)}"


def is_property(defn: SymbolNode) -> bool:
if isinstance(defn, Decorator):
return defn.func.is_property
if isinstance(defn, OverloadedFuncDef):
if defn.items and isinstance(defn.items[0], Decorator):
return defn.items[0].func.is_property
return False


def get_property_type(t: ProperType) -> ProperType:
if isinstance(t, CallableType):
return get_proper_type(t.ret_type)
if isinstance(t, Overloaded):
return get_proper_type(t.items[0].ret_type)
return t


def is_subtype_no_promote(left: Type, right: Type) -> bool:
return is_subtype(left, right, ignore_promotions=True)

Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-abstract.test
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ class A(metaclass=ABCMeta):
def x(self) -> int: pass
class B(A):
@property
def x(self) -> str: pass # E: Return type "str" of "x" incompatible with return type "int" in supertype "A"
def x(self) -> str: pass # E: Signature of "x" incompatible with supertype "A"
b = B()
b.x() # E: "str" not callable
[builtins fixtures/property.pyi]
Expand Down
23 changes: 23 additions & 0 deletions test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -7366,3 +7366,26 @@ class D(C[List[T]]): ...
di: D[int]
reveal_type(di) # N: Revealed type is "Tuple[builtins.list[builtins.int], builtins.list[builtins.int], fallback=__main__.D[builtins.int]]"
[builtins fixtures/tuple.pyi]

[case testOverrideAttrWithSettableProperty]
class Foo:
def __init__(self) -> None:
self.x = 42

class Bar(Foo):
@property
def x(self) -> int: ...
@x.setter
def x(self, value: int) -> None: ...
[builtins fixtures/property.pyi]

[case testOverrideAttrWithSettablePropertyAnnotation]
class Foo:
x: int

class Bar(Foo):
@property
def x(self) -> int: ...
@x.setter
def x(self, value: int) -> None: ...
[builtins fixtures/property.pyi]
2 changes: 1 addition & 1 deletion test-data/unit/check-dataclasses.test
Original file line number Diff line number Diff line change
Expand Up @@ -1286,7 +1286,7 @@ class A:
@dataclass
class B(A):
@property
def foo(self) -> int: pass # E: Signature of "foo" incompatible with supertype "A"
def foo(self) -> int: pass

reveal_type(B) # N: Revealed type is "def (foo: builtins.int) -> __main__.B"

Expand Down
3 changes: 1 addition & 2 deletions test-data/unit/check-inference.test
Original file line number Diff line number Diff line change
Expand Up @@ -1475,9 +1475,8 @@ class A:
self.x = [] # E: Need type annotation for "x" (hint: "x: List[<type>] = ...")

class B(A):
# TODO?: This error is kind of a false positive, unfortunately
@property
def x(self) -> List[int]: # E: Signature of "x" incompatible with supertype "A"
def x(self) -> List[int]: # E: Cannot override writeable attribute with read-only property
return [123]
[builtins fixtures/list.pyi]

Expand Down