Skip to content

Commit

Permalink
Enable support for decorated properties where possible (#13409)
Browse files Browse the repository at this point in the history
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 <>
  • Loading branch information
ilevkivskyi committed Aug 14, 2022
1 parent 3a13b8e commit b805551
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 28 deletions.
3 changes: 3 additions & 0 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
23 changes: 16 additions & 7 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1168,16 +1170,18 @@ 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":
# 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
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)
Expand Down Expand Up @@ -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"):
Expand Down Expand Up @@ -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
Expand All @@ -1304,15 +1307,21 @@ 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:
dec.var.info = self.type
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)

Expand Down
20 changes: 20 additions & 0 deletions test-data/unit/check-abstract.test
Original file line number Diff line number Diff line change
Expand Up @@ -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]
8 changes: 4 additions & 4 deletions test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand All @@ -7058,7 +7058,7 @@ class B:
return A

class C:
@property
@final
@staticmethod
def A() -> Type[A]:
return A
Expand Down
85 changes: 85 additions & 0 deletions test-data/unit/check-functions.test
Original file line number Diff line number Diff line change
Expand Up @@ -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]
4 changes: 2 additions & 2 deletions test-data/unit/check-functools.test
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions test-data/unit/fixtures/property.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class type:
class function: pass

property = object() # Dummy definition
class classmethod: pass

class dict: pass
class int: pass
Expand Down
20 changes: 5 additions & 15 deletions test-data/unit/semanal-errors.test
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand Down

0 comments on commit b805551

Please sign in to comment.