Merge pull request #459 from rectorphp/action-inject-to-constructor

Improve Action Inject to work with container

src/Bridge/Contract/AnalyzedApplicationContainerInterface.php

@@ -0,0 +1,10 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Bridge\Contract;
+
+interface AnalyzedApplicationContainerInterface
+{
+    public function getTypeForName(string $name): ?string;
+
+    public function hasService(string $name): bool;
+}

src/Bridge/Contract/ServiceTypeForNameProviderInterface.php

@@ -1,8 +0,0 @@
-<?php declare(strict_types=1);
-
-namespace Rector\Bridge\Contract;
-
-interface ServiceTypeForNameProviderInterface
-{
-    public function provideTypeForName(string $name): ?string;
-}

src/Bridge/Symfony/DefaultAnalyzedSymfonyApplicationContainer.php

@@ -2,12 +2,13 @@
 
 namespace Rector\Bridge\Symfony;
 
-use Rector\Bridge\Contract\ServiceTypeForNameProviderInterface;
+use Rector\Bridge\Contract\AnalyzedApplicationContainerInterface;
 use Rector\Bridge\Symfony\DependencyInjection\ContainerFactory;
 use Rector\Configuration\Option;
+use Symfony\Component\DependencyInjection\Container;
 use Symplify\PackageBuilder\Parameter\ParameterProvider;
 
-final class DefaultServiceTypeForNameProvider implements ServiceTypeForNameProviderInterface
+final class DefaultAnalyzedSymfonyApplicationContainer implements AnalyzedApplicationContainerInterface
 {
     /**
      * @var ParameterProvider
@@ -34,16 +35,9 @@ final class DefaultServiceTypeForNameProvider implements ServiceTypeForNameProvi
         $this->containerFactory = $containerFactory;
     }
 
-    public function provideTypeForName(string $name): ?string
+    public function getTypeForName(string $name): ?string
     {
-        $kernelClass = $this->parameterProvider->provideParameter(Option::KERNEL_CLASS_PARAMETER);
-        $this->symfonyKernelParameterGuard->ensureKernelClassIsValid($kernelClass);
-
-        // make this default, register and require kernel_class paramter, see:
-        // https://github.com/rectorphp/rector/issues/428
-
-        /** @var string $kernelClass */
-        $container = $this->containerFactory->createFromKernelClass($kernelClass);
+        $container = $this->getContainer();
 
         if ($container->has($name)) {
             $definition = $container->get($name);
@@ -53,4 +47,21 @@ final class DefaultServiceTypeForNameProvider implements ServiceTypeForNameProvi
 
         return null;
     }
+
+    public function hasService(string $name): bool
+    {
+        $container = $this->getContainer();
+
+        return $container->has($name);
+    }
+
+    private function getContainer(): Container
+    {
+        $kernelClass = $this->parameterProvider->provideParameter(Option::KERNEL_CLASS_PARAMETER);
+
+        $this->symfonyKernelParameterGuard->ensureKernelClassIsValid($kernelClass);
+
+        /** @var string $kernelClass */
+        return $this->containerFactory->createFromKernelClass($kernelClass);
+    }
 }

src/Bridge/Symfony/DependencyInjection/ContainerFactory.php

@@ -2,8 +2,11 @@
 
 namespace Rector\Bridge\Symfony\DependencyInjection;
 
+use Rector\DependencyInjection\CompilerPass\MakeServicesPublicCompilerPass;
 use Symfony\Component\DependencyInjection\Container;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\HttpKernel\Kernel;
+use Symplify\PackageBuilder\Reflection\PrivatesCaller;
 
 final class ContainerFactory
 {
@@ -14,19 +17,26 @@ final class ContainerFactory
 
     public function createFromKernelClass(string $kernelClass): Container
     {
-        if ($this->containersByKernelClass[$kernelClass]) {
+        if (isset($this->containersByKernelClass[$kernelClass])) {
             return $this->containersByKernelClass[$kernelClass];
         }
 
         return $this->containersByKernelClass[$kernelClass] = $this->createContainerFromKernelClass($kernelClass);
     }
 
+    /**
+     * Mimics https://github.com/symfony/symfony/blob/226e2f3949c5843b67826aca4839c2c6b95743cf/src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.php#L200-L203
+     */
     private function createContainerFromKernelClass(string $kernelClass): Container
     {
         $kernel = $this->createKernelFromKernelClass($kernelClass);
-        $kernel->boot();
 
-        return $kernel->getContainer();
+        /** @var ContainerBuilder $containerBuilder */
+        $containerBuilder = (new PrivatesCaller())->callPrivateMethod($kernel, 'buildContainer');
+        $containerBuilder->getCompilerPassConfig()->addPass(new MakeServicesPublicCompilerPass());
+        $containerBuilder->compile();
+
+        return $containerBuilder;
     }
 
     private function createKernelFromKernelClass(string $kernelClass): Kernel

src/DependencyInjection/CompilerPass/MakeServicesPublicCompilerPass.php

@@ -0,0 +1,20 @@
+<?php declare(strict_types=1);
+
+namespace Rector\DependencyInjection\CompilerPass;
+
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+final class MakeServicesPublicCompilerPass implements CompilerPassInterface
+{
+    public function process(ContainerBuilder $containerBuilder): void
+    {
+        foreach ($containerBuilder->getDefinitions() as $definition) {
+            $definition->setPublic(true);
+        }
+
+        foreach ($containerBuilder->getAliases() as $definition) {
+            $definition->setPublic(true);
+        }
+    }
+}

src/Rector/Architecture/DependencyInjection/ActionInjectionToConstructorInjectionRector.php

@@ -7,11 +7,13 @@ use PhpParser\Node;
 use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassMethod;
+use Rector\Bridge\Contract\AnalyzedApplicationContainerInterface;
 use Rector\Builder\Class_\VariableInfo;
 use Rector\Builder\Class_\VariableInfoFactory;
 use Rector\Builder\ConstructorMethodBuilder;
 use Rector\Builder\PropertyBuilder;
 use Rector\Configuration\Rector\Architecture\DependencyInjection\VariablesToPropertyFetchCollection;
+use Rector\Node\Attribute;
 use Rector\Rector\AbstractRector;
 use Rector\RectorDefinition\CodeSample;
 use Rector\RectorDefinition\RectorDefinition;
@@ -38,16 +40,23 @@ final class ActionInjectionToConstructorInjectionRector extends AbstractRector
      */
     private $variablesToPropertyFetchCollection;
 
+    /**
+     * @var AnalyzedApplicationContainerInterface
+     */
+    private $analyzedApplicationContainer;
+
     public function __construct(
         PropertyBuilder $propertyBuilder,
         ConstructorMethodBuilder $constructorMethodBuilder,
         VariableInfoFactory $variableInfoFactory,
-        VariablesToPropertyFetchCollection $variablesToPropertyFetchCollection
+        VariablesToPropertyFetchCollection $variablesToPropertyFetchCollection,
+        AnalyzedApplicationContainerInterface $analyzedApplicationContainer
     ) {
         $this->propertyBuilder = $propertyBuilder;
         $this->constructorMethodBuilder = $constructorMethodBuilder;
         $this->variableInfoFactory = $variableInfoFactory;
         $this->variablesToPropertyFetchCollection = $variablesToPropertyFetchCollection;
+        $this->analyzedApplicationContainer = $analyzedApplicationContainer;
     }
 
     public function isCandidate(Node $node): bool
@@ -120,7 +129,7 @@ CODE_SAMPLE
 
             $variableInfo = $this->variableInfoFactory->createFromNameAndTypes(
                 $paramNode->var->name,
-                [(string) $paramNode->type]
+                $paramNode->getAttribute(Attribute::TYPES)
             );
 
             $this->addConstructorDependencyToClassNode($classNode, $variableInfo);
@@ -140,7 +149,8 @@ CODE_SAMPLE
             return false;
         }
 
-        if (Strings::endsWith($typehint, 'Request')) {
+        $typehint = (string) $paramNode->getAttribute(Attribute::TYPES)[0] ?? null;
+        if ($typehint === null) {
             return false;
         }
 
@@ -149,7 +159,7 @@ CODE_SAMPLE
             return false;
         }
 
-        return true;
+        return $this->analyzedApplicationContainer->hasService($typehint);
     }
 
     private function addConstructorDependencyToClassNode(Class_ $classNode, VariableInfo $variableInfo): void

src/Rector/Contrib/Symfony/FrameworkBundle/AbstractToConstructorInjectionRector.php

@@ -7,7 +7,7 @@ use PhpParser\Node\Expr\ClassConstFetch;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Name;
 use PhpParser\Node\Scalar\String_;
-use Rector\Bridge\Contract\ServiceTypeForNameProviderInterface;
+use Rector\Bridge\Contract\AnalyzedApplicationContainerInterface;
 use Rector\Builder\Class_\ClassPropertyCollector;
 use Rector\Naming\PropertyNaming;
 use Rector\Node\Attribute;
@@ -33,9 +33,9 @@ abstract class AbstractToConstructorInjectionRector extends AbstractRector
     protected $propertyFetchNodeFactory;
 
     /**
-     * @var ServiceTypeForNameProviderInterface
+     * @var AnalyzedApplicationContainerInterface
      */
-    protected $serviceTypeForNameProvider;
+    protected $analyzedApplicationContainer;
 
     /**
      * @var MethodCallAnalyzer
@@ -46,13 +46,13 @@ abstract class AbstractToConstructorInjectionRector extends AbstractRector
         PropertyNaming $propertyNaming,
         ClassPropertyCollector $classPropertyCollector,
         PropertyFetchNodeFactory $propertyFetchNodeFactory,
-        ServiceTypeForNameProviderInterface $serviceTypeForNameProvider,
+        AnalyzedApplicationContainerInterface $analyzedApplicationContainer,
         MethodCallAnalyzer $methodCallAnalyzer
     ) {
         $this->propertyNaming = $propertyNaming;
         $this->classPropertyCollector = $classPropertyCollector;
         $this->propertyFetchNodeFactory = $propertyFetchNodeFactory;
-        $this->serviceTypeForNameProvider = $serviceTypeForNameProvider;
+        $this->analyzedApplicationContainer = $analyzedApplicationContainer;
         $this->methodCallAnalyzer = $methodCallAnalyzer;
     }
 
@@ -91,7 +91,7 @@ abstract class AbstractToConstructorInjectionRector extends AbstractRector
 
         if ($argument instanceof String_) {
             $serviceName = $argument->value;
-            return $this->serviceTypeForNameProvider->provideTypeForName($serviceName);
+            return $this->analyzedApplicationContainer->getTypeForName($serviceName);
         }
 
         if (! $argument instanceof ClassConstFetch) {

tests/Rector/Architecture/DependencyInjection/ActionInjectionToConstructorInjectionRector/ActionInjectionToConstructorInjectionRectorTest.php

@@ -21,6 +21,7 @@ final class ActionInjectionToConstructorInjectionRectorTest extends AbstractRect
     public function provideWrongToFixedFiles(): Iterator
     {
         yield [__DIR__ . '/Wrong/wrong.php.inc', __DIR__ . '/Correct/correct.php.inc'];
+        yield [__DIR__ . '/Wrong/wrong2.php.inc', __DIR__ . '/Correct/correct2.php.inc'];
     }
 
     protected function provideConfig(): string

tests/Rector/Architecture/DependencyInjection/ActionInjectionToConstructorInjectionRector/Correct/correct.php.inc

@@ -1,12 +1,14 @@
 <?php declare(strict_types=1);
 
+use Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository;
+
 final class SomeController
 {
     /**
-     * @var \ProductRepository
+     * @var \Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository
      */
     private $productRepository;
-    public function __construct(\ProductRepository $productRepository)
+    public function __construct(\Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository $productRepository)
     {
         $this->productRepository = $productRepository;
     }

tests/Rector/Architecture/DependencyInjection/ActionInjectionToConstructorInjectionRector/Correct/correct2.php.inc

@@ -0,0 +1,23 @@
+<?php declare(strict_types=1);
+
+use Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository;
+
+final class SomeController
+{
+    /**
+     * @var \Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository
+     */
+    private $productRepository;
+    public function __construct(\Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository $productRepository)
+    {
+        $this->productRepository = $productRepository;
+    }
+    public function default()
+    {
+        $products = $this->productRepository->fetchAll();
+    }
+
+    public function detail(Request $request, Product $product)
+    {
+    }
+}

tests/Rector/Architecture/DependencyInjection/ActionInjectionToConstructorInjectionRector/Source/ProductRepository.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source;
+
+final class ProductRepository
+{
+
+}

tests/Rector/Architecture/DependencyInjection/ActionInjectionToConstructorInjectionRector/Source/SomeKernelClass.php

@@ -0,0 +1,35 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source;
+
+use Symfony\Component\Config\Loader\LoaderInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\HttpKernel\Kernel;
+
+final class SomeKernelClass extends Kernel
+{
+    public function registerBundles(): iterable
+    {
+        return [];
+    }
+
+    public function registerContainerConfiguration(LoaderInterface $loader): void
+    {
+    }
+
+    protected function build(ContainerBuilder $containerBuilder): void
+    {
+        $containerBuilder->register(ProductRepository::class);
+    }
+
+    public function getCacheDir()
+    {
+        return sys_get_temp_dir() . '/_tmp';
+    }
+
+    public function getLogDir()
+    {
+        return sys_get_temp_dir() . '/_tmp';
+    }
+}

tests/Rector/Architecture/DependencyInjection/ActionInjectionToConstructorInjectionRector/Wrong/wrong.php.inc

@@ -1,5 +1,7 @@
 <?php declare(strict_types=1);
 
+use Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository;
+
 final class SomeController
 {
     public function default(ProductRepository $productRepository)

tests/Rector/Architecture/DependencyInjection/ActionInjectionToConstructorInjectionRector/Wrong/wrong2.php.inc

@@ -0,0 +1,15 @@
+<?php declare(strict_types=1);
+
+use Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository;
+
+final class SomeController
+{
+    public function default(ProductRepository $productRepository)
+    {
+        $products = $productRepository->fetchAll();
+    }
+
+    public function detail(Request $request, Product $product)
+    {
+    }
+}

tests/Rector/Architecture/DependencyInjection/ActionInjectionToConstructorInjectionRector/config.yml

@@ -1,2 +1,5 @@
+parameters:
+    kernel_class: 'Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\SomeKernelClass'
+
 imports:
     - { resource: '../../../../../config/level/architecture/action-injection-to-constructor-injection.yml' }

tests/Rector/Contrib/Symfony/FrameworkBundle/AbstractToConstructorInjectionRectorSource/SomeKernelClass.php

@@ -20,9 +20,7 @@ final class SomeKernelClass extends Kernel
 
     protected function build(ContainerBuilder $containerBuilder): void
     {
-        $someServiceDefinition = $containerBuilder->register('some_service', 'stdClass');
-        // so we can get it by the string name
-        $someServiceDefinition->setPublic(true);
+        $containerBuilder->register('some_service', 'stdClass');
     }
 
     public function getCacheDir()