From 417ee5c74986a10468bac0b17db6c8a17c265c41 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 21 Sep 2019 21:06:31 +0200 Subject: [PATCH] decouple --- CHANGELOG.md | 4 +- config/set/symfony/symfony-phpunit.yaml | 2 +- docs/AllRectorsOverview.md | 4 +- .../src/AssertManipulator.php | 2 +- .../src/Route/RouteInfoFactory.php | 2 +- packages/SymfonyPHPUnit/config/config.yaml | 7 + .../src/Naming/ServiceNaming.php | 29 ++ .../src/Node/KernelTestCaseNodeAnalyzer.php | 46 ++ .../src/Node/KernelTestCaseNodeFactory.php | 157 +++++++ .../MultipleServiceGetToSetUpMethodRector.php | 414 ------------------ ...tMethodCallFromTestToSetUpMethodRector.php | 255 +++++++++++ .../src/SelfContainerMethodCallCollector.php | 77 ++++ .../Fixture/existing_setup.php.inc | 60 --- ...extends_parent_class_with_property.php.inc | 47 -- .../Fixture/fixture.php.inc | 54 --- .../Fixture/instant_call.php.inc | 52 --- .../Fixture/string_service_name.php.inc | 52 --- .../Source/DummyKernelTestCase.php | 8 - .../Source/ItemRepository.php | 8 - .../Fixture/existing_setup.php.inc | 60 +++ ...extends_parent_class_with_property.php.inc | 47 ++ .../Fixture/fixture.php.inc | 54 +++ .../Fixture/instant_call.php.inc | 52 +++ .../Fixture/skip_sessions.php.inc | 4 +- .../Fixture/string_service_name.php.inc | 41 ++ ...odCallFromTestToSetUpMethodRectorTest.php} | 10 +- .../Source/DummyKernelTestCase.php | 8 + .../Source/ItemRepository.php | 8 + .../ParentClassWithPropertyKernelTestCase.php | 2 +- .../Manipulator/ClassMethodManipulator.php | 2 +- src/PhpParser/Node/Value/ValueResolver.php | 2 +- .../AbstractRector/ValueResolverTrait.php | 2 +- .../Node/Value/ValueResolverTest.php | 2 +- 33 files changed, 860 insertions(+), 714 deletions(-) create mode 100644 packages/SymfonyPHPUnit/config/config.yaml create mode 100644 packages/SymfonyPHPUnit/src/Naming/ServiceNaming.php create mode 100644 packages/SymfonyPHPUnit/src/Node/KernelTestCaseNodeAnalyzer.php create mode 100644 packages/SymfonyPHPUnit/src/Node/KernelTestCaseNodeFactory.php delete mode 100644 packages/SymfonyPHPUnit/src/Rector/Class_/MultipleServiceGetToSetUpMethodRector.php create mode 100644 packages/SymfonyPHPUnit/src/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector.php create mode 100644 packages/SymfonyPHPUnit/src/SelfContainerMethodCallCollector.php delete mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/existing_setup.php.inc delete mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/extends_parent_class_with_property.php.inc delete mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/fixture.php.inc delete mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/instant_call.php.inc delete mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/string_service_name.php.inc delete mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Source/DummyKernelTestCase.php delete mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Source/ItemRepository.php create mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/existing_setup.php.inc create mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/extends_parent_class_with_property.php.inc create mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/fixture.php.inc create mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/instant_call.php.inc rename packages/SymfonyPHPUnit/tests/Rector/Class_/{MultipleServiceGetToSetUpMethodRector => SelfContainerGetMethodCallFromTestToSetUpMethodRector}/Fixture/skip_sessions.php.inc (63%) create mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/string_service_name.php.inc rename packages/SymfonyPHPUnit/tests/Rector/Class_/{MultipleServiceGetToSetUpMethodRector/MultipleServiceGetToSetUpMethodRectorTest.php => SelfContainerGetMethodCallFromTestToSetUpMethodRector/SelfContainerGetMethodCallFromTestToSetUpMethodRectorTest.php} (65%) create mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Source/DummyKernelTestCase.php create mode 100644 packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Source/ItemRepository.php rename packages/SymfonyPHPUnit/tests/Rector/Class_/{MultipleServiceGetToSetUpMethodRector => SelfContainerGetMethodCallFromTestToSetUpMethodRector}/Source/ParentClassWithPropertyKernelTestCase.php (56%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d7c9542e750..5a86a8144af3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -189,7 +189,7 @@ PRs and issues are linked, so you can find more about it. Thanks to [ChangelogLi - [#1753] [DeadCode] Rector `RemoveDeadConstructorRector` should skip `private` method, Thanks to [@stloyd] - [#1687] [Symfony] Set few default common service names for Symfony App Analyzer, Thanks to [@stloyd] - [#1675] [Symfony] Make set symfony42 refactor get(...) in former container aware commands -- [#1666] Skip session in `MultipleServiceGetToSetUpMethodRector` +- [#1666] Skip session in `SelfContainerGetMethodCallFromTestToSetUpMethodRector` - [#1757] make SymfonyContainer factory configurable with "kernel_environment" parameter in rector.yaml - [#1707] Don't mess with lines between docblock comment and var type., Thanks to [@ravanscafi] - [#1699] Update composer scripts, Thanks to [@ravanscafi] @@ -233,7 +233,7 @@ PRs and issues are linked, so you can find more about it. Thanks to [ChangelogLi - [#1584] [DeadCode] Add `RemoveDeadZeroAndOneOperationRector` - [#1586] [DeadCode] Add `RemoveDelegatingParentCallRector` - [#1603] [DeadCode] Add `RemoveDuplicatedInstanceOfRector` -- [#1656] [SymfonyPHPUnit] Add `MultipleServiceGetToSetUpMethodRector` +- [#1656] [SymfonyPHPUnit] Add `SelfContainerGetMethodCallFromTestToSetUpMethodRector` - [#1589] Add assign ref support to `AddDefaultValueForUndefinedVariableRector` - [#1609] Add `ElasticSearchDSL` package, Thanks to [@shyim] - [#1611] Add rector for ShopRegistration, Thanks to [@shyim] diff --git a/config/set/symfony/symfony-phpunit.yaml b/config/set/symfony/symfony-phpunit.yaml index 913249b2366b..d80313b328ec 100644 --- a/config/set/symfony/symfony-phpunit.yaml +++ b/config/set/symfony/symfony-phpunit.yaml @@ -1,2 +1,2 @@ services: - Rector\SymfonyPHPUnit\Rector\Class_\MultipleServiceGetToSetUpMethodRector: ~ + Rector\SymfonyPHPUnit\Rector\Class_\SelfContainerGetMethodCallFromTestToSetUpMethodRector: ~ diff --git a/docs/AllRectorsOverview.md b/docs/AllRectorsOverview.md index 92c8d766c525..e7ce8f0fae1c 100644 --- a/docs/AllRectorsOverview.md +++ b/docs/AllRectorsOverview.md @@ -5830,9 +5830,9 @@ Change Symfony Event listener class to Event Subscriber based on configuration i ## SymfonyPHPUnit -### `MultipleServiceGetToSetUpMethodRector` +### `SelfContainerGetMethodCallFromTestToSetUpMethodRector` -- class: `Rector\SymfonyPHPUnit\Rector\Class_\MultipleServiceGetToSetUpMethodRector` +- class: `Rector\SymfonyPHPUnit\Rector\Class_\SelfContainerGetMethodCallFromTestToSetUpMethodRector` ```diff use ItemRepository; diff --git a/packages/NetteTesterToPHPUnit/src/AssertManipulator.php b/packages/NetteTesterToPHPUnit/src/AssertManipulator.php index 284e08abb950..7837b8238e5e 100644 --- a/packages/NetteTesterToPHPUnit/src/AssertManipulator.php +++ b/packages/NetteTesterToPHPUnit/src/AssertManipulator.php @@ -228,7 +228,7 @@ private function processTruthyOrFalseyCall(StaticCall $staticCall): Expr private function processTypeCall(StaticCall $staticCall): void { - $value = $this->valueResolver->resolve($staticCall->args[0]->value); + $value = $this->valueResolver->getValue($staticCall->args[0]->value); $typeToMethod = [ 'list' => 'assertIsArray', diff --git a/packages/NetteToSymfony/src/Route/RouteInfoFactory.php b/packages/NetteToSymfony/src/Route/RouteInfoFactory.php index f34b3b7b273f..74a66f8c3999 100644 --- a/packages/NetteToSymfony/src/Route/RouteInfoFactory.php +++ b/packages/NetteToSymfony/src/Route/RouteInfoFactory.php @@ -81,7 +81,7 @@ public function createFromNode(Node $node): ?RouteInfo private function createRouteInfoFromArgs(Node $node, array $methods = []): ?RouteInfo { $pathArgument = $node->args[0]->value; - $routePath = $this->valueResolver->resolve($pathArgument); + $routePath = $this->valueResolver->getValue($pathArgument); // route path is needed if ($routePath === null || ! is_string($routePath)) { diff --git a/packages/SymfonyPHPUnit/config/config.yaml b/packages/SymfonyPHPUnit/config/config.yaml new file mode 100644 index 000000000000..a52e8816fe98 --- /dev/null +++ b/packages/SymfonyPHPUnit/config/config.yaml @@ -0,0 +1,7 @@ +services: + _defaults: + public: true + autowire: true + + Rector\SymfonyPHPUnit\: + resource: '../src/' diff --git a/packages/SymfonyPHPUnit/src/Naming/ServiceNaming.php b/packages/SymfonyPHPUnit/src/Naming/ServiceNaming.php new file mode 100644 index 000000000000..4db92da5afb1 --- /dev/null +++ b/packages/SymfonyPHPUnit/src/Naming/ServiceNaming.php @@ -0,0 +1,29 @@ +propertyNaming = $propertyNaming; + } + + public function resolvePropertyNameFromServiceType(string $serviceType): string + { + if (Strings::contains($serviceType, '_') && ! Strings::contains($serviceType, '\\')) { + return $this->propertyNaming->underscoreToName($serviceType); + } + + return $this->propertyNaming->fqnToVariableName(new ObjectType($serviceType)); + } +} diff --git a/packages/SymfonyPHPUnit/src/Node/KernelTestCaseNodeAnalyzer.php b/packages/SymfonyPHPUnit/src/Node/KernelTestCaseNodeAnalyzer.php new file mode 100644 index 000000000000..04fdbb223499 --- /dev/null +++ b/packages/SymfonyPHPUnit/src/Node/KernelTestCaseNodeAnalyzer.php @@ -0,0 +1,46 @@ +nameResolver = $nameResolver; + } + + /** + * Matches: + * self::$container->get() + */ + public function isSelfContainerGetMethodCall(Node $node): bool + { + if (! $node instanceof MethodCall) { + return false; + } + + if (! $node->var instanceof StaticPropertyFetch) { + return false; + } + + if (! $this->nameResolver->isName($node->var->class, 'self')) { + return false; + } + + if (! $this->nameResolver->isName($node->var->name, 'container')) { + return false; + } + + return $this->nameResolver->isName($node->name, 'get'); + } +} diff --git a/packages/SymfonyPHPUnit/src/Node/KernelTestCaseNodeFactory.php b/packages/SymfonyPHPUnit/src/Node/KernelTestCaseNodeFactory.php new file mode 100644 index 000000000000..8c74a097c4b3 --- /dev/null +++ b/packages/SymfonyPHPUnit/src/Node/KernelTestCaseNodeFactory.php @@ -0,0 +1,157 @@ +builderFactory = $builderFactory; + $this->phpUnitTypeDeclarationDecorator = $phpUnitTypeDeclarationDecorator; + $this->nodeFactory = $nodeFactory; + $this->serviceNaming = $serviceNaming; + } + + /** + * @param string[] $serviceTypes + */ + public function createSetUpClassMethodWithGetTypes(Class_ $class, array $serviceTypes): ?ClassMethod + { + $assigns = $this->createSelfContainerGetWithTypeAssigns($class, $serviceTypes); + if (count($assigns) === 0) { + return null; + } + + $stmts = array_merge([new StaticCall(new Name('parent'), 'setUp')], $assigns); + + $classMethodBuilder = $this->builderFactory->method('setUp'); + $classMethodBuilder->makeProtected(); + $classMethodBuilder->addStmts($stmts); + $classMethod = $classMethodBuilder->getNode(); + + $this->phpUnitTypeDeclarationDecorator->decorate($classMethod); + + return $classMethod; + } + + /** + * @param string[] $serviceTypes + * + * @return Property[] + */ + public function createPrivatePropertiesFromTypes(Class_ $class, array $serviceTypes): array + { + $properties = []; + + /** @var string $className */ + $className = $class->getAttribute(AttributeKey::CLASS_NAME); + + foreach ($serviceTypes as $serviceType) { + $propertyName = $this->serviceNaming->resolvePropertyNameFromServiceType($serviceType); + + // skip existing properties + if (property_exists($className, $propertyName)) { + continue; + } + + $serviceType = new ObjectType($serviceType); + $properties[] = $this->nodeFactory->createPrivatePropertyFromNameAndType($propertyName, $serviceType); + } + + return $properties; + } + + /** + * @param string[] $serviceTypes + * + * @return Expression[] + * + * E.g. "['SomeService']" → "$this->someService = self::$container->get(SomeService::class);" + */ + public function createSelfContainerGetWithTypeAssigns(Class_ $class, array $serviceTypes): array + { + $stmts = []; + + /** @var string $className */ + $className = $class->getAttribute(AttributeKey::CLASS_NAME); + + foreach ($serviceTypes as $serviceType) { + $propertyName = $this->serviceNaming->resolvePropertyNameFromServiceType($serviceType); + // skip existing properties + if (property_exists($className, $propertyName)) { + continue; + } + + $propertyFetch = new PropertyFetch(new Variable('this'), $propertyName); + $methodCall = $this->createSelfContainerGetWithTypeMethodCall($serviceType); + + $assign = new Assign($propertyFetch, $methodCall); + $stmts[] = new Expression($assign); + } + return $stmts; + } + + private function createSelfContainerGetWithTypeMethodCall(string $serviceType): MethodCall + { + $staticPropertyFetch = new StaticPropertyFetch(new Name('self'), 'container'); + $methodCall = new MethodCall($staticPropertyFetch, 'get'); + + if (Strings::contains($serviceType, '_') && ! Strings::contains($serviceType, '\\')) { + // keep string + $getArgumentValue = new String_($serviceType); + } else { + $getArgumentValue = new ClassConstFetch(new FullyQualified($serviceType), 'class'); + } + + $methodCall->args[] = new Arg($getArgumentValue); + + return $methodCall; + } +} diff --git a/packages/SymfonyPHPUnit/src/Rector/Class_/MultipleServiceGetToSetUpMethodRector.php b/packages/SymfonyPHPUnit/src/Rector/Class_/MultipleServiceGetToSetUpMethodRector.php deleted file mode 100644 index 82aa935af8a6..000000000000 --- a/packages/SymfonyPHPUnit/src/Rector/Class_/MultipleServiceGetToSetUpMethodRector.php +++ /dev/null @@ -1,414 +0,0 @@ -kernelTestCaseClass = $kernelTestCaseClass; - $this->propertyNaming = $propertyNaming; - $this->phpUnitTypeDeclarationDecorator = $phpUnitTypeDeclarationDecorator; - } - - public function getDefinition(): RectorDefinition - { - return new RectorDefinition('', [ - new CodeSample( - <<<'PHP' -use ItemRepository; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; - -class SomeTest extends KernelTestCase -{ - public function testOne() - { - $itemRepository = self::$container->get(ItemRepository::class); - $itemRepository->doStuff(); - } - - public function testTwo() - { - $itemRepository = self::$container->get(ItemRepository::class); - $itemRepository->doAnotherStuff(); - } -} -PHP - , - <<<'PHP' -use ItemRepository; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; - -class SomeTest extends KernelTestCase -{ - /** - * @var \ItemRepository - */ - private $itemRepository; - - protected function setUp() - { - parent::setUp(); - $this->itemRepository = self::$container->get(ItemRepository::class); - } - - public function testOne() - { - $this->itemRepository->doStuff(); - } - - public function testTwo() - { - $this->itemRepository->doAnotherStuff(); - } -} -PHP - ), - ]); - } - - /** - * @return string[] - */ - public function getNodeTypes(): array - { - return [Class_::class]; - } - - /** - * @param Class_ $node - */ - public function refactor(Node $node): ?Node - { - if ($node->extends === null) { - return null; - } - - if (! $this->isObjectType($node, $this->kernelTestCaseClass)) { - return null; - } - - // 1. find self::$container->get(x) that are called more than in 1 method - $serviceTypes = $this->collectContainerGetServiceTypes($node); - if (count($serviceTypes) === 0) { - return null; - } - - // 2. put them to setUp() method - $setUpClassMethod = $node->getMethod('setUp'); - if ($setUpClassMethod === null) { - $setUpClassMethod = $this->createSetUpClassMethodWithGetTypes($node, $serviceTypes); - if ($setUpClassMethod !== null) { - $node->stmts = array_merge([$setUpClassMethod], $node->stmts); - } - } else { - $assigns = $this->createSelfContainerGetWithTypeAssigns($node, $serviceTypes); - $setUpClassMethod->stmts = array_merge((array) $setUpClassMethod->stmts, $assigns); - } - - // 3. create private properties with this types - $privateProperties = $this->createPrivatePropertiesFromTypes($node, $serviceTypes); - $node->stmts = array_merge($privateProperties, $node->stmts); - - // 4. remove old in-method $property assigns - $formerVariablesByMethods = $this->removeAndCollectFormerAssignedVariables($node); - - // 5. replace former variables by $this->someProperty - $this->replaceFormerVariablesWithPropertyFetch($node, $formerVariablesByMethods); - - return $node; - } - - private function isSelfContainerGetMethodCall(Node $node): bool - { - if (! $node instanceof MethodCall) { - return false; - } - - if (! $node->var instanceof StaticPropertyFetch) { - return false; - } - - if (! $this->isName($node->var->class, 'self')) { - return false; - } - - if (! $this->isName($node->var->name, 'container')) { - return false; - } - - return $this->isName($node->name, 'get'); - } - - /** - * @param string[] $serviceTypes - */ - private function createSetUpClassMethodWithGetTypes(Class_ $class, array $serviceTypes): ?ClassMethod - { - $assigns = $this->createSelfContainerGetWithTypeAssigns($class, $serviceTypes); - if (count($assigns) === 0) { - return null; - } - - $stmts = array_merge([new StaticCall(new Name('parent'), 'setUp')], $assigns); - - $classMethodBuilder = $this->builderFactory->method('setUp'); - $classMethodBuilder->makeProtected(); - $classMethodBuilder->addStmts($stmts); - $classMethod = $classMethodBuilder->getNode(); - - $this->phpUnitTypeDeclarationDecorator->decorate($classMethod); - - return $classMethod; - } - - private function createSelfContainerGetWithTypeMethodCall(string $serviceType): MethodCall - { - $staticPropertyFetch = new StaticPropertyFetch(new Name('self'), 'container'); - - $methodCall = new MethodCall($staticPropertyFetch, 'get'); - if (Strings::contains($serviceType, '_') && ! Strings::contains($serviceType, '\\')) { - // keep string - $getArgumentValue = new String_($serviceType); - } else { - $getArgumentValue = $this->createClassConstantReference($serviceType); - } - - $methodCall->args[] = new Arg($getArgumentValue); - - return $methodCall; - } - - /** - * @param string[] $serviceTypes - * - * @return Expression[] - * - * E.g. "['SomeService']" → "$this->someService = self::$container->get(SomeService::class);" - */ - private function createSelfContainerGetWithTypeAssigns(Class_ $class, array $serviceTypes): array - { - $stmts = []; - - /** @var string $className */ - $className = $class->getAttribute(AttributeKey::CLASS_NAME); - - foreach ($serviceTypes as $serviceType) { - $propertyName = $this->resolvePropertyNameFromServiceType($serviceType); - - // skip existing properties - if (property_exists($className, $propertyName)) { - continue; - } - - $propertyFetch = new PropertyFetch(new Variable('this'), $propertyName); - - $methodCall = $this->createSelfContainerGetWithTypeMethodCall($serviceType); - - $assign = new Assign($propertyFetch, $methodCall); - $stmts[] = new Expression($assign); - } - - return $stmts; - } - - /** - * @param string[] $serviceTypes - * @return Property[] - */ - private function createPrivatePropertiesFromTypes(Class_ $class, array $serviceTypes): array - { - $properties = []; - - /** @var string $className */ - $className = $class->getAttribute(AttributeKey::CLASS_NAME); - - foreach ($serviceTypes as $serviceType) { - $propertyName = $this->resolvePropertyNameFromServiceType($serviceType); - - // skip existing properties - if (property_exists($className, $propertyName)) { - continue; - } - - $serviceType = new ObjectType($serviceType); - $properties[] = $this->nodeFactory->createPrivatePropertyFromNameAndType($propertyName, $serviceType); - } - - return $properties; - } - - /** - * @return string[] - */ - private function collectContainerGetServiceTypes(Class_ $class): array - { - $serviceTypes = []; - - $this->traverseNodesWithCallable($class->stmts, function (Node $node) use (&$serviceTypes) { - if (! $this->isSelfContainerGetMethodCall($node)) { - return null; - } - - // skip setUp() method - $methodName = $node->getAttribute(AttributeKey::METHOD_NAME); - if ($methodName === 'setUp' || $methodName === null) { - return null; - } - - /** @var MethodCall $node */ - $serviceType = $this->getValue($node->args[0]->value); - if ($this->shouldSkipServiceType($serviceType)) { - return null; - } - - $serviceTypes[] = $serviceType; - }); - - return array_unique($serviceTypes); - } - - /** - * @return string[][] - */ - private function removeAndCollectFormerAssignedVariables(Class_ $class): array - { - $formerVariablesByMethods = []; - - $this->traverseNodesWithCallable($class->stmts, function (Node $node) use ( - &$formerVariablesByMethods - ): ?PropertyFetch { - if (! $node instanceof MethodCall) { - return null; - } - - // skip setUp() method - $methodName = $node->getAttribute(AttributeKey::METHOD_NAME); - if ($methodName === 'setUp' || $methodName === null) { - return null; - } - - if (! $this->isSelfContainerGetMethodCall($node)) { - return null; - } - - $type = $this->getValue($node->args[0]->value); - if ($type === null) { - return null; - } - - $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE); - if ($parentNode instanceof Assign) { - $variableName = $this->getName($parentNode->var); - if ($variableName === null) { - return null; - } - - $formerVariablesByMethods[$methodName][$variableName] = $type; - - $this->removeNode($parentNode); - return null; - } - - $propertyName = $this->resolvePropertyNameFromServiceType($type); - - return new PropertyFetch(new Variable('this'), $propertyName); - }); - - return $formerVariablesByMethods; - } - - /** - * @param string[][] $formerVariablesByMethods - */ - private function replaceFormerVariablesWithPropertyFetch(Class_ $class, array $formerVariablesByMethods): void - { - $this->traverseNodesWithCallable($class->stmts, function (Node $node) use ( - $formerVariablesByMethods - ): ?PropertyFetch { - if (! $node instanceof Variable) { - return null; - } - - /** @var string $methodName */ - $methodName = $node->getAttribute(AttributeKey::METHOD_NAME); - $variableName = $this->getName($node); - if ($variableName === null) { - return null; - } - - if (! isset($formerVariablesByMethods[$methodName][$variableName])) { - return null; - } - - $serviceType = $formerVariablesByMethods[$methodName][$variableName]; - - $propertyName = $this->resolvePropertyNameFromServiceType($serviceType); - - return new PropertyFetch(new Variable('this'), $propertyName); - }); - } - - private function shouldSkipServiceType(string $serviceType): bool - { - return $serviceType === 'Symfony\Component\HttpFoundation\Session\SessionInterface'; - } - - private function resolvePropertyNameFromServiceType(string $serviceType): string - { - if (Strings::contains($serviceType, '_') && ! Strings::contains($serviceType, '\\')) { - return $this->propertyNaming->underscoreToName($serviceType); - } - - return $this->propertyNaming->fqnToVariableName(new ObjectType($serviceType)); - } -} diff --git a/packages/SymfonyPHPUnit/src/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector.php b/packages/SymfonyPHPUnit/src/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector.php new file mode 100644 index 000000000000..b94bb9574c61 --- /dev/null +++ b/packages/SymfonyPHPUnit/src/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector.php @@ -0,0 +1,255 @@ +kernelTestCaseClass = $kernelTestCaseClass; + $this->kernelTestCaseNodeAnalyzer = $kernelTestCaseNodeAnalyzer; + $this->kernelTestCaseNodeFactory = $kernelTestCaseNodeFactory; + $this->selfContainerMethodCallCollector = $selfContainerMethodCallCollector; + $this->serviceNaming = $serviceNaming; + } + + public function getDefinition(): RectorDefinition + { + return new RectorDefinition('Move self::$container service fetching from test methods up to setUp method', [ + new CodeSample( + <<<'PHP' +use ItemRepository; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; + +class SomeTest extends KernelTestCase +{ + public function testOne() + { + $itemRepository = self::$container->get(ItemRepository::class); + $itemRepository->doStuff(); + } + + public function testTwo() + { + $itemRepository = self::$container->get(ItemRepository::class); + $itemRepository->doAnotherStuff(); + } +} +PHP + , + <<<'PHP' +use ItemRepository; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; + +class SomeTest extends KernelTestCase +{ + /** + * @var \ItemRepository + */ + private $itemRepository; + + protected function setUp() + { + parent::setUp(); + $this->itemRepository = self::$container->get(ItemRepository::class); + } + + public function testOne() + { + $this->itemRepository->doStuff(); + } + + public function testTwo() + { + $this->itemRepository->doAnotherStuff(); + } +} +PHP + ), + ]); + } + + /** + * @return string[] + */ + public function getNodeTypes(): array + { + return [Class_::class]; + } + + /** + * @param Class_ $node + */ + public function refactor(Node $node): ?Node + { + if ($node->extends === null) { + return null; + } + + if (! $this->isObjectType($node, $this->kernelTestCaseClass)) { + return null; + } + + // 1. find self::$container->get(x) that are called more than in 1 method + $serviceTypes = $this->selfContainerMethodCallCollector->collectContainerGetServiceTypes($node); + if (count($serviceTypes) === 0) { + return null; + } + + // 2. put them to setUp() method + $setUpClassMethod = $node->getMethod('setUp'); + if ($setUpClassMethod === null) { + $setUpClassMethod = $this->kernelTestCaseNodeFactory->createSetUpClassMethodWithGetTypes( + $node, + $serviceTypes + ); + if ($setUpClassMethod !== null) { + $node->stmts = array_merge([$setUpClassMethod], $node->stmts); + } + } else { + $assigns = $this->kernelTestCaseNodeFactory->createSelfContainerGetWithTypeAssigns($node, $serviceTypes); + $setUpClassMethod->stmts = array_merge((array) $setUpClassMethod->stmts, $assigns); + } + + // 3. create private properties with this types + $privateProperties = $this->kernelTestCaseNodeFactory->createPrivatePropertiesFromTypes($node, $serviceTypes); + $node->stmts = array_merge($privateProperties, $node->stmts); + + // 4. remove old in-method $property assigns + $formerVariablesByMethods = $this->removeAndCollectFormerAssignedVariables($node); + + // 5. replace former variables by $this->someProperty + $this->replaceFormerVariablesWithPropertyFetch($node, $formerVariablesByMethods); + + return $node; + } + + /** + * @return string[][] + */ + private function removeAndCollectFormerAssignedVariables(Class_ $class): array + { + $formerVariablesByMethods = []; + + $this->traverseNodesWithCallable($class->stmts, function (Node $node) use ( + &$formerVariablesByMethods + ): ?PropertyFetch { + if (! $node instanceof MethodCall) { + return null; + } + + // skip setUp() method + $methodName = $node->getAttribute(AttributeKey::METHOD_NAME); + if ($methodName === 'setUp' || $methodName === null) { + return null; + } + + if (! $this->kernelTestCaseNodeAnalyzer->isSelfContainerGetMethodCall($node)) { + return null; + } + + $type = $this->getValue($node->args[0]->value); + if ($type === null) { + return null; + } + + $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE); + if ($parentNode instanceof Assign) { + $variableName = $this->getName($parentNode->var); + if ($variableName === null) { + return null; + } + + $formerVariablesByMethods[$methodName][$variableName] = $type; + + $this->removeNode($parentNode); + return null; + } + + $propertyName = $this->serviceNaming->resolvePropertyNameFromServiceType($type); + + return new PropertyFetch(new Variable('this'), $propertyName); + }); + + return $formerVariablesByMethods; + } + + /** + * @param string[][] $formerVariablesByMethods + */ + private function replaceFormerVariablesWithPropertyFetch(Class_ $class, array $formerVariablesByMethods): void + { + $this->traverseNodesWithCallable($class->stmts, function (Node $node) use ( + $formerVariablesByMethods + ): ?PropertyFetch { + if (! $node instanceof Variable) { + return null; + } + + /** @var string $methodName */ + $methodName = $node->getAttribute(AttributeKey::METHOD_NAME); + $variableName = $this->getName($node); + if ($variableName === null) { + return null; + } + + if (! isset($formerVariablesByMethods[$methodName][$variableName])) { + return null; + } + + $serviceType = $formerVariablesByMethods[$methodName][$variableName]; + $propertyName = $this->serviceNaming->resolvePropertyNameFromServiceType($serviceType); + + return new PropertyFetch(new Variable('this'), $propertyName); + }); + } +} diff --git a/packages/SymfonyPHPUnit/src/SelfContainerMethodCallCollector.php b/packages/SymfonyPHPUnit/src/SelfContainerMethodCallCollector.php new file mode 100644 index 000000000000..ee51757732ac --- /dev/null +++ b/packages/SymfonyPHPUnit/src/SelfContainerMethodCallCollector.php @@ -0,0 +1,77 @@ +kernelTestCaseNodeAnalyzer = $kernelTestCaseNodeAnalyzer; + $this->callableNodeTraverser = $callableNodeTraverser; + $this->valueResolver = $valueResolver; + } + + /** + * @return string[] + */ + public function collectContainerGetServiceTypes(Class_ $class): array + { + $serviceTypes = []; + + $this->callableNodeTraverser->traverseNodesWithCallable($class->stmts, function (Node $node) use ( + &$serviceTypes + ) { + if (! $this->kernelTestCaseNodeAnalyzer->isSelfContainerGetMethodCall($node)) { + return null; + } + + // skip setUp() method + $methodName = $node->getAttribute(AttributeKey::METHOD_NAME); + if ($methodName === 'setUp' || $methodName === null) { + return null; + } + + /** @var MethodCall $node */ + $serviceType = $this->valueResolver->getValue($node->args[0]->value); + if ($this->shouldSkipServiceType($serviceType)) { + return null; + } + + $serviceTypes[] = $serviceType; + }); + + return array_unique($serviceTypes); + } + + private function shouldSkipServiceType(string $serviceType): bool + { + return $serviceType === SessionInterface::class; + } +} diff --git a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/existing_setup.php.inc b/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/existing_setup.php.inc deleted file mode 100644 index fced8f4c149f..000000000000 --- a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/existing_setup.php.inc +++ /dev/null @@ -1,60 +0,0 @@ -get(ItemRepository::class); - $itemRepository->doStuff(); - } - - public function testTwo() - { - $itemRepository = self::$container->get(ItemRepository::class); - $itemRepository->doAnotherStuff(); - } -} - -?> ------ -itemRepository = self::$container->get(\Rector\SymfonyPHPUnit\Tests\Rector\Class_\MultipleServiceGetToSetUpMethodRector\Source\ItemRepository::class); - } - - public function testOne() - { - $this->itemRepository->doStuff(); - } - - public function testTwo() - { - $this->itemRepository->doAnotherStuff(); - } -} - -?> diff --git a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/extends_parent_class_with_property.php.inc b/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/extends_parent_class_with_property.php.inc deleted file mode 100644 index 189e7a45e5c2..000000000000 --- a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/extends_parent_class_with_property.php.inc +++ /dev/null @@ -1,47 +0,0 @@ -get(ItemRepository::class); - $itemRepository->doStuff(); - } - - public function testTwo() - { - $itemRepository = self::$container->get(ItemRepository::class); - $itemRepository->doAnotherStuff(); - } -} - -?> ------ -itemRepository->doStuff(); - } - - public function testTwo() - { - $this->itemRepository->doAnotherStuff(); - } -} - -?> diff --git a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/fixture.php.inc b/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/fixture.php.inc deleted file mode 100644 index 6e3aad4abec0..000000000000 --- a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/fixture.php.inc +++ /dev/null @@ -1,54 +0,0 @@ -get(ItemRepository::class); - $itemRepository->doStuff(); - } - - public function testTwo() - { - $itemRepository = self::$container->get(ItemRepository::class); - $itemRepository->doAnotherStuff(); - } -} - -?> ------ -itemRepository = self::$container->get(\Rector\SymfonyPHPUnit\Tests\Rector\Class_\MultipleServiceGetToSetUpMethodRector\Source\ItemRepository::class); - } - public function testOne() - { - $this->itemRepository->doStuff(); - } - - public function testTwo() - { - $this->itemRepository->doAnotherStuff(); - } -} - -?> diff --git a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/instant_call.php.inc b/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/instant_call.php.inc deleted file mode 100644 index 4102a33b2fc0..000000000000 --- a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/instant_call.php.inc +++ /dev/null @@ -1,52 +0,0 @@ -get(ItemRepository::class)->get(1); - } - - public function testTwo() - { - $item = self::$container->get(ItemRepository::class)->get(2); - } -} - -?> ------ -itemRepository = self::$container->get(\Rector\SymfonyPHPUnit\Tests\Rector\Class_\MultipleServiceGetToSetUpMethodRector\Source\ItemRepository::class); - } - public function testOne() - { - $item = $this->itemRepository->get(1); - } - - public function testTwo() - { - $item = $this->itemRepository->get(2); - } -} - -?> diff --git a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/string_service_name.php.inc b/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/string_service_name.php.inc deleted file mode 100644 index 29aef1537566..000000000000 --- a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/string_service_name.php.inc +++ /dev/null @@ -1,52 +0,0 @@ -get('some_value'); - $someValue->doStuff(); - } - - public function testTwo() - { - $someValue = self::$container->get('some_value'); - $someValue->doAnotherStuff(); - } -} - -?> ------ -someValue = self::$container->get('some_value'); - } - public function testOne() - { - $this->someValue->doStuff(); - } - - public function testTwo() - { - $this->someValue->doAnotherStuff(); - } -} - -?> diff --git a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Source/DummyKernelTestCase.php b/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Source/DummyKernelTestCase.php deleted file mode 100644 index 39cb4900525d..000000000000 --- a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Source/DummyKernelTestCase.php +++ /dev/null @@ -1,8 +0,0 @@ -get(ItemRepository::class); + $itemRepository->doStuff(); + } + + public function testTwo() + { + $itemRepository = self::$container->get(ItemRepository::class); + $itemRepository->doAnotherStuff(); + } +} + +?> +----- +itemRepository = self::$container->get(\Rector\SymfonyPHPUnit\Tests\Rector\Class_\SelfContainerGetMethodCallFromTestToSetUpMethodRector\Source\ItemRepository::class); + } + + public function testOne() + { + $this->itemRepository->doStuff(); + } + + public function testTwo() + { + $this->itemRepository->doAnotherStuff(); + } +} + +?> diff --git a/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/extends_parent_class_with_property.php.inc b/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/extends_parent_class_with_property.php.inc new file mode 100644 index 000000000000..e31a9525a28d --- /dev/null +++ b/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/extends_parent_class_with_property.php.inc @@ -0,0 +1,47 @@ +get(ItemRepository::class); + $itemRepository->doStuff(); + } + + public function testTwo() + { + $itemRepository = self::$container->get(ItemRepository::class); + $itemRepository->doAnotherStuff(); + } +} + +?> +----- +itemRepository->doStuff(); + } + + public function testTwo() + { + $this->itemRepository->doAnotherStuff(); + } +} + +?> diff --git a/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/fixture.php.inc b/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/fixture.php.inc new file mode 100644 index 000000000000..8b719cb0bae7 --- /dev/null +++ b/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/fixture.php.inc @@ -0,0 +1,54 @@ +get(ItemRepository::class); + $itemRepository->doStuff(); + } + + public function testTwo() + { + $itemRepository = self::$container->get(ItemRepository::class); + $itemRepository->doAnotherStuff(); + } +} + +?> +----- +itemRepository = self::$container->get(\Rector\SymfonyPHPUnit\Tests\Rector\Class_\SelfContainerGetMethodCallFromTestToSetUpMethodRector\Source\ItemRepository::class); + } + public function testOne() + { + $this->itemRepository->doStuff(); + } + + public function testTwo() + { + $this->itemRepository->doAnotherStuff(); + } +} + +?> diff --git a/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/instant_call.php.inc b/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/instant_call.php.inc new file mode 100644 index 000000000000..442913f2ea7d --- /dev/null +++ b/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/instant_call.php.inc @@ -0,0 +1,52 @@ +get(ItemRepository::class)->get(1); + } + + public function testTwo() + { + $item = self::$container->get(ItemRepository::class)->get(2); + } +} + +?> +----- +itemRepository = self::$container->get(\Rector\SymfonyPHPUnit\Tests\Rector\Class_\SelfContainerGetMethodCallFromTestToSetUpMethodRector\Source\ItemRepository::class); + } + public function testOne() + { + $item = $this->itemRepository->get(1); + } + + public function testTwo() + { + $item = $this->itemRepository->get(2); + } +} + +?> diff --git a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/skip_sessions.php.inc b/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/skip_sessions.php.inc similarity index 63% rename from packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/skip_sessions.php.inc rename to packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/skip_sessions.php.inc index 87b89541f9ec..81b86b992529 100644 --- a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/Fixture/skip_sessions.php.inc +++ b/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Fixture/skip_sessions.php.inc @@ -1,8 +1,8 @@ get('some_value'); + $someValue->doStuff(); + } +} + +?> +----- +someValue = self::$container->get('some_value'); + } + public function testOne() + { + $this->someValue->doStuff(); + } +} + +?> diff --git a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/MultipleServiceGetToSetUpMethodRectorTest.php b/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/SelfContainerGetMethodCallFromTestToSetUpMethodRectorTest.php similarity index 65% rename from packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/MultipleServiceGetToSetUpMethodRectorTest.php rename to packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/SelfContainerGetMethodCallFromTestToSetUpMethodRectorTest.php index ca54d9f2b04a..266058ba82ea 100644 --- a/packages/SymfonyPHPUnit/tests/Rector/Class_/MultipleServiceGetToSetUpMethodRector/MultipleServiceGetToSetUpMethodRectorTest.php +++ b/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/SelfContainerGetMethodCallFromTestToSetUpMethodRectorTest.php @@ -1,12 +1,12 @@ [ + SelfContainerGetMethodCallFromTestToSetUpMethodRector::class => [ '$kernelTestCaseClass' => DummyKernelTestCase::class, ], ]; diff --git a/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Source/DummyKernelTestCase.php b/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Source/DummyKernelTestCase.php new file mode 100644 index 000000000000..dc673ff008f7 --- /dev/null +++ b/packages/SymfonyPHPUnit/tests/Rector/Class_/SelfContainerGetMethodCallFromTestToSetUpMethodRector/Source/DummyKernelTestCase.php @@ -0,0 +1,8 @@ +args as $arg) { - $value = $this->valueResolver->resolve($arg->value); + $value = $this->valueResolver->getValue($arg->value); if ($value) { $arguments[] = $value; diff --git a/src/PhpParser/Node/Value/ValueResolver.php b/src/PhpParser/Node/Value/ValueResolver.php index c7d05fcb289e..87392cba0ef0 100644 --- a/src/PhpParser/Node/Value/ValueResolver.php +++ b/src/PhpParser/Node/Value/ValueResolver.php @@ -56,7 +56,7 @@ public function __construct( /** * @return mixed|null */ - public function resolve(Expr $expr) + public function getValue(Expr $expr) { try { $value = $this->getConstExprEvaluator()->evaluateDirectly($expr); diff --git a/src/Rector/AbstractRector/ValueResolverTrait.php b/src/Rector/AbstractRector/ValueResolverTrait.php index 7dc4fbf1dc6a..356aa60e51a4 100644 --- a/src/Rector/AbstractRector/ValueResolverTrait.php +++ b/src/Rector/AbstractRector/ValueResolverTrait.php @@ -29,7 +29,7 @@ public function setValueResolver(ValueResolver $valueResolver): void */ protected function getValue(Expr $expr) { - return $this->valueResolver->resolve($expr); + return $this->valueResolver->getValue($expr); } /** diff --git a/tests/PhpParser/Node/Value/ValueResolverTest.php b/tests/PhpParser/Node/Value/ValueResolverTest.php index a16718451e71..b85c491e0f3e 100644 --- a/tests/PhpParser/Node/Value/ValueResolverTest.php +++ b/tests/PhpParser/Node/Value/ValueResolverTest.php @@ -31,7 +31,7 @@ protected function setUp(): void */ public function test($expected, Expr $expr): void { - $this->assertSame($expected, $this->valueResolver->resolve($expr)); + $this->assertSame($expected, $this->valueResolver->getValue($expr)); } public function dataProvider(): Iterator