From 5955113bd495f8128070347a58e5efedb103c297 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20Ro=CC=88=C3=9Fner?= Date: Mon, 8 May 2023 17:45:22 +0200 Subject: [PATCH 1/4] make class checks and test names match the requirements. --- .../Symfony/ContainerInterfaceUnknownServiceRuleTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php b/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php index 4bd233e9..65b938e0 100644 --- a/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php +++ b/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php @@ -41,8 +41,8 @@ public function testGetPrivateService(): void public function testGetPrivateServiceInAbstractController(): void { - if (!class_exists('Symfony\Bundle\FrameworkBundle\Controller\Controller')) { - self::markTestSkipped(); + if (!class_exists('Symfony\Bundle\FrameworkBundle\Controller\AbstractController')) { + self::markTestSkipped('The test needs Symfony\Bundle\FrameworkBundle\Controller\AbstractController class.'); } $this->analyse( @@ -58,7 +58,7 @@ public function testGetPrivateServiceInAbstractController(): void ); } - public function testGetPrivateServiceInLegacyServiceSubscriber(): void + public function testGetPrivateServiceInServiceSubscriber(): void { if (!interface_exists('Symfony\Contracts\Service\ServiceSubscriberInterface')) { self::markTestSkipped('The test needs Symfony\Contracts\Service\ServiceSubscriberInterface class.'); From b03b35c463a042e7c8bb68e29750417557bbbdfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20Ro=CC=88=C3=9Fner?= Date: Mon, 8 May 2023 17:46:07 +0200 Subject: [PATCH 2/4] create a test szenario for a service locator where the locator key is not the service name. --- ...ntainerInterfaceUnknownServiceRuleTest.php | 14 ++++++++ ...ExampleServiceSubscriberWithLocatorKey.php | 34 +++++++++++++++++++ tests/Rules/Symfony/container.xml | 5 +++ 3 files changed, 53 insertions(+) create mode 100644 tests/Rules/Symfony/ExampleServiceSubscriberWithLocatorKey.php diff --git a/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php b/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php index 65b938e0..7a19c30d 100644 --- a/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php +++ b/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php @@ -72,6 +72,20 @@ public function testGetPrivateServiceInServiceSubscriber(): void ); } + public function testGetPrivateServiceInServiceSubscriberWithAnotherKey(): void + { + if (!interface_exists('Symfony\Contracts\Service\ServiceSubscriberInterface')) { + self::markTestSkipped('The test needs Symfony\Contracts\Service\ServiceSubscriberInterface class.'); + } + + $this->analyse( + [ + __DIR__ . '/ExampleServiceSubscriberWithLocatorKey.php', + ], + [] + ); + } + public static function getAdditionalConfigFiles(): array { return [ diff --git a/tests/Rules/Symfony/ExampleServiceSubscriberWithLocatorKey.php b/tests/Rules/Symfony/ExampleServiceSubscriberWithLocatorKey.php new file mode 100644 index 00000000..0e53565a --- /dev/null +++ b/tests/Rules/Symfony/ExampleServiceSubscriberWithLocatorKey.php @@ -0,0 +1,34 @@ +locator = $locator; + } + + public function privateService(): void + { + $this->locator->get('foobar'); + } + + /** + * @return string[] + */ + public static function getSubscribedServices(): array + { + return []; + } + +} diff --git a/tests/Rules/Symfony/container.xml b/tests/Rules/Symfony/container.xml index f3261e0a..5d5269ed 100644 --- a/tests/Rules/Symfony/container.xml +++ b/tests/Rules/Symfony/container.xml @@ -8,5 +8,10 @@ + + + + + From ffb77271ec9844c2361c7892020f03c4c314aeae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20Ro=CC=88=C3=9Fner?= Date: Mon, 8 May 2023 19:01:56 +0200 Subject: [PATCH 3/4] allow non-service service keys in service locators. --- .../ContainerInterfaceUnknownServiceRule.php | 15 ++++++++++++++- .../ExampleServiceSubscriberWithLocatorKey.php | 11 +++++++---- tests/Rules/Symfony/container.xml | 5 +++-- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/Rules/Symfony/ContainerInterfaceUnknownServiceRule.php b/src/Rules/Symfony/ContainerInterfaceUnknownServiceRule.php index 417f51f5..4887cbe3 100644 --- a/src/Rules/Symfony/ContainerInterfaceUnknownServiceRule.php +++ b/src/Rules/Symfony/ContainerInterfaceUnknownServiceRule.php @@ -9,6 +9,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleError; use PHPStan\Symfony\ServiceMap; +use PHPStan\TrinaryLogic; use PHPStan\Type\ObjectType; use PHPStan\Type\Symfony\Helper; use function sprintf; @@ -72,7 +73,11 @@ public function processNode(Node $node, Scope $scope): array if ($serviceId !== null) { $service = $this->serviceMap->getService($serviceId); $serviceIdType = $scope->getType($node->getArgs()[0]->value); - if ($service === null && !$scope->getType(Helper::createMarkerNode($node->var, $serviceIdType, $this->printer))->equals($serviceIdType)) { + if ( + $service === null && + !$this->isContainerCallInServiceSubscriber($isContainerType, $isPsrContainerType, $scope) && + !$scope->getType(Helper::createMarkerNode($node->var, $serviceIdType, $this->printer))->equals($serviceIdType) + ) { return [sprintf('Service "%s" is not registered in the container.', $serviceId)]; } } @@ -80,4 +85,12 @@ public function processNode(Node $node, Scope $scope): array return []; } + private function isContainerCallInServiceSubscriber(TrinaryLogic $isContainerType, TrinaryLogic $isPsrContainerType, Scope $scope): bool + { + $scopeClassReflection = $scope->getClassReflection(); + return $scopeClassReflection !== null && + $scopeClassReflection->implementsInterface('Symfony\Contracts\Service\ServiceSubscriberInterface') && + ($isContainerType->yes() || $isPsrContainerType->yes()); + } + } diff --git a/tests/Rules/Symfony/ExampleServiceSubscriberWithLocatorKey.php b/tests/Rules/Symfony/ExampleServiceSubscriberWithLocatorKey.php index 0e53565a..2fc4517e 100644 --- a/tests/Rules/Symfony/ExampleServiceSubscriberWithLocatorKey.php +++ b/tests/Rules/Symfony/ExampleServiceSubscriberWithLocatorKey.php @@ -3,9 +3,7 @@ namespace PHPStan\Rules\Symfony; use Psr\Container\ContainerInterface; -use Symfony\Component\DependencyInjection\ParameterBag\ContainerBag; use Symfony\Contracts\Service\ServiceSubscriberInterface; -use function PHPStan\Rules\Symfony\doFoo; final class ExampleServiceSubscriberWithLocatorKey implements ServiceSubscriberInterface { @@ -18,9 +16,14 @@ public function __construct(ContainerInterface $locator) $this->locator = $locator; } - public function privateService(): void + public function privateAliasService(): void { - $this->locator->get('foobar'); + $this->locator->get('private_alias'); + } + + public function publicAliasService(): void + { + $this->locator->get('public_alias'); } /** diff --git a/tests/Rules/Symfony/container.xml b/tests/Rules/Symfony/container.xml index 5d5269ed..63aa2d87 100644 --- a/tests/Rules/Symfony/container.xml +++ b/tests/Rules/Symfony/container.xml @@ -8,9 +8,10 @@ - + - + + From 0cf58643853e33222ef8db842caa8dce349520d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20Ro=CC=88=C3=9Fner?= Date: Mon, 8 May 2023 19:23:32 +0200 Subject: [PATCH 4/4] add support for legacy ServiceSubscriberInterface, as well. --- .../ContainerInterfaceUnknownServiceRule.php | 10 ++++- ...ntainerInterfaceUnknownServiceRuleTest.php | 14 +++++++ ...eLegacyServiceSubscriberWithLocatorKey.php | 37 +++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 tests/Rules/Symfony/ExampleLegacyServiceSubscriberWithLocatorKey.php diff --git a/src/Rules/Symfony/ContainerInterfaceUnknownServiceRule.php b/src/Rules/Symfony/ContainerInterfaceUnknownServiceRule.php index 4887cbe3..9cb851ff 100644 --- a/src/Rules/Symfony/ContainerInterfaceUnknownServiceRule.php +++ b/src/Rules/Symfony/ContainerInterfaceUnknownServiceRule.php @@ -89,8 +89,14 @@ private function isContainerCallInServiceSubscriber(TrinaryLogic $isContainerTyp { $scopeClassReflection = $scope->getClassReflection(); return $scopeClassReflection !== null && - $scopeClassReflection->implementsInterface('Symfony\Contracts\Service\ServiceSubscriberInterface') && - ($isContainerType->yes() || $isPsrContainerType->yes()); + ( + $scopeClassReflection->implementsInterface('Symfony\Contracts\Service\ServiceSubscriberInterface') || + $scopeClassReflection->implementsInterface('Symfony\Component\DependencyInjection\ServiceSubscriberInterface') + ) && + ( + $isContainerType->yes() || + $isPsrContainerType->yes() + ); } } diff --git a/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php b/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php index 7a19c30d..b0e22922 100644 --- a/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php +++ b/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php @@ -86,6 +86,20 @@ public function testGetPrivateServiceInServiceSubscriberWithAnotherKey(): void ); } + public function testGetPrivateServiceInLegacyServiceSubscriberWithAnotherKey(): void + { + if (!interface_exists('Symfony\Component\DependencyInjection\ServiceSubscriberInterface')) { + self::markTestSkipped('The test needs Symfony\Component\DependencyInjection\ServiceSubscriberInterface class.'); + } + + $this->analyse( + [ + __DIR__ . '/ExampleLegacyServiceSubscriberWithLocatorKey.php', + ], + [] + ); + } + public static function getAdditionalConfigFiles(): array { return [ diff --git a/tests/Rules/Symfony/ExampleLegacyServiceSubscriberWithLocatorKey.php b/tests/Rules/Symfony/ExampleLegacyServiceSubscriberWithLocatorKey.php new file mode 100644 index 00000000..f4fb5b40 --- /dev/null +++ b/tests/Rules/Symfony/ExampleLegacyServiceSubscriberWithLocatorKey.php @@ -0,0 +1,37 @@ +locator = $locator; + } + + public function privateAliasService(): void + { + $this->locator->get('private_alias'); + } + + public function publicAliasService(): void + { + $this->locator->get('public_alias'); + } + + /** + * @return string[] + */ + public static function getSubscribedServices(): array + { + return []; + } + +}