Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InterfaceClass subclass not recognized as interface #21

Closed
jaraco opened this issue Sep 18, 2020 · 9 comments
Closed

InterfaceClass subclass not recognized as interface #21

jaraco opened this issue Sep 18, 2020 · 9 comments

Comments

@jaraco
Copy link
Contributor

jaraco commented Sep 18, 2020

The project foolscap relies on zope.interface and creates a RemoteInterface as an instance of a subclass of zope.interface.interface.InterfaceClass. When tested against mypy with this plugin, the interface isn't recognized as such an results in "Method must have at least one argument" failed validations.

As an example:

draft $ cat > mypy.ini
[mypy]
ignore_missing_imports = True
plugins=mypy_zope:plugin
draft $ cat > example.py
from foolscap.api import RemoteInterface

      
class Api(RemoteInterface):
    def open():
        "open something"
draft $ pip-run -q mypy foolscap mypy-zope -- -m mypy example.py
example.py:5: error: Method must have at least one argument
Found 1 error in 1 file (checked 1 source file)

I've attempted to create an even more minimal example that doesn't involve foolscap, but I've been unsuccessful:

draft $ cat > example.py
from zope.interface import interface


RemoteInterface = interface.InterfaceClass("RemoteInterface")


class Api(RemoteInterface):
    def open():
        "open something"
draft $ pip-run -q mypy mypy-zope -- -m mypy example.py
example.py:7: error: Variable "example.RemoteInterface" is not valid as a type
example.py:7: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
example.py:7: error: Invalid base class "RemoteInterface"
example.py:8: error: Method must have at least one argument
Found 3 errors in 1 file (checked 1 source file)

I don't understand why

Variable "example.RemoteInterface" is not valid as a type

The note there isn't particularly helpful, as I'm creating neither an alias nor a variable. I'm creating a proper type and in the same way that foolscap does, so why does this example error where foolscap doesn't?

And surely the example error isn't valid until mypy recogognizes RemoteInterface correctly.

Can you help to determine a solution here to support foolscap.api.RemoteInterface subclasses within mypy?

@kedder
Copy link
Member

kedder commented Sep 18, 2020

I don't understand why

Variable "example.RemoteInterface" is not valid as a type

Mainly because RemoteInterface looks like an instance of a class (if you don't know about the runtime magic behind InterfaceClass). And instances of a class are not valid as a type.

mypy-zope is simply don't know how InterfaceClass behaves at runtime and treats it as a regular class. I think it should be possible to teach mypy-zope to treat such expressions, but it is simply not there.

What is supported is class declarations, like:

class RemoteInterface(interface.Interface):
    pass

class Api(RemoteInterface):
    def open():
        "open something"

I'm not completely sure that is equivalent to your case though.

I can't give you a recipe how to implement the support for this use case, but perhaps it would involve adding a special case in get_function_hook() and returning an instance of a type when zope.interface.InterfaceClass is called.

Another hack to try is to add a special __new__ signature to the stub of InterfaceClass in src/zope-stubs/interface/interface.pyi.

@jaraco
Copy link
Contributor Author

jaraco commented Sep 24, 2020

I don't understand why

Variable "example.RemoteInterface" is not valid as a type

Mainly because RemoteInterface looks like an instance of a class... And instances of a class are not valid as a type.

But the same is true of RemoteInterface in foolscap, and the error isn't emitted in my first example. Even if I copy the full implementation of RemoteInterfaceClass and RemoteInterface from foolscap to my example, the error continues. I wish I understood why the implementation in foolscap doesn't trigger this error. Regardless, that issue is secondary to my concern. It would be nice to have a small reproducer for the issue, but the primary issue is that in foolscap, the RemoteInterface isn't recognized as an InterfaceClass instance (and allowing omission of self).

Oh, maybe I know - since I'm only running mypy on example.py, it's not checking anything in foolscap, and that's why the error isn't emitted.

I'll try your recommendations for addressing the foolscap RemoteInterface.

@jaraco
Copy link
Contributor Author

jaraco commented Sep 24, 2020

perhaps it would involve adding a special case in get_function_hook() and returning an instance of a type when zope.interface.InterfaceClass is called.

This approach doesn't have any effect in the foolscap case, so I'm not sure it's relevant.

Another hack to try is to add a special new signature to the stub of InterfaceClass in src/zope-stubs/interface/interface.pyi.

I explored this approach, but quickly realized that there's no suitable return type for __new__:

class InterfaceClass(type, Element, InterfaceBase, Specification):
    ....
    def __new__(self, name: Any, bases: Any = ..., attrs: Optional[Any] = ..., __doc__: Optional[Any] = ..., __module__: Optional[Any] = ...) -> Interface: ...

I tried that line, but at the point where __new__ is defined, Interface is not yet defined, and cannot be until InterfaceClass is defined. Moreover, Interface isn't the correct return type. What __new__ returns is an instance of InterfaceClass and Interface is just one such instantiation.

I did try an alternative syntax for the RemoteInterfaceClass creation:

from zope.interface import interface


class RemoteInterfaceClass(interface.InterfaceClass):
    pass


class RemoteInterface(metaclass=RemoteInterfaceClass):
    pass


class Api(RemoteInterface):
    def open():
        "open something"

Running mypy-zope on this produces:

draft $ pip-run -q mypy-zope -- -m mypy example.py
example.py:13: error: Method must have at least one argument
Found 1 error in 1 file (checked 1 source file)

This error seems to capture the limitation without foolscap.

Do you have any suggestions for addressing this limitation?

@jaraco
Copy link
Contributor Author

jaraco commented Sep 24, 2020

An even simpler implementation triggers the same error:

from zope.interface import interface


class RemoteInterface(metaclass=interface.InterfaceClass):
    pass


class Api(RemoteInterface):
    def open():
        "open something"

@jaraco
Copy link
Contributor Author

jaraco commented Sep 24, 2020

I've found with this diff:

mypy-zope master $ git diff
diff --git a/src/mypy_zope/plugin.py b/src/mypy_zope/plugin.py
index 6a28b5c..2d8bdd7 100644
--- a/src/mypy_zope/plugin.py
+++ b/src/mypy_zope/plugin.py
@@ -258,7 +258,11 @@ class ZopeInterfacePlugin(Plugin):
         self, fullname: str
     ) -> Optional[Callable[[ClassDefContext], None]]:
         # print(f"get_metaclass_hook: {fullname}")
-        return None
+        def analyze_metaclass(ctx: ClassDefContext) -> Callable[[ClassDefContext], None]:
+            if ctx.cls.metaclass.name == 'InterfaceClass':
+                md = self._get_metadata(ctx.cls.info)
+                md["is_interface"] = True
+        return analyze_metaclass
 
     def get_base_class_hook(
         self, fullname: str

Running the checks against that example no longer fail.

It's not a proper fix, but starts to work toward a solution. I couldn't figure out how to resolve that ctx.cls.metaclass to zope.interface.interface.InterfaceClass. And it still doesn't address the fact that InterfaceClass() can't be called with parameters to construct a new Interface type.

Any help you can lend in filling in the gaps and adding support for this use case would be most appreciated.

@kedder
Copy link
Member

kedder commented Sep 26, 2020

"Method must have at least one argument" error indicates that the interface is not recognized as interface. There is a hack that adds "self" argument to all the methods of the interface, so that further mypy checks would not emit this error. See _adjust_interface_function.

Currently the plugin has a special treatment for zope.interface.interface.Interface: it is recognized as a base for all interfaces. interface.InterfaceClass("RemoteInterface") creates another kind of interfaces and it is not recognized.

However, zope.interface.interface.Interface itself is is also defined as

class Interface(metaclass=InterfaceClass): ...

So perhaps it is possible to generalize this so that. And actually your patch makes sense from that point of view. The only thing is missing here: the base zope.interface.interface.Interface gets special treatment: a fake constructor is created for that class to support adaptation pattern (see analyze_interface_base)

@jaraco
Copy link
Contributor Author

jaraco commented Oct 31, 2020

Running the checks against that example no longer fail.

I'm no longer able to replicate this observation. I think the observation I'd encountered may have been due to a false negative from a cached result.

@jaraco
Copy link
Contributor Author

jaraco commented Oct 31, 2020

Aha. I had forgotten to enable the plugin. Enabling the plugin (which apparently can only be done by creating a config file), I can once again replicate the success.

kedder pushed a commit that referenced this issue Dec 27, 2020
* Indicate any instance whose class type is named InterfaceClass is an interface.

* Rely on fullname to more precisely detect an InterfaceClass instance.

* Apply black style to patch

* Inline the check. Avoids need to define types for a function.

* Satisfy type checks through casting.

* Remove analyze_direct, no longer needed now that the metaclass is handled.

* Add test capturing failed expectation. Ref #21.
@kedder
Copy link
Member

kedder commented Dec 27, 2020

Closing after merging of #24

@kedder kedder closed this as completed Dec 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants