From 4a7aa8e0154a772ad7ad3a22318573a0a0564794 Mon Sep 17 00:00:00 2001 From: Kevin Bond Date: Fri, 23 Jul 2021 08:44:26 -0400 Subject: [PATCH] [DependencyInjection] Add `SubscribedService` attribute, deprecate current `ServiceSubscriberTrait` usage --- .../RegisterServiceSubscribersPassTest.php | 121 +++++++++++++++++- .../LegacyTestServiceSubscriberChild.php | 29 +++++ .../LegacyTestServiceSubscriberParent.php | 16 +++ .../LegacyTestServiceSubscriberTrait.php | 11 ++ .../Fixtures/TestServiceSubscriberChild.php | 15 ++- .../Fixtures/TestServiceSubscriberParent.php | 6 + .../Fixtures/TestServiceSubscriberTrait.php | 9 +- .../Service/Attribute/SubscribedService.php | 33 +++++ .../Service/ServiceSubscriberTrait.php | 57 ++++++++- src/Symfony/Contracts/Service/composer.json | 3 +- .../Service/ServiceSubscriberTraitTest.php | 48 ++++++- 11 files changed, 334 insertions(+), 14 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/LegacyTestServiceSubscriberChild.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/LegacyTestServiceSubscriberParent.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/LegacyTestServiceSubscriberTrait.php create mode 100644 src/Symfony/Contracts/Service/Attribute/SubscribedService.php diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php index af81202bd754a..fd7a9f1e7af3e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php @@ -24,6 +24,8 @@ use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition; +use Symfony\Component\DependencyInjection\Tests\Fixtures\LegacyTestServiceSubscriberChild; +use Symfony\Component\DependencyInjection\Tests\Fixtures\LegacyTestServiceSubscriberParent; use Symfony\Component\DependencyInjection\Tests\Fixtures\TestDefinition1; use Symfony\Component\DependencyInjection\Tests\Fixtures\TestDefinition2; use Symfony\Component\DependencyInjection\Tests\Fixtures\TestDefinition3; @@ -31,6 +33,7 @@ use Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriberChild; use Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriberParent; use Symfony\Component\DependencyInjection\TypedReference; +use Symfony\Contracts\Service\Attribute\SubscribedService; use Symfony\Contracts\Service\ServiceSubscriberInterface; use Symfony\Contracts\Service\ServiceSubscriberTrait; @@ -143,11 +146,14 @@ public function testExtraServiceSubscriber() $container->compile(); } + /** + * @group legacy + */ public function testServiceSubscriberTrait() { $container = new ContainerBuilder(); - $container->register('foo', TestServiceSubscriberChild::class) + $container->register('foo', LegacyTestServiceSubscriberChild::class) ->addMethodCall('setContainer', [new Reference(PsrContainerInterface::class)]) ->addTag('container.service_subscriber') ; @@ -159,15 +165,18 @@ public function testServiceSubscriberTrait() $locator = $container->getDefinition((string) $foo->getMethodCalls()[0][1][0]); $expected = [ - TestServiceSubscriberChild::class.'::invalidDefinition' => new ServiceClosureArgument(new TypedReference('Symfony\Component\DependencyInjection\Tests\Fixtures\InvalidDefinition', 'Symfony\Component\DependencyInjection\Tests\Fixtures\InvalidDefinition', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)), - TestServiceSubscriberChild::class.'::testDefinition2' => new ServiceClosureArgument(new TypedReference(TestDefinition2::class, TestDefinition2::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)), - TestServiceSubscriberChild::class.'::testDefinition3' => new ServiceClosureArgument(new TypedReference(TestDefinition3::class, TestDefinition3::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)), - TestServiceSubscriberParent::class.'::testDefinition1' => new ServiceClosureArgument(new TypedReference(TestDefinition1::class, TestDefinition1::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)), + LegacyTestServiceSubscriberChild::class.'::invalidDefinition' => new ServiceClosureArgument(new TypedReference('Symfony\Component\DependencyInjection\Tests\Fixtures\InvalidDefinition', 'Symfony\Component\DependencyInjection\Tests\Fixtures\InvalidDefinition', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)), + LegacyTestServiceSubscriberChild::class.'::testDefinition2' => new ServiceClosureArgument(new TypedReference(TestDefinition2::class, TestDefinition2::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)), + LegacyTestServiceSubscriberChild::class.'::testDefinition3' => new ServiceClosureArgument(new TypedReference(TestDefinition3::class, TestDefinition3::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)), + LegacyTestServiceSubscriberParent::class.'::testDefinition1' => new ServiceClosureArgument(new TypedReference(TestDefinition1::class, TestDefinition1::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)), ]; $this->assertEquals($expected, $container->getDefinition((string) $locator->getFactory()[0])->getArgument(0)); } + /** + * @group legacy + */ public function testServiceSubscriberTraitWithGetter() { $container = new ContainerBuilder(); @@ -195,6 +204,108 @@ public function getFoo(): \stdClass $this->assertEquals($expected, $container->getDefinition((string) $locator->getFactory()[0])->getArgument(0)); } + /** + * @requires PHP 8 + */ + public function testServiceSubscriberTraitWithSubscribedServiceAttribute() + { + if (!class_exists(SubscribedService::class)) { + $this->markTestSkipped('SubscribedService attribute not available.'); + } + + $container = new ContainerBuilder(); + + $container->register('foo', TestServiceSubscriberChild::class) + ->addMethodCall('setContainer', [new Reference(PsrContainerInterface::class)]) + ->addTag('container.service_subscriber') + ; + + (new RegisterServiceSubscribersPass())->process($container); + (new ResolveServiceSubscribersPass())->process($container); + + $foo = $container->getDefinition('foo'); + $locator = $container->getDefinition((string) $foo->getMethodCalls()[0][1][0]); + + $expected = [ + TestServiceSubscriberChild::class.'::invalidDefinition' => new ServiceClosureArgument(new TypedReference('Symfony\Component\DependencyInjection\Tests\Fixtures\InvalidDefinition', 'Symfony\Component\DependencyInjection\Tests\Fixtures\InvalidDefinition')), + TestServiceSubscriberChild::class.'::testDefinition2' => new ServiceClosureArgument(new TypedReference(TestDefinition2::class, TestDefinition2::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)), + TestServiceSubscriberChild::class.'::testDefinition4' => new ServiceClosureArgument(new TypedReference(TestDefinition3::class, TestDefinition3::class)), + TestServiceSubscriberParent::class.'::testDefinition1' => new ServiceClosureArgument(new TypedReference(TestDefinition1::class, TestDefinition1::class)), + 'custom_name' => new ServiceClosureArgument(new TypedReference(TestDefinition3::class, TestDefinition3::class, ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, 'custom_name')), + ]; + + $this->assertEquals($expected, $container->getDefinition((string) $locator->getFactory()[0])->getArgument(0)); + } + + /** + * @requires PHP 8 + */ + public function testServiceSubscriberTraitWithSubscribedServiceAttributeOnStaticMethod() + { + if (!class_exists(SubscribedService::class)) { + $this->markTestSkipped('SubscribedService attribute not available.'); + } + + $subscriber = new class() implements ServiceSubscriberInterface { + use ServiceSubscriberTrait; + + #[SubscribedService] + public static function method(): TestDefinition1 + { + } + }; + + $this->expectException(\LogicException::class); + + $subscriber::getSubscribedServices(); + } + + /** + * @requires PHP 8 + */ + public function testServiceSubscriberTraitWithSubscribedServiceAttributeOnMethodWithRequiredParameters() + { + if (!class_exists(SubscribedService::class)) { + $this->markTestSkipped('SubscribedService attribute not available.'); + } + + $subscriber = new class() implements ServiceSubscriberInterface { + use ServiceSubscriberTrait; + + #[SubscribedService] + public function method($param1, $param2 = null): TestDefinition1 + { + } + }; + + $this->expectException(\LogicException::class); + + $subscriber::getSubscribedServices(); + } + + /** + * @requires PHP 8 + */ + public function testServiceSubscriberTraitWithSubscribedServiceAttributeOnMethodMissingReturnType() + { + if (!class_exists(SubscribedService::class)) { + $this->markTestSkipped('SubscribedService attribute not available.'); + } + + $subscriber = new class() implements ServiceSubscriberInterface { + use ServiceSubscriberTrait; + + #[SubscribedService] + public function method() + { + } + }; + + $this->expectException(\LogicException::class); + + $subscriber::getSubscribedServices(); + } + public function testServiceSubscriberWithSemanticId() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/LegacyTestServiceSubscriberChild.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/LegacyTestServiceSubscriberChild.php new file mode 100644 index 0000000000000..ea8a8cc64f65a --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/LegacyTestServiceSubscriberChild.php @@ -0,0 +1,29 @@ +container->get(__METHOD__); + } + + private function invalidDefinition(): InvalidDefinition + { + return $this->container->get(__METHOD__); + } + + private function privateFunction1(): string + { + } + + private function privateFunction2(): string + { + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/LegacyTestServiceSubscriberParent.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/LegacyTestServiceSubscriberParent.php new file mode 100644 index 0000000000000..7109958863ba7 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/LegacyTestServiceSubscriberParent.php @@ -0,0 +1,16 @@ +container->get(__METHOD__); + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/LegacyTestServiceSubscriberTrait.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/LegacyTestServiceSubscriberTrait.php new file mode 100644 index 0000000000000..514f05f50eafb --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/LegacyTestServiceSubscriberTrait.php @@ -0,0 +1,11 @@ +container->get(__CLASS__.'::'.__FUNCTION__); + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberChild.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberChild.php index c585a6643be32..ee2df273996b6 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberChild.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberChild.php @@ -2,6 +2,7 @@ namespace Symfony\Component\DependencyInjection\Tests\Fixtures; +use Symfony\Contracts\Service\Attribute\SubscribedService; use Symfony\Contracts\Service\ServiceSubscriberTrait; class TestServiceSubscriberChild extends TestServiceSubscriberParent @@ -9,11 +10,19 @@ class TestServiceSubscriberChild extends TestServiceSubscriberParent use ServiceSubscriberTrait; use TestServiceSubscriberTrait; - private function testDefinition2(): TestDefinition2 + #[SubscribedService] + private function testDefinition2(): ?TestDefinition2 { return $this->container->get(__METHOD__); } + #[SubscribedService('custom_name')] + private function testDefinition3(): TestDefinition3 + { + return $this->container->get('custom_name'); + } + + #[SubscribedService] private function invalidDefinition(): InvalidDefinition { return $this->container->get(__METHOD__); @@ -26,4 +35,8 @@ private function privateFunction1(): string private function privateFunction2(): string { } + + private function privateFunction3(): AnotherClass + { + } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberParent.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberParent.php index 04a024e4ecd81..95fbfd35218c6 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberParent.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberParent.php @@ -2,6 +2,7 @@ namespace Symfony\Component\DependencyInjection\Tests\Fixtures; +use Symfony\Contracts\Service\Attribute\SubscribedService; use Symfony\Contracts\Service\ServiceSubscriberInterface; use Symfony\Contracts\Service\ServiceSubscriberTrait; @@ -9,6 +10,11 @@ class TestServiceSubscriberParent implements ServiceSubscriberInterface { use ServiceSubscriberTrait; + public function publicFunction1(): SomeClass + { + } + + #[SubscribedService] private function testDefinition1(): TestDefinition1 { return $this->container->get(__METHOD__); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberTrait.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberTrait.php index ea98a0f2f13e1..52e946ff1f0bc 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberTrait.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberTrait.php @@ -2,9 +2,16 @@ namespace Symfony\Component\DependencyInjection\Tests\Fixtures; +use Symfony\Contracts\Service\Attribute\SubscribedService; + trait TestServiceSubscriberTrait { - private function testDefinition3(): TestDefinition3 + protected function protectedFunction1(): SomeClass + { + } + + #[SubscribedService] + private function testDefinition4(): TestDefinition3 { return $this->container->get(__CLASS__.'::'.__FUNCTION__); } diff --git a/src/Symfony/Contracts/Service/Attribute/SubscribedService.php b/src/Symfony/Contracts/Service/Attribute/SubscribedService.php new file mode 100644 index 0000000000000..10d1bc38e8bf5 --- /dev/null +++ b/src/Symfony/Contracts/Service/Attribute/SubscribedService.php @@ -0,0 +1,33 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Contracts\Service\Attribute; + +use Symfony\Contracts\Service\ServiceSubscriberTrait; + +/** + * Use with {@see ServiceSubscriberTrait} to mark a method's return type + * as a subscribed service. + * + * @author Kevin Bond + */ +#[\Attribute(\Attribute::TARGET_METHOD)] +final class SubscribedService +{ + /** + * @param string|null $key The key to use for the service + * If null, use "ClassName::methodName" + */ + public function __construct( + public ?string $key = null + ) { + } +} diff --git a/src/Symfony/Contracts/Service/ServiceSubscriberTrait.php b/src/Symfony/Contracts/Service/ServiceSubscriberTrait.php index cba072937d0a0..f0b2806f286b0 100644 --- a/src/Symfony/Contracts/Service/ServiceSubscriberTrait.php +++ b/src/Symfony/Contracts/Service/ServiceSubscriberTrait.php @@ -12,10 +12,11 @@ namespace Symfony\Contracts\Service; use Psr\Container\ContainerInterface; +use Symfony\Contracts\Service\Attribute\SubscribedService; /** * Implementation of ServiceSubscriberInterface that determines subscribed services from - * private method return types. Service ids are available as "ClassName::methodName". + * method return types. Service ids are available as "ClassName::methodName". * * @author Kevin Bond */ @@ -36,13 +37,59 @@ public static function getSubscribedServices(): array } $services = \is_callable(['parent', __FUNCTION__]) ? parent::getSubscribedServices() : []; + $attributeOptIn = false; - foreach ((new \ReflectionClass(self::class))->getMethods() as $method) { - if ($method->isStatic() || $method->isAbstract() || $method->isGenerator() || $method->isInternal() || $method->getNumberOfRequiredParameters()) { - continue; + if (\PHP_VERSION_ID >= 80000) { + 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)); + } + + $serviceId = $returnType instanceof \ReflectionNamedType ? $returnType->getName() : (string) $returnType; + + if ($returnType->allowsNull()) { + $serviceId = '?'.$serviceId; + } + + $services[$attribute->newInstance()->key ?? self::class.'::'.$method->name] = $serviceId; + $attributeOptIn = true; } + } + + if (!$attributeOptIn) { + foreach ((new \ReflectionClass(self::class))->getMethods() as $method) { + if ($method->isStatic() || $method->isAbstract() || $method->isGenerator() || $method->isInternal() || $method->getNumberOfRequiredParameters()) { + continue; + } + + if (self::class !== $method->getDeclaringClass()->name) { + continue; + } + + if (!$returnType = $method->getReturnType()) { + continue; + } + + if ($returnType->isBuiltin()) { + continue; + } + + if (\PHP_VERSION_ID >= 80000) { + trigger_deprecation('symfony/service-contracts', '2.5', 'Using "%s" in "%s" without using the "%s" attribute on any method is deprecated.', ServiceSubscriberTrait::class, self::class, SubscribedService::class); + } - if (self::class === $method->getDeclaringClass()->name && ($returnType = $method->getReturnType()) && !$returnType->isBuiltin()) { $services[self::class.'::'.$method->name] = '?'.($returnType instanceof \ReflectionNamedType ? $returnType->getName() : $returnType); } } diff --git a/src/Symfony/Contracts/Service/composer.json b/src/Symfony/Contracts/Service/composer.json index 617512a9fb99e..e680798d73926 100644 --- a/src/Symfony/Contracts/Service/composer.json +++ b/src/Symfony/Contracts/Service/composer.json @@ -17,7 +17,8 @@ ], "require": { "php": ">=7.2.5", - "psr/container": "^1.1" + "psr/container": "^1.1", + "symfony/deprecation-contracts": "^2.1" }, "conflict": { "ext-psr": "<1.1|>=2" diff --git a/src/Symfony/Contracts/Tests/Service/ServiceSubscriberTraitTest.php b/src/Symfony/Contracts/Tests/Service/ServiceSubscriberTraitTest.php index 344bba8772dc6..d6a5b41192beb 100644 --- a/src/Symfony/Contracts/Tests/Service/ServiceSubscriberTraitTest.php +++ b/src/Symfony/Contracts/Tests/Service/ServiceSubscriberTraitTest.php @@ -13,15 +13,34 @@ use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; +use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir\Component1\Dir1\Service1; +use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir\Component1\Dir2\Service2; +use Symfony\Contracts\Service\Attribute\SubscribedService; use Symfony\Contracts\Service\ServiceLocatorTrait; use Symfony\Contracts\Service\ServiceSubscriberInterface; use Symfony\Contracts\Service\ServiceSubscriberTrait; class ServiceSubscriberTraitTest extends TestCase { + /** + * @group legacy + */ + public function testLegacyMethodsOnParentsAndChildrenAreIgnoredInGetSubscribedServices() + { + $expected = [LegacyTestService::class.'::aService' => '?'.Service2::class]; + + $this->assertEquals($expected, LegacyChildTestService::getSubscribedServices()); + } + + /** + * @requires PHP 8 + */ public function testMethodsOnParentsAndChildrenAreIgnoredInGetSubscribedServices() { - $expected = [TestService::class.'::aService' => '?Symfony\Contracts\Tests\Service\Service2']; + $expected = [ + TestService::class.'::aService' => Service2::class, + TestService::class.'::nullableService' => '?'.Service2::class, + ]; $this->assertEquals($expected, ChildTestService::getSubscribedServices()); } @@ -48,18 +67,45 @@ public function setContainer(ContainerInterface $container) } } +class LegacyTestService extends ParentTestService implements ServiceSubscriberInterface +{ + use ServiceSubscriberTrait; + + public function aService(): Service2 + { + } +} + +class LegacyChildTestService extends LegacyTestService +{ + public function aChildService(): Service3 + { + } +} + class TestService extends ParentTestService implements ServiceSubscriberInterface { use ServiceSubscriberTrait; + #[SubscribedService] public function aService(): Service2 { } + + #[SubscribedService] + public function nullableService(): ?Service2 + { + } } class ChildTestService extends TestService { + #[SubscribedService] public function aChildService(): Service3 { } } + +class Service3 +{ +}