Skip to content

[DependencyInjection] Suggestion for autowiring parameters #22755

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

Closed
zmitic opened this issue May 18, 2017 · 5 comments
Closed

[DependencyInjection] Suggestion for autowiring parameters #22755

zmitic opened this issue May 18, 2017 · 5 comments

Comments

@zmitic
Copy link

zmitic commented May 18, 2017

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? yes
Symfony version 3.3

When services are autowired, parameter values still needs to be declared in definition like:

    app.my_service:
        class: AppBundle\Service\MyService
        arguments: 
            - $clientHostParameter: "%client_host%"

What about guessing them based on variable name? For example:

class MyService
{
    public function __construct(TwigEngine $twigEngine, $clientHostParameter)
    {
        //...
    }

So if variable ends with word parameter, S3 would tableize clientHost, look for parameter client_host and inject it.

An example taken from Symfony Flex demo would then become:

class RecipeLoader
{
    public function __construct(LoggerInterface $logger, $kernelDebugParameter)
    {
        //...
    }

What do you think?


Sorry if someone posted this idea before.

@jvasseur
Copy link
Contributor

jvasseur commented May 18, 2017

Something similar was proposed in #20738 and rejected in favor of the current argument binding by name.

@zmitic
Copy link
Author

zmitic commented May 19, 2017

@jvasseur Thank you, didn't see it before.

I am guessing doing something like this (the @param annotation):

<?php
class RecipeLoader
{
    /**
     * @param LoggerInterface $logger
     * @param bool $kernelDebugParameter %kernel.debug%
     */
    public function __construct(LoggerInterface $logger, $kernelDebugParameter)
    {
        //...
    }
}

has also been rejected?

A bit offtopic:

I still like the idea of autowiring params and tried to understand the code in Symfony\Component\DependencyInjection\Compiler\AutowirePass. It is complicated but I think I could figure it.

So before I try to make a bundle to do that, can you tell me if it is even possible? I am not looking for help but given I am not S3 expert, I would just like to know if it is even possible to do.

@weaverryan
Copy link
Member

Hey!

Yea, it's certainly possible, but I don't think a proposal like this will be accepted into core, at least not for a little while: we need to see how these things work in the wild. And so far, autowiring is very non-magic: it relies on type-hints only. I also proposed a PR that used annotations to wire arguments, but it was rejected.

Another option - especially to serve as a proof of concept - is to create a bundle and do this there. You would not be able to use AutowirePass, you'd create your own compiler pass that looked at missing arguments to autowired methods and check the docs or whatever you want. Itams non-trivial in large part due to parent child definition stuff (that's what the AbstractRecursivePass relates to), but doable. Also, your pass wouldn't need to worry about the "types" in the container, so that also simplifies things.

So that's my advice for now: try it in a bundle. Then of course, you can always submit a PR here - the temperament for a feature may or may not change in the future. We don't want to add things that really look magic :)

@zmitic
Copy link
Author

zmitic commented May 21, 2017

Thanks @weaverryan , I will surely do try. I watched your presentation of symfony flex and like how services are auto-registered and tagged with no yml definition (twig extension thing).

Being able to autowire parameters would be even better.

@zmitic zmitic closed this as completed May 21, 2017
@TomasVotruba
Copy link
Contributor

TomasVotruba commented Dec 9, 2017

What do you think about making this per-config optional? Like autowire is
#21699 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants