diff --git a/mypy/checker.py b/mypy/checker.py index c69b80a55fd9..0f71079e7080 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -172,6 +172,7 @@ ANY_STRATEGY, MYPYC_NATIVE_INT_NAMES, OVERLOAD_NAMES, + PROPERTY_DECORATOR_NAMES, AnyType, BoolTypeQuery, CallableType, @@ -641,6 +642,13 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None: if not defn.items: # In this case we have already complained about none of these being # valid overloads. + # We only want to add more helpful note if possible. + if defn.info and defn.unanalyzed_items: + first_item = defn.unanalyzed_items[0] + if isinstance(first_item, Decorator): + for base in defn.info.mro[1:]: + if defn.name in base.names: + self.check_property_component_override(first_item, defn.name, base) return if len(defn.items) == 1: self.fail(message_registry.MULTIPLE_OVERLOADS_REQUIRED, defn) @@ -2034,6 +2042,7 @@ def check_method_or_accessor_override_for_base( if base: name = defn.name base_attr = base.names.get(name) + is_bad_property = False if base_attr: # First, check if we override a final (always an error, even with Any types). if is_final_node(base_attr.node) and not is_private(name): @@ -2041,8 +2050,10 @@ def check_method_or_accessor_override_for_base( # Second, final can't override anything writeable independently of types. if defn.is_final: self.check_if_final_var_override_writable(name, base_attr.node, defn) + if isinstance(defn, (OverloadedFuncDef, Decorator)): + is_bad_property = self.check_property_component_override(defn, name, base) found_base_method = True - if check_override_compatibility: + if not is_bad_property and check_override_compatibility: # Check compatibility of the override signature # (__init__, __new__, __init_subclass__ are special). if self.check_method_override_for_base_with_name(defn, name, base): @@ -2096,6 +2107,35 @@ def check_setter_type_override( if not is_subtype(original_type, typ): self.msg.incompatible_setter_override(defn.items[1], typ, original_type, base) + def check_property_component_override( + self, defn: OverloadedFuncDef | Decorator, name: str, base: TypeInfo + ) -> bool: + """Can the override refer to property setter/deleter?""" + if isinstance(defn, OverloadedFuncDef): + if isinstance(defn.items[0], Decorator): + return self.check_property_component_override(defn.items[0], name, base) + return False + + deco_type = next( + ( + deco.name + for deco in defn.decorators + if isinstance(deco, MemberExpr) and deco.name in ("setter", "deleter") + ), + None, + ) + if deco_type is None: + return False + + base_attr = base.names.get(name) + if not base_attr or base_attr.node is None or not is_property(base_attr.node): + return False + self.msg.fail( + f"Property {deco_type} overrides are not supported.", defn.func, code=codes.MISC + ) + self.msg.note("Consider overriding getter explicitly with super() call.", defn.func) + return True + def check_method_override_for_base_with_name( self, defn: FuncDef | OverloadedFuncDef | Decorator, name: str, base: TypeInfo ) -> bool: @@ -2279,6 +2319,20 @@ def check_method_override_for_base_with_name( ) return False + def get_property_instance(self, method: Decorator) -> Instance | None: + property_deco_name = next( + ( + name + for d in method.original_decorators + for name in PROPERTY_DECORATOR_NAMES + if refers_to_fullname(d, name) + ), + None, + ) + if property_deco_name is not None: + return self.named_type(property_deco_name) + return None + def bind_and_map_method( self, sym: SymbolTableNode, typ: FunctionLike, sub_info: TypeInfo, super_info: TypeInfo ) -> FunctionLike: @@ -5285,7 +5339,7 @@ def visit_decorator_inner(self, e: Decorator, allow_empty: bool = False) -> None # For overloaded functions we already checked override for overload as a whole. if allow_empty: return - if e.func.info and not e.func.is_dynamic() and not e.is_overload: + if e.func.info and not e.is_overload: found_method_base_classes = self.check_method_override(e) if ( e.func.is_explicit_override diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 4b7e39d2042a..ed73654be51c 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -378,12 +378,15 @@ def analyze_ref_expr(self, e: RefExpr, lvalue: bool = False) -> Type: # Reference to a global function. result = function_type(node, self.named_type("builtins.function")) elif isinstance(node, OverloadedFuncDef): + result = node.type if node.type is None: if self.chk.in_checked_function() and node.items: self.chk.handle_cannot_determine_type(node.name, e) result = AnyType(TypeOfAny.from_error) - else: - result = node.type + elif isinstance(node.items[0], Decorator): + property_type = self.chk.get_property_instance(node.items[0]) + if property_type is not None: + result = property_type elif isinstance(node, TypeInfo): # Reference to a type object. if node.typeddict_type: @@ -409,7 +412,11 @@ def analyze_ref_expr(self, e: RefExpr, lvalue: bool = False) -> Type: # Reference to a module object. result = self.module_type(node) elif isinstance(node, Decorator): - result = self.analyze_var_ref(node.var, e) + property_type = self.chk.get_property_instance(node) + if property_type is not None: + result = property_type + else: + result = self.analyze_var_ref(node.var, e) elif isinstance(node, TypeAlias): # Something that refers to a type alias appears in runtime context. # Note that we suppress bogus errors for alias redefinitions, diff --git a/mypy/checkmember.py b/mypy/checkmember.py index f6b5e6be2c53..3f7e746d6068 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -1122,6 +1122,16 @@ def analyze_class_attribute_access( # C[int].x -> int t = erase_typevars(expand_type_by_instance(t, isuper), {tv.id for tv in def_vars}) + if isinstance(node.node, Decorator) and node.node.func.is_property: + property_type = mx.chk.get_property_instance(node.node) + if property_type is not None: + return property_type + if isinstance(node.node, OverloadedFuncDef) and node.node.is_property: + assert isinstance(node.node.items[0], Decorator) + property_type = mx.chk.get_property_instance(node.node.items[0]) + if property_type is not None: + return property_type + is_classmethod = (is_decorated and cast(Decorator, node.node).func.is_class) or ( isinstance(node.node, FuncBase) and node.node.is_class ) diff --git a/mypy/semanal.py b/mypy/semanal.py index 8463e07e61cb..ec9aef27da4f 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -261,6 +261,7 @@ NEVER_NAMES, OVERLOAD_NAMES, OVERRIDE_DECORATOR_NAMES, + PROPERTY_DECORATOR_NAMES, PROTOCOL_NAMES, REVEAL_TYPE_NAMES, TPDICT_NAMES, @@ -1650,16 +1651,7 @@ def visit_decorator(self, dec: Decorator) -> None: removed.append(i) dec.func.is_explicit_override = True self.check_decorated_function_is_method("override", dec) - elif refers_to_fullname( - d, - ( - "builtins.property", - "abc.abstractproperty", - "functools.cached_property", - "enum.property", - "types.DynamicClassAttribute", - ), - ): + elif refers_to_fullname(d, PROPERTY_DECORATOR_NAMES): removed.append(i) dec.func.is_property = True dec.var.is_property = True @@ -7166,6 +7158,40 @@ def already_defined( f'{noun} "{unmangle(name)}" already defined{extra_msg}', ctx, code=codes.NO_REDEF ) + if ( + isinstance(ctx, (OverloadedFuncDef, Decorator)) + and node is not None + and self.maybe_property_setter_or_deleter(ctx) + and self.maybe_property_definition(node) + ): + self.note("Property setter and deleter must be adjacent to the getter.", ctx) + + def maybe_property_setter_or_deleter(self, node: SymbolNode) -> bool: + if isinstance(node, OverloadedFuncDef) and node.unanalyzed_items: + # Use unanalyzed_items: setter+deletter would have empty .items + # due to previous error + node = node.unanalyzed_items[0] + return isinstance(node, Decorator) and any( + isinstance(dec, MemberExpr) and dec.name in ("setter", "deleter") + for dec in node.decorators + ) + + def maybe_property_definition(self, node: SymbolNode) -> bool: + if isinstance(node, Decorator) and node.func.is_property: + return True + elif isinstance(node, OverloadedFuncDef): + if node.is_property: + # Already analyzed + return True + elif isinstance(node.items[0], Decorator): + for dec in node.items[0].decorators: + if isinstance(dec, (NameExpr, MemberExpr)): + if not dec.fullname: + self.accept(dec) + if dec.fullname in PROPERTY_DECORATOR_NAMES: + return True + return False + def name_already_defined( self, name: str, ctx: Context, original_ctx: SymbolTableNode | SymbolNode | None = None ) -> None: diff --git a/mypy/types.py b/mypy/types.py index f3745695889f..b4b8d7a1e9a7 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -172,6 +172,15 @@ # Supported @override decorator names. OVERRIDE_DECORATOR_NAMES: Final = ("typing.override", "typing_extensions.override") +# Supported property decorators +PROPERTY_DECORATOR_NAMES: Final = ( + "builtins.property", + "abc.abstractproperty", + "functools.cached_property", + "enum.property", + "types.DynamicClassAttribute", +) + # A placeholder used for Bogus[...] parameters _dummy: Final[Any] = object() diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index cf401bc2aece..5513ec79af9e 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -1621,8 +1621,7 @@ class A: self.x = 1 self.x = '' # E: Incompatible types in assignment (expression has type "str", variable has type "int") return '' -[builtins fixtures/property.pyi] -[out] +[builtins fixtures/property-full.pyi] [case testDynamicallyTypedProperty] import typing @@ -1632,7 +1631,7 @@ class A: a = A() a.f.xx a.f = '' # E: Property "f" defined in "A" is read-only -[builtins fixtures/property.pyi] +[builtins fixtures/property-full.pyi] [case testPropertyWithSetter] import typing @@ -1649,7 +1648,7 @@ a.f.x # E: "int" has no attribute "x" a.f = '' # E: Incompatible types in assignment (expression has type "str", variable has type "int") a.f = 1 reveal_type(a.f) # N: Revealed type is "builtins.int" -[builtins fixtures/property.pyi] +[builtins fixtures/property-full.pyi] [case testPropertyWithDeleterButNoSetter] import typing @@ -1663,7 +1662,367 @@ class A: a = A() a.f = a.f # E: Property "f" defined in "A" is read-only a.f.x # E: "int" has no attribute "x" -[builtins fixtures/property.pyi] +[builtins fixtures/property-full.pyi] + +[case testPropertyAccessOnClass] +class Foo: + @property + def bar(self) -> bool: + return True + + reveal_type(bar) # N: Revealed type is "builtins.property" + +reveal_type(Foo.bar) # N: Revealed type is "builtins.property" +reveal_type(Foo.bar(Foo())) # E: "property" not callable \ + # N: Revealed type is "Any" +reveal_type(Foo.bar.fget(Foo())) # E: "None" not callable \ + # N: Revealed type is "Any" + +class Bar: + @property + def bar(self) -> bool: + return True + @bar.setter + def bar(self, bar: bool) -> None: + pass + + reveal_type(bar) # N: Revealed type is "builtins.property" + +reveal_type(Bar.bar) # N: Revealed type is "builtins.property" +reveal_type(Bar.bar(Bar())) # E: "property" not callable \ + # N: Revealed type is "Any" +reveal_type(Bar.bar.fget(Bar())) # E: "None" not callable \ + # N: Revealed type is "Any" +[builtins fixtures/property-full.pyi] + +[case testPropertyAccessOnClass2] +import functools +from functools import cached_property + +class Foo: + @cached_property + def foo(self) -> bool: + return True + + @functools.cached_property + def bar(self) -> bool: + return True + + reveal_type(foo) # N: Revealed type is "functools.cached_property[Any]" + reveal_type(bar) # N: Revealed type is "functools.cached_property[Any]" + +reveal_type(Foo.foo) # N: Revealed type is "functools.cached_property[Any]" +reveal_type(Foo.bar) # N: Revealed type is "functools.cached_property[Any]" +Foo.foo(Foo()) # E: "cached_property[Any]" not callable +Foo.bar(Foo()) # E: "cached_property[Any]" not callable +[builtins fixtures/property-full.pyi] + +[case testPropertySetterDefinedInSubclass] +# See https://github.com/python/mypy/issues/5936 +# Ideally we should support this, but at least be explicit that it isn't supported +class Base: + @property + def value(self) -> int: + return 0 + + reveal_type(value) # N: Revealed type is "builtins.property" + reveal_type(value.setter) # N: Revealed type is "def (def (Any, Any)) -> builtins.property" + reveal_type(value.fset) # N: Revealed type is "Union[def (Any, Any), None]" + reveal_type(value.fget) # N: Revealed type is "Union[def (Any) -> Any, None]" + +class Sub(Base): + @Base.value.setter + def value(self, value: int) -> None: # E: Property setter overrides are not supported. \ + # N: Consider overriding getter explicitly with super() call. + self._value = value + +class Sub2(Base): + @Base.value.setter + def value(self, value: int) -> None: # E: Property setter overrides are not supported. \ + # N: Consider overriding getter explicitly with super() call. + self._value = value + @Base.value.deleter # E: Name "value" already defined on line 19 + def value(self) -> None: ... + +reveal_type(Base.value) # N: Revealed type is "builtins.property" +reveal_type(Base().value) # N: Revealed type is "builtins.int" +Base().value = 2 # E: Property "value" defined in "Base" is read-only + +reveal_type(Sub.value) # N: Revealed type is "Any" +reveal_type(Sub().value) # N: Revealed type is "Any" +Sub().value = 2 +[builtins fixtures/property-full.pyi] + +[case testPropertySetterOverrideInSubclass] +# See https://github.com/python/mypy/issues/5936 +# Ideally we should support this, but at least be explicit that it isn't supported +class Base: + _value: int + + @property + def value(self) -> int: + return self._value + @value.setter + def value(self, value: int) -> None: + self._value = value + + reveal_type(value) # N: Revealed type is "builtins.property" + reveal_type(value.setter) # N: Revealed type is "def (def (Any, Any)) -> builtins.property" + +class Sub(Base): + @Base.value.setter + def value(self, value: int) -> None: # E: Property setter overrides are not supported. \ + # N: Consider overriding getter explicitly with super() call. + self._value = value + +class Sub2(Base): + @Base.value.setter + def value(self, value: int) -> None: # E: Property setter overrides are not supported. \ + # N: Consider overriding getter explicitly with super() call. + self._value = value + @Base.value.deleter # E: Name "value" already defined on line 22 + def value(self) -> None: ... + +reveal_type(Base.value) # N: Revealed type is "builtins.property" +reveal_type(Base().value) # N: Revealed type is "builtins.int" +Base().value = 2 + +reveal_type(Sub.value) # N: Revealed type is "Any" +reveal_type(Sub().value) # N: Revealed type is "Any" +Sub().value = 2 +[builtins fixtures/property-full.pyi] + +[case testUntypedPropertySetterOverrideInSubclass] +# See https://github.com/python/mypy/issues/5936 +# Ideally we should support this, but at least be explicit that it isn't supported +class Base: + _value: int + + @property + def value(self): + return self._value + @value.setter + def value(self, value): + self._value = value + + reveal_type(value) # N: Revealed type is "builtins.property" + reveal_type(value.setter) # N: Revealed type is "def (def (Any, Any)) -> builtins.property" + +class Sub(Base): + @Base.value.setter + def value(self, value): # E: Property setter overrides are not supported. \ + # N: Consider overriding getter explicitly with super() call. + self._value = value + +class Sub2(Base): + @Base.value.deleter + def value(self): # E: Property deleter overrides are not supported. \ + # N: Consider overriding getter explicitly with super() call. + self._value = value + +class Sub3(Base): + @Base.value.setter + def value(self, value): # E: Property setter overrides are not supported. \ + # N: Consider overriding getter explicitly with super() call. + self._value = value + @Base.value.deleter # E: Name "value" already defined on line 27 + def value(self): + self._value = value + +reveal_type(Base.value) # N: Revealed type is "builtins.property" +reveal_type(Base().value) # N: Revealed type is "Any" +Base().value = 2 + +reveal_type(Sub.value) # N: Revealed type is "Any" +reveal_type(Sub().value) # N: Revealed type is "Any" +Sub().value = 2 +[builtins fixtures/property-full.pyi] + +[case testPropertySetterNonAdjacent] +# See https://github.com/python/mypy/issues/1465 +# We want to support this later, but at least fail explicitly for now. +class A: + @property + def val(self) -> int: + return 0 + + reveal_type(val) # N: Revealed type is "builtins.property" + + def _other(self) -> None: ... + + @val.setter # E: Name "val" already defined on line 4 \ + # N: Property setter and deleter must be adjacent to the getter. + def val(self, val: int) -> None: ... + + reveal_type(val) # N: Revealed type is "builtins.property" + +class B: + @property + def val(self) -> int: + return 0 + @val.setter + def val(self, val: int) -> None: ... + + reveal_type(val) # N: Revealed type is "builtins.property" + + def _other(self) -> None: ... + + @val.deleter # E: Name "val" already defined on line 18 \ + # N: Property setter and deleter must be adjacent to the getter. + def val(self) -> None: ... + + reveal_type(val) # N: Revealed type is "builtins.property" + +class C: + @property + def val(self): + return 0 + + reveal_type(val) # N: Revealed type is "builtins.property" + + def _other(self): ... + + @val.setter # E: Name "val" already defined on line 34 \ + # N: Property setter and deleter must be adjacent to the getter. + def val(self, new): ... + + reveal_type(val) # N: Revealed type is "builtins.property" + +class D: + @property + def val(self): ... + @val.setter + def val(self, val): ... + + reveal_type(val) # N: Revealed type is "builtins.property" + + def _other(self) -> None: ... + + @val.deleter # E: Name "val" already defined on line 48 \ + # N: Property setter and deleter must be adjacent to the getter. + def val(self): ... + + reveal_type(val) # N: Revealed type is "builtins.property" + +class E: + @property + def val(self): ... + + reveal_type(val) # N: Revealed type is "builtins.property" + + def _other(self) -> None: ... + + @val.setter # E: Name "val" already defined on line 63 \ + # N: Property setter and deleter must be adjacent to the getter. + def val(self, val): ... + @val.deleter # E: Name "val" already defined on line 70 + def val(self): ... + + reveal_type(val) # N: Revealed type is "builtins.property" + +class F: + @property + def val(self): ... + + reveal_type(val) # N: Revealed type is "builtins.property" + def _other(self) -> None: ... + + @val.setter # E: Name "val" already defined on line 78 \ + # N: Property setter and deleter must be adjacent to the getter. + def val(self, val): ... + + reveal_type(val) # N: Revealed type is "builtins.property" + def _other2(self) -> None: ... + + @val.deleter # E: Name "val" already defined on line 78 \ + # N: Property setter and deleter must be adjacent to the getter. + def val(self): ... + + reveal_type(val) # N: Revealed type is "builtins.property" +[builtins fixtures/property-full.pyi] + +[case testPropertyRedefinition] +class A1: + @property + def fn(self) -> None: ... + + @property # E: Only supported top decorator is @fn.setter + def fn(self) -> None: ... + +class B1: + @property + def fn(self) -> None: ... + + def foo(self) -> None: ... + + @property # E: Name "fn" already defined on line 9 + def fn(self) -> None: ... + +class A2: + @property + def fn(self) -> None: ... + @fn.setter + def fn(self, _: None) -> None: ... + + @property # E: Only supported top decorator is @fn.setter + def fn(self) -> None: ... + +class B2: + @property + def fn(self) -> None: ... + @fn.setter + def fn(self, _: None) -> None: ... + + def foo(self) -> None: ... + + @property # E: Name "fn" already defined on line 27 + def fn(self) -> None: ... + +class A3: + @property + def fn(self) -> None: ... + @fn.setter + def fn(self, _: None) -> None: ... + + @property # E: Only supported top decorator is @fn.setter + def fn(self) -> None: ... + @fn.setter + def fn(self, _: None) -> None: ... + +class B3: + @property + def fn(self) -> None: ... + @fn.setter + def fn(self, _: None) -> None: ... + + def foo(self) -> None: ... + + @property # E: Name "fn" already defined on line 49 + def fn(self) -> None: ... + @fn.setter + def fn(self, _: None) -> None: ... + +class A4: + @property + def fn(self) -> None: ... + @fn.setter + def fn(self, _: None) -> None: ... + + @fn.setter + def fn(self, _: None) -> None: ... + +class B4: + @property + def fn(self) -> None: ... + @fn.setter + def fn(self, _: None) -> None: ... + + def foo(self) -> None: ... + + @fn.setter # E: Name "fn" already defined on line 71 \ + # N: Property setter and deleter must be adjacent to the getter. + def fn(self, _: None) -> None: ... +[builtins fixtures/property-full.pyi] -- Descriptors -- ----------- diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index 58973307a1ae..606e0cf27c45 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -3276,7 +3276,7 @@ class A: @decorator def f(self) -> int: ... -reveal_type(A.f) # N: Revealed type is "__main__.something_callable" +reveal_type(A.f) # N: Revealed type is "builtins.property" reveal_type(A().f) # N: Revealed type is "builtins.str" [builtins fixtures/property.pyi] diff --git a/test-data/unit/fixtures/property-full.pyi b/test-data/unit/fixtures/property-full.pyi new file mode 100644 index 000000000000..378a7b91028f --- /dev/null +++ b/test-data/unit/fixtures/property-full.pyi @@ -0,0 +1,43 @@ +from typing import Any, Callable, Generic, TypeVar + +_T = TypeVar('_T') + +class object: + def __init__(self) -> None: pass + +class type: + def __init__(self, x: Any) -> None: pass + +class function: pass + +class property: + fget: Callable[[Any], Any] | None + fset: Callable[[Any, Any], None] | None + fdel: Callable[[Any], None] | None + __isabstractmethod__: bool + + def __init__( + self, + fget: Callable[[Any], Any] | None = ..., + fset: Callable[[Any, Any], None] | None = ..., + fdel: Callable[[Any], None] | None = ..., + doc: str | None = ..., + ) -> None: ... + def getter(self, fget: Callable[[Any], Any], /) -> property: ... + def setter(self, fset: Callable[[Any, Any], None], /) -> property: ... + def deleter(self, fdel: Callable[[Any], None], /) -> property: ... + def __get__(self, instance: Any, owner: type | None = None, /) -> Any: ... + def __set__(self, instance: Any, value: Any, /) -> None: ... + def __delete__(self, instance: Any, /) -> None: ... +class classmethod: pass + +class list: pass +class dict: pass +class int: pass +class float: pass +class str: pass +class bytes: pass +class bool: pass +class ellipsis: pass + +class tuple(Generic[_T]): pass diff --git a/test-data/unit/fixtures/property.pyi b/test-data/unit/fixtures/property.pyi index 933868ac9907..9d3c0005b7fe 100644 --- a/test-data/unit/fixtures/property.pyi +++ b/test-data/unit/fixtures/property.pyi @@ -10,7 +10,7 @@ class type: class function: pass -property = object() # Dummy definition +class property: pass # Dummy definition class classmethod: pass class list(typing.Generic[_T]): pass