[DependencyInjection] Add #[AutowireMethodOf] attribute to autowire a method of a service as a callable

[nicolas-grekas]

Q	A
Branch?	7.1
Bug fix?	no
New feature?	yes
Deprecations?	no
Issues	-
License	MIT

Let's take a controller for example, with one action that has this argument:

        #[AutowireMethodOf(CommentRepository::class)]
        \Closure $getCommentPaginator,

The proposed attribute tells Symfony to create a closure that will call CommentRepository::getCommentPaginator(). The name of the method to be called is inferred from the name of the parameter.

This is already doable with this syntax, so that the proposed attribute is just a shortcut for this:

        #[AutowireCallable(service: CommentRepository::class, method: 'getCommentPaginator')]
        \Closure $getCommentPaginator,

Using this style allows turning e.g. entity repositories into query functions, which are way more flexible. But because the existing syntax is quite verbose, i looked for a more concise alternative, so here we are with this proposal.

Benefits:

• Increased Flexibility: Allows developers to inject specific methods as Closures, providing greater control over what functionality is injected into
• Improved Readability: By using the attribute, the intention behind the dependency injection is made explicit, improving code clarity.
• Enhanced Modularity: Facilitates a more modular architecture by decoupling services from direct dependencies on specific class methods, making the codebase more maintainable and testable.

Because we leverage the existing code infrastructure for AutowireCallable, if I declare this interface:

interface GetCommentPaginatorInterface
{
    public function __invoke(Conference $conference, int $page): Paginator;
}

then I can also do native types (vs a closure) without doing anything else:

        #[AutowireMethodOf(CommentRepository::class)]
        GetCommentPaginatorInterface $getCommentPaginator,

[GromNaN]

What is the reason to create a new attribute instead of making method optional?

[nicolas-grekas]

What is the reason to create a new attribute instead of making method optional?

Because method is already optional and it currently means __invoke, so that would be confusing.
A new attribute makes it explicit that some convention is used also IMHO (inferring the name of the method from the name of the parameter)

[alexandre-daubois]

Really subjective point of view here, I definitely see an improvement of "instant understanding of what's being done". I think having this "alias attribute" helps to get what's happening at first sight, where the existing notation needs a bit of reflection before getting it.

It feels more human friendly to me

[Kocal]

What are the benefits about AutowireMethodOf and AutowireCallable, versus directly injecting CommentRepository?

[OskarStark]

Only one con IMHO, if you refactor the method name via IDE this code breaks, I know it’s the same problem for the long attribute version. Not sure I would use it, because if you have no tests and even with static analysis this bug can go to prod. Sounds like a footgun but indeed nice DX

[OskarStark]

@Kocal my repositories are final so I cannot mock them, so I would need an interface. This way you don’t need an interface, can have a final class and you can easily test it.

[nicolas-grekas]

versus directly injecting CommentRepository

think about testing a service that has CommentRepository as a dependency: you cannot test it without having a database or without heavy mocking. Query functions are just trivial to replace instead.

[Kocal]

Hum alright, I can understand the issue, but then doesn't it make sense to create by yourself a invokable class that you can easily mock?

IINW you lose the autocompletion/static analysis by using AutowireMethodOf/AutowireCallable no?

[nicolas-grekas]

doesn't it make sense to create by yourself a invokable class that you can easily mock

It's like invokable controllers vs action methods: everyone their preferences 🤷 Mine is this attribute because of the low verbosity: just write a repository, and still get decoupling without creating a myriad of class boilerplate.

you lose the autocompletion/static analysis by using AutowireMethodOf/AutowireCallable no?

@param Closure(Conference, int): Paginator $getCommentPaginator to the rescue in my example (or inline @var Paginator $result)). The attribute doesn't introduce this concern also since the current alternative works the same.

Only one con IMHO, if you refactor the method name via IDE this code breaks

true with the current tooling, but this is fixable since the tools can very well be taught about the attribute. On this topic also the current situation is similar and the proposed attribute doesn't bring anything new.

[Kocal]

Alright thanks for the explanations :)

[smnandre]

You mentionned "query" but -naive question- would this work with setters / persisters too ?

Could this work ?

public function __invoke(
    #[AutowireMethodOf(CommentRepository::class)] \Closure $saveComment,
    #[MapRequestPayload] Comment $comment,
) {
    $saveComment($comment);
}

[nicolas-grekas]

@smnandre yes of course

[nicolas-grekas]

PR updated with better error handling and a test case.

[nicolas-grekas]

FTR, if I define this interface:

interface GetCommentPaginatorInterface
{
    public function __invoke(Conference $conference, int $page): Paginator;
}

Then I can also do native types (vs @param + closure) without doing anything else:

        #[AutowireMethodOf(CommentRepository::class)]
        GetCommentPaginatorInterface $getCommentPaginator,