From ee66a12380a02984679501a0d873da5b5fe1e736 Mon Sep 17 00:00:00 2001 From: Grzegorz Korba Date: Tue, 8 Feb 2022 00:44:00 +0100 Subject: [PATCH 1/2] Remove invalid scenario for retrieving services from service locator It does not check what it should, even after removing `ServiceLocator` condition in `ContainerInterfacePrivateServiceRule` tests are green. --- tests/Rules/Symfony/ExampleController.php | 5 ----- tests/Rules/Symfony/container.xml | 5 ----- 2 files changed, 10 deletions(-) diff --git a/tests/Rules/Symfony/ExampleController.php b/tests/Rules/Symfony/ExampleController.php index edbdaaf5..65d1359b 100644 --- a/tests/Rules/Symfony/ExampleController.php +++ b/tests/Rules/Symfony/ExampleController.php @@ -39,9 +39,4 @@ public function unknownGuardedServiceOutsideOfContext(): void $this->get('unknown'); } - public function privateServiceFromServiceLocator(): void - { - $this->get('service_locator')->get('private'); - } - } diff --git a/tests/Rules/Symfony/container.xml b/tests/Rules/Symfony/container.xml index f3261e0a..f21aeae3 100644 --- a/tests/Rules/Symfony/container.xml +++ b/tests/Rules/Symfony/container.xml @@ -3,10 +3,5 @@ - - - - - From 8ce5970ae07a6d052c35030a7bfe8fcc1861410c Mon Sep 17 00:00:00 2001 From: Grzegorz Korba Date: Tue, 8 Feb 2022 01:07:38 +0100 Subject: [PATCH 2/2] Support for `Symfony\Contracts\Service\ServiceProviderInterface` Accessing services from `ServiceProviderInterface` (which extends `ContainerInterface`) should not trigger "Service is private". --- .../ContainerInterfacePrivateServiceRule.php | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/src/Rules/Symfony/ContainerInterfacePrivateServiceRule.php b/src/Rules/Symfony/ContainerInterfacePrivateServiceRule.php index 4c9a6af1..2fcfd38f 100644 --- a/src/Rules/Symfony/ContainerInterfacePrivateServiceRule.php +++ b/src/Rules/Symfony/ContainerInterfacePrivateServiceRule.php @@ -56,8 +56,16 @@ public function processNode(Node $node, Scope $scope): array $isTestContainerType = (new ObjectType('Symfony\Bundle\FrameworkBundle\Test\TestContainer'))->isSuperTypeOf($argType); $isOldServiceSubscriber = (new ObjectType('Symfony\Component\DependencyInjection\ServiceSubscriberInterface'))->isSuperTypeOf($argType); $isServiceSubscriber = $this->isServiceSubscriber($argType, $scope); + $isServiceProvider = $this->isServiceProvider($argType, $scope); + // ServiceLocator is an implementation of ServiceProviderInterface but let's keep explicit rule for it to keep full BC $isServiceLocator = (new ObjectType('Symfony\Component\DependencyInjection\ServiceLocator'))->isSuperTypeOf($argType); - if ($isTestContainerType->yes() || $isOldServiceSubscriber->yes() || $isServiceSubscriber->yes() || $isServiceLocator->yes()) { + if ( + $isTestContainerType->yes() + || $isOldServiceSubscriber->yes() + || $isServiceSubscriber->yes() + || $isServiceProvider->yes() + || $isServiceLocator->yes() + ) { return []; } @@ -87,14 +95,31 @@ public function processNode(Node $node, Scope $scope): array private function isServiceSubscriber(Type $containerType, Scope $scope): TrinaryLogic { - $serviceSubscriberInterfaceType = new ObjectType('Symfony\Contracts\Service\ServiceSubscriberInterface'); - $isContainerServiceSubscriber = $serviceSubscriberInterfaceType->isSuperTypeOf($containerType); + return $this->isInstanceOf( + new ObjectType('Symfony\Contracts\Service\ServiceSubscriberInterface'), + $containerType, + $scope + ); + } + + private function isServiceProvider(Type $containerType, Scope $scope): TrinaryLogic + { + return $this->isInstanceOf( + new ObjectType('Symfony\Contracts\Service\ServiceProviderInterface'), + $containerType, + $scope + ); + } + + private function isInstanceOf(ObjectType $interfaceType, Type $containerType, Scope $scope): TrinaryLogic + { + $isImplementation = $interfaceType->isSuperTypeOf($containerType); $classReflection = $scope->getClassReflection(); if ($classReflection === null) { - return $isContainerServiceSubscriber; + return $isImplementation; } $containedClassType = new ObjectType($classReflection->getName()); - return $isContainerServiceSubscriber->or($serviceSubscriberInterfaceType->isSuperTypeOf($containedClassType)); + return $isImplementation->or($interfaceType->isSuperTypeOf($containedClassType)); } }