From 2d46724afd6e29dd36bbfa20547c6aec2e45b0ef Mon Sep 17 00:00:00 2001 From: Olda Salek Date: Fri, 29 Dec 2017 18:54:40 +0100 Subject: [PATCH 1/6] Add support for symfony Controllers, replace getClassName with instanceOf --- src/Rules/ContainerInterfacePrivateServiceRule.php | 7 +++++-- src/Rules/ContainerInterfaceUnknownServiceRule.php | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Rules/ContainerInterfacePrivateServiceRule.php b/src/Rules/ContainerInterfacePrivateServiceRule.php index e1b344f..b79ea17 100644 --- a/src/Rules/ContainerInterfacePrivateServiceRule.php +++ b/src/Rules/ContainerInterfacePrivateServiceRule.php @@ -8,6 +8,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Type\ObjectType; +use PHPStan\Type\ThisType; use PhpParser\Node; use PhpParser\Node\Arg; use PhpParser\Node\Expr\MethodCall; @@ -34,8 +35,10 @@ public function processNode(Node $node, Scope $scope): array { if ($node instanceof MethodCall && $node->name === 'get') { $type = $scope->getType($node->var); - if ($type instanceof ObjectType - && \in_array($type->getClassName(), ['Symfony\Component\DependencyInjection\ContainerInterface', 'Symfony\Bundle\FrameworkBundle\Controller\Controller'], \true) + $baseController = new ObjectType('Symfony\Bundle\FrameworkBundle\Controller\Controller'); + $isInstanceOfController = $type instanceof ThisType && $baseController->isSuperTypeOf($type)->yes(); + $isContainerInterface = $type instanceof ObjectType && $type->getClassName() === 'Symfony\Component\DependencyInjection\ContainerInterface'; + if (($isContainerInterface || $isInstanceOfController) && isset($node->args[0]) && $node->args[0] instanceof Arg ) { diff --git a/src/Rules/ContainerInterfaceUnknownServiceRule.php b/src/Rules/ContainerInterfaceUnknownServiceRule.php index ed51cc2..baf0a91 100644 --- a/src/Rules/ContainerInterfaceUnknownServiceRule.php +++ b/src/Rules/ContainerInterfaceUnknownServiceRule.php @@ -8,6 +8,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Type\ObjectType; +use PHPStan\Type\ThisType; use PhpParser\Node; use PhpParser\Node\Arg; use PhpParser\Node\Expr\MethodCall; @@ -34,8 +35,10 @@ public function processNode(Node $node, Scope $scope): array { if ($node instanceof MethodCall && $node->name === 'get') { $type = $scope->getType($node->var); - if ($type instanceof ObjectType - && \in_array($type->getClassName(), ['Symfony\Component\DependencyInjection\ContainerInterface', 'Symfony\Bundle\FrameworkBundle\Controller\Controller'], \true) + $baseController = new ObjectType('Symfony\Bundle\FrameworkBundle\Controller\Controller'); + $isInstanceOfController = $type instanceof ThisType && $baseController->isSuperTypeOf($type)->yes(); + $isContainerInterface = $type instanceof ObjectType && $type->getClassName() === 'Symfony\Component\DependencyInjection\ContainerInterface'; + if (($isInstanceOfController || $isContainerInterface) && isset($node->args[0]) && $node->args[0] instanceof Arg ) { From 7ece628dc1b8676b638e910a72ed8d7d2cc7b5fe Mon Sep 17 00:00:00 2001 From: Olda Salek Date: Fri, 29 Dec 2017 19:06:51 +0100 Subject: [PATCH 2/6] Add support for symfony Controllers, test get private service. --- ...ntainerInterfacePrivateServiceRuleTest.php | 30 +++++++++++++++++++ tests/Rules/data/Controller.php | 10 +++++++ tests/Rules/data/ExampleController.php | 17 +++++++++++ 3 files changed, 57 insertions(+) create mode 100644 tests/Rules/ContainerInterfacePrivateServiceRuleTest.php create mode 100644 tests/Rules/data/Controller.php create mode 100644 tests/Rules/data/ExampleController.php diff --git a/tests/Rules/ContainerInterfacePrivateServiceRuleTest.php b/tests/Rules/ContainerInterfacePrivateServiceRuleTest.php new file mode 100644 index 0000000..46104dc --- /dev/null +++ b/tests/Rules/ContainerInterfacePrivateServiceRuleTest.php @@ -0,0 +1,30 @@ +analyse([__DIR__ . '/data/ExampleController.php'], [ + [ + 'Service "private" is private.', + 14, + ], + ]); + } + +} diff --git a/tests/Rules/data/Controller.php b/tests/Rules/data/Controller.php new file mode 100644 index 0000000..1b22bb9 --- /dev/null +++ b/tests/Rules/data/Controller.php @@ -0,0 +1,10 @@ +get('private'); + $service->noMethod(); + } +} From 5f5f5e59f37f98823b534b33b901b7ac19698d3f Mon Sep 17 00:00:00 2001 From: Olda Salek Date: Fri, 29 Dec 2017 19:11:00 +0100 Subject: [PATCH 3/6] Add support for symfony Controllers: Test get unknown service from container --- ...ntainerInterfaceUnknownServiceRuleTest.php | 30 +++++++++++++++++++ tests/Rules/data/ExampleController.php | 23 +++++++++----- 2 files changed, 46 insertions(+), 7 deletions(-) create mode 100644 tests/Rules/ContainerInterfaceUnknownServiceRuleTest.php diff --git a/tests/Rules/ContainerInterfaceUnknownServiceRuleTest.php b/tests/Rules/ContainerInterfaceUnknownServiceRuleTest.php new file mode 100644 index 0000000..346f497 --- /dev/null +++ b/tests/Rules/ContainerInterfaceUnknownServiceRuleTest.php @@ -0,0 +1,30 @@ +analyse([__DIR__ . '/data/ExampleController.php'], [ + [ + 'Service "service.not.found" is not registered in the container.', + 20, + ], + ]); + } + +} diff --git a/tests/Rules/data/ExampleController.php b/tests/Rules/data/ExampleController.php index 2ab540f..82d7157 100644 --- a/tests/Rules/data/ExampleController.php +++ b/tests/Rules/data/ExampleController.php @@ -1,4 +1,6 @@ -get('private'); - $service->noMethod(); - } + public function getPrivateServiceAction() + { + $service = $this->get('private'); + $service->noMethod(); + } + + public function getUnknownService() + { + $service = $this->get('service.not.found'); + $service->noMethod(); + } + } From 8ef093301e3c11f34ddf326196a9243239ab94b1 Mon Sep 17 00:00:00 2001 From: Olda Salek Date: Fri, 29 Dec 2017 19:39:24 +0100 Subject: [PATCH 4/6] improve include, removed side effects. A file should declare new symbols (classes, functions, constants, etc.) and cause no other side effects, or it should execute logic with side effects, but should not do both. The first symbol is defined on line 11 and the first side effect is on line 9. (PSR1.Files.SideEffects.FoundWithSymbols) --- tests/Rules/ContainerInterfacePrivateServiceRuleTest.php | 5 +++++ tests/Rules/ContainerInterfaceUnknownServiceRuleTest.php | 5 +++++ tests/Rules/data/ExampleController.php | 2 -- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/Rules/ContainerInterfacePrivateServiceRuleTest.php b/tests/Rules/ContainerInterfacePrivateServiceRuleTest.php index 46104dc..f60d35a 100644 --- a/tests/Rules/ContainerInterfacePrivateServiceRuleTest.php +++ b/tests/Rules/ContainerInterfacePrivateServiceRuleTest.php @@ -10,6 +10,11 @@ final class ContainerInterfacePrivateServiceRuleTest extends \PHPStan\Testing\RuleTestCase { + protected function setUp() + { + include_once __DIR__ . '/data/Controller.php'; + } + protected function getRule(): Rule { $serviceMap = new ServiceMap(__DIR__ . '/../container.xml'); diff --git a/tests/Rules/ContainerInterfaceUnknownServiceRuleTest.php b/tests/Rules/ContainerInterfaceUnknownServiceRuleTest.php index 346f497..d31d0fa 100644 --- a/tests/Rules/ContainerInterfaceUnknownServiceRuleTest.php +++ b/tests/Rules/ContainerInterfaceUnknownServiceRuleTest.php @@ -10,6 +10,11 @@ final class ContainerInterfaceUnknownServiceRuleTest extends \PHPStan\Testing\RuleTestCase { + protected function setUp() + { + include_once __DIR__ . '/data/Controller.php'; + } + protected function getRule(): Rule { $serviceMap = new ServiceMap(__DIR__ . '/../container.xml'); diff --git a/tests/Rules/data/ExampleController.php b/tests/Rules/data/ExampleController.php index 82d7157..9c7a1d9 100644 --- a/tests/Rules/data/ExampleController.php +++ b/tests/Rules/data/ExampleController.php @@ -6,8 +6,6 @@ use Symfony\Bundle\FrameworkBundle\Controller\Controller; -include __DIR__ . '/Controller.php'; - final class ExampleController extends Controller { From efaff1f3283bbad4a5ef0ca1b47944ffd3fd7595 Mon Sep 17 00:00:00 2001 From: Olda Salek Date: Fri, 29 Dec 2017 19:54:05 +0100 Subject: [PATCH 5/6] phpstan: ci: exclude data from analytics --- phpstan.neon | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/phpstan.neon b/phpstan.neon index b2b86dc..edcc938 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,3 +1,7 @@ includes: - vendor/phpstan/phpstan-phpunit/extension.neon - vendor/phpstan/phpstan-phpunit/rules.neon + +parameters: + excludes_analyse: + - %rootDir%/../../../tests/*/data/* From 93f60f0d4f97bd2682cbba30e0a27fa68fdb846a Mon Sep 17 00:00:00 2001 From: Olda Salek Date: Mon, 15 Jan 2018 18:34:04 +0100 Subject: [PATCH 6/6] ignore PhpParser\Node\Expr\Variable in ContainerInterfaceUnknownServiceRule --- src/Rules/ContainerInterfaceUnknownServiceRule.php | 2 ++ tests/Rules/data/ExampleController.php | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/src/Rules/ContainerInterfaceUnknownServiceRule.php b/src/Rules/ContainerInterfaceUnknownServiceRule.php index baf0a91..90a32b6 100644 --- a/src/Rules/ContainerInterfaceUnknownServiceRule.php +++ b/src/Rules/ContainerInterfaceUnknownServiceRule.php @@ -5,6 +5,7 @@ namespace Lookyman\PHPStan\Symfony\Rules; use Lookyman\PHPStan\Symfony\ServiceMap; +use PhpParser\Node\Expr\Variable; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Type\ObjectType; @@ -41,6 +42,7 @@ public function processNode(Node $node, Scope $scope): array if (($isInstanceOfController || $isContainerInterface) && isset($node->args[0]) && $node->args[0] instanceof Arg + && !$node->args[0]->value instanceof Variable ) { $service = $this->serviceMap->getServiceFromNode($node->args[0]->value); if ($service === \null) { diff --git a/tests/Rules/data/ExampleController.php b/tests/Rules/data/ExampleController.php index 9c7a1d9..98eea86 100644 --- a/tests/Rules/data/ExampleController.php +++ b/tests/Rules/data/ExampleController.php @@ -21,4 +21,10 @@ public function getUnknownService() $service->noMethod(); } + public function getVariableService(string $serviceKey) + { + $service = $this->get($serviceKey); + $service->noMethod(); + } + }