Skip to content

[DependencyInjection] Autoconfigurable attributes #39897

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

Merged

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Jan 19, 2021

Q A
Branch? 5.x
Bug fix? no
New feature? yes
Deprecations? no
Tickets N/A
License MIT
Doc PR TODO

Alternative to #39776. Please have a look at that PR as well to see the full discussion.

With this PR, I propose to introduce a way to autoconfigure services by using PHP Attributes. The feature is enabled on all autoconfigured service definitions. The heart of this feature is a new way to register autoconfiguration rules:

$container->registerAttributeForAutoconfiguration(
    MyAttribute::class,
    static function (ChildDefinition $definition, MyAttribute $attribute, \ReflectionClass $reflector): void {
        $definition->addTag('my_tag', ['some_property' => $attribute->someProperty]);
    }
);

An example for such an attribute is shipped with this PR with the EventListener attribute. This piece of code is a fully functional autoconfigurable event listener:

use Symfony\Component\EventDispatcher\Attribute\EventListener;
use Symfony\Component\HttpKernel\Event\RequestEvent;

#[EventListener]
class MyListener
{
    public function __invoke(RequestEvent $event): void
    {
        // …
    }
}

What makes attributes interesting for this kind of configuration is that they can transport meta information that can be evaluated during autoconfiguration. For instance, if we wanted to change the priority of the listener, we can just pass it to the attribute.

#[EventListener(priority: 42)]

The attribute itself is a dumb data container and is unaware of the DI component.

This PR provides applications and bundles with the necessary tools to build own attributes and autoconfiguration rules.

@nicolas-grekas
Copy link
Member

Thanks for giving this a try. I can't have a look immediately, but I'm just thinking that we should make this ready to support all kind of attributes, eg on params, methods, etc.

Copy link
Member

@jderusse jderusse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like it.

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I really like how $container->registerAttributeForAutoconfiguration() allows mapping random attributes to DI concepts.

There are some important design issues right now IMHO, let's resolve them :)

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jan 21, 2021

A wish from Twitter ;)

public function __construct(
    #[Tagged('my.tag')] ContainerInterface/iterable $tagged,
)

@derrabus
Copy link
Member Author

A wish from Twitter ;)

I want to have this as well, but maybe as a follow-up to this PR.

@derrabus derrabus force-pushed the feature/autoconfigurable-attributes branch 7 times, most recently from a904953 to cbfcc0a Compare February 11, 2021 18:18
@derrabus
Copy link
Member Author

derrabus commented Feb 11, 2021

Sorry for stalling so long. I've had some rough weeks juggeling day job and homeschooling. 😞

I have updated the PR to address the comments:

  • The Reset attribute is gone, so this PR only ships EventListener as a POC.
  • We can set a tag to disable attribute inspection for a definition, as suggested by @nicolas-grekas.
  • I'm using ContainerBuiler::getReflectionClass() now.
  • I'm passing the reflector to the autoconfigurator now.

Most importantly, the autoconfigurator now receives an empty ChildDefinition that will eventually be merged with the original definition. Most of the merging logic is missing; so far, only tags work. I'm working on getting the other autoconfiguration features to work now.

@derrabus derrabus force-pushed the feature/autoconfigurable-attributes branch from cbfcc0a to ba62349 Compare February 12, 2021 00:07
@derrabus derrabus force-pushed the feature/autoconfigurable-attributes branch 2 times, most recently from f99223e to 894d77d Compare February 16, 2021 00:10
@derrabus derrabus force-pushed the feature/autoconfigurable-attributes branch from bd6d13e to 5af5333 Compare February 16, 2021 10:44
@derrabus
Copy link
Member Author

Copy link
Member

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this a lot! Thanks for the great and detailed explanation of this feature in the PR description.

@wouterj
Copy link
Member

wouterj commented Feb 17, 2021

Regarding @javiereguiluz's comment: I wonder how autocompletion works here. That's not a reason to block merging this PR, but something to keep an eye out for. E.g. even though it has this special #[Attribute] marking, it can be used as a normal PHP class: https://3v4l.org/W0hq5
Given the confusion around "event listeners vs event subscribers" for people new to Symfony, I wonder if this doesn't cause confusion if the autocompletion pop-up includes EventListener when doing ... extends Event<TAB>.

Btw, I quite like the more action oriented #[ListenTo] used in https://stitcher.io/blog/attributes-in-php-8 . We may can take inspiration from that, if we find that EventListener is confusing.

@weaverryan
Copy link
Member

I really love this PR - congrats @derrabus!

Btw, I quite like the more action oriented #[ListenTo] used in https://stitcher.io/blog/attributes-in-php-8 . We may can take inspiration from that, if we find that EventListener is confusing.

This is a good catch... and it may be a perfect opportunity to use a different naming convention for these attributes. ListenTo only sounds strange because it's missing the "what". Listen to... what? ... since the event name itself won't always be included (and I like the invokable version where it's not included). We could use #[ListenToEvent].

Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it!

I also like @weaverryan's suggestion. However, it's just a naming thing, so here is an approval for all code changes (we can also make the rename in a separate PR, I think we should come up with a general attribute-naming strategy: action based or object based?)

@nicolas-grekas
Copy link
Member

On my side, I'd vote for keeping EventListener. That's what makes most sense. The autosuggestion issue should be solved by IDEs, which could skip suggesting attributes in a non-attribute context.

@derrabus derrabus force-pushed the feature/autoconfigurable-attributes branch from 5744933 to 2ab3caf Compare February 17, 2021 19:42
@derrabus
Copy link
Member Author

I mean, we could also finalize the EventListener class, if we fear that people will extend it for the wrong reasons. Apart from that, I don't have strong opinions whether we class the class EventListener or ListenToEvent. Both would work for me.

@nicolas-grekas
Copy link
Member

Thank you @derrabus.

@nicolas-grekas nicolas-grekas merged commit 22c2f1a into symfony:5.x Feb 18, 2021
@derrabus derrabus deleted the feature/autoconfigurable-attributes branch February 18, 2021 17:18
@fabpot fabpot mentioned this pull request Apr 18, 2021
ruudk added a commit to ruudk/symfony that referenced this pull request Jul 9, 2021
Based on the implementation of symfony#39897 is made it possible to autoconfigure method attributes.
ruudk added a commit to ruudk/symfony that referenced this pull request Jul 9, 2021
Based on the implementation of symfony#39897 I made it possible to autoconfigure method attributes.
ruudk added a commit to ruudk/symfony that referenced this pull request Jul 9, 2021
Based on the implementation of symfony#39897 I made it possible to autoconfigure method attributes.
ruudk added a commit to ruudk/symfony that referenced this pull request Jul 9, 2021
Based on the implementation of symfony#39897 I made it possible to autoconfigure method attributes.
ruudk added a commit to ruudk/symfony that referenced this pull request Jul 9, 2021
Based on the implementation of symfony#39897 I made it possible to autoconfigure method attributes.
ruudk added a commit to ruudk/symfony that referenced this pull request Jul 15, 2021
Based on the implementation of symfony#39897 I made it possible to autoconfigure method attributes.
fabpot added a commit that referenced this pull request Aug 30, 2021
…ethods, properties and parameters (ruudk)

This PR was merged into the 5.4 branch.

Discussion
----------

[DependencyInjection] Autoconfigurable attributes on methods, properties and parameters

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      |no
| New feature?  | yes
| Deprecations? |no
| Tickets       |
| License       | MIT
| Doc PR        |

## Introduction

#39897 introduced the possibility auto configure classes that were annotated with attributes:
```php
$container->registerAttributeForAutoconfiguration(
    MyAttribute::class,
    static function (ChildDefinition $definition, MyAttribute $attribute, \ReflectionClass $reflector): void {
        $definition->addTag('my_tag', ['some_property' => $attribute->someProperty]);
    }
);
```

This works great. But it only works when the attribute is added on the class.

With this PR, it's now possible to also auto configure methods, properties and parameters.

## How does it work?

Let's say you have an attribute that targets classes and methods like this:

```php
#[Attribute(Attribute::TARGET_CLASS | Attribute::TARGET_METHOD | Attribute::TARGET_PROPERTY)]
final class MyAttribute
{
}
```

You have two services that use them:
```php
#[MyAttribute]
class MyService {
}

class MyOtherService {
    #[MyAttribute]
    public function myMethod() {}
}
```

You can now use `registerAttributeForAutoconfiguration` in your extension, together with a union of the types that you want to seach for. In this example, the extension only cares for classes and methods, so it uses  `\ReflectionClass|\ReflectionMethod $reflector`:
```php
final class MyBundleExtension extends Extension
{
    public function load(array $configs, ContainerBuilder $container) : void
    {
        $container->registerAttributeForAutoconfiguration(
            MyAttribute::class,
            static function (ChildDefinition $definition, MyAttribute $attribute, \ReflectionClass|\ReflectionMethod $reflector) : void {
                $args = [];
                if ($reflector instanceof \ReflectionMethod) {
                    $args['method'] = $reflector->getName();
                }
                $definition->addTag('my.tag', $args);
            }
        );
    }
}
```

This will tag `MyService` with `my.tag` and it will tag `MyOtherService` with `my.tag, method: myMethod`

If the extension also wants to target the properties that are annotated with attributes, it can either change the union to `\ReflectionClass|\ReflectionMethod|\ReflectionProperty $reflector` or it can just use `\Reflector $reflector` and do the switching in the callable.

## Another example

Let's say you have an attribute like this:
```php
#[Attribute(Attribute::TARGET_CLASS)]
final class MyAttribute
{
}
```

and you use it like this:
```php
$container->registerAttributeForAutoconfiguration(
    MyAttribute::class,
    static function (ChildDefinition $definition, MyAttribute $attribute, \ReflectionClass|\ReflectionMethod $reflector) : void {
        $definition->addTag('my.tag');
    }
);
```

you'll get an error saying that `ReflectionMethod` is not possible as the attribute only targets classes.

Commits
-------

917fcc0 [DependencyInjection] Autoconfigurable attributes on methods, properties and parameters
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request Aug 30, 2021
…ethods, properties and parameters (ruudk)

This PR was merged into the 5.4 branch.

Discussion
----------

[DependencyInjection] Autoconfigurable attributes on methods, properties and parameters

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      |no
| New feature?  | yes
| Deprecations? |no
| Tickets       |
| License       | MIT
| Doc PR        |

## Introduction

symfony/symfony#39897 introduced the possibility auto configure classes that were annotated with attributes:
```php
$container->registerAttributeForAutoconfiguration(
    MyAttribute::class,
    static function (ChildDefinition $definition, MyAttribute $attribute, \ReflectionClass $reflector): void {
        $definition->addTag('my_tag', ['some_property' => $attribute->someProperty]);
    }
);
```

This works great. But it only works when the attribute is added on the class.

With this PR, it's now possible to also auto configure methods, properties and parameters.

## How does it work?

Let's say you have an attribute that targets classes and methods like this:

```php
#[Attribute(Attribute::TARGET_CLASS | Attribute::TARGET_METHOD | Attribute::TARGET_PROPERTY)]
final class MyAttribute
{
}
```

You have two services that use them:
```php
#[MyAttribute]
class MyService {
}

class MyOtherService {
    #[MyAttribute]
    public function myMethod() {}
}
```

You can now use `registerAttributeForAutoconfiguration` in your extension, together with a union of the types that you want to seach for. In this example, the extension only cares for classes and methods, so it uses  `\ReflectionClass|\ReflectionMethod $reflector`:
```php
final class MyBundleExtension extends Extension
{
    public function load(array $configs, ContainerBuilder $container) : void
    {
        $container->registerAttributeForAutoconfiguration(
            MyAttribute::class,
            static function (ChildDefinition $definition, MyAttribute $attribute, \ReflectionClass|\ReflectionMethod $reflector) : void {
                $args = [];
                if ($reflector instanceof \ReflectionMethod) {
                    $args['method'] = $reflector->getName();
                }
                $definition->addTag('my.tag', $args);
            }
        );
    }
}
```

This will tag `MyService` with `my.tag` and it will tag `MyOtherService` with `my.tag, method: myMethod`

If the extension also wants to target the properties that are annotated with attributes, it can either change the union to `\ReflectionClass|\ReflectionMethod|\ReflectionProperty $reflector` or it can just use `\Reflector $reflector` and do the switching in the callable.

## Another example

Let's say you have an attribute like this:
```php
#[Attribute(Attribute::TARGET_CLASS)]
final class MyAttribute
{
}
```

and you use it like this:
```php
$container->registerAttributeForAutoconfiguration(
    MyAttribute::class,
    static function (ChildDefinition $definition, MyAttribute $attribute, \ReflectionClass|\ReflectionMethod $reflector) : void {
        $definition->addTag('my.tag');
    }
);
```

you'll get an error saying that `ReflectionMethod` is not possible as the attribute only targets classes.

Commits
-------

917fcc09f7 [DependencyInjection] Autoconfigurable attributes on methods, properties and parameters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants