feature #54496 [Contracts] Rename ServiceSubscriberTrait to ServiceMe…

…thodsSubscriberTrait (nicolas-grekas)

This PR was merged into the 7.1 branch.

Discussion
----------

[Contracts] Rename ServiceSubscriberTrait to ServiceMethodsSubscriberTrait

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Issues        | Fix #54490
| License       | MIT

As described in the linked PR, AbstractController is incompatible with ServiceSubscriberTrait because of the added type to the AbstractController::$container property, while ServiceSubscriberTrait's $container property cannot have a type without a BC break.

There are two parts to this PR:
- Deprecate ServiceSubscriberTrait in favor if ServiceMethodsSubscriberTrait, which declares the type of the $container property. The new name better conveys its purpose as a bonus.
- Fix the incompatibility with AbstractController by removing the property declaration on ServiceSubscriberTrait. This means the
 trait will create a dynamic property. Those are deprecated, but since the trait is also deprecated, the upgrade path is clear.

I also tried to improve the description of the trait in the meantime.

/cc `@kbond`

Commits
-------

8f47ced [Contracts] Rename ServiceSubscriberTrait to ServiceMethodsSubscriberTrait

src/Symfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php

@@ -43,8 +43,8 @@
 use Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriberUnionWithTrait;
 use Symfony\Component\DependencyInjection\TypedReference;
 use Symfony\Contracts\Service\Attribute\SubscribedService;
+use Symfony\Contracts\Service\ServiceMethodsSubscriberTrait;
 use Symfony\Contracts\Service\ServiceSubscriberInterface;
-use Symfony\Contracts\Service\ServiceSubscriberTrait;
 
 require_once __DIR__.'/../Fixtures/includes/classes.php';
 
@@ -221,7 +221,7 @@ public function testExtraServiceSubscriber()
         $container->compile();
     }
 
-    public function testServiceSubscriberTraitWithSubscribedServiceAttribute()
+    public function testServiceMethodsSubscriberTraitWithSubscribedServiceAttribute()
     {
         if (!class_exists(SubscribedService::class)) {
             $this->markTestSkipped('SubscribedService attribute not available.');
@@ -251,14 +251,14 @@ public function testServiceSubscriberTraitWithSubscribedServiceAttribute()
         $this->assertEquals($expected, $container->getDefinition((string) $locator->getFactory()[0])->getArgument(0));
     }
 
-    public function testServiceSubscriberTraitWithSubscribedServiceAttributeOnStaticMethod()
+    public function testServiceMethodsSubscriberTraitWithSubscribedServiceAttributeOnStaticMethod()
     {
         if (!class_exists(SubscribedService::class)) {
             $this->markTestSkipped('SubscribedService attribute not available.');
         }
 
         $subscriber = new class() implements ServiceSubscriberInterface {
-            use ServiceSubscriberTrait;
+            use ServiceMethodsSubscriberTrait;
 
             #[SubscribedService]
             public static function method(): TestDefinition1
@@ -271,14 +271,14 @@ public static function method(): TestDefinition1
         $subscriber::getSubscribedServices();
     }
 
-    public function testServiceSubscriberTraitWithSubscribedServiceAttributeOnMethodWithRequiredParameters()
+    public function testServiceMethodsSubscriberTraitWithSubscribedServiceAttributeOnMethodWithRequiredParameters()
     {
         if (!class_exists(SubscribedService::class)) {
             $this->markTestSkipped('SubscribedService attribute not available.');
         }
 
         $subscriber = new class() implements ServiceSubscriberInterface {
-            use ServiceSubscriberTrait;
+            use ServiceMethodsSubscriberTrait;
 
             #[SubscribedService]
             public function method($param1, $param2 = null): TestDefinition1
@@ -291,14 +291,14 @@ public function method($param1, $param2 = null): TestDefinition1
         $subscriber::getSubscribedServices();
     }
 
-    public function testServiceSubscriberTraitWithSubscribedServiceAttributeOnMethodMissingReturnType()
+    public function testServiceMethodsSubscriberTraitWithSubscribedServiceAttributeOnMethodMissingReturnType()
     {
         if (!class_exists(SubscribedService::class)) {
             $this->markTestSkipped('SubscribedService attribute not available.');
         }
 
         $subscriber = new class() implements ServiceSubscriberInterface {
-            use ServiceSubscriberTrait;
+            use ServiceMethodsSubscriberTrait;
 
             #[SubscribedService]
             public function method()
@@ -311,7 +311,7 @@ public function method()
         $subscriber::getSubscribedServices();
     }
 
-    public function testServiceSubscriberTraitWithUnionReturnType()
+    public function testServiceMethodsSubscriberTraitWithUnionReturnType()
     {
         if (!class_exists(SubscribedService::class)) {
             $this->markTestSkipped('SubscribedService attribute not available.');
@@ -338,7 +338,7 @@ public function testServiceSubscriberTraitWithUnionReturnType()
         $this->assertEquals($expected, $container->getDefinition((string) $locator->getFactory()[0])->getArgument(0));
     }
 
-    public function testServiceSubscriberTraitWithIntersectionReturnType()
+    public function testServiceMethodsSubscriberTraitWithIntersectionReturnType()
     {
         if (!class_exists(SubscribedService::class)) {
             $this->markTestSkipped('SubscribedService attribute not available.');

src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberTrait.php → src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceMethodsSubscriberTrait.php

@@ -4,7 +4,7 @@
 
 use Symfony\Contracts\Service\Attribute\SubscribedService;
 
-trait TestServiceSubscriberTrait
+trait TestServiceMethodsSubscriberTrait
 {
     protected function protectedFunction1(): SomeClass
     {

src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberChild.php

@@ -3,12 +3,12 @@
 namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
 
 use Symfony\Contracts\Service\Attribute\SubscribedService;
-use Symfony\Contracts\Service\ServiceSubscriberTrait;
+use Symfony\Contracts\Service\ServiceMethodsSubscriberTrait;
 
 class TestServiceSubscriberChild extends TestServiceSubscriberParent
 {
-    use ServiceSubscriberTrait;
-    use TestServiceSubscriberTrait;
+    use ServiceMethodsSubscriberTrait;
+    use TestServiceMethodsSubscriberTrait;
 
     #[SubscribedService]
     private function testDefinition2(): ?TestDefinition2

src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberIntersectionWithTrait.php

@@ -3,12 +3,12 @@
 namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
 
 use Symfony\Contracts\Service\Attribute\SubscribedService;
+use Symfony\Contracts\Service\ServiceMethodsSubscriberTrait;
 use Symfony\Contracts\Service\ServiceSubscriberInterface;
-use Symfony\Contracts\Service\ServiceSubscriberTrait;
 
 class TestServiceSubscriberIntersectionWithTrait implements ServiceSubscriberInterface
 {
-    use ServiceSubscriberTrait;
+    use ServiceMethodsSubscriberTrait;
 
     #[SubscribedService]
     private function method1(): TestDefinition1&TestDefinition2

src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberParent.php

@@ -3,12 +3,12 @@
 namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
 
 use Symfony\Contracts\Service\Attribute\SubscribedService;
+use Symfony\Contracts\Service\ServiceMethodsSubscriberTrait;
 use Symfony\Contracts\Service\ServiceSubscriberInterface;
-use Symfony\Contracts\Service\ServiceSubscriberTrait;
 
 class TestServiceSubscriberParent implements ServiceSubscriberInterface
 {
-    use ServiceSubscriberTrait;
+    use ServiceMethodsSubscriberTrait;
 
     public function publicFunction1(): SomeClass
     {

src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberUnionWithTrait.php

@@ -3,12 +3,12 @@
 namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
 
 use Symfony\Contracts\Service\Attribute\SubscribedService;
+use Symfony\Contracts\Service\ServiceMethodsSubscriberTrait;
 use Symfony\Contracts\Service\ServiceSubscriberInterface;
-use Symfony\Contracts\Service\ServiceSubscriberTrait;
 
 class TestServiceSubscriberUnionWithTrait implements ServiceSubscriberInterface
 {
-    use ServiceSubscriberTrait;
+    use ServiceMethodsSubscriberTrait;
 
     #[SubscribedService]
     private function method1(): TestDefinition1|TestDefinition2|null

src/Symfony/Contracts/CHANGELOG.md

@@ -5,6 +5,7 @@ CHANGELOG
 ---
 
  * Add `ServiceCollectionInterface`
+ * Deprecate `ServiceSubscriberTrait`, use `ServiceMethodsSubscriberTrait` instead
 
 3.4
 ---

src/Symfony/Contracts/Service/Attribute/SubscribedService.php

@@ -11,15 +11,15 @@
 
 namespace Symfony\Contracts\Service\Attribute;
 
+use Symfony\Contracts\Service\ServiceMethodsSubscriberTrait;
 use Symfony\Contracts\Service\ServiceSubscriberInterface;
-use Symfony\Contracts\Service\ServiceSubscriberTrait;
 
 /**
  * For use as the return value for {@see ServiceSubscriberInterface}.
  *
  * @example new SubscribedService('http_client', HttpClientInterface::class, false, new Target('githubApi'))
  *
- * Use with {@see ServiceSubscriberTrait} to mark a method's return type
+ * Use with {@see ServiceMethodsSubscriberTrait} to mark a method's return type
  * as a subscribed service.
  *
  * @author Kevin Bond <kevinbond@gmail.com>

src/Symfony/Contracts/Service/ServiceMethodsSubscriberTrait.php

@@ -0,0 +1,80 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Contracts\Service;
+
+use Psr\Container\ContainerInterface;
+use Symfony\Contracts\Service\Attribute\Required;
+use Symfony\Contracts\Service\Attribute\SubscribedService;
+
+/**
+ * Implementation of ServiceSubscriberInterface that determines subscribed services
+ * from methods that have the #[SubscribedService] attribute.
+ *
+ * Service ids are available as "ClassName::methodName" so that the implementation
+ * of subscriber methods can be just `return $this->container->get(__METHOD__);`.
+ *
+ * @author Kevin Bond <kevinbond@gmail.com>
+ */
+trait ServiceMethodsSubscriberTrait
+{
+    protected ContainerInterface $container;
+
+    public static function getSubscribedServices(): array
+    {
+        $services = method_exists(get_parent_class(self::class) ?: '', __FUNCTION__) ? parent::getSubscribedServices() : [];
+
+        foreach ((new \ReflectionClass(self::class))->getMethods() as $method) {
+            if (self::class !== $method->getDeclaringClass()->name) {
+                continue;
+            }
+
+            if (!$attribute = $method->getAttributes(SubscribedService::class)[0] ?? null) {
+                continue;
+            }
+
+            if ($method->isStatic() || $method->isAbstract() || $method->isGenerator() || $method->isInternal() || $method->getNumberOfRequiredParameters()) {
+                throw new \LogicException(sprintf('Cannot use "%s" on method "%s::%s()" (can only be used on non-static, non-abstract methods with no parameters).', SubscribedService::class, self::class, $method->name));
+            }
+
+            if (!$returnType = $method->getReturnType()) {
+                throw new \LogicException(sprintf('Cannot use "%s" on methods without a return type in "%s::%s()".', SubscribedService::class, $method->name, self::class));
+            }
+
+            /* @var SubscribedService $attribute */
+            $attribute = $attribute->newInstance();
+            $attribute->key ??= self::class.'::'.$method->name;
+            $attribute->type ??= $returnType instanceof \ReflectionNamedType ? $returnType->getName() : (string) $returnType;
+            $attribute->nullable = $returnType->allowsNull();
+
+            if ($attribute->attributes) {
+                $services[] = $attribute;
+            } else {
+                $services[$attribute->key] = ($attribute->nullable ? '?' : '').$attribute->type;
+            }
+        }
+
+        return $services;
+    }
+
+    #[Required]
+    public function setContainer(ContainerInterface $container): ?ContainerInterface
+    {
+        $ret = null;
+        if (method_exists(get_parent_class(self::class) ?: '', __FUNCTION__)) {
+            $ret = parent::setContainer($container);
+        }
+
+        $this->container = $container;
+
+        return $ret;
+    }
+}

src/Symfony/Contracts/Service/ServiceSubscriberTrait.php

@@ -15,17 +15,23 @@
 use Symfony\Contracts\Service\Attribute\Required;
 use Symfony\Contracts\Service\Attribute\SubscribedService;
 
+trigger_deprecation('symfony/contracts', 'v3.5', '"%s" is deprecated, use "ServiceMethodsSubscriberTrait" instead.', ServiceSubscriberTrait::class);
+
 /**
- * Implementation of ServiceSubscriberInterface that determines subscribed services from
- * method return types. Service ids are available as "ClassName::methodName".
+ * Implementation of ServiceSubscriberInterface that determines subscribed services
+ * from methods that have the #[SubscribedService] attribute.
+ *
+ * Service ids are available as "ClassName::methodName" so that the implementation
+ * of subscriber methods can be just `return $this->container->get(__METHOD__);`.
+ *
+ * @property ContainerInterface $container
  *
  * @author Kevin Bond <kevinbond@gmail.com>
+ *
+ * @deprecated since symfony/contracts v3.5, use ServiceMethodsSubscriberTrait instead
  */
 trait ServiceSubscriberTrait
 {
-    /** @var ContainerInterface */
-    protected $container;
-
     public static function getSubscribedServices(): array
     {
         $services = method_exists(get_parent_class(self::class) ?: '', __FUNCTION__) ? parent::getSubscribedServices() : [];

src/Symfony/Contracts/Service/composer.json

@@ -17,7 +17,8 @@
     ],
     "require": {
         "php": ">=8.1",
-        "psr/container": "^1.1|^2.0"
+        "psr/container": "^1.1|^2.0",
+        "symfony/deprecation-contracts": "^2.5|^3"
     },
     "conflict": {
         "ext-psr": "<1.1|>=2"

src/Symfony/Contracts/Tests/Service/LegacyTestService.php

@@ -0,0 +1,93 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Contracts\Tests\Service;
+
+use Psr\Container\ContainerInterface;
+use Symfony\Contracts\Service\Attribute\Required;
+use Symfony\Contracts\Service\Attribute\SubscribedService;
+use Symfony\Contracts\Service\ServiceSubscriberInterface;
+use Symfony\Contracts\Service\ServiceSubscriberTrait;
+
+class LegacyParentTestService
+{
+    public function aParentService(): Service1
+    {
+    }
+
+    public function setContainer(ContainerInterface $container): ?ContainerInterface
+    {
+        return $container;
+    }
+}
+
+class LegacyTestService extends LegacyParentTestService implements ServiceSubscriberInterface
+{
+    use ServiceSubscriberTrait;
+
+    #[SubscribedService]
+    public function aService(): Service2
+    {
+        return $this->container->get(__METHOD__);
+    }
+
+    #[SubscribedService]
+    public function nullableService(): ?Service2
+    {
+        return $this->container->get(__METHOD__);
+    }
+
+    #[SubscribedService(attributes: new Required())]
+    public function withAttribute(): ?Service2
+    {
+        return $this->container->get(__METHOD__);
+    }
+}
+
+class LegacyChildTestService extends LegacyTestService
+{
+    #[SubscribedService()]
+    public function aChildService(): LegacyService3
+    {
+        return $this->container->get(__METHOD__);
+    }
+}
+
+class LegacyParentWithMagicCall
+{
+    public function __call($method, $args)
+    {
+        throw new \BadMethodCallException('Should not be called.');
+    }
+
+    public static function __callStatic($method, $args)
+    {
+        throw new \BadMethodCallException('Should not be called.');
+    }
+}
+
+class LegacyService3
+{
+}
+
+class LegacyParentTestService2
+{
+    /** @var ContainerInterface */
+    protected $container;
+
+    public function setContainer(ContainerInterface $container)
+    {
+        $previous = $this->container ?? null;
+        $this->container = $container;
+
+        return $previous;
+    }
+}