ivankorobkov / python-inject

Python dependency injection
Apache License 2.0
671 stars 77 forks source link

Type hint broken for inject.attr when using ABCMeta #69

Open TheEdgeOfRage opened 3 years ago

TheEdgeOfRage commented 3 years ago

I'm not sure whether it's possible to do fix this at all, or whether it's an inherent Python thing. Also, just a heads up. This issue is only related to static type checking. It doesn't manifest itself in runtime.

I have an interface class (let's say BaseService) that has ABCMeta as its metaclass with @abstractmethod decorated empty methods. Several other classes (in my example Service) inherit from it and implement the methods. Then I have a factory function that instantiates one of the Implementations and returns it with the BaseService type hint. I use inject bind the factory to the BaseService type.

To resolve the dependency in another class I used inject.attr(BaseService), but the returned value is type hinted as Union[object, Any], instead of BaseService. This leads mypy to flag any code that uses the resolved dependency, saying that the object doesn't have any of the attributes that BaseService has.

Here is a simple reproducible example:

Click to expand ```python from abc import ABCMeta, abstractmethod import inject # Dummy service interface (abstract base) class class BaseService(metaclass=ABCMeta): @abstractmethod def f(self) -> None: pass # Dummy service implementation class Service(BaseService): def f(self) -> None: print('f') # Dummy service factory (for multiple different implementations) def service_factory() -> BaseService: return Service() # Simple Test class to trigger the issue class Test: service = inject.attr(BaseService) def test(self) -> None: self.service.f() # mypy error: Item "object" of "Union[object, Any]" has no attribute "f" # Binding the factory as a constructor (not sure if this is necessary) def bindings(binder: inject.Binder) -> None: binder.bind_to_constructor(BaseService, service_factory) inject.configure(bindings) Test().test() ```

As I said, this code works just fine when I run it through the CPython (3.8) interpreter. The problem is only related to mypy static checking. Running mypy on the code yields the following error:

test.py:29: error: Item "object" of "Union[object, Any]" has no attribute "f"

If i instead remove metaclass=ABCMeta and the @abstractmethod decorator from BaseService and use it as a normal base class, there is no issue. My guess is that the ABCMeta metaclass breaks the type hint for some reason, but I don't know enough about how it works to even make an educated guess.

If this issue is not in fact rooted in this library, let me know and I'll open the issue elsewhere.

Thanks :)

ivankorobkov commented 3 years ago

Hi!

Thank you for a detailed explanation. I'll try to research the issue today or tomorrow and write back.

TheEdgeOfRage commented 3 years ago

I have found a workaround for this, but it's not pretty.

from typing import cast

class Test:
    service = cast(BaseService, inject.attr(BaseService))

    def test(self) -> None:
        self.service.f()  # No error occurs now

By using the cast function from the typing module, mypy correctly identifies the type. It works, but it requires code duplication and isn't really as neat as just having inject.attr(Type).

JosXa commented 3 years ago

I suppose another workaround could be:

T = TypeVar("T", bound=Type)

inject.attr = cast(Callable[[Type[T]], T], inject.attr)

so you only have to do it once. Haven't tested that though, I'm on a phone and only just stumbled over this library. Seriously considering to switch from Haps...

JosXa commented 3 years ago

With these type declarations: https://github.com/ivankorobkov/python-inject/blob/05ad3e87bb179ce56cabb272bd5f75cca2880f7c/inject/__init__.py#L107-L108

In combination with abstract base classes it seems that we're running into a very common problem: https://github.com/python/mypy/issues/5374#issuecomment-582093112

ivankorobkov commented 3 years ago

@TheEdgeOfRage I haven't found any way how to solve this in inject. Also I'm not a mypy expert. If anyone knows how to make this work, please, make a PR.

Rahix commented 3 years ago

I'm hitting this problem in an unrelated project and found a workaround which might help here as well. In https://github.com/python/mypy/issues/4717, someone found out that you can use Callable[..., T] as a "replacement" for Type[T] which allows abstract types. So something like this could work:

T = TypeVar("T", bound=Union[object, Any])
def attr(t: Callable[..., T]) -> T:
    ...

It does smell like a horrible hack though ...

AlTosterino commented 2 years ago

Hey!

Any update on this? I also stumbled upon this issue today ;) I see that https://github.com/python/mypy/issues/5374 is still Open :/

ivankorobkov commented 2 years ago

@AlTosterino Hi! I still know no way to fix this :-( Maybe, someone will find a way and make a PR.