RFC: Autowiring scalar parameters

[maldoinc]

Hi all,

In a large symfony codebase we are in the process of upgrading it from 3.4 to 4, one of the tasks is to remove the manual service declarations (we used xml) and fetching them from the container in favor of autowiring. During this process, DI works for services but we still need to declare into a yaml file the values for scalar arguments. Yet again having some part of the service declaration in a separate file.

Description

What I would like to propose is the addition of a new mechanism (attribute/annotation), which would bind parameters to the arguments removing the need to declare a part of the service in a separate file (I am aware of the possibility of binding specific argument names to a parameter, but personally I am not a fan of said functionality due to having to memorize what argument name belongs to what parameter).

The advantages of this approach would be that the service can be fully self contained, with its entire declaration in the constructor. Below are two examples using a php attribute and an annotation. The names used for the attribute/annotation are just an example, what I'm looking forward to hearing from is about the idea in general.

If this looks like a positive change, I can contribute the necessary patch to implement it.

Looking forward to your thoughts

Example

// Example using php8 attribute to bind the kernel.debug parameter to the $isDebug argument
class DummyService
{
    public function __construct(
        TranslatorInterface $translator,
        LoggerInterface $logger,

        #[Parameter("kernel.debug")] 
        bool $isDebug
    )
    {
    }
}


// Similar to the above example but instead an annotation is used.
class DummyService
{
    /**
     * @BindParameter("kernel.debug", to="$isDebug")
     */
    public function __construct(TranslatorInterface $translator, LoggerInterface $logger, bool $isDebug)
    {
    }
}```

[derrabus]

You can already achieve this using bind:

services:
    _defaults:
        autowire: true
        autoconfigure: true
        bind:
            bool $isDebug: '%kernel.debug%'

But we can look into this Parameter attribute. It certainly looks useful.

[jkufner]

I like it, but I'm a bit worried. It looks like a good idea which places all inputs of a class into one place . Over all, it is not very different from the usual type-based injection. However, it introduces a new coupling between the DI container and the classes. Such an annotation requires a knowledge about the DI container, and this knowledge is not supposed to be anywhere except the DI configuration. Therefore, the "kernel.debug" is clearly out of scope here (the parameter name is internal/private to the DI container).

Similar mechanism we already have is interface-based service tagging. An interface is a tag and DI then processes such classes somehow. Such interfaces are not specific to DI, so there is no additional coupling. (Internally, a compiler pass generates DI-specific tags, but that is an implementation detail.)

So, what if we tag parameters too?

Syntax in the class would be about the same: __construct(..., #[ParameterTag("debug") bool $isDebug]). (No framework-specific parameter name.)

Then, when configuring the service, a compiler pass will ask for all parameters tagged with a specific tag and bind them to container parameter, other service, or do whatever it wants.

The consequence is, that the coupling between the container and the class/service is more loose. We can have a standardized set of tags (like a contract or interface) for libraries, and application can define their own tags as needed.

Another benefit is, that we can do something like this: __constuct(DatabaseConnection $primaryConnection, #[ParameterTag('read-only')] DatabaseConnection $readOnlyMirrorConnection). And then let a custom compiler pass to decide what DB connection we want where.

[ahundiak]

I have never really cared for the named argument approach myself but I'd like to point out that PHP8 now supports the notion of passing arguments by name. So good or bad, in the future we can expect argument names to stay 'fixed' for longer and perhaps have more thought out names when first created. Which in turn makes the bind stuff more reliable and standard.

[ro0NL]

generally spoken argument tagging seems appropriate.

im curious if we can widen default autowiring to include required arguments by matching name+type ... because why not :)

bool $debug // implicit matches kernel.debug
Type $serviceId // matches <service class="Type" id="service.id"/>

[jkufner]

im curious if we can widen default autowiring [...]

Not by default, but it would be reasonable to include something simple and predictable that can be easily enabled. Autowiring also needs to be enabled explicitly, and that is a good thing.

[ro0NL]

well, it's a naming convention to autowire things. So autowire: true would be the trigger yes. Any existing binding would take precedence either way.

bool $debug, string $projectDir, string $cacheDir to me are all candicates for default framework provisioning. I add those bindings in projects 10/10.

[jkufner]

@ro0NL Type-based autoriwing has a type system to guard for mistakes. The types provide (some) semantics. But when using scalar types, there is no safeguard like that. There is no way to know that a given string represents what the service expects. The name of a parameter is much weaker than its type, and there is much much more space for unexpected conflicts and duplicates, which nobody was thinking about and compiler will not detect them.

Parameter binding is a project-wide declaration of a convention. We cannot expect 3rd-party libraries to follow it. Therefore, it must not be automatic with a simple autowire: true.

But we can safely autowire tagged parameters, and we can have a contract library (or even a PSR standard) with attributes (annotations) to tag parameters like bool $debug, so that libraries can utilize these features.

[ro0NL]

we could also configure some global parameter bindings in Kernel at the recipe level, for users to learn&discover.

[jkufner]

@ro0NL A compiler pass can do almost anything to the container, so it opens many possibilities, or a really big can of worms. Important rule is, that all of these features must be explicitly enabled in config files.

[nicolas-grekas]

Although it's not exactly the same, adding eg #[Autoconfigure(bind: '%kernel.debug%')] on a class works already.

I'm not super sold on this, because bindings are centralized, whereas the proposed attribute would need to be repeated everywhere needed.

I'm still open to discuss this. The implementation should probably take inspiration from #40406.

[maldoinc]

The goal here is to have services be self contained rather than having their declarations be split into multiple files.

Parameter binding creates a argument name -> symfony parameter mapping which you have to memorize or look up when creating services, maybe this is fine for a small amount of parameters but for large applications I feel this would get out of hand.

The attribute would indeed be repeated on every service declaration as needed but that is also the point. If you're not using bindings then you have to specify the argument names on the service declaration via the parameters key which repeats not only the constructor parameter name but also the symfony parameter.

Acme\Service\Dummy:
    arguments:
        $isDebug: "%kernel.debug%"
        $otherParam: "%other.parameter%"

Declarations such as these would benefit from the new attribute. Regarding the container, this would only expose the parameter bag. I implemented the feature recently and it uses the same AutowirePass as TaggedIterator and TaggedLocator attributes.

[nicolas-grekas]

I'm closing here this is moot on my side. PR welcome if you think we should really do it.