Merge pull request #22 from TomasVotruba/command-as-service

[Symfony] Command as service with constructor injection

easy-coding-standard.neon

@@ -43,12 +43,10 @@ parameters:
     skip:
         Symplify\CodingStandard\Fixer\Php\ClassStringToClassConstantFixer:
             # classes might not exist
-            - */src/Rector/Contrib/Nette/*Rector.php
-            - src/Rector/Contrib/Symfony/StringFormTypeToClassRector.php
-            - src/Rector/Contrib/Symfony/FormIsValidRector.php
+            - */src/Rector/Contrib/*/*Rector.php
         Symplify\CodingStandard\Sniffs\Debug\CommentedOutCodeSniff:
             # examples of code to be found
-            - src/Rector/Contrib/Symfony/GetterToPropertyRector.php
+            - src/Rector/Contrib/SymfonyExtra/CommandToConstructorInjectionRector.php
         SlevomatCodingStandard\Sniffs\Classes\UnusedPrivateElementsSniff:
             # will be used soon
             - packages/NodeTypeResolver/src/TypeContext.php

src/Builder/Kernel/ServiceFromKernelResolver.php

@@ -2,23 +2,29 @@
 
 namespace Rector\Builder\Kernel;
 
+use PhpParser\Node\Arg;
 use Psr\Container\ContainerInterface;
 use Symfony\Component\HttpKernel\Kernel;
 
 final class ServiceFromKernelResolver
 {
-    public function resolveServiceClassByNameFromKernel(string $serviceName, string $kernelClass): ?string
+    public function resolveServiceClassFromArgument(Arg $argNode, string $kernelClass): ?string
     {
-        /** @var Kernel $kernel */
-        $kernel = new $kernelClass('dev', true);
-        $kernel->boot();
+        $serviceName = $argNode->value->value;
+        $serviceType = $this->resolveServiceClassByNameFromKernel($serviceName, $kernelClass);
 
-        // @todo: cache
-        // @todo: initialize without creating cache or log directory
-        // @todo: call only loadBundles() and initializeContainer() methods
+        if ($serviceType === null) {
+            return null;
+        }
+
+        return $serviceType;
+    }
+
+
+    private function resolveServiceClassByNameFromKernel(string $serviceName, string $kernelClass): ?string
+    {
+        $container = $this->createContainerFromKernelClass($kernelClass);
 
-        /** @var ContainerInterface $container */
-        $container = $kernel->getContainer();
         if (! $container->has($serviceName)) {
             // service name could not be found
             return null;
@@ -28,4 +34,24 @@ final class ServiceFromKernelResolver
 
         return get_class($service);
     }
+
+    private function createContainerFromKernelClass(string $kernelClass): ContainerInterface
+    {
+        $kernel = $this->createKernelFromKernelClass($kernelClass);
+
+        return $kernel->getContainer();
+    }
+
+    private function createKernelFromKernelClass(string $kernelClass): Kernel
+    {
+        /** @var Kernel $kernel */
+        $kernel = new $kernelClass('dev', true);
+        $kernel->boot();
+
+        return $kernel;
+
+        // @todo: cache
+        // @todo: initialize without creating cache or log directory
+        // @todo: call only loadBundles() and initializeContainer() methods
+    }
 }

src/Deprecation/SetNames.php

@@ -13,4 +13,9 @@ final class SetNames
      * @var string
      */
     public const SYMFONY = 'Symfony';
+
+    /**
+     * @var string
+     */
+    public const SYMFONY_EXTRA = 'SymfonyExtra';
 }

src/NodeFactory/NodeFactory.php

@@ -0,0 +1,22 @@
+<?php declare(strict_types=1);
+
+namespace Rector\NodeFactory;
+
+use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\Variable;
+
+final class NodeFactory
+{
+    /**
+     * Creates "$this->propertyName"
+     */
+    public function createLocalPropertyFetch(string $propertyName): PropertyFetch
+    {
+        return new PropertyFetch(
+            new Variable('this', [
+                'name' => $propertyName,
+            ]),
+            $propertyName
+        );
+    }
+}

src/Rector/AbstractRector.php

@@ -3,6 +3,7 @@
 namespace Rector\Rector;
 
 use PhpParser\Node;
+use PhpParser\Node\Stmt\Class_;
 use PhpParser\NodeTraverser;
 use PhpParser\NodeVisitorAbstract;
 use Rector\Contract\Deprecation\DeprecationInterface;
@@ -10,6 +11,34 @@ use Rector\Contract\Rector\RectorInterface;
 
 abstract class AbstractRector extends NodeVisitorAbstract implements DeprecationInterface, RectorInterface
 {
+    /**
+     * @var Class_|null
+     */
+    protected $classNode;
+
+    /**
+     * @param Node[] $nodes
+     * @return null|Node[]
+     */
+    public function beforeTraverse(array $nodes): ?array
+    {
+        $this->classNode = null;
+
+        foreach ($nodes as $node) {
+            if ($node instanceof Class_) {
+                $this->classNode = $node;
+                break;
+            }
+        }
+
+        return null;
+    }
+
+    protected function getClassName(): string
+    {
+        return $this->classNode->namespacedName->toString();
+    }
+
     /**
      * @return null|int|Node
      */

src/Rector/Contrib/Nette/RemoveConfiguratorConstantsRector.php

@@ -13,7 +13,7 @@ final class RemoveConfiguratorConstantsRector extends AbstractRector
     public function isCandidate(Node $node): bool
     {
         if ($node instanceof ClassConstFetch) {
-            $className = $this->getClassName($node);
+            $className = $this->getClassNameFromClassConstFetch($node);
 
             if ($className !== $this->getDesiredClass()) {
                 return false;
@@ -50,7 +50,7 @@ final class RemoveConfiguratorConstantsRector extends AbstractRector
         return 2.3;
     }
 
-    private function getClassName(ClassConstFetch $classConstFetchNode): string
+    private function getClassNameFromClassConstFetch(ClassConstFetch $classConstFetchNode): string
     {
         /** @var Node\Name\FullyQualified $fqnName */
         $fqnName = $classConstFetchNode->class->getAttribute('resolvedName');

src/Rector/Contrib/Symfony/VarDumperTestTraitMethodArgsRector.php

@@ -49,9 +49,13 @@ final class VarDumperTestTraitMethodArgsRector extends AbstractRector
 
     public function isCandidate(Node $node): bool
     {
-        if (! $this->methodCallAnalyzer->isStaticMethodCallTypeAndMethods($node,self::TRAIT_NAME, ['assertDumpEquals', 'assertDumpMatchesFormat'])) {
+        if (! $this->methodCallAnalyzer->isStaticMethodCallTypeAndMethods(
+            $node,
+            self::TRAIT_NAME,
+            ['assertDumpEquals', 'assertDumpMatchesFormat']
+        )) {
             return false;
-       }
+        }
 
         /** @var StaticCall $node */
         if (count($node->args) <= 2) {

src/Rector/Contrib/SymfonyExtra/CommandToConstructorInjectionRector.php

@@ -0,0 +1,132 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Rector\Contrib\SymfonyExtra;
+
+use Nette\Utils\Strings;
+use PhpParser\Node;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Name;
+use PhpParser\Node\Scalar\String_;
+use Rector\Builder\Class_\ClassPropertyCollector;
+use Rector\Builder\Kernel\ServiceFromKernelResolver;
+use Rector\Builder\Naming\NameResolver;
+use Rector\Deprecation\SetNames;
+use Rector\NodeFactory\NodeFactory;
+use Rector\Rector\AbstractRector;
+use Rector\Tests\Rector\Contrib\SymfonyExtra\GetterToPropertyRector\Source\LocalKernel;
+
+/**
+ * Ref: https://github.com/symfony/symfony/blob/master/UPGRADE-4.0.md#console
+ *
+ * Similar to @see \Rector\Rector\Contrib\Symfony\GetterToPropertyRector
+ * @todo Extract common logic!
+ *
+ * Before:
+ * class MyCommand extends ContainerAwareCommand
+ *
+ * $this->getContainer()->get('some_service');
+ *
+ * After:
+ * class MyCommand extends Command
+ *
+ * public function construct(SomeService $someService)
+ * {
+ *     $this->someService = $someService;
+ * }
+ *
+ * ...
+ *
+ * $this->someService
+ */
+final class CommandToConstructorInjectionRector extends AbstractRector
+{
+    /**
+     * @var ServiceFromKernelResolver
+     */
+    private $serviceFromKernelResolver;
+
+    /**
+     * @var ClassPropertyCollector
+     */
+    private $classPropertyCollector;
+
+    /**
+     * @var NameResolver
+     */
+    private $nameResolver;
+
+    /**
+     * @var NodeFactory
+     */
+    private $nodeFactory;
+
+    public function __construct(
+        ServiceFromKernelResolver $serviceFromKernelResolver,
+        ClassPropertyCollector $classPropertyCollector,
+        NameResolver $nameResolver,
+        NodeFactory $nodeFactory
+    ) {
+        $this->serviceFromKernelResolver = $serviceFromKernelResolver;
+        $this->classPropertyCollector = $classPropertyCollector;
+        $this->nameResolver = $nameResolver;
+        $this->nodeFactory = $nodeFactory;
+    }
+
+    public function getSetName(): string
+    {
+        return SetNames::SYMFONY_EXTRA;
+    }
+
+    public function sinceVersion(): float
+    {
+        return 3.3;
+    }
+
+    public function isCandidate(Node $node): bool
+    {
+        if (! Strings::endsWith($this->getClassName(), 'Command')) {
+            return false;
+        }
+
+        // finds **$this->getContainer()->get**('some_service');
+        if (! $node instanceof MethodCall || ! $node->var instanceof MethodCall) {
+            return false;
+        }
+
+        // finds **$this**->getContainer()->**get**('some_service');
+        if ((string) $node->var->var->name !== 'this' || (string) $node->name !== 'get') {
+            return false;
+        }
+
+        // finds $this->getContainer()->get**('some_service')**;
+        if (count($node->args) !== 1 || ! $node->args[0]->value instanceof String_) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * @param MethodCall $node
+     */
+    public function refactor(Node $node): ?Node
+    {
+        $this->replaceParentContainerAwareCommandWithCommand();
+
+        $serviceType = $this->serviceFromKernelResolver->resolveServiceClassFromArgument($node->args[0], LocalKernel::class);
+        if ($serviceType === null) {
+            return null;
+        }
+
+        $propertyName = $this->nameResolver->resolvePropertyNameFromType($serviceType);
+
+        $this->classPropertyCollector->addPropertyForClass($this->getClassName(), $serviceType, $propertyName);
+
+        return $this->nodeFactory->createLocalPropertyFetch($propertyName);
+    }
+
+    private function replaceParentContainerAwareCommandWithCommand(): void
+    {
+        $this->classNode->extends = new Name('\Symfony\Component\Console\Command\Command');
+    }
+}

src/Rector/Contrib/SymfonyExtra/GetterToPropertyRector.php

@@ -1,20 +1,19 @@
 <?php declare(strict_types=1);
 
-namespace Rector\Rector\Contrib\Symfony;
+namespace Rector\Rector\Contrib\SymfonyExtra;
 
 use PhpParser\Node;
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\PropertyFetch;
-use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Scalar\String_;
-use PhpParser\Node\Stmt\Class_;
 use Rector\Builder\Class_\ClassPropertyCollector;
 use Rector\Builder\Kernel\ServiceFromKernelResolver;
 use Rector\Builder\Naming\NameResolver;
 use Rector\Deprecation\SetNames;
+use Rector\NodeFactory\NodeFactory;
 use Rector\Rector\AbstractRector;
-use Rector\Tests\Rector\Contrib\Symfony\GetterToPropertyRector\Source\LocalKernel;
+use Rector\Tests\Rector\Contrib\SymfonyExtra\GetterToPropertyRector\Source\LocalKernel;
 
 /**
  * Converts all:
@@ -25,11 +24,6 @@ use Rector\Tests\Rector\Contrib\Symfony\GetterToPropertyRector\Source\LocalKerne
  */
 final class GetterToPropertyRector extends AbstractRector
 {
-    /**
-     * @var string
-     */
-    private $className;
-
     /**
      * @var NameResolver
      */
@@ -45,44 +39,34 @@ final class GetterToPropertyRector extends AbstractRector
      */
     private $classPropertyCollector;
 
+    /**
+     * @var NodeFactory
+     */
+    private $nodeFactory;
+
     public function __construct(
         NameResolver $nameResolver,
         ServiceFromKernelResolver $serviceFromKernelResolver,
-        ClassPropertyCollector $classPropertyCollector
+        ClassPropertyCollector $classPropertyCollector,
+        NodeFactory $nodeFactory
     ) {
         $this->nameResolver = $nameResolver;
         $this->serviceFromKernelResolver = $serviceFromKernelResolver;
         $this->classPropertyCollector = $classPropertyCollector;
-    }
-
-    /**
-     * @param Node[] $nodes
-     * @return null|Node[]
-     */
-    public function beforeTraverse(array $nodes): ?array
-    {
-        $this->className = null;
-
-        foreach ($nodes as $node) {
-            if ($node instanceof Class_) {
-                $this->className = (string) $node->name;
-            }
-        }
-
-        return null;
+        $this->nodeFactory = $nodeFactory;
     }
 
     public function isCandidate(Node $node): bool
     {
-        // $var = $this->get('some_service');
-        // $var = $this->get('some_service')->getData();
+        // finds $var = $this->get('some_service');
+        // finds $var = $this->get('some_service')->getData();
         if ($node instanceof Assign && ($node->expr instanceof MethodCall || $node->var instanceof MethodCall)) {
             if ($this->isContainerGetCall($node->expr)) {
                 return true;
             }
         }
 
-        // ['var => $this->get('some_service')->getData()]
+        // finds ['var => $this->get('some_service')->getData()]
         if ($node instanceof MethodCall && $node->var instanceof MethodCall) {
             if ($this->isContainerGetCall($node->var)) {
                 return true;
@@ -113,7 +97,7 @@ final class GetterToPropertyRector extends AbstractRector
 
     public function getSetName(): string
     {
-        return SetNames::SYMFONY;
+        return SetNames::SYMFONY_EXTRA;
     }
 
     public function sinceVersion(): float
@@ -143,36 +127,15 @@ final class GetterToPropertyRector extends AbstractRector
 
     private function processMethodCallNode(MethodCall $methodCall): ?PropertyFetch
     {
-        /** @var String_ $argument */
-        $argument = $methodCall->args[0]->value;
-        $serviceName = $argument->value;
-
-        $serviceType = $this->serviceFromKernelResolver->resolveServiceClassByNameFromKernel(
-            $serviceName,
-            LocalKernel::class
-        );
-
+        $serviceType = $this->serviceFromKernelResolver->resolveServiceClassFromArgument($methodCall->args[0], LocalKernel::class);
         if ($serviceType === null) {
             return null;
         }
 
         $propertyName = $this->nameResolver->resolvePropertyNameFromType($serviceType);
 
-        $this->classPropertyCollector->addPropertyForClass($this->className, $serviceType, $propertyName);
+        $this->classPropertyCollector->addPropertyForClass($this->getClassName(), $serviceType, $propertyName);
 
-        return $this->createPropertyFetch($propertyName);
-    }
-
-    /**
-     * Creates "$this->propertyName".
-     */
-    private function createPropertyFetch(string $propertyName): PropertyFetch
-    {
-        return new PropertyFetch(
-            new Variable('this', [
-                'name' => $propertyName,
-            ]),
-            $propertyName
-        );
+        return $this->nodeFactory->createLocalPropertyFetch($propertyName);
     }
 }

tests/Rector/Contrib/SymfonyExtra/CommandToConstructorInjectionRector/Correct/correct.php.inc

@@ -0,0 +1,19 @@
+<?php declare(strict_types=1);
+
+use Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand;
+
+final class MyCommand extends \Symfony\Component\Console\Command\Command
+{
+    /**
+     * @var stdClass
+     */
+    private $stdClass;
+    public function __construct(stdClass $stdClass)
+    {
+        $this->stdClass = $stdClass;
+    }
+    protected function execute()
+    {
+        $someService = $this->stdClass;
+    }
+}

tests/Rector/Contrib/SymfonyExtra/CommandToConstructorInjectionRector/Test.php

@@ -0,0 +1,25 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Tests\Rector\Contrib\SymfonyExtra\CommandToConstructorInjectionRector;
+
+use Rector\Rector\Contrib\SymfonyExtra\CommandToConstructorInjectionRector;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class Test extends AbstractRectorTestCase
+{
+    public function test(): void
+    {
+        $this->doTestFileMatchesExpectedContent(
+            __DIR__ . '/Wrong/wrong.php.inc',
+            __DIR__ . '/Correct/correct.php.inc'
+        );
+    }
+
+    /**
+     * @return string[]
+     */
+    protected function getRectorClasses(): array
+    {
+        return [CommandToConstructorInjectionRector::class];
+    }
+}

tests/Rector/Contrib/SymfonyExtra/CommandToConstructorInjectionRector/Wrong/wrong.php.inc

@@ -0,0 +1,11 @@
+<?php declare(strict_types=1);
+
+use Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand;
+
+final class MyCommand extends ContainerAwareCommand
+{
+    protected function execute()
+    {
+        $someService = $this->getContainer()->get('some_service');
+    }
+}

tests/Rector/Contrib/SymfonyExtra/GetterToPropertyRector/Correct/correct4.php.inc

@@ -0,0 +1,19 @@
+<?php declare (strict_types=1);
+
+class ClassWithNamedService implements ContainerAwareInterface
+{
+    /**
+     * @var stdClass
+     */
+    private $stdClass;
+    public function __construct(stdClass $stdClass)
+    {
+        $this->stdClass = $stdClass;
+    }
+    public function render()
+    {
+        $this->renderTwig([
+            'posts' => $this->stdClass->callMe()
+        ]);
+    }
+}

tests/Rector/Contrib/SymfonyExtra/GetterToPropertyRector/Source/LocalKernel.php

@@ -1,6 +1,6 @@
 <?php declare(strict_types=1);
 
-namespace Rector\Tests\Rector\Contrib\Symfony\GetterToPropertyRector\Source;
+namespace Rector\Tests\Rector\Contrib\SymfonyExtra\GetterToPropertyRector\Source;
 
 use Symfony\Component\Config\Loader\LoaderInterface;
 use Symfony\Component\HttpKernel\Bundle\BundleInterface;

tests/Rector/Contrib/SymfonyExtra/GetterToPropertyRector/Source/services.yml

@@ -1,3 +1,3 @@
 services:
-    some.class:
+    some_service:
         class: stdClass

tests/Rector/Contrib/SymfonyExtra/GetterToPropertyRector/Test.php

@@ -1,8 +1,8 @@
 <?php declare(strict_types=1);
 
-namespace Rector\Tests\Rector\Contrib\Symfony\GetterToPropertyRector;
+namespace Rector\Tests\Rector\Contrib\SymfonyExtra\GetterToPropertyRector;
 
-use Rector\Rector\Contrib\Symfony\GetterToPropertyRector;
+use Rector\Rector\Contrib\SymfonyExtra\GetterToPropertyRector;
 use Rector\Testing\PHPUnit\AbstractRectorTestCase;
 
 final class Test extends AbstractRectorTestCase
@@ -23,6 +23,11 @@ final class Test extends AbstractRectorTestCase
             __DIR__ . '/Wrong/wrong3.php.inc',
             __DIR__ . '/Correct/correct3.php.inc'
         );
+
+        $this->doTestFileMatchesExpectedContent(
+            __DIR__ . '/Wrong/wrong4.php.inc',
+            __DIR__ . '/Correct/correct4.php.inc'
+        );
     }
 
     /**

tests/Rector/Contrib/SymfonyExtra/GetterToPropertyRector/Wrong/wrong.php.inc

@@ -4,6 +4,6 @@ class ClassWithNamedService extends Controller
 {
     public function render()
     {
-        $this->get('some.class')->render();
+        $this->get('some_service')->render();
     }
 }

tests/Rector/Contrib/SymfonyExtra/GetterToPropertyRector/Wrong/wrong2.php.inc

@@ -4,6 +4,6 @@ class ClassWithNamedService implements ContainerAwareInterface
 {
     public function render()
     {
-        $someService = $this->get('some.class');
+        $someService = $this->get('some_service');
     }
 }

tests/Rector/Contrib/SymfonyExtra/GetterToPropertyRector/Wrong/wrong3.php.inc

@@ -0,0 +1,10 @@
+<?php declare (strict_types=1);
+
+class ClassWithNamedService implements ContainerAwareInterface
+{
+    public function render()
+    {
+        $someService = $this->get('some_service');
+        $someResult = $this->get('some_service')->callMe();
+    }
+}

tests/Rector/Contrib/SymfonyExtra/GetterToPropertyRector/Wrong/wrong4.php.inc

@@ -4,7 +4,8 @@ class ClassWithNamedService implements ContainerAwareInterface
 {
     public function render()
     {
-        $someService = $this->get('some.class');
-        $someResult = $this->get('some.class')->callMe();
+        $this->renderTwig([
+            'posts' => $this->get('some_service')->callMe()
+        ]);
     }
 }