From b8055512bd3cc54a0b82f8382ac39b3430cf9941 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sun, 14 Aug 2022 01:00:06 +0100 Subject: [PATCH] Enable support for decorated properties where possible (#13409) Fixes #1362 I delete the old cryptic error message and instead: * Enable the situations that we can already handle (cover 95% of what was discussed in the issue) * For the rest give more precise error messages Co-authored-by: graingert <> --- mypy/checker.py | 3 + mypy/semanal.py | 23 +++++--- test-data/unit/check-abstract.test | 20 +++++++ test-data/unit/check-classes.test | 8 +-- test-data/unit/check-functions.test | 85 ++++++++++++++++++++++++++++ test-data/unit/check-functools.test | 4 +- test-data/unit/fixtures/property.pyi | 1 + test-data/unit/semanal-errors.test | 20 ++----- 8 files changed, 136 insertions(+), 28 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index bc9bdf5390ec..ab938e8a7b3c 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -4310,6 +4310,9 @@ def visit_decorator(self, e: Decorator) -> None: e.var.type = sig e.var.is_ready = True if e.func.is_property: + if isinstance(sig, CallableType): + 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(): self.check_method_override(e) diff --git a/mypy/semanal.py b/mypy/semanal.py index 88565058e146..4c8a143d15e0 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1049,6 +1049,8 @@ def analyze_overload_sigs_and_impl( else: item.func.is_overload = True types.append(callable) + if item.var.is_property: + self.fail("An overload can not be a property", item) elif isinstance(item, FuncDef): if i == len(defn.items) - 1 and not self.is_stub_file: impl = item @@ -1168,7 +1170,7 @@ def analyze_property_with_multi_part_definition(self, defn: OverloadedFuncDef) - deleted_items = [] for i, item in enumerate(items[1:]): if isinstance(item, Decorator): - if len(item.decorators) == 1: + if len(item.decorators) >= 1: node = item.decorators[0] if isinstance(node, MemberExpr): if node.name == "setter": @@ -1176,8 +1178,10 @@ def analyze_property_with_multi_part_definition(self, defn: OverloadedFuncDef) - first_item.var.is_settable_property = True # Get abstractness from the original definition. item.func.abstract_status = first_item.func.abstract_status - else: - self.fail("Decorated property not supported", item) + else: + 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) @@ -1258,6 +1262,7 @@ def visit_decorator(self, dec: Decorator) -> None: d.accept(self) removed: List[int] = [] no_type_check = False + could_be_decorated_property = False for i, d in enumerate(dec.decorators): # A bunch of decorators are special cased here. if refers_to_fullname(d, "abc.abstractmethod"): @@ -1288,8 +1293,6 @@ def visit_decorator(self, dec: Decorator) -> None: elif refers_to_fullname(d, "functools.cached_property"): dec.var.is_settable_property = True self.check_decorated_function_is_method("property", dec) - if len(dec.func.arguments) > 1: - self.fail("Too many arguments", dec.func) elif refers_to_fullname(d, "typing.no_type_check"): dec.var.type = AnyType(TypeOfAny.special_form) no_type_check = True @@ -1304,6 +1307,10 @@ def visit_decorator(self, dec: Decorator) -> None: removed.append(i) else: self.fail("@final cannot be used with non-method functions", d) + elif not dec.var.is_property: + # We have seen a "non-trivial" decorator before seeing @property, if + # we will see a @property later, give an error, as we don't support this. + could_be_decorated_property = True for i in reversed(removed): del dec.decorators[i] if (not dec.is_overload or dec.var.is_property) and self.type: @@ -1311,8 +1318,10 @@ def visit_decorator(self, dec: Decorator) -> None: dec.var.is_initialized_in_class = True if not no_type_check and self.recurse_into_functions: dec.func.accept(self) - if dec.decorators and dec.var.is_property: - self.fail("Decorated property not supported", dec) + if could_be_decorated_property and dec.decorators and dec.var.is_property: + self.fail("Decorators on top of @property are not supported", dec) + if (dec.func.is_static or dec.func.is_class) and dec.var.is_property: + self.fail("Only instance methods can be decorated with @property", dec) if dec.func.abstract_status == IS_ABSTRACT and dec.func.is_final: self.fail(f"Method {dec.func.name} is both abstract and final", dec) diff --git a/test-data/unit/check-abstract.test b/test-data/unit/check-abstract.test index c782609f7ec0..e384cb89120b 100644 --- a/test-data/unit/check-abstract.test +++ b/test-data/unit/check-abstract.test @@ -1030,3 +1030,23 @@ def deco(cls: Type[T]) -> Type[T]: ... class A(metaclass=ABCMeta): @abstractmethod def foo(self, x: int) -> None: ... + +[case testAbstractPropertiesAllowed] +from abc import abstractmethod + +class B: + @property + @abstractmethod + def x(self) -> int: ... + @property + @abstractmethod + def y(self) -> int: ... + @y.setter + @abstractmethod + def y(self, value: int) -> None: ... + +B() # E: Cannot instantiate abstract class "B" with abstract attributes "x" and "y" +b: B +b.x = 1 # E: Property "x" defined in "B" is read-only +b.y = 1 +[builtins fixtures/property.pyi] diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 488531845be5..6f302144d7bc 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -7006,8 +7006,8 @@ class A: def y(self) -> int: ... @y.setter def y(self, value: int) -> None: ... - @dec - def y(self) -> None: ... # TODO: This should generate an error + @dec # E: Only supported top decorator is @y.setter + def y(self) -> None: ... reveal_type(A().y) # N: Revealed type is "builtins.int" [builtins fixtures/property.pyi] @@ -7044,7 +7044,7 @@ reveal_type(D1() + 0.5) # N: Revealed type is "__main__.D1" [builtins fixtures/primitives.pyi] [case testRefMethodWithDecorator] -from typing import Type +from typing import Type, final class A: pass @@ -7058,7 +7058,7 @@ class B: return A class C: - @property + @final @staticmethod def A() -> Type[A]: return A diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index e4cbdc076996..32d531ebbe99 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -2589,3 +2589,88 @@ def a(b: any): pass # E: Function "builtins.any" is not valid as a type \ def a(b: callable): pass # E: Function "builtins.callable" is not valid as a type \ # N: Perhaps you meant "typing.Callable" instead of "callable"? [builtins fixtures/callable.pyi] + +[case testDecoratedProperty] +from typing import TypeVar, Callable, final + +T = TypeVar("T") + +def dec(f: Callable[[T], int]) -> Callable[[T], str]: ... +def dec2(f: T) -> T: ... + +class A: + @property + @dec + def f(self) -> int: pass + @property + @dec2 + def g(self) -> int: pass +reveal_type(A().f) # N: Revealed type is "builtins.str" +reveal_type(A().g) # N: Revealed type is "builtins.int" + +class B: + @final + @property + @dec + def f(self) -> int: pass +reveal_type(B().f) # N: Revealed type is "builtins.str" + +class C: + @property # E: Only instance methods can be decorated with @property + @classmethod + def f(cls) -> int: pass +reveal_type(C().f) # N: Revealed type is "builtins.int" +[builtins fixtures/property.pyi] +[out] + +[case testDecoratedPropertySetter] +from typing import TypeVar, Callable, final + +T = TypeVar("T") +def dec(f: T) -> T: ... + +class A: + @property + @dec + def f(self) -> int: pass + @f.setter + @dec + def f(self, v: int) -> None: pass +reveal_type(A().f) # N: Revealed type is "builtins.int" + +class B: + @property + @dec + def f(self) -> int: pass + @dec # E: Only supported top decorator is @f.setter + @f.setter + def f(self, v: int) -> None: pass + +class C: + @dec # E: Decorators on top of @property are not supported + @property + def f(self) -> int: pass + @f.setter + @dec + def f(self, v: int) -> None: pass +[builtins fixtures/property.pyi] +[out] + +[case testInvalidArgCountForProperty] +from typing import Callable, TypeVar + +T = TypeVar("T") +def dec(f: Callable[[T], int]) -> Callable[[T, int], int]: ... + +class A: + @property # E: Too many arguments for property + def f(self, x) -> int: pass + @property # E: Too many arguments for property + @dec + def e(self) -> int: pass + @property + def g() -> int: pass # E: Method must have at least one argument + @property + def h(self, *args, **kwargs) -> int: pass # OK +[builtins fixtures/property.pyi] +[out] diff --git a/test-data/unit/check-functools.test b/test-data/unit/check-functools.test index 9233ece6ccfc..f95b823a5291 100644 --- a/test-data/unit/check-functools.test +++ b/test-data/unit/check-functools.test @@ -119,8 +119,8 @@ class Child(Parent): def f(self) -> str: pass @cached_property def g(self) -> int: pass - @cached_property - def h(self, arg) -> int: pass # E: Too many arguments + @cached_property # E: Too many arguments for property + def h(self, arg) -> int: pass reveal_type(Parent().f) # N: Revealed type is "builtins.str" reveal_type(Child().f) # N: Revealed type is "builtins.str" reveal_type(Child().g) # N: Revealed type is "builtins.int" diff --git a/test-data/unit/fixtures/property.pyi b/test-data/unit/fixtures/property.pyi index b3f60abaf8a0..9dca0d50a3be 100644 --- a/test-data/unit/fixtures/property.pyi +++ b/test-data/unit/fixtures/property.pyi @@ -11,6 +11,7 @@ class type: class function: pass property = object() # Dummy definition +class classmethod: pass class dict: pass class int: pass diff --git a/test-data/unit/semanal-errors.test b/test-data/unit/semanal-errors.test index ace95bcdced9..f7ec4040a5b1 100644 --- a/test-data/unit/semanal-errors.test +++ b/test-data/unit/semanal-errors.test @@ -1205,23 +1205,13 @@ def f() -> int: pass [builtins fixtures/property.pyi] [out] -[case testInvalidArgCountForProperty] -import typing -class A: - @property - def f(self, x) -> int: pass # E: Too many arguments - @property - def g() -> int: pass # E: Method must have at least one argument -[builtins fixtures/property.pyi] -[out] - [case testOverloadedProperty] from typing import overload class A: - @overload # E: Decorated property not supported + @overload # E: Decorators on top of @property are not supported @property def f(self) -> int: pass - @property # E: Decorated property not supported + @property # E: Only supported top decorator is @f.setter @overload def f(self) -> int: pass [builtins fixtures/property.pyi] @@ -1232,7 +1222,7 @@ from typing import overload class A: @overload # E: An overloaded function outside a stub file must have an implementation def f(self) -> int: pass - @property # E: Decorated property not supported + @property # E: An overload can not be a property @overload def f(self) -> int: pass [builtins fixtures/property.pyi] @@ -1242,10 +1232,10 @@ class A: import typing def dec(f): pass class A: - @dec # E: Decorated property not supported + @dec # E: Decorators on top of @property are not supported @property def f(self) -> int: pass - @property # E: Decorated property not supported + @property # OK @dec def g(self) -> int: pass [builtins fixtures/property.pyi]