Skip to content

Consistently store settable property type #18774

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

Merged
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
64 changes: 46 additions & 18 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -654,23 +654,34 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
assert isinstance(defn.items[0], Decorator)
self.visit_decorator(defn.items[0])
if defn.items[0].var.is_settable_property:
# TODO: here and elsewhere we assume setter immediately follows getter.
assert isinstance(defn.items[1], Decorator)
self.visit_func_def(defn.items[1].func)
setter_type = self.function_type(defn.items[1].func)
assert isinstance(setter_type, CallableType)
if len(setter_type.arg_types) != 2:
# Perform a reduced visit just to infer the actual setter type.
self.visit_decorator_inner(defn.items[1], skip_first_item=True)
setter_type = get_proper_type(defn.items[1].var.type)
# Check if the setter can accept two positional arguments.
any_type = AnyType(TypeOfAny.special_form)
fallback_setter_type = CallableType(
arg_types=[any_type, any_type],
arg_kinds=[ARG_POS, ARG_POS],
arg_names=[None, None],
ret_type=any_type,
fallback=self.named_type("builtins.function"),
)
if setter_type and not is_subtype(setter_type, fallback_setter_type):
self.fail("Invalid property setter signature", defn.items[1].func)
any_type = AnyType(TypeOfAny.from_error)
setter_type = setter_type.copy_modified(
arg_types=[any_type, any_type],
arg_kinds=[ARG_POS, ARG_POS],
arg_names=[None, None],
)
if not isinstance(setter_type, CallableType) or len(setter_type.arg_types) != 2:
# TODO: keep precise type for callables with tricky but valid signatures.
setter_type = fallback_setter_type
defn.items[0].var.setter_type = setter_type
for fdef in defn.items:
for i, fdef in enumerate(defn.items):
assert isinstance(fdef, Decorator)
if defn.is_property:
self.check_func_item(fdef.func, name=fdef.func.name, allow_empty=True)
assert isinstance(defn.items[0], Decorator)
settable = defn.items[0].var.is_settable_property
# Do not visit the second time the items we checked above.
if (settable and i > 1) or (not settable and i > 0):
self.check_func_item(fdef.func, name=fdef.func.name, allow_empty=True)
else:
# Perform full check for real overloads to infer type of all decorated
# overload variants.
Expand All @@ -692,6 +703,13 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
item_types.append(item_type)
if item_types:
defn.type = Overloaded(item_types)
elif defn.type is None:
# We store the getter type as an overall overload type, as some
# code paths are getting property type this way.
assert isinstance(defn.items[0], Decorator)
var_type = get_proper_type(defn.items[0].var.type)
assert isinstance(var_type, CallableType)
defn.type = Overloaded([var_type])
# Check override validity after we analyzed current definition.
if defn.info:
found_method_base_classes = self.check_method_override(defn)
Expand Down Expand Up @@ -5277,25 +5295,34 @@ def visit_decorator(self, e: Decorator) -> None:
return
self.visit_decorator_inner(e)

def visit_decorator_inner(self, e: Decorator, allow_empty: bool = False) -> None:
def visit_decorator_inner(
self, e: Decorator, allow_empty: bool = False, skip_first_item: bool = False
) -> None:
if self.recurse_into_functions:
with self.tscope.function_scope(e.func):
self.check_func_item(e.func, name=e.func.name, allow_empty=allow_empty)

# 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):
non_trivial_decorator = False
# For settable properties skip the first decorator (that is @foo.setter).
for d in reversed(e.decorators[1:] if skip_first_item else e.decorators):
if refers_to_fullname(d, "abc.abstractmethod"):
# This is a hack to avoid spurious errors because of incomplete type
# of @abstractmethod in the test fixtures.
continue
if refers_to_fullname(d, OVERLOAD_NAMES):
if not allow_empty:
self.fail(message_registry.MULTIPLE_OVERLOADS_REQUIRED, e)
continue
non_trivial_decorator = True
dec = self.expr_checker.accept(d)
temp = self.temp_node(sig, context=d)
fullname = None
if isinstance(d, RefExpr):
fullname = d.fullname or None
# if this is a expression like @b.a where b is an object, get the type of b
# if this is an 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: Type | None = None
if fullname is None and isinstance(d, MemberExpr) and self.has_type(d.expr):
Expand All @@ -5305,7 +5332,8 @@ def visit_decorator_inner(self, e: Decorator, allow_empty: bool = False) -> None
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)
if non_trivial_decorator:
self.check_untyped_after_decorator(sig, e.func)
sig = set_callable_name(sig, e.func)
e.var.type = sig
e.var.is_ready = True
Expand All @@ -5314,8 +5342,8 @@ def visit_decorator_inner(self, e: Decorator, allow_empty: bool = False) -> 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)
# For overloaded functions we already checked override for overload as a whole.
if allow_empty:
# For overloaded functions/properties we already checked override for overload as a whole.
if allow_empty or skip_first_item:
return
if e.func.info and not e.func.is_dynamic() and not e.is_overload:
found_method_base_classes = self.check_method_override(e)
Expand Down
25 changes: 21 additions & 4 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -1246,10 +1246,11 @@ def analyze_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
with self.overload_item_set(0):
first_item.accept(self)

bare_setter_type = None
if isinstance(first_item, Decorator) and first_item.func.is_property:
# This is a property.
first_item.func.is_overload = True
self.analyze_property_with_multi_part_definition(defn)
bare_setter_type = 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]
Expand Down Expand Up @@ -1283,6 +1284,11 @@ def analyze_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
# * Put decorator everywhere, use "bare" types in overloads.
defn.type = Overloaded(types)
defn.type.line = defn.line
# In addition, we can set the getter/setter type for valid properties as some
# code paths may either use the above type, or var.type etc. of the first item.
if isinstance(first_item, Decorator) and bare_setter_type:
first_item.var.type = types[0]
first_item.var.setter_type = bare_setter_type

if not defn.items:
# It was not a real overload after all, but function redefinition. We've
Expand Down Expand Up @@ -1502,26 +1508,37 @@ def process_static_or_class_method_in_overload(self, defn: OverloadedFuncDef) ->
defn.is_class = class_status[0]
defn.is_static = static_status[0]

def analyze_property_with_multi_part_definition(self, defn: OverloadedFuncDef) -> None:
def analyze_property_with_multi_part_definition(
self, defn: OverloadedFuncDef
) -> CallableType | None:
"""Analyze a property defined using multiple methods (e.g., using @x.setter).

Assume that the first method (@property) has already been analyzed.
Return bare setter type (without any other decorators applied), this may be used
by the caller for performance optimizations.
"""
defn.is_property = True
items = defn.items
first_item = defn.items[0]
assert isinstance(first_item, Decorator)
deleted_items = []
bare_setter_type = None
for i, item in enumerate(items[1:]):
if isinstance(item, Decorator):
if len(item.decorators) >= 1:
item.func.accept(self)
if item.decorators:
first_node = item.decorators[0]
if isinstance(first_node, MemberExpr):
if first_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.abstract_status = first_item.func.abstract_status
setter_func_type = function_type(
item.func, self.named_type("builtins.function")
)
assert isinstance(setter_func_type, CallableType)
bare_setter_type = setter_func_type
if first_node.name == "deleter":
item.func.abstract_status = first_item.func.abstract_status
for other_node in item.decorators[1:]:
Expand All @@ -1530,7 +1547,6 @@ def analyze_property_with_multi_part_definition(self, defn: OverloadedFuncDef) -
self.fail(
f"Only supported top decorator is @{first_item.func.name}.setter", item
)
item.func.accept(self)
else:
self.fail(f'Unexpected definition for property "{first_item.func.name}"', item)
deleted_items.append(i + 1)
Expand All @@ -1544,6 +1560,7 @@ def analyze_property_with_multi_part_definition(self, defn: OverloadedFuncDef) -
item.func.deprecated = (
f"function {item.fullname} is deprecated: {deprecated}"
)
return bare_setter_type

def add_function_to_symbol_table(self, func: FuncDef | OverloadedFuncDef) -> None:
if self.is_class_scope():
Expand Down
74 changes: 74 additions & 0 deletions test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -8464,3 +8464,77 @@ def deco(fn: Callable[[], list[T]]) -> Callable[[], T]: ...
@deco
def f() -> list[str]: ...
[builtins fixtures/property.pyi]

[case testPropertySetterSuperclassDeferred2]
import a
[file a.py]
import b
class D(b.C):
@property
def foo(self) -> str: ...
@foo.setter # E: Incompatible override of a setter type \
# N: (base class "C" defined the type as "str", \
# N: override has type "int")
def foo(self, x: int) -> None: ...
[file b.py]
from a import D
class C:
@property
def foo(self) -> str: ...
@foo.setter
def foo(self, x: str) -> None: ...
[builtins fixtures/property.pyi]

[case testPropertySetterDecorated]
from typing import Callable, TypeVar

class B:
def __init__(self) -> None:
self.foo: str
self.bar: int

class C(B):
@property
def foo(self) -> str: ...
@foo.setter # E: Incompatible override of a setter type \
# N: (base class "B" defined the type as "str", \
# N: override has type "int")
@deco
def foo(self, x: int, y: int) -> None: ...

@property
def bar(self) -> int: ...
@bar.setter
@deco
def bar(self, x: int, y: int) -> None: ...

@property
def baz(self) -> int: ...
@baz.setter
@deco_untyped
def baz(self, x: int) -> None: ...

c: C
c.baz = "yes" # OK, because of untyped decorator

T = TypeVar("T")
def deco(fn: Callable[[T, int, int], None]) -> Callable[[T, int], None]: ...
def deco_untyped(fn): ...
[builtins fixtures/property.pyi]

[case testPropertyDeleterBodyChecked]
class C:
@property
def foo(self) -> int: ...
@foo.deleter
def foo(self) -> None:
1() # E: "int" not callable

@property
def bar(self) -> int: ...
@bar.setter
def bar(self, x: str) -> None: ...
@bar.deleter
def bar(self) -> None:
1() # E: "int" not callable
[builtins fixtures/property.pyi]