ksindi / implements

:snake: Pythonic interfaces using decorators
http://implements.readthedocs.io/
Apache License 2.0
33 stars 4 forks source link

return signature isn't check on properties #22

Closed cat-0 closed 3 years ago

cat-0 commented 3 years ago

Hi. Wonderful library, why do I only discover it now :wink: ?

Using implements-0.2.0, I have the following code which IMHO should prevented by implements, but is executed without error :

from implements import Interface, implements

class A(Interface):
    @property
    def foo(self) -> int:
        pass

    @foo.setter
    def set_foo(self, x: int) -> None:
        pass

@implements(A)
class B():
    @property
    def foo(self) -> str:
        pass

    @foo.setter
    def set_foo(self, x: str) -> int:
        pass

@implements(A)
class C():
    @property
    def foo(self):
        pass

    @foo.setter
    def set_foo(self, x):
        pass

Both the foo and set_foo signatures don't seem to be checked. I guess this is related to #10 and #12.

Should the library cover this case or am I doing something wrong ?

pshirali commented 3 years ago

@cat-0 thank you for filing this. Yes, you have found something that this library should address !!

This is related to #10. Data descriptors themselves are not callable, hence, signature checks will fail against them. However, I see scope for an additional check that could be performed by comparing signatures of the getter, setter and deleter (via fget, fset and fdel attributes within a property). This could address the problem that you've mentioned in your post.

>>> inspect.signature(A.foo)               <---- already known from #10 
*** TypeError: <property object at 0x1074c5e50> is not a callable object

----

# However, the following can be fetched and verified (currently unsupported)

>>> inspect.signature(A.foo.fget)
<Signature (self) -> int>

>>> inspect.signature(A.set_foo.fset)
<Signature (self, x: int) -> None>

The library already fetches getters, setters and deleters from data descriptors and compares them for their presence in the implementation. A signature comparison could be additionally implemented after this if-block.

Would you be interested in working on and contributing a fix for this? Would love to support you.

Thanks again! 👍