Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define alias with an attribute #41188

Closed
garak opened this issue May 12, 2021 · 7 comments
Closed

Define alias with an attribute #41188

garak opened this issue May 12, 2021 · 7 comments

Comments

@garak
Copy link
Contributor

garak commented May 12, 2021

Description

Scenario: you have a concrete class that implements an interface. Currently, you need to add an alias in your services configuration,
The idea is to avoid it with a new php8 attribute, that can be applied to the class.

Example

<?php

namespace Foo;

use Symfony\Component\DependencyInjection\Attribute\Alias;

#[Alias]
final class MyClass implements \Bar\MyInterface
{
    // ...
}

the following then can be avoided:

services:
    Bar\MyInterface: '@Foo\MyClass'
@nicolas-grekas
Copy link
Member

This issue with this approach is that many classes can have the attribute, which will create collisions.
Also, the class might implement many interfaces. Which ones should be aliased?
Maybe all these questions mean that the attribute should be on the interface instead.
Or how would you solve these concerns?

@garak
Copy link
Contributor Author

garak commented May 12, 2021

We could ask for a mandatory parameter in case of multiple interfaces.
About the possibility of collisions, I guess that it already exists now (I hope it's handled by container).
I don't like the idea of putting annotation on the interface itself, since it can be out of your control

@nicolas-grekas
Copy link
Member

nicolas-grekas commented May 12, 2021

About the possibility of collisions, I guess that it already exists now (I hope it's handled by container).

Actually not, there is no need for such a mechanism currently as no collision can happen, since only one service or alias can be named "foo".

I don't like the idea of putting annotation on the interface itself, since it can be out of your control

Can you review actual use cases of yours and give us some examples where such aliases are need, both when you have control over the interface, and where you don't?

Because putting the alias on the interface would really be the most appropriate.

Maybe we can accept that when you don't have control over the interface, you should/could not use the attribute?

@garak
Copy link
Contributor Author

garak commented May 13, 2021

About the possibility of collisions, I guess that it already exists now (I hope it's handled by container).

Actually not, there is no need for such a mechanism currently as no collision can happen, since only one service or alias can be named "foo".

Indeed, in yaml it's impossible just because you can't use the same keys twice.
But I just tried with PHP configuration:

<?php
return static function (ContainerConfigurator $configurator): void {
    $services->alias('foo', MyClass1)->alias('foo', MyClass2);
}

And it's allowed: the first alias is overwritten by the second one.

Can you review actual use cases of yours and give us some examples where such aliases are need, both when you have control over the interface, and where you don't?

The typical use case is when the interface is in a vendor.
But also when it's under my control, in my typical project (using DDD) I keep interfaces in the Domain layer and concrete classes in the Infrastructure layer: adding annotations in the former is wrong, because service definitions belong to the latter.

@HypeMC
Copy link
Contributor

HypeMC commented May 13, 2021

Because putting the alias on the interface would really be the most appropriate.

There's also the case when the alias is not an interface, but a arbitrary string. This and then case when you don't have control over the interface make me think it's better to use this attribute on a concrete implementation since you can't cover every case on the interface. You could make this attribute work on both interface and classes to cover this cases, but that makes the usage inconsistent IMO.

@nicolas-grekas
Copy link
Member

Closing as discussed in #41207

@nicolas-grekas
Copy link
Member

See #49361

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants