I didn't fix all the tests yet (is there any script to update all autogenerated DI-fixtures?), need feedback. ping@nicolas-grekas

UPD: target branch is incorrect. Anyway, any chance to merge it in 3.4? If so, I'll reopen PR.

I think I'm 👎: this will introduce a non trivial amount of complexity (it's broken already), when the current way just works (just give that decorator a name.)

What about just anonymous services without decorators?

What about just anonymous services without decorators?

I would do it without the "anonymous" helper:->set(null, Fqcn::class)

I would do it without the "anonymous" helper

Well, a lot (probably, most) of my services are anonymous: listeners, command handlers, API controllers, console commands, cache/monitoring/logging decorators. I can useset(SendEmailForRegisteredUserListener::class, ...) for most of them, but they include namespace and service name becomes very long. On other hand, typingset(null, ...) again and again doesn't make me happy as well.

My fear is that ppl will get confused when reading
->set(Foo::class)
vs
->anonymous(Foo::class)
the difference is not obvious if you're not used to anonymous services (how many of us are, of many new comers...)
->set(null, Foo::class) on the contrary is "more" different than->set(Foo::class)

And I'm also fearing that writing actually might become more confusing: "anonymous" would become a new item on the autocompletion list, which ppl might have a hard time figure out what it means (because it's not that common) - and even scarier: ppl doing the mistake of selecting the wrong set/anonymous choice might have an even harder time figuring out their mistake.

the difference is not obvious if you're not used to anonymous services
And I'm also fearing that writing actually might become more confusing

Let them learn the difference. They won't hurt themselves, it's not a gun.

(how many of us are, of many new comers...)

I think most of us have such services in our codebases, we probably just don't realize it. It's yet another reason to make it explicit in my opinion.

I've removed decorators, tests are OK except some unrelated ones.

Let's focus on theanonymous() helper. Do you really want to remove it? I'd like to see it as explicit way to register a service, I really don't like nullables, it's almost always better to introduce new straightforward method. I also really doubt it's sort of "forbidden knowledge" that may bring any problems to people. They'd rather discover something new for themselves.

Offtopic:

just give that decorator a name

This annoys in the same way as requirement to specify name for "anonymous" services: I don't need it. Considering we have name convention for service names likegame.infrastructure.players_qs_memcache_decorator.inner, it becomes a problem. We also have sometimes 3-4 similar services with decorators, so I copy-paste definitions and trying to find the difference in names which I don't even need, ugh.

I'm still really unsold on the topic, if anything,->set(null, Foo::class) should be the way to IMHO.
Services don'thave to be anonymous. Giving them a name is fine (esp. when the name is automatically set as is now.)

@nicolas-grekas it looks like astraw man for me: I didn't say that serviceshave to be anonymous. Just many of themcould be. Giving name is fine, but sometimes is totally pointless like rituals.

->set(null, Foo::class)

Giving a name for this construction is fine, isn't it?->set(null, Foo::class) is WTF syntax and it's harder to google it.

The whole discussion is about explicit vs implicit syntax for the same concept. I don't understand your point why this kind of syntax should be implicit.

Oh! :)
could be would need a reason. When doing->set(Foo::class) is actually fine, I really don't see benefit of doing->anonymous(Foo::class). The first providing a name is a side effect that benefits everyone when needing a name, and doesn't hurt anyone when a name is actually not needed. That's my reasoning. Just let SF give a name for you, and ignore it if you don't need it.
The only case where the name is actually boilerplate if when you need several services implemented by the same class. Which is a case where you have to type some configuration anyway, and doing->set(null, Foo::class) would just work and not look like impossible boilerplate IMHO.

It should be quite easy to define your own DSL, by creating a different set of configurator classes, and type-hinting for them in the closure.

What do you think of decorators forContainerConfigurator?

@@ -23,6 +25,16 @@ use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigura  */ class PhpFileLoader extends FileLoader {+    private $configuratorDecorator;++    public static function withConfiguratorDecorator(ContainerBuilder $container, FileLocatorInterface $locator, callable $configuratorDecorator): PhpFileLoader+    {+        $loader = new self($container, $locator);+        $loader->configuratorDecorator = $configuratorDecorator;++        return $loader;+    }+     /**      * {@inheritdoc}      */@@ -44,7 +56,13 @@ class PhpFileLoader extends FileLoader         $callback = $load($path);         if ($callback instanceof \Closure) {-            $callback(new ContainerConfigurator($this->container, $this, $this->instanceof, $path, $resource), $this->container, $this);+            $configurator = new ContainerConfigurator($this->container, $this, $this->instanceof, $path, $resource);++            if ($this->configuratorDecorator) {+                $configurator = call_user_func($this->configuratorDecorator, $configurator);+            }++            $callback($configurator, $this->container, $this);         }

set(null, Foo::class) doesnt work in 4.0 duestring $id type :) which is also the phpdoc in 3.4, this is actually consistent with the current loaders right.

Anyway, just walked into this trying to register a event listener twice. Now in a codebase full ofset(Fqcn::class) theset(null, Fqcn::class) (if it worked) looks a bit odd... but im really thinking of a good ID value now 🤔 (the "naming is hard" issue also counts a bit IMHO).

naming is hard

Especially when you don't need a name (anonymous/lambda functions is a very close example).

set(null, Foo::class) doesnt work in 4.0 due string $id type

which is not an issue since the method is final, so can be changed without any BC break

Hm looking at failing tests locally

+    :\n+        class: App\BarService\n+        public: true\n

that's bad right? :)

@ro0NL if you mean my branch, it's possible if you merged with 3.4/master becauseContainerBuilderTest was changed (GitHub shows the conflict). I could resolve it, but I don't understand the state of PR.

Do I understand correctly that in order to merge it I have to replace->anonymous() with->set(null, ...)?

right. I tested on 3.4 :) i thought it worked today already. If we can settle withset(null, Fqcn::class) im fine 👍

@nicolas-grekas can you change base branch on master?

You can do it yourself with theedit button at the top of the issue.

@curry684 thanks, didn't know it.

OK, looks good.

Cannot be merged as tests are broken.

Tests fixed, should be green in a few minutes.

Thank you@unkind.

Nice one@unkind. Thx super useful :)