Scanning all methods of all autoconfigured services might be a perf hog on real apps.
We should understand the impact this could have before going this way.
Can you run some benchmark and report your findings please?
IIRC, Doctrine does it with an extra annotation on the class, basically saying "have a look at methods".

@nicolas-grekas Not sure how to properly benchmark this, but I used a Stopwatch in my large application and these are the results:
Class attributes took 3.000000 ms for 4469 classes
Method attributes took 18.500000 ms for 4469 classes and total of 41920 methods

  private Stopwatch $stopwatch;
  private int $classes = 0;
  private int $methods = 0;

  public function __destruct()
  {
      dump(sprintf(
          'Class attributes took %f ms for %d classes',
          $this->stopwatch->getEvent('class')->getDuration(),
          $this->classes
      ));
      dump(sprintf(
          'Method attributes took %f ms for %d classes and total of %d methods',
          $this->stopwatch->getEvent('method')->getDuration(),
          $this->classes,
          $this->methods
      ));
  }

  protected function processValue($value, bool $isRoot = false)
  {
      // ...
      $this->stopwatch->start('class');
      ++$this->classes;
      foreach ($reflector->getAttributes() as $attribute) {
          if ($configurator = $autoconfiguredAttributes[$attribute->getName()] ?? null) {
              $configurator($conditionals, $attribute->newInstance(), $reflector);
          }
      }
      $this->stopwatch->stop('class');
      $this->stopwatch->start('method');
      foreach ($reflector->getMethods() as $method) {
          ++$this->methods;
          foreach ($method->getAttributes() as $attribute) {
              if ($configurator = $autoconfiguredAttributes[$attribute->getName()] ?? null) {
                  $configurator($conditionals, $attribute->newInstance(), $method);
              }
          }
      }
      $this->stopwatch->stop('method');
      // ...
  }

Not sure if this is acceptable and how it can be improved.

Thanks for the bench, this looks acceptable!

In order to play safe and optimize perf a bit, I'd suggest doing some reflection in the process() method to classify configurators in 4 groups: the ones that work on classes (ie the ones that have only two arguments or that have a 3rd arg that accept ReflectionClass), the ones that work on methods, properties, and parameters. If a group is empty, we would skip scanning for the corresponding type of attributes.

@nicolas-grekas I applied this feedback and pushed a WIP commit.

Since Attributes only work on PHP 8 I thought it would be nice to support Union types when you want to register an attribute for multiple methods.

So for my example of the AsEventListener I want to typehint that with ReflectionClass | ReflectionMethod.

This works, but it will produce a fatal error on PHP 7.4 because it doesn't support unions there. How can we fix that?
Or should we target the AsEventListener change for Symfony 6 as that requires PHP 8 and up?

I'm also not sure how ReflectionParameter should be implemented. Should that work on all methods? Or only promoted parameters in the constructor? Aren't those already picked by ReflectionProperty?

Todo

• How to handle AsEventListener on method change? Move to Symfony 6 or find alternative for Union?
• Add and fix all tests
• ReflectionParameter

Much better!

@ruudk Can you work on a PR for the docs, or at least describe the feature with examples in the PR description? Thank you.

@fabpot I updated the PR description and added a few examples. Hope this helps!

@fabpot I updated the PR description and added a few examples. Hope this helps!

That's perfect, thank you.

Thank you @ruudk.

@nicolas-grekas This PHPStan annotation doesn't work as we expect it to work

See phpstan/phpstan#6200

How can we solve this?

$container->registerAttributeForAutoconfiguration(
            WithFlow::class,
            static function (ChildDefinition $definition, WithFlow $attribute, ReflectionMethod $reflector) : void {
                $definition->addTag(ServiceTag::WITH_FLOW, [
                    'flow' => $attribute->getFlowName(),
                ]);
            }
        );

gives this error:

phpstan: Parameter #2 $configurator of method 
Symfony\Component\DependencyInjection\ContainerBuilder::registerAttributeForAutoconfiguration() 
expects callable(Symfony\Component\DependencyInjection\ChildDefinition, WithoutFlow, Reflector): 
void, Closure(Symfony\Component\DependencyInjection\ChildDefinition, WithoutFlow, 
ReflectionMethod): void given.

Closure(Reflector)|Closure(ReflectionMethod)|Closure(ReflectionMethod|ReflectionProperty)|etc

I'm not sure phpstan supports any simple way to express the closures we support here. Ignoring the error might be just fine.