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

Retuning an instance of the class being defined which provides an interface is not handled #39

Open
wsanchez opened this issue Mar 16, 2021 · 6 comments

Comments

@wsanchez
Copy link

In twisted/klein#471, I'm running into the following error in CI:

src/klein/_dihttp.py:62:16: error: Incompatible return value type (got "RequestURL", expected "IDependencyInjector")  [return-value]
            return cls()

The code in question is:

@provider(IRequiredParameter, IDependencyInjector)
class RequestURL:
    """
    Require a hyperlink L{DecodedURL} object from a L{Requirer}.

    @since: Klein NEXT
    """

    @classmethod
    def registerInjector(
        cls,
        injectionComponents: Componentized,
        parameterName: str,
        requestLifecycle: IRequestLifecycle,
    ) -> IDependencyInjector:
        return cls()

…

So here, we return a RequestURL, which is declared as a provider of IDependencyInjector, so I think the above error is incorrect.

I'm guessing this is due to the fact that the definition of RequestURL isn't complete yet?

@wsanchez
Copy link
Author

I think the actual issue is that @provider isn't handled, unlike @implementer.

@kedder
Copy link
Member

kedder commented Mar 16, 2021

Yes, mypy-zope doesn't know anything about @provider. That said, I think your declaration is not quite correct? If instance of RequestURL implements IDependencyInjector, as registerInjector suggests, shouldn't RequestURL be declared with @implementer instead?

@wsanchez
Copy link
Author

Heh… I had to ask @glyph about this also. @implementer says that instances of the class will provide the interface. @provider says that the class itself does so. (Note that the methods are all class methods.)

@wsanchez
Copy link
Author

Super happy to be getting real typing via mypy-zope, by the way. It's great. And thanks for the quick replies to these tickets.

@kedder
Copy link
Member

kedder commented Mar 16, 2021

Heh… I had to ask @glyph about this also. @implementer says that instances of the class will provide the interface. @provider says that the class itself does so. (Note that the methods are all class methods.)

Right, but registerInjector returns an instance of RequestURL. So, in order for registerInjector return type to be correct, an instance of RequestURL must provide the IDependencyInjector . However, it does not provide that interface. Only the class itself does, according to @provider declaration.

So, even though @provider is currently not supported by mypy-zope, I suspect the type hint on registerInjector is not entirely correct. I'd say registerInjector has to return cls, not cls(). Or, alterntively, RequestURL must implement IDependencyInjector, not provide it.

@kedder
Copy link
Member

kedder commented Mar 16, 2021

Super happy to be getting real typing via mypy-zope, by the way. It's great. And thanks for the quick replies to these tickets.

Thanks! I'm glad mypy exists and it was possible to marry zope with it. Interfaces become so much more meaningful with it! :)

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