I like it.
Should be made to work with method calls.
Missing resources tracking for now.
With now two alts to#20738, I think we should close it :)

Note that this conflicts with the implementation of#21313 that was just merged.

Note that this conflicts with the implementation of#21313 that was just merged.

Not really (AFAIU). But can't be used along with.

Side-note: I've already seen some projects using arguments keys, despite it had no effect until now (simply for "increasing readability"). Shouldn't it be considered as a BC break?

@ogizanagi in my opinion no, it was explicitly documented to support only packed arrays (see my changes inDefinition).
If it really matters, I can skip named parameters not matching instead of throwing, but it will be harder to debug.

Not really (AFAIU). But can't be used along with.

Indeed, I'll try to adapt my PR to be able to use both syntaxes together.

This would be nice but it actually doesn't work:

services:Acme\NewsletterManager:{ apiKey: "%mandril_api_key%" }

But this works:

services:Acme\NewsletterManager:arguments:{ apiKey: "%mandril_api_key%" }

@nicolas-grekas

Should be made to work with method calls.

It would be a nice addition, I propose to do it in another PR when this one will be reviewed and merged.

Missing resources tracking for now.

Done, I rely on@weaverryan's implementation for now because conditions to rebuild the container will be the same if we addcalls support later.

With now two alts to#20738, I think we should close it :)

Done!

@kasparsklavins we are trying to find the best way to inject parameters when using autowiring and service auto-registration (#21289). It will be a brand new way to use Symfony that will change drastically the first experience for new users and allow to develop faster.
For now, there is no way to do it. Please consider the big picture. Btw, you can already define a service using XML, YAML, PHP, your own loader, autowiring and so on...

@fabpot@ogizanagi I've tried something to support the short syntax:b8c0e7c WDYT?

Looks good to me, as soon as we warn in the documentation no argument should be named the same as one of the definition keywords, otherwise it won't work.
I don't know if we can do better for now :)

Re-thinking about it, it also means introducing a new keyword someday means a potential BC break for someone using the short syntax along with named arguments 😕
So maybe this tradeoff isn't worth it and the short syntax should be kept for most simple usages.

Enforce a leading$, eg$apiKey: '%foo%'?
About method calls, is it that a big change ? Because that feels incomplete to me to not have it right now...

One minor issue I see with this: when you rename a variable in php, it will now crash, but this won't be detected with applications (or bundles) with only unit-tests.

@iltar you're right but it's already the case for traditional definitions (if you add or remove an argument it will fail) and forcalls definitions (if you rename the method it will fail).

It's why unit testing is not enough and apps need to have a pyramid of tests including some functional and UI ones.

@dunglas correct, however, there's a bunch of refactoring tools and find/replaces that might modify the name of a variable. Correctly removing one without causing parse errors is a lot more complex, hence this issue is faster to present itself.

PR updated to make named arguments starting with a$ as suggested by@nicolas-grekas. It solves problems raided by@ogizanagi (no conflict possible now) and partially the one solved by@iltar (the $ makes it explicit that there is a special handling, and IDEs can add support for this new convention to allow fluent refactoring).

I don't like the$ at all. It makes the YAML file look a mix of PHP and YAML contents. Example:

arguments:$apiKey:"%mandrill_api_key%"

I think the$ is appropriate. It makes it much more obvious that the keys are directly referencing PHP variables/parameters (as defined in the method definition) rather than some non-trivial structure defined by something like the config component.
Making the arguments configuration seem a little less "magic" to newer developers (meaning a lower learning curve) is, to me personally, more important than clean looking configuration files.

@weaverryan Can we close#21376?

@dunglas Looks like another one where a doc PR is needed :)

Thank you@dunglas.

I've at least got a docs issue opened for this!symfony/symfony-docs#7482

... and I closed my other PR -#21376

The$ prefix is unfortunate. PHP-ism seeping into YAML.

For reasons unknown to me, my currently client had tons of service definitions that looked like:

service.id:class:Fooarguments:foo:somethingbar:somethingelse

<?phpclass Foo {publicfunction__construct($foo,$bar) {// $foo would be something// $bar would be somethingelse  }}

All of those broke with the introduction of these changes in 3.3.

Invalid key "X" found in arguments of method "X" for service "X": only integer or $named arguments are allowed.

Was this intentional? Was their use-case undocumented and thus not supported? Cause it did work exactly as expected before 3.3...

Afaik, the keys were unused and therefore treated by sequence. Writing it down like this wasn't a supported feature either.

That's what I figured. They were using the keys themselves though in some cases, by retrieving the arguments of the service definition, in some CompilerPass related stuff. Took me a bit to sort everything out 😅