From 222557c12220d4df75462cce0116b0356c046e84 Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Wed, 19 Jul 2017 18:52:54 +0200 Subject: [PATCH] [NamedServicesToContructor] split NodeVisitors to do particular job --- easy-coding-standard.neon | 2 + src/Console/Command/ReconstructCommand.php | 5 +- ...jectAnnotationToConstructorNodeVisitor.php | 6 +- .../NamedServicesToConstructorNodeVisitor.php | 14 +- .../GetterToPropertyNodeVisitor.php | 174 ++++++++++++++++++ .../Test.php | 3 +- 6 files changed, 189 insertions(+), 15 deletions(-) create mode 100644 src/NodeVisitor/DependencyInjection/NamedServicesToConstrutor/GetterToPropertyNodeVisitor.php diff --git a/easy-coding-standard.neon b/easy-coding-standard.neon index afe9c18b53f..b90bf2b0190 100644 --- a/easy-coding-standard.neon +++ b/easy-coding-standard.neon @@ -1 +1,3 @@ includes: +# todo: +# - vendor/symplify/easy-coding-standard/config/ \ No newline at end of file diff --git a/src/Console/Command/ReconstructCommand.php b/src/Console/Command/ReconstructCommand.php index ad2a90f6764..6284b10bde9 100644 --- a/src/Console/Command/ReconstructCommand.php +++ b/src/Console/Command/ReconstructCommand.php @@ -38,8 +38,6 @@ final class ReconstructCommand extends Command { $this->setName(self::NAME); $this->setDescription('Reconstruct set of your code.'); - - // @todo: use modular configure from ApiGen $this->addArgument( self::ARGUMENT_SOURCE_NAME, InputArgument::REQUIRED | InputArgument::IS_ARRAY, @@ -57,9 +55,10 @@ final class ReconstructCommand extends Command } /** + * @param string[] $directories * @return SplFileInfo[] array */ - private function findPhpFilesInDirectories(string ...$directories): array + private function findPhpFilesInDirectories(array $directories): array { $finder = Finder::find('*.php') ->in($directories); diff --git a/src/NodeVisitor/DependencyInjection/InjectAnnotationToConstructorNodeVisitor.php b/src/NodeVisitor/DependencyInjection/InjectAnnotationToConstructorNodeVisitor.php index b890577a602..845ee5d2f85 100644 --- a/src/NodeVisitor/DependencyInjection/InjectAnnotationToConstructorNodeVisitor.php +++ b/src/NodeVisitor/DependencyInjection/InjectAnnotationToConstructorNodeVisitor.php @@ -7,12 +7,12 @@ use PhpParser\Comment\Doc; use PhpParser\Node; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\Property; -use PhpParser\NodeTraverser; use PhpParser\NodeVisitorAbstract; use Rector\Builder\ConstructorMethodBuilder; final class InjectAnnotationToConstructorNodeVisitor extends NodeVisitorAbstract { + const ANNOTATION_INJECT = 'inject'; /** * @var ConstructorMethodBuilder */ @@ -40,7 +40,7 @@ final class InjectAnnotationToConstructorNodeVisitor extends NodeVisitorAbstract $propertyNode = $classElementStatement; $propertyDocBlock = $this->createDocBlockFromProperty($propertyNode); - $injectAnnotations = $propertyDocBlock->getAnnotationsOfType('inject'); + $injectAnnotations = $propertyDocBlock->getAnnotationsOfType(self::ANNOTATION_INJECT); if (! $injectAnnotations) { continue; } @@ -68,7 +68,7 @@ final class InjectAnnotationToConstructorNodeVisitor extends NodeVisitorAbstract private function removeInjectAnnotationFromProperty(Property $propertyNode, DocBlock $propertyDocBlock): void { - $injectAnnotations = $propertyDocBlock->getAnnotationsOfType('inject'); + $injectAnnotations = $propertyDocBlock->getAnnotationsOfType(self::ANNOTATION_INJECT); foreach ($injectAnnotations as $injectAnnotation) { $injectAnnotation->remove(); diff --git a/src/NodeVisitor/DependencyInjection/NamedServicesToConstructorNodeVisitor.php b/src/NodeVisitor/DependencyInjection/NamedServicesToConstructorNodeVisitor.php index 549f90328c7..e3bff4c616c 100644 --- a/src/NodeVisitor/DependencyInjection/NamedServicesToConstructorNodeVisitor.php +++ b/src/NodeVisitor/DependencyInjection/NamedServicesToConstructorNodeVisitor.php @@ -11,7 +11,6 @@ use PhpParser\Node\Expr\Variable; use PhpParser\Node\Scalar\String_; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; -use PhpParser\NodeTraverser; use PhpParser\NodeVisitorAbstract; use Rector\Builder\ConstructorMethodBuilder; use Rector\Builder\Naming\NameResolver; @@ -96,10 +95,12 @@ final class NamedServicesToConstructorNodeVisitor extends NodeVisitorAbstract return; } - $refactoredMethodCall = $this->processMethodCallNode($classNode, $assignNode->expr); - if ($refactoredMethodCall) { - $assignNode->expr = $refactoredMethodCall; - } + $this->processMethodCallNode($classNode, $assignNode->expr); + + /*$refactoredMethodCall = */ +// if ($refactoredMethodCall) { +// $assignNode->expr = $refactoredMethodCall; +// } } /** @@ -212,9 +213,8 @@ final class NamedServicesToConstructorNodeVisitor extends NodeVisitorAbstract { if ($this->isCandidate($node)) { $this->reconstruct($node); - return; } - return NodeTraverser::DONT_TRAVERSE_CHILDREN; + return null; } } diff --git a/src/NodeVisitor/DependencyInjection/NamedServicesToConstrutor/GetterToPropertyNodeVisitor.php b/src/NodeVisitor/DependencyInjection/NamedServicesToConstrutor/GetterToPropertyNodeVisitor.php new file mode 100644 index 00000000000..36dfd38249a --- /dev/null +++ b/src/NodeVisitor/DependencyInjection/NamedServicesToConstrutor/GetterToPropertyNodeVisitor.php @@ -0,0 +1,174 @@ +get('some_service') + * + * into: + * $this->someService + */ +final class GetterToPropertyNodeVisitor extends NodeVisitorAbstract +{ + /** + * Return value semantics: + * * null + * => $node stays as-is + * * NodeTraverser::DONT_TRAVERSE_CHILDREN + * => Children of $node are not traversed. $node stays as-is + * * NodeTraverser::STOP_TRAVERSAL + * => Traversal is aborted. $node stays as-is + * * otherwise + * => $node is set to the return value + * + * @return null|int|Node + */ + public function enterNode(Node $node) + { + if ($this->isCandidate($node)) { + $this->reconstruct($node); + } + + return null; + } + + /** + * @var NameResolver + */ + private $nameResolver; + + public function __construct(NameResolver $nameResolver) + { + $this->nameResolver = $nameResolver; + } + + private function isCandidate(Node $node): bool + { + // $var = $this->get('some_service'); + // $var = $this->get('some_service')->getData(); + if ($node instanceof Assign) { + if ($node->expr instanceof MethodCall || $node->var instanceof MethodCall) { + if ($this->isContainerGetCall($node->expr)) { + return true; + } + } + } + + // ['var => $this->get('some_service')->getData()] + if ($node instanceof MethodCall && $node->var instanceof MethodCall) { + if ($this->isContainerGetCall($node->var)) { + return true; + } + } + + return false; + } + + /** + * @param Assign|MethodCall $assignOrMethodCallNode + */ + public function reconstruct(Node $assignOrMethodCallNode): void + { + if ($assignOrMethodCallNode instanceof Assign) { + $this->processAssignment($assignOrMethodCallNode); + } + } + + private function processAssignment(Assign $assignNode): void + { + $refactoredMethodCall = $this->processMethodCallNode($assignNode->expr); + if ($refactoredMethodCall) { + $assignNode->expr = $refactoredMethodCall; + } + } + + /** + * @todo extract to helper service, LocalKernelProvider::get...() + */ + private function getContainerFromKernelClass(): ContainerInterface + { + /** @var Kernel $kernel */ + $kernel = new LocalKernel('dev', true); + $kernel->boot(); + + // @todo: initialize without creating cache or log directory + // @todo: call only loadBundles() and initializeContainer() methods + + return $kernel->getContainer(); + } + + /** + * Is "$this->get('string')" statements? + */ + private function isContainerGetCall(MethodCall $methodCall): bool + { + if ($methodCall->var->name !== 'this') { + return false; + } + + if ((string) $methodCall->name !== 'get') { + return false; + } + + if (! $methodCall->args[0]->value instanceof String_) { + return false; + } + + return true; + } + + /** + * @param MethodCall|Expr $methodCallNode + */ + private function resolveServiceTypeFromMethodCall($methodCallNode): ?string + { + /** @var String_ $argument */ + $argument = $methodCallNode->args[0]->value; + $serviceName = $argument->value; + + $container = $this->getContainerFromKernelClass(); + if (! $container->has($serviceName)) { + // service name could not be found + return null; + } + + $service = $container->get($serviceName); + + return get_class($service); + } + + private function processMethodCallNode(MethodCall $methodCall): ?PropertyFetch + { + // Get service type + $serviceType = $this->resolveServiceTypeFromMethodCall($methodCall); + if ($serviceType === null) { + return null; + } + + // Get property name + $propertyName = $this->nameResolver->resolvePropertyNameFromType($serviceType); + + // creates "$this->propertyName" + return new PropertyFetch( + new Variable('this', [ + 'name' => $propertyName + ]), $propertyName + ); + + } +} diff --git a/tests/NodeVisitor/DependencyInjection/NamedServicesToConstructorReconstructor/Test.php b/tests/NodeVisitor/DependencyInjection/NamedServicesToConstructorReconstructor/Test.php index 59ba7e24a63..752f1321d7c 100644 --- a/tests/NodeVisitor/DependencyInjection/NamedServicesToConstructorReconstructor/Test.php +++ b/tests/NodeVisitor/DependencyInjection/NamedServicesToConstructorReconstructor/Test.php @@ -10,8 +10,7 @@ final class Test extends AbstractReconstructorTestCase public function test(): void { $this->doTestFileMatchesExpectedContent(__DIR__ . '/wrong/wrong.php.inc', __DIR__ . '/correct/correct.php.inc'); -// $this->doTestFileMatchesExpectedContent(__DIR__ . '/wrong/wrong2.php.inc', __DIR__ . '/correct/correct2.php.inc' -// ); + $this->doTestFileMatchesExpectedContent(__DIR__ . '/wrong/wrong2.php.inc', __DIR__ . '/correct/correct2.php.inc'); // $this->doTestFileMatchesExpectedContent(__DIR__ . '/wrong/wrong3.php.inc', __DIR__ . '/correct/correct3.php.inc'); }