👍

The syntax for the XML loader is:

<?xml version="1.0" encoding="utf-8"?>
<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
    <services>
        <service id="autowire_star" class="Foo" autowire="*" />
        <service id="autowire_array" class="Foo">
            <autowire>setFoo</autowire>
            <autowire>bar</autowire>
        </service>
    </services>
</container>

Does it looks good to everybody?

ping @symfony/deciders. We need to choose between merging this one or reverting setter autowiring before the 3.2 release.

The XML loader should probably forbid to use both the autowire attribute and elements at the same time. Stuff like that doesn't make sense:

<?xml version="1.0" encoding="utf-8"?>
<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
    <services>
        <service id="autowire_star" class="Foo" autowire="*">
           <autowire>setFoo</autowire>
            <autowire>bar</autowire>
        </service>
        <service id="autowire_array" class="Foo"  autowire="false">
            <autowire>setFoo</autowire>
            <autowire>bar</autowire>
        </service>
    </services>
</container>

So if you explicitly autowire a method that has no arguments it is basically the same as a normal call configuration to this method? I suspect people will then misuse autowiring just to call certain methods since there are now 2 ways to do that.

@Tobion thanks for the review, comments fixed now.

So if you explicitly autowire a method that has no arguments it is basically the same as a normal call configuration to this method? I suspect people will then misuse autowiring just to call certain methods since there are now 2 ways to do that.

I think that it's not the same intent:

• When you use <autowire>, it means "always fill parameters of this method, if I add or remove some, it must not fail"
• When you use <call>, it's the old good call without any magic

But again, and as pointed in the doc, I would discourage end users to use setter autowiring. It's a feature designed for creators of RAD frameworks and bundles.

When you use , it means "always fill parameters of this method, if I add or remove some, it must not fail"

Fine for me.

One more thing. I think we should make the programmatic Definition methods more explicit and not use a multivalued @param bool|string|string[] $autowired parameter. IMO we need to distinguish between configuration and programmatic API.

autowire: true # constructor autowiring
autowire: [__construct, setFoo, setBar] # autowire whitelisted methods only
autowire: '*' # autowire constructor + every setters (following existing rules for setters autowiring)

makes sense as @dunglas and @nicolas-grekas pointed out. But using a magic * in a programmers API like

$fooDefinition = new Definition('Foo');
$fooDefinition->setAutowired('*');

does not makse sense.

So I'd prefer to change the methods on the Definition class similar to what @ro0NL suggested in #19631 (comment).

$definition->setAutowired(Definition::CONSTRUCTOR|Definition::SETTERS);
$definition->setAutowiredMethods(array('setFoo', 'notASetter'));
$definition->disableAutowiring();

The current yaml/xml loaders would just map to this explicit API.

It will be difficult to make this new API backward compatible with the old one. Currently autowired is a boolean.

@dunglas what about this approach?

$def->setAutowired(false); // no autowiring
$def->setAutorwired(true/**Def::CONSTRUCTOR*/); // constructor autowiring
$def->setAutowired(Def::SETTERS); // setter autowiring
$def->setAutorwired(array('foo')); // whitelist autowiring
$def->setAutorwired(Def::CONSTRUCTOR|Def::SETTERS); // combined autowiring

// optional: combined + whitelist autowiring
$def->setAutorwired(Def::CONSTRUCTOR|Def::SETTERS|Def::WHITELIST, array('foo'));

autowire: false # no autowiring
autowire: true | 'constructor' | constant('Def::CONSTRUCTOR') # constructor autowiring
autowire: 'setters' | constant('Def::SETTERS') # setter autowiring
autowire: # whitelist autowiring
  - foo
autowire: # combined autowiring
  constructor: true
  setters: true

# optional: combined + whitelist autowiring
autowire:
  constructor: true
  setters: true
  whitelist:
    - foo

@ro0NL I don't want to change the format for XML and YAML because we already have reached a consensus.

Your proposal for the PHP API doesn't fix the issue raised by @Tobion: the setAutowire method still accept several types of parameters (string, bool and string[]); exactly like the current implementation.

Using a bit field isn't the right choice here: you cannot accumulate options. You can only choose one option between constructor autowiring, constructor + setter autowiring or a list of methods to autowire.

IMO the current implementation is good enough even if I would prefer accepting only one type for setAutowire. It's expressive, easy to learn and flexible. All loaders (PHP, XML and YAML) also have a similar API (better learning curve).

Maybe can we just introduce a Definition::SETTER_AUTOWIRING = '*' to avoid using a special raw string in the PHP API.

Fair enough. Just saying im not sure * meaning constructor + setters is intuitive.. especially as we shouldnt promote setter injection.. right? This is making it more or less special.

autowire: '*' vs. autowire: { constructor: true, setters: true }

I'd prefer the latter in terms of less magic, extensibility and verbose configuration. But you're right, we've already reached consensus before. Just thought of this today :)

Perhaps should be avoided, but in this case (to me) it makes most sense from a configuring perspective.

Eventually a YAML/XML/PHP definition represents the same thing, and therefore should have similar (if not the same) schema/API imo.

edit: on 2nd thought :):) maybe choose the desired PHP API now (eg. setAutowiredMethods(array $methods)) and extract PHP configuration (eg. return [];) later on? Does that make sense?

My vote would be for reverting #17608 and merge this one for 3.3.
This one has a high impact on DX, yet DX needs some time to mature IMHO, and this maybe be too fresh for 3.2.

I agree, it's too late for 3.2.

I've rebased the PR and updated method's names.

I've added two new methods setAutowiredMethods(array $autowiredMethods) and getAutowiredMethods() : array because the naming is better and it allows to remove the union type.

However I've kept isAutowired() : bool and setAutowired(bool $autowired) (without deprecating them) because it doesn't hurt and it looks better in term of DX when not using method autowiring. And method autowiring should almost never be used when not developing a library or a framework.

@fabpot can I merge this one? It will help for #18193 and #20738.

👍 for merging asap, so that we can play with the feat asap and tweak it if needed in the coming months.