Skip to content

Recognize different setter type for properties. #11643

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

Closed
wants to merge 7 commits into from
Closed
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
49 changes: 26 additions & 23 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,9 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:

if defn.is_property:
# HACK: Infer the type of the property.
self.visit_decorator(cast(Decorator, defn.items[0]))
self.visit_decorator(cast(Decorator, defn.items[0]), overloaded_property=True)
if defn.property_setter is not None:
self.visit_decorator(defn.property_setter, overloaded_property=True)
for fdef in defn.items:
assert isinstance(fdef, Decorator)
self.check_func_item(fdef.func, name=fdef.func.name)
Expand Down Expand Up @@ -3913,7 +3915,7 @@ def visit_del_stmt(self, s: DelStmt) -> None:
self.binder.assign_type(elt, DeletedType(source=elt.name),
get_declaration(elt), False)

def visit_decorator(self, e: Decorator) -> None:
def visit_decorator(self, e: Decorator, overloaded_property: bool = False) -> None:
for d in e.decorators:
if isinstance(d, RefExpr):
if d.fullname == 'typing.no_type_check':
Expand All @@ -3928,33 +3930,34 @@ def visit_decorator(self, e: Decorator) -> None:
# Process decorators from the inside out to determine decorated signature, which
# may be different from the declared signature.
sig: Type = self.function_type(e.func)
for d in reversed(e.decorators):
if refers_to_fullname(d, 'typing.overload'):
self.fail(message_registry.MULTIPLE_OVERLOADS_REQUIRED, e)
continue
dec = self.expr_checker.accept(d)
temp = self.temp_node(sig, context=e)
fullname = None
if isinstance(d, RefExpr):
fullname = d.fullname
# if this is a expression like @b.a where b is an object, get the type of b
# so we can pass it the method hook in the plugins
object_type: Optional[Type] = None
if fullname is None and isinstance(d, MemberExpr) and d.expr in self.type_map:
object_type = self.type_map[d.expr]
fullname = self.expr_checker.method_fullname(object_type, d.name)
self.check_for_untyped_decorator(e.func, dec, d)
sig, t2 = self.expr_checker.check_call(dec, [temp],
[nodes.ARG_POS], e,
callable_name=fullname,
object_type=object_type)
if not overloaded_property:
for d in reversed(e.decorators):
if refers_to_fullname(d, 'typing.overload'):
self.fail(message_registry.MULTIPLE_OVERLOADS_REQUIRED, e)
continue
dec = self.expr_checker.accept(d)
temp = self.temp_node(sig, context=e)
fullname = None
if isinstance(d, RefExpr):
fullname = d.fullname
# if this is a expression like @b.a where b is an object, get the type of b
# so we can pass it the method hook in the plugins
object_type: Optional[Type] = None
if fullname is None and isinstance(d, MemberExpr) and d.expr in self.type_map:
object_type = self.type_map[d.expr]
fullname = self.expr_checker.method_fullname(object_type, d.name)
self.check_for_untyped_decorator(e.func, dec, d)
sig, t2 = self.expr_checker.check_call(dec, [temp],
[nodes.ARG_POS], e,
callable_name=fullname,
object_type=object_type)
self.check_untyped_after_decorator(sig, e.func)
sig = set_callable_name(sig, e.func)
e.var.type = sig
e.var.is_ready = True
if e.func.is_property:
self.check_incompatible_property_override(e)
if e.func.info and not e.func.is_dynamic():
if e.func.info and not e.func.is_dynamic() and not overloaded_property:
self.check_method_override(e)

if e.func.info and e.func.name in ('__init__', '__new__'):
Expand Down
31 changes: 23 additions & 8 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -2084,15 +2084,17 @@ def check_union_call(self,
def visit_member_expr(self, e: MemberExpr, is_lvalue: bool = False) -> Type:
"""Visit member expression (of form e.id)."""
self.chk.module_refs.update(extract_refexpr_names(e))
result = self.analyze_ordinary_member_access(e, is_lvalue)
return self.narrow_type_from_binder(e, result)
return self.analyze_ordinary_member_access(e, is_lvalue, narrow=True)

def analyze_ordinary_member_access(self, e: MemberExpr,
is_lvalue: bool) -> Type:
def analyze_ordinary_member_access(self,
e: MemberExpr,
is_lvalue: bool,
narrow: bool = False) -> Type:
"""Analyse member expression or member lvalue."""
skip_non_overlapping_narrow = False
if e.kind is not None:
# This is a reference to a module attribute.
return self.analyze_ref_expr(e)
result_type = self.analyze_ref_expr(e)
else:
# This is a reference to a non-module attribute.
original_type = self.accept(e.expr)
Expand All @@ -2102,13 +2104,26 @@ def analyze_ordinary_member_access(self, e: MemberExpr,
if isinstance(base, RefExpr) and isinstance(base.node, MypyFile):
module_symbol_table = base.node.names

member_type = analyze_member_access(
result_type = analyze_member_access(
e.name, original_type, e, is_lvalue, False, False,
self.msg, original_type=original_type, chk=self.chk,
in_literal_context=self.is_literal_context(),
module_symbol_table=module_symbol_table)

return member_type
# Properties with a setter should not narrow if the set type does
# not overlap with the get type
proper_type = get_proper_type(original_type)
if isinstance(proper_type, Instance):
method = proper_type.type.get_method(e.name)
if isinstance(method, OverloadedFuncDef) and method.property_setter:
skip_non_overlapping_narrow = True

if narrow:
narrow_type = self.narrow_type_from_binder(
e, result_type,
skip_non_overlapping=skip_non_overlapping_narrow)
return narrow_type if narrow_type is not None else result_type

return result_type

def analyze_external_member_access(self, member: str, base_type: Type,
context: Context) -> Type:
Expand Down
13 changes: 10 additions & 3 deletions mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,11 @@ def analyze_instance_member_access(name: str,
if method and not isinstance(method, Decorator):
if method.is_property:
assert isinstance(method, OverloadedFuncDef)
first_item = cast(Decorator, method.items[0])
return analyze_var(name, first_item.var, typ, info, mx)
if mx.is_lvalue and method.property_setter is not None:
var = method.property_setter.var
else:
var = cast(Decorator, method.items[0]).var
return analyze_var(name, var, typ, info, mx)
if mx.is_lvalue:
mx.msg.cant_assign_to_method(mx.context)
signature = function_type(method, mx.named_type('builtins.function'))
Expand Down Expand Up @@ -598,7 +601,11 @@ def analyze_var(name: str,
if var.is_property:
# A property cannot have an overloaded type => the cast is fine.
assert isinstance(expanded_signature, CallableType)
result = expanded_signature.ret_type
if mx.is_lvalue and var.is_property_setter_different:
assert var.is_settable_property
result = expanded_signature.arg_types[0]
else:
result = expanded_signature.ret_type
else:
result = expanded_signature
else:
Expand Down
2 changes: 2 additions & 0 deletions mypy/fixup.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ def visit_overloaded_func_def(self, o: OverloadedFuncDef) -> None:
item.accept(self)
if o.impl:
o.impl.accept(self)
if o.property_setter:
o.property_setter.accept(self)

def visit_decorator(self, d: Decorator) -> None:
if self.current_info is not None:
Expand Down
18 changes: 14 additions & 4 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -545,11 +545,12 @@ class OverloadedFuncDef(FuncBase, SymbolNode, Statement):
Overloaded variants must be consecutive in the source file.
"""

__slots__ = ('items', 'unanalyzed_items', 'impl')
__slots__ = ('items', 'unanalyzed_items', 'impl', 'property_setter')

items: List[OverloadPart]
unanalyzed_items: List[OverloadPart]
impl: Optional[OverloadPart]
property_setter: 'Optional[Decorator]'

def __init__(self, items: List['OverloadPart']) -> None:
super().__init__()
Expand All @@ -559,6 +560,7 @@ def __init__(self, items: List['OverloadPart']) -> None:
if len(items) > 0:
self.set_line(items[0].line, items[0].column)
self.is_final = False
self.property_setter = None

@property
def name(self) -> str:
Expand All @@ -579,6 +581,8 @@ def serialize(self) -> JsonDict:
'fullname': self._fullname,
'impl': None if self.impl is None else self.impl.serialize(),
'flags': get_flags(self, FUNCBASE_FLAGS),
'property_setter': (None if self.property_setter is None else
self.property_setter.serialize()),
}

@classmethod
Expand All @@ -598,6 +602,10 @@ def deserialize(cls, data: JsonDict) -> 'OverloadedFuncDef':
res.type = typ
res._fullname = data['fullname']
set_flags(res, data['flags'])
if data.get('property_setter') is not None:
property_setter = SymbolNode.deserialize(data['property_setter'])
assert isinstance(property_setter, Decorator)
res.property_setter = property_setter
# NOTE: res.info will be set in the fixup phase.
return res

Expand Down Expand Up @@ -849,9 +857,9 @@ def deserialize(cls, data: JsonDict) -> 'Decorator':

VAR_FLAGS: Final = [
'is_self', 'is_initialized_in_class', 'is_staticmethod',
'is_classmethod', 'is_property', 'is_settable_property', 'is_suppressed_import',
'is_classvar', 'is_abstract_var', 'is_final', 'final_unset_in_class', 'final_set_in_init',
'explicit_self_type', 'is_ready', 'from_module_getattr',
'is_classmethod', 'is_property', 'is_settable_property', 'is_property_setter_different',
'is_suppressed_import', 'is_classvar', 'is_abstract_var', 'is_final', 'final_unset_in_class',
'final_set_in_init', 'explicit_self_type', 'is_ready', 'from_module_getattr',
'has_explicit_value',
]

Expand All @@ -875,6 +883,7 @@ class Var(SymbolNode):
'is_classmethod',
'is_property',
'is_settable_property',
'is_property_setter_different',
'is_classvar',
'is_abstract_var',
'is_final',
Expand Down Expand Up @@ -904,6 +913,7 @@ def __init__(self, name: str, type: 'Optional[mypy.types.Type]' = None) -> None:
self.is_classmethod = False
self.is_property = False
self.is_settable_property = False
self.is_property_setter_different = False
self.is_classvar = False
self.is_abstract_var = False
# Set to true when this variable refers to a module we were unable to
Expand Down
27 changes: 22 additions & 5 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,9 +764,14 @@ def analyze_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
# This is a property.
first_item.func.is_overload = True
self.analyze_property_with_multi_part_definition(defn)
typ = function_type(first_item.func, self.named_type('builtins.function'))
assert isinstance(typ, CallableType)
types = [typ]
getter = function_type(first_item.func, self.named_type('builtins.function'))
assert isinstance(getter, CallableType)
types = [getter]
if defn.property_setter is not None:
setter = function_type(defn.property_setter.func,
self.named_type('builtins.function'))
assert isinstance(setter, CallableType)
types.append(setter)
else:
# This is an a normal overload. Find the item signatures, the
# implementation (if outside a stub), and any missing @overload
Expand Down Expand Up @@ -949,10 +954,22 @@ def analyze_property_with_multi_part_definition(self, defn: OverloadedFuncDef) -
node = item.decorators[0]
if isinstance(node, MemberExpr):
if node.name == 'setter':
# The first item represents the entire property.
first_item.var.is_settable_property = True
# Get abstractness from the original definition.
item.func.is_abstract = first_item.func.is_abstract
item.func.is_property = True
# Only record the specific setter if it is actually
# valid setter, this way the property is still kept
# "settable", but will use whatever the get type
# is.
if len(item.func.arguments) < 2:
self.fail('Too few arguments', item.func)
else:
defn.property_setter = item
item.var.is_settable_property = True
item.var.is_property_setter_different = True
item.var.is_initialized_in_class = True
item.var.is_property = True
item.var.info = first_item.var.info
else:
self.fail("Decorated property not supported", item)
item.func.accept(self)
Expand Down
20 changes: 10 additions & 10 deletions mypyc/irbuild/function.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,16 +387,16 @@ def handle_ext_method(builder: IRBuilder, cdef: ClassDef, fdef: FuncDef) -> None
fdef.line)

if fdef.is_property:
# If there is a property setter, it will be processed after the getter,
# We populate the optional setter field with none for now.
assert name not in class_ir.properties
class_ir.properties[name] = (func_ir, None)

elif fdef in builder.prop_setters:
# The respective property getter must have been processed already
assert name in class_ir.properties
getter_ir, _ = class_ir.properties[name]
class_ir.properties[name] = (getter_ir, func_ir)
if fdef not in builder.prop_setters:
# If there is a property setter, it will be processed after the getter,
# We populate the optional setter field with none for now.
assert name not in class_ir.properties
class_ir.properties[name] = (func_ir, None)
else:
# The respective property getter must have been processed already
assert name in class_ir.properties
getter_ir, _ = class_ir.properties[name]
class_ir.properties[name] = (getter_ir, func_ir)

class_ir.methods[func_ir.decl.name] = func_ir

Expand Down
2 changes: 1 addition & 1 deletion mypyc/irbuild/prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def prepare_method_def(ir: ClassIR, module_name: str, cdef: ClassDef, mapper: Ma
decl.is_prop_setter = True
ir.method_decls[PROPSET_PREFIX + node.name] = decl

if node.func.is_property:
if node.func.is_property and not decl.is_prop_setter:
assert node.func.type, f"Expected return type annotation for property '{node.name}'"
decl.is_prop_getter = True
ir.property_types[node.name] = decl.sig.ret_type
Expand Down
10 changes: 9 additions & 1 deletion test-data/unit/check-abstract.test
Original file line number Diff line number Diff line change
Expand Up @@ -850,12 +850,20 @@ class A(metaclass=ABCMeta):
def x(self, v: int) -> None: pass
class B(A):
@property # E
def x(self) -> int: pass
def x(self) -> int: pass # E
b = B()
b.x.y # E
[builtins fixtures/property.pyi]
[out]
main:8: error: Read-only property cannot override read-write property
main:9: error: Signature of "x" incompatible with supertype "A"
main:9: note: Superclass:
main:9: note: @overload
main:9: note: def x(self) -> int
main:9: note: @overload
main:9: note: def x(self, v: int) -> None
main:9: note: Subclass:
main:9: note: def x(self) -> int
main:11: error: "int" has no attribute "y"

[case testDynamicallyTypedReadOnlyAbstractProperty]
Expand Down
41 changes: 41 additions & 0 deletions test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -1383,6 +1383,47 @@ a.f = 1
reveal_type(a.f) # N: Revealed type is "builtins.int"
[builtins fixtures/property.pyi]

[case testPropertyWithInvalidSetter]
import typing
class A:
@property
def f(self) -> int: ...
@f.setter
def f(self) -> None: ... # E: Too few arguments
a = A()
reveal_type(a.f) # N: Revealed type is "builtins.int"
[builtins fixtures/property.pyi]

[case testPropertyWithNonOverlappingSetter]
import typing
class A:
@property
def f(self) -> int: ...
@f.setter
def f(self, x: str) -> None: ...
a = A()
reveal_type(a.f) # N: Revealed type is "builtins.int"
a.f = ''
reveal_type(a.f) # N: Revealed type is "builtins.int"
a.f = 1 # E: Incompatible types in assignment (expression has type "int", variable has type "str")
reveal_type(a.f) # N: Revealed type is "builtins.int"
[builtins fixtures/property.pyi]

[case testPropertyWithOverlappingSetter]
import typing
class A:
@property
def f(self) -> typing.Union[int, str]: ...
@f.setter
def f(self, x: int) -> None: ...
a = A()
reveal_type(a.f) # N: Revealed type is "Union[builtins.int, builtins.str]"
a.f = '' # E: Incompatible types in assignment (expression has type "str", variable has type "int")
reveal_type(a.f) # N: Revealed type is "Union[builtins.int, builtins.str]"
a.f = 1
reveal_type(a.f) # N: Revealed type is "builtins.int"
[builtins fixtures/property.pyi]

[case testPropertyWithDeleterButNoSetter]
import typing
class A:
Expand Down
Loading