PoC Injectable type hint

[dkraczkowski]

No description provided.

[dkraczkowski]

This is the clue of this PR.

[dkraczkowski]

I am not really happy about it, but tried __class_getitem__ and generic class, but IDE is not able to recognize it properly. mypy is throwing errors, and pylint is exploding :D. So it is what it is.

[dkraczkowski]

TypeAlias is required by mypy, otherwise, it will start complaining that Injectable is not a valid type.

[dkraczkowski]

I have used pretty much the simplified scenario from the issue.

[dkraczkowski]

Here we have Injectable usage.

[and3rson]

Awesome! Thanks @dkraczkowski!

I did some testing and, for some reason, mypy fails to properly see Injectable[X] as it is, and instead infers it as Any, thus preventing the plugin from spotting injected args:

def foo(x: Injectable[str]):
    pass

reveal_type(foo)
# Revealed type is "def (x: Any) -> Any"

When I was doing some local testing few days ago, I made my own Injectable by stealing the typing.Optional as follows:

from typing import _SpecialForm
@_SpecialForm
def Injectable(self, parameters):
    # Stolen from `typing.Optional`
    arg = _type_check(parameters, f"{self} requires a single type.")
    return arg

def foo(x: Injectable[str]):
    pass

Revealed type is "def (x: Injectable?[builtins.str]) -> Any"

...however this makes mypy complain that Function "1.Injectable" is not valid as a type - weird, considering that typing.Optional has same implementation and works fine.

Any thoughts on that?

[and3rson]

Update: I've removed #type: ignore from your code but it still resolves to Any:

reveal_type(Injectable[IRepository])
# Any

Possibly relevant: python/mypy#11501

[and3rson]

Update 2: seems like some metaclass magic could do the trick:

class M(type):
    def __getitem__(self, typ: Type[I]) -> I:
        return typ

class Injectable(Generic[I], metaclass=M): pass

reveal_type(Injectable[X])
# Revealed type is "def () -> kink.inject.Injectable[1.IRepository]"

That still has some issues, trying to find a workaround for it.

Reference: python/mypy@e0e9c2e#diff-1c26dc582ed5f36723414080dfb646bc71cf20736b73ae1ccff9b90a5d3befaf

[and3rson]

Okay, I think I've finally found a way to do this:

from typing import Generic, TypeVar
from typing_extensions import reveal_type
import inspect


class Foo:
    pass


T = TypeVar('T')


class M(type):
    def __getitem__(self, item: T) -> T:
        return item


class Injectable(Generic[T], metaclass=M):
    pass


def foo(a: str, b: Injectable[Foo]):
    pass


# mypy:
reveal_type(foo)  # Revealed type is "def (a: builtins.str, b: test.Injectable[test.Foo]) -> Any"

# runtime:
print(Injectable[Foo])  # <class '__main__.Foo'>
print(inspect.signature(foo))  # (a: str, b: __main__.Foo)

[and3rson]

I think I finally managed to make it work.

Some changes are required though:

I = TypeVar('I')

if TYPE_CHECKING:
    # Give mypy a generic version of `Injectable[I]` that also inherits `I`.
    # Do not implement `__getattr__` to prevent downgrading `Injectable[I]` to `I`.
    class Injectable(I, Generic[I]):
        pass

else:
    # Give python a working version of `Injectable[I]` that actually returns `I`.
    # This will convert `Injectable[I]` to `I` during runtime which is compatible with inject logic.
    # (Taken from your PR)
    class _Injectable:
        def __getitem__(self, item: Type[I]) -> I:  # FIXME: Shouldn't this be Type[I] -> Type[I]?
            return item  # type: ignore
    Injectable: TypeAlias = _Injectable()  # type: ignore[misc]

    # Alternative version using metaclasses:
    class M(type):
        def __getitem__(self, item: Type[I]) -> Type[I]:
            return item
    class Injectable(metaclass=M):
        pass

Corresponding plugin code:

from mypy.nodes import ARG_OPT
from mypy.plugin import FunctionContext, Plugin
from mypy.types import Instance

# Set this to contain full names of corresponding items
INJECT_FULLNAME = 'kink.inject.inject'
INJECTABLE_FULLNAME = 'kink.inject.Injectable'


class KinkPlugin(Plugin):
    def get_function_hook(self, fullname: str):
        if fullname == INJECT_FULLNAME:
            return self._inject_hook
        return super().get_function_hook(fullname)

    def _inject_hook(self, ctx: FunctionContext):
        try:
            func = ctx.arg_types[0][0]
        except IndexError:
            # FIXME: This is not an `@inject` form, but `@inject()`.
            # Do nothing since we don't have the function signature yet.
            return ctx.default_return_type
        for i, (arg_name, arg_type) in enumerate(zip(func.arg_names, func.arg_types)):
            # Check if argument is of type `Injectable[T]`
            if (
                arg_name not in ('self', 'cls')
                and isinstance(arg_type, Instance)
                and arg_type.type.fullname == INJECTABLE_FULLNAME
            ):
                # Mark as optional & replace with inner arg
                func.arg_kinds[i] = ARG_OPT
                # func.arg_types[i] = arg_type.args[0]  # Not necessary since Injectable[T] implements T
        return func


def plugin(version: str):
    return KinkPlugin

This still doesn't work with @inject(), only with @inject, but this could possible be worked around by https://docs.python.org/3/library/typing.html#typing.overload

What do you think?

[and3rson]

@dkraczkowski I just gave it some more thought, and I think there's a more universal solution for this.
The problem with my idea of Injectable as a type hint is that it requires plugins to work properly. While we may succeed at making this work for mypy, other tools like pylance & pylint will still complain about the signature. Feels like the type hint might have been a bad idea...

However, I was thinking of adding a different way to mark injected values, but with the help of injected function with a slightly different purpose than Injectable: by using default value rather than type hint. Consider this example:

T = TypeVar('T')

def injected(typ: Type[T]) -> T:
    # Stub for setting default values of injected arguments to make checkers happy.
    # This eliminates "missing argument" errors.
    # This is totally optional - most users will not use this, it's for those who want
    # to make mypy & pylint work properly.
    pass

# ...

class UserService:
    @inject()
    def register_new_user(self, username, repo: IRepository = injected(IRepository)):
        user = User(username)
        repo.save_user(user)

This passes mypy and pylint checks. This is also fully backwards-compatible - it preserves raising of exceptions like kink.errors.execution_error.ExecutionError: Cannot execute function without required parameters..
Some additional changes might be required to def inject(...): ... signature to properly work with both parametrized & bare decorator forms by using @overload: https://docs.python.org/3/library/typing.html#typing.overload

Pros:

• Less type juggling comparing to Injectable[T]
• Friendly to all popular checkers
• Plays well with differing signatures comparing to Injectable[T] approach (does not trigger Liskov violation when, for example, UserService extends another class that defines register_new_user as def register_new_user(self, username): ...)
• No need for a plugin
• Pluggable & optional

Cons:

• Does not work when injecting instances that extend typing.Protocol (or other abstract classes): Only concrete class can be given where "Type[IRepository]" is expected. However, this is currently being addressed:
  • abstract dataclass inheritance gives Only concrete class can be given where "Type[Abstract]" is expected python/mypy#5374
  • Use case for typing.Type with abstract types python/mypy#4717 (workaround in last comment - --disable-error-code=type-abstract will be available in the next release of mypy)

[dkraczkowski]

@and3rson For some reason, GitHub didn't notify me about your comment o_0. What if we use your first solution with a minor tweak:

from typing import _SpecialForm
@_SpecialForm
def _Injectable(self, parameters):
    # Stolen from `typing.Optional`
    arg = _type_check(parameters, f"{self} requires a single type.")
    return arg

Injectable: TypeAlias = _Injectable  # type: ignore

def foo(x: Injectable[str]):
    pass

I think this should sort out mypy and work how we want it too. From my perspective, assigning a default value is not the best idea (not from my perspective, maybe I would need to get used to it). Other explorations were fascinating (thank you for that).

I also had a bit of a problem myself sorting it out, so it is practical and not disturbing.

Ping me in the issue if I don't respond, Thanks ;)

[dkraczkowski]

OK. I have checked the solution and seems like IDE is not going to regonize it and mypy is complaining a lot. I will think how to solve it differently.

[and3rson]

@dkraczkowski JFYI - I've been using service: IService = injected(IService) notation for the past few days and had a lot of luck making both mypy & pylint happy with it. I'm still not sure if it's the best solution and I agree with you on that, but it seems like it's really very portable. However it works well only with @inject() form, not @inject. I think that if kink had @overload definitions for callable and non-callable versions of inject decorator, it would make injected() work even better.

Some docs I've found:

• https://docs.python.org/3/library/typing.html#typing.overload
• https://stackoverflow.com/questions/71736179/typing-an-overloaded-decorator-wrapped-in-partial

Furthermore, having @overload would bring better typing to callable and non-callable versions of inject decorator, so I think you could consider using them to provide better static typing for the decorator itself irregardless of which way we decide to proceed with the original issue.

P.S. I'm not in any way pushing you to accept my proposal with injected - I'm only sharing my findings so far just in case they are helpful in your research. :)

Thanks for the time & efforts you're putting into this! Kink has become our go-to library for dependency injection in one of our new healthcare projects and is going to production next week!

[dkraczkowski]

@and3rson Thank you for the links I will have a look in my spare time. I am really happy to see another projects in production using kink, this is really an awesome news and I wish you all the best with deployment ;).

[adamlogan73]

A couple of things:
1: how can I help move this along? I have time I can use to noodle this, but its been dead for about a month in this conversation and I don't want to waste work if progress has been made in the mean time.
2: I actually end up putting a comment everywhere we inject to make it explicit that we are injecting, maybe the default parameter isn't a bad thing?
2a: I am pretty sure this behavior is intended to be optional, thus backwards compatible, and only necessary if you want to use it.

[adamlogan73]

@dkraczkowski @and3rson Any progress on this? Willing to help but don't want to duplicate effort and need to get this fixed.

[dkraczkowski]

@adamlogan73 I am happy to share the effort, lately I did not had much time to look into issues that the community is facing.