Skip to content

Commit 04f38f5

Browse files
Fix crash on settable property alias (#19615)
Fixes #19572 Surprisingly, when working on this I found another inconsistency in how `.definiton` is set. So after all I decided to do some cleanup, now `.definiton` should always point do the `Decortor` if the definition is a decorated function (no matter whether it is a trivial decorator like `@abstractmethod` or `@overload` or a "real" one). --------- Co-authored-by: Stanislav Terliakov <[email protected]>
1 parent 0d791b2 commit 04f38f5

File tree

11 files changed

+111
-117
lines changed

11 files changed

+111
-117
lines changed

mypy/checker.py

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@
130130
WhileStmt,
131131
WithStmt,
132132
YieldExpr,
133+
get_func_def,
133134
is_final_node,
134135
)
135136
from mypy.operators import flip_ops, int_op_to_method, neg_ops
@@ -703,6 +704,12 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
703704
# TODO: keep precise type for callables with tricky but valid signatures.
704705
setter_type = fallback_setter_type
705706
defn.items[0].var.setter_type = setter_type
707+
if isinstance(defn.type, Overloaded):
708+
# Update legacy property type for decorated properties.
709+
getter_type = self.extract_callable_type(defn.items[0].var.type, defn)
710+
if getter_type is not None:
711+
getter_type.definition = defn.items[0]
712+
defn.type.items[0] = getter_type
706713
for i, fdef in enumerate(defn.items):
707714
assert isinstance(fdef, Decorator)
708715
if defn.is_property:
@@ -730,7 +737,7 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
730737
assert isinstance(item, Decorator)
731738
item_type = self.extract_callable_type(item.var.type, item)
732739
if item_type is not None:
733-
item_type.definition = item.func
740+
item_type.definition = item
734741
item_types.append(item_type)
735742
if item_types:
736743
defn.type = Overloaded(item_types)
@@ -2501,8 +2508,9 @@ def check_override(
25012508

25022509
override_ids = override.type_var_ids()
25032510
type_name = None
2504-
if isinstance(override.definition, FuncDef):
2505-
type_name = override.definition.info.name
2511+
definition = get_func_def(override)
2512+
if isinstance(definition, FuncDef):
2513+
type_name = definition.info.name
25062514

25072515
def erase_override(t: Type) -> Type:
25082516
return erase_typevars(t, ids_to_erase=override_ids)
@@ -3509,6 +3517,7 @@ def check_compatibility_all_supers(self, lvalue: RefExpr, rvalue: Expression) ->
35093517
continue
35103518

35113519
base_type, base_node = self.node_type_from_base(lvalue_node.name, base, lvalue)
3520+
# TODO: if the r.h.s. is a descriptor, we should check setter override as well.
35123521
custom_setter = is_custom_settable_property(base_node)
35133522
if isinstance(base_type, PartialType):
35143523
base_type = None
@@ -4494,6 +4503,8 @@ def set_inferred_type(self, var: Var, lvalue: Lvalue, type: Type) -> None:
44944503
if isinstance(p_type, Overloaded):
44954504
# TODO: in theory we can have a property with a deleter only.
44964505
var.is_settable_property = True
4506+
assert isinstance(definition, Decorator), definition
4507+
var.setter_type = definition.var.setter_type
44974508

44984509
def set_inference_error_fallback_type(self, var: Var, lvalue: Lvalue, type: Type) -> None:
44994510
"""Store best known type for variable if type inference failed.
@@ -5356,6 +5367,8 @@ def visit_decorator_inner(
53565367
self.check_untyped_after_decorator(sig, e.func)
53575368
self.require_correct_self_argument(sig, e.func)
53585369
sig = set_callable_name(sig, e.func)
5370+
if isinstance(sig, CallableType):
5371+
sig.definition = e
53595372
e.var.type = sig
53605373
e.var.is_ready = True
53615374
if e.func.is_property:
@@ -8654,17 +8667,21 @@ def visit_type_alias_type(self, t: TypeAliasType) -> Type:
86548667
return t.copy_modified(args=[a.accept(self) for a in t.args])
86558668

86568669

8657-
def is_classmethod_node(node: Node | None) -> bool | None:
8670+
def is_classmethod_node(node: SymbolNode | None) -> bool | None:
86588671
"""Find out if a node describes a classmethod."""
8672+
if isinstance(node, Decorator):
8673+
node = node.func
86598674
if isinstance(node, FuncDef):
86608675
return node.is_class
86618676
if isinstance(node, Var):
86628677
return node.is_classmethod
86638678
return None
86648679

86658680

8666-
def is_node_static(node: Node | None) -> bool | None:
8681+
def is_node_static(node: SymbolNode | None) -> bool | None:
86678682
"""Find out if a node describes a static function method."""
8683+
if isinstance(node, Decorator):
8684+
node = node.func
86688685
if isinstance(node, FuncDef):
86698686
return node.is_static
86708687
if isinstance(node, Var):

mypy/checkmember.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -976,8 +976,15 @@ def expand_and_bind_callable(
976976
freeze_all_type_vars(expanded)
977977
if not var.is_property:
978978
return expanded
979-
# TODO: a decorated property can result in Overloaded here.
980-
assert isinstance(expanded, CallableType)
979+
if isinstance(expanded, Overloaded):
980+
# Legacy way to store settable properties is with overloads. Also in case it is
981+
# an actual overloaded property, selecting first item that passed check_self_arg()
982+
# is a good approximation, long-term we should use check_call() inference below.
983+
if not expanded.items:
984+
# A broken overload, error should be already reported.
985+
return AnyType(TypeOfAny.from_error)
986+
expanded = expanded.items[0]
987+
assert isinstance(expanded, CallableType), expanded
981988
if var.is_settable_property and mx.is_lvalue and var.setter_type is not None:
982989
if expanded.variables:
983990
type_ctx = mx.rvalue or TempNode(AnyType(TypeOfAny.special_form), context=mx.context)

mypy/fixup.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,7 @@ def visit_overloaded_func_def(self, o: OverloadedFuncDef) -> None:
181181
if isinstance(o.type, Overloaded):
182182
# For error messages we link the original definition for each item.
183183
for typ, item in zip(o.type.items, o.items):
184-
if isinstance(item, Decorator):
185-
typ.definition = item.func
184+
typ.definition = item
186185

187186
def visit_decorator(self, d: Decorator) -> None:
188187
if self.current_info is not None:
@@ -193,8 +192,9 @@ def visit_decorator(self, d: Decorator) -> None:
193192
d.var.accept(self)
194193
for node in d.decorators:
195194
node.accept(self)
196-
if isinstance(d.var.type, ProperType) and isinstance(d.var.type, CallableType):
197-
d.var.type.definition = d.func
195+
typ = d.var.type
196+
if isinstance(typ, ProperType) and isinstance(typ, CallableType):
197+
typ.definition = d.func
198198

199199
def visit_class_def(self, c: ClassDef) -> None:
200200
for v in c.type_vars:

mypy/messages.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
SymbolTable,
5656
TypeInfo,
5757
Var,
58+
get_func_def,
5859
reverse_builtin_aliases,
5960
)
6061
from mypy.operators import op_methods, op_methods_to_symbols
@@ -2938,10 +2939,11 @@ def format_single(arg: Type) -> str:
29382939

29392940
def pretty_class_or_static_decorator(tp: CallableType) -> str | None:
29402941
"""Return @classmethod or @staticmethod, if any, for the given callable type."""
2941-
if tp.definition is not None and isinstance(tp.definition, SYMBOL_FUNCBASE_TYPES):
2942-
if tp.definition.is_class:
2942+
definition = get_func_def(tp)
2943+
if definition is not None and isinstance(definition, SYMBOL_FUNCBASE_TYPES):
2944+
if definition.is_class:
29432945
return "@classmethod"
2944-
if tp.definition.is_static:
2946+
if definition.is_static:
29452947
return "@staticmethod"
29462948
return None
29472949

@@ -2991,12 +2993,13 @@ def [T <: int] f(self, x: int, y: T) -> None
29912993
slash = True
29922994

29932995
# If we got a "special arg" (i.e: self, cls, etc...), prepend it to the arg list
2996+
definition = get_func_def(tp)
29942997
if (
2995-
isinstance(tp.definition, FuncDef)
2996-
and hasattr(tp.definition, "arguments")
2998+
isinstance(definition, FuncDef)
2999+
and hasattr(definition, "arguments")
29973000
and not tp.from_concatenate
29983001
):
2999-
definition_arg_names = [arg.variable.name for arg in tp.definition.arguments]
3002+
definition_arg_names = [arg.variable.name for arg in definition.arguments]
30003003
if (
30013004
len(definition_arg_names) > len(tp.arg_names)
30023005
and definition_arg_names[0]
@@ -3005,7 +3008,7 @@ def [T <: int] f(self, x: int, y: T) -> None
30053008
if s:
30063009
s = ", " + s
30073010
s = definition_arg_names[0] + s
3008-
s = f"{tp.definition.name}({s})"
3011+
s = f"{definition.name}({s})"
30093012
elif tp.name:
30103013
first_arg = get_first_arg(tp)
30113014
if first_arg:
@@ -3051,9 +3054,10 @@ def [T <: int] f(self, x: int, y: T) -> None
30513054

30523055

30533056
def get_first_arg(tp: CallableType) -> str | None:
3054-
if not isinstance(tp.definition, FuncDef) or not tp.definition.info or tp.definition.is_static:
3057+
definition = get_func_def(tp)
3058+
if not isinstance(definition, FuncDef) or not definition.info or definition.is_static:
30553059
return None
3056-
return tp.definition.original_first_arg
3060+
return definition.original_first_arg
30573061

30583062

30593063
def variance_string(variance: int) -> str:

mypy/nodes.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4380,6 +4380,13 @@ def is_final_node(node: SymbolNode | None) -> bool:
43804380
return isinstance(node, (Var, FuncDef, OverloadedFuncDef, Decorator)) and node.is_final
43814381

43824382

4383+
def get_func_def(typ: mypy.types.CallableType) -> SymbolNode | None:
4384+
definition = typ.definition
4385+
if isinstance(definition, Decorator):
4386+
definition = definition.func
4387+
return definition
4388+
4389+
43834390
def local_definitions(
43844391
names: SymbolTable, name_prefix: str, info: TypeInfo | None = None
43854392
) -> Iterator[Definition]:

mypy/semanal.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1243,6 +1243,7 @@ def analyze_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
12431243
bare_setter_type = self.analyze_property_with_multi_part_definition(defn)
12441244
typ = function_type(first_item.func, self.named_type("builtins.function"))
12451245
assert isinstance(typ, CallableType)
1246+
typ.definition = first_item
12461247
types = [typ]
12471248
else:
12481249
# This is a normal overload. Find the item signatures, the
@@ -1374,6 +1375,7 @@ def analyze_overload_sigs_and_impl(
13741375
if isinstance(item, Decorator):
13751376
callable = function_type(item.func, self.named_type("builtins.function"))
13761377
assert isinstance(callable, CallableType)
1378+
callable.definition = item
13771379
if not any(refers_to_fullname(dec, OVERLOAD_NAMES) for dec in item.decorators):
13781380
if i == len(defn.items) - 1 and not self.is_stub_file:
13791381
# Last item outside a stub is impl

mypy/types.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1887,6 +1887,9 @@ def __init__(
18871887
self.fallback = fallback
18881888
assert not name or "<bound method" not in name
18891889
self.name = name
1890+
# The rules for what exactly is considered a definition:
1891+
# * If it is a non-decorated function, FuncDef is the definition
1892+
# * If it is a decorated function, enclosing Decorator is the definition
18901893
self.definition = definition
18911894
self.variables = variables
18921895
self.is_ellipsis_args = is_ellipsis_args

test-data/unit/check-classes.test

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9211,3 +9211,50 @@ class CGT(BG[T]): ...
92119211

92129212
reveal_type(CGI) # N: Revealed type is "def () -> __main__.CGI"
92139213
reveal_type(CGT) # N: Revealed type is "def [T] () -> __main__.CGT[T`1]"
9214+
9215+
[case testSettablePropertyAlias]
9216+
from typing import Any, TypeVar
9217+
9218+
class A:
9219+
@property
9220+
def prop(self: Any) -> str: ...
9221+
@prop.setter
9222+
def prop(self, val: str) -> None: ...
9223+
9224+
T = TypeVar("T")
9225+
class AT:
9226+
@property
9227+
def prop(self: T) -> T: ...
9228+
@prop.setter
9229+
def prop(self: T, val: list[T]) -> None: ...
9230+
9231+
class B:
9232+
prop: str
9233+
prop_t: str
9234+
9235+
class C(B):
9236+
prop = A.prop
9237+
prop_t = AT.prop # E: Incompatible types in assignment (expression has type "C", base class "B" defined the type as "str")
9238+
9239+
reveal_type(C().prop) # N: Revealed type is "builtins.str"
9240+
C().prop = "no" # E: Invalid self argument "C" to attribute function "prop" with type "Callable[[A, str], None]"
9241+
reveal_type(C().prop_t) # N: Revealed type is "__main__.C"
9242+
C().prop_t = 1 # E: Incompatible types in assignment (expression has type "int", variable has type "list[C]")
9243+
[builtins fixtures/property.pyi]
9244+
9245+
[case testClassEqDecoratedAbstractNote]
9246+
from abc import abstractmethod
9247+
9248+
class C:
9249+
@abstractmethod
9250+
def __eq__(self, other: C) -> bool: ...
9251+
[builtins fixtures/plugin_attrs.pyi]
9252+
[out]
9253+
main:5: error: Argument 1 of "__eq__" is incompatible with supertype "builtins.object"; supertype defines the argument type as "object"
9254+
main:5: note: This violates the Liskov substitution principle
9255+
main:5: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
9256+
main:5: note: It is recommended for "__eq__" to work with arbitrary objects, for example:
9257+
main:5: note: def __eq__(self, other: object) -> bool:
9258+
main:5: note: if not isinstance(other, C):
9259+
main:5: note: return NotImplemented
9260+
main:5: note: return <logic to compare two C instances>

test-data/unit/check-dataclasses.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2055,7 +2055,7 @@ from dataclasses import dataclass, replace, InitVar
20552055
from typing import ClassVar
20562056

20572057
@dataclass
2058-
class A:
2058+
class A: # N: "replace" of "A" defined here
20592059
x: int
20602060
q: InitVar[int]
20612061
q2: InitVar[int] = 0

0 commit comments

Comments
 (0)