Looks good to me! 👍

What about allowing a collection of setters to autowire;
setSettersAutowired(['setBar', 'setBaz']); // but not setFoo()?

@ro0NL I don't think there is much benefit compared to the existing

calls:            -[setMailer, ['@app.mailer']]

To clarify on why this change is needed, enabling auto-wiring without this flag could cause a BC break. Given following example service:

class Foo{private$baz;publicfunction__construct(Bar$bar)    {$this->baz =$bar;    }publicfunctionsetBaz(Taz$taz)    {$this->baz =$taz;    }}

Turning auto-wiring on in earlierDependencyInjection versions calls__construct.

Currentmaster calls__construct and thensetBaz (the risk is injecting a different instance here).

This PR reverts the behavior to just__construct, whilesetBaz is to be enabled by the consumer.

@dunglas guess so, it's considerable though. There will be situations you want toideally autowire 9/10 methods and face a all or nothing choice: go with autowiring or configure 9 services. We fully lose the autowiring benefit then.. which can be a, pragmatic, pain :-)

which can be a, pragmatic, pain :-)

It's still out of the 80/20 use-case, from the looks of it.

Agree :)

edit: consider

calls:  -[setBar, [@arg]]  -setFoo# autowired

Yeah, i think i like

autowire:true[autowire_]calls:   -setBar   -setFoo

better than

setters_autowired:true

Orsetters_autowired: true should be an equivalent ofcalls: <all_setters>

👍

Should we consider this for inheritance? At least makechildren inherit the flag and maybe allow themto change it? Actually doing it afterwards for theautowire flag in#19643.

👍

I'm worried about usability. If you wantfull autowiring, it'll now be:

autowire:trueautowire_setters:true

That's a lot of work for something that's purely for rapid development. But, we need to keep BC also.

What about this suggestion, which includes part of@ro0NL's idea:

# A) deprecated, but would only autowire the constructor, allows BCautowire:true# B) would autowire everything, including settersautowiring:true# C) or, use an array to opt into only certain methodsautowiring:[__construct]

This is also - accidentally - more flexible.

Even more flexible, IMHO having to list the methods to autowire would indeed reduce the benefit of using this feature rather than fully fillcalls, as pointed out by@dunglas.

The proposed flag sounds nice, keeping autowire managing only constructors per default which stay logic to me.

Otherwise, what about a choice?

autowiring:constructorautowiring:settersautowiring:[constructor, setters]

(Depreciating the current flag)

@weaverryan regarding usability, this is a reminder that setter injection is generally harmful, and while it's a nice-to-have, it's usually to be discouraged.

this is a reminder that setter injection is generally harmful

This, imo, explains why it should be verbose.autowire_setters: true #magic happens

edit: i rather be explicit in what methods are called (autowired or not). The shortcut (true) will be the nice-to-have imo.

I also agree with@Ocramius, we should discourage setter injection (it should be opt-in).

I added initial support for setter autowiring (#17608) to ease the creation of Symfony controllers that don't depend of the container (#18193). Such classes will basically be automatically registered (e.g.:DunglasActionBundle), the user will not have to care about enabling setters autowiring by itself.

Developpers wanting to always enable setters autowiring in other contexts can create a userland a compiler pass to do it.

this is a reminder that setter injection is generally harmful

Is it harmful? The user is already opting into magic. I'm not saying this is wrong, but I want to challenge the statement. Is there some evidence we can look at to support / contest this?

I added initial support for setter autowiring (#17608) to ease the creation of Symfony controllers that don't depend of the container

I'm interested in setter autowiring because of the potential to have traits with setters and helper methods - e.g.RouterTrait withsetRouter andgenerateUrl methods.

My proposal (#19631 (comment)) offers BC, ease-of-use and control... but assumes the best default mode for autowiring isconstructors and setters.

Is it harmful?

Yes, due to basic design of service dependency immutability.

The user is already opting into magic.

The comment about setter injection can be taken completely out of the scope of this PR or of the dependency injection component.

I'm interested in setter autowiring because of the potential to have traits with setters and helper methods - e.g. RouterTrait with setRouter and generateUrl methods.

Yes, it's basically the same use case. But for such cases, a compiler pass can automatically enable setters autowiring (for instance if the service implements a given interface).

Ok, I'm getting out-voted, and the arguments are sound.

👍 for the current approach

Just to clarify; this will call any definedset..() method, after you configuredautowire_settings: true, right/?. I can live with it, no problem, but I hope you understand my concernwhy that's tricky and bad DX.

IMO, it's the same trickiness what we currently try to avoid.

@ro0NL anyapplicable setter. See#17608 and tests for details.

I see. How does this solve optional dependencies? I don't wantsetFoo(Bar $bar = null) to be called... im stuck?

Point is: you need to know at all time, whenever you define a setter, on whatever level in the class-chain, it's gonna be called (if enabled). It's tricky, period. :)

edit: i understand it's somewhat the current behavior, hence i can live with this approach. Just saying ;-)

What do you think about the following config, it should satisfy everybody's needs and is backward compatible:

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

What about

autowire:true# enables constructor autowiring by default, if disabled no autowiring whatsoever happensautowire_calls:[autowireThis, andThat]# additional method calls to be autowiredautowire_setters:true# adds all setters to autowire_calls

This looks intuitive.. but is limiting in a way, you can only autowire method calls when the constructor is autowired.

I prefer@dunglas proposal because the various configurations are mutually exclusive, but having several keys allows setting several of them at once, and from a DX perspective requires remembering 3 keys.

Okay then.. let's do it :) Thanks for letting me clarify my point.

@dunglas just thought of using bitwise flags, on php level it's pretty verbose;

$definition->setAutowired(Definition::CONSTRUCTOR|Definition::SETTERS);// true = Definition::CONSTRUCTOR// array = whitelistif ($definition->getAutowired() & Definition::SETTERS) { }

For yamlDefinition::FULL could work (CONSTRUCTOR|SETTERS), ie;

autowire:!php/const:Definition::FULL

or allow the* shortcut anyway.

autowire: !php/const:Definition::FULL is a no go for me.

Understand, hence i proposed allowing the* shortcut. It's more about the PHP structure for this. The * can be allowed for file loaders, or directly inDefinition::setAutowired.

My proposal is more flexible and easier to learn (though)

Easier to learn, i guess so ;-) my proposal allows for addingDefinition::SOME_FUTURE_GROUP_OF_METHODS, which will never happen.. but if you wanna talk about flexibility; it's their).

Anyway, lets move on.true|array|"*" is 👍

@ro0NL your proposal shows the wrong Yaml snippet. It would be like this:

autowire:!php/const:Symfony\Component\DependencyInjection\Definition::FULL

Left that out intentionally 👼 i know it's a no-go in some ways (too verbose i guess), but im looking at this from the php side first.

Status: needs work

As said, it would be a BC break to don't get this merged for 3.2.
@dunglas Are you fine or do you want some help?

I'll do it for the 3.2 release. If it's not ready, we'll revert the introduction of setter autowiring (and reintroduce it in 3.3) but I'll find the time to finish this PR.

I opened a new PR implementing#19631 (comment):#20167