[DependencyInjection] Extract GetByTypeMethodCallToConstructorInjecti…

…onRector to make migration smoother
rectorphp · Dec 1, 2024 · 2d57041 · 2d57041
1 parent cd07337
commit 2d57041
Show file tree

Hide file tree

Showing 8 changed files with 296 additions and 27 deletions.
diff --git a/...yTypeToConstructorInjectionRector/ControllerGetByTypeToConstructorInjectionRectorTest.php b/...yTypeToConstructorInjectionRector/ControllerGetByTypeToConstructorInjectionRectorTest.php
@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\Tests\DependencyInjection\Rector\Class_\ControllerGetByTypeToConstructorInjectionRector;
+
+use Iterator;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class ControllerGetByTypeToConstructorInjectionRectorTest extends AbstractRectorTestCase
+{
+    #[DataProvider('provideData')]
+    public function test(string $filePath): void
+    {
+        $this->doTestFile($filePath);
+    }
+
+    public static function provideData(): Iterator
+    {
+        return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
+    }
+
+    public function provideConfigFilePath(): string
+    {
+        return __DIR__ . '/config/configured_rule.php';
+    }
+}
diff --git a/.../ControllerGetByTypeToConstructorInjectionRector/Fixture/controller_get_with_type.php.inc b/.../ControllerGetByTypeToConstructorInjectionRector/Fixture/controller_get_with_type.php.inc
@@ -0,0 +1,36 @@
+<?php
+
+namespace Rector\Symfony\Tests\DependencyInjection\Rector\Class_\ControllerGetByTypeToConstructorInjectionRector\Fixture;
+
+use Symfony\Bundle\FrameworkBundle\Controller\Controller;
+
+final class ControllerGetWithType extends Controller
+{
+    public function configure()
+    {
+        $someType = $this->get(SomeType::class);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Symfony\Tests\DependencyInjection\Rector\Class_\ControllerGetByTypeToConstructorInjectionRector\Fixture;
+
+use Symfony\Bundle\FrameworkBundle\Controller\Controller;
+
+final class ControllerGetWithType extends Controller
+{
+    public function __construct(private SomeType $someType)
+    {
+
+    }
+    public function configure()
+    {
+        $someType = $this->someType;
+    }
+}
+
+?>
+
diff --git a/.../Rector/Class_/ControllerGetByTypeToConstructorInjectionRector/config/configured_rule.php b/.../Rector/Class_/ControllerGetByTypeToConstructorInjectionRector/config/configured_rule.php
@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+
+return static function (RectorConfig $rectorConfig): void {
+    $rectorConfig->rule(
+        \Rector\Symfony\DependencyInjection\Rector\Class_\ControllerGetByTypeToConstructorInjectionRector::class
+    );
+};
diff --git a/rules/DependencyInjection/NodeDecorator/CommandConstructorDecorator.php b/rules/DependencyInjection/NodeDecorator/CommandConstructorDecorator.php
@@ -0,0 +1,44 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\DependencyInjection\NodeDecorator;
+
+use PhpParser\Node\Expr\StaticCall;
+use PhpParser\Node\Name;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Expression;
+use PHPStan\Type\ObjectType;
+use Rector\NodeTypeResolver\NodeTypeResolver;
+use Rector\ValueObject\MethodName;
+
+final class CommandConstructorDecorator
+{
+    public function __construct(
+        private NodeTypeResolver $nodeTypeResolver
+    ) {
+    }
+
+    public function decorate(Class_ $class): void
+    {
+        // special case for command to keep parent constructor call
+        if (! $this->nodeTypeResolver->isObjectType(
+            $class,
+            new ObjectType('Symfony\Component\Console\Command\Command')
+        )) {
+            return;
+        }
+
+        $constuctClassMethod = $class->getMethod(MethodName::CONSTRUCT);
+        if (! $constuctClassMethod instanceof ClassMethod) {
+            return;
+        }
+
+        // empty stmts? add parent::__construct() to setup command
+        if ((array) $constuctClassMethod->stmts === []) {
+            $parentConstructStaticCall = new StaticCall(new Name('parent'), '__construct');
+            $constuctClassMethod->stmts[] = new Expression($parentConstructStaticCall);
+        }
+    }
+}
diff --git a/rules/DependencyInjection/Rector/Class_/ControllerGetByTypeToConstructorInjectionRector.php b/rules/DependencyInjection/Rector/Class_/ControllerGetByTypeToConstructorInjectionRector.php
@@ -0,0 +1,167 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\DependencyInjection\Rector\Class_;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\ClassConstFetch;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Stmt\Class_;
+use PHPStan\Reflection\ClassReflection;
+use PHPStan\Type\ObjectType;
+use Rector\NodeManipulator\ClassDependencyManipulator;
+use Rector\PHPStan\ScopeFetcher;
+use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer;
+use Rector\PostRector\ValueObject\PropertyMetadata;
+use Rector\Rector\AbstractRector;
+use Rector\StaticTypeMapper\ValueObject\Type\FullyQualifiedObjectType;
+use Rector\Symfony\Enum\SymfonyClass;
+use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
+use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+
+/**
+ * @see \Rector\Symfony\Tests\DependencyInjection\Rector\Class_\ControllerGetByTypeToConstructorInjectionRector\ControllerGetByTypeToConstructorInjectionRectorTest
+ */
+final class ControllerGetByTypeToConstructorInjectionRector extends AbstractRector
+{
+    public function __construct(
+        private readonly TestsNodeAnalyzer $testsNodeAnalyzer,
+        private readonly ClassDependencyManipulator $classDependencyManipulator,
+    ) {
+    }
+
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition(
+            'From `$container->get(SomeType::class)` in controllers to constructor injection (step 1/x)',
+            [
+                new CodeSample(
+                    <<<'CODE_SAMPLE'
+use Symfony\Bundle\FrameworkBundle\Controller\Controller;
+
+final class SomeCommand extends Controller
+{
+    public function someMethod()
+    {
+        $someType = $this->get(SomeType::class);
+    }
+}
+CODE_SAMPLE
+                    ,
+                    <<<'CODE_SAMPLE'
+use Symfony\Bundle\FrameworkBundle\Controller\Controller;
+
+final class SomeCommand extends Controller
+{
+    public function __construct(private SomeType $someType)
+    {
+    }
+
+    public function someMethod()
+    {
+        $someType = $this->someType;
+    }
+}
+CODE_SAMPLE
+                ),
+            ]
+        );
+    }
+
+    /**
+     * @return array<class-string<Node>>
+     */
+    public function getNodeTypes(): array
+    {
+        return [Class_::class];
+    }
+
+    /**
+     * @param Class_ $node
+     */
+    public function refactor(Node $node): ?Node
+    {
+        if ($this->shouldSkipClass($node)) {
+            return null;
+        }
+
+        $class = $node;
+        $propertyMetadatas = [];
+
+        $this->traverseNodesWithCallable($class, function (Node $node) use (&$propertyMetadatas): ?Node {
+            if (! $node instanceof MethodCall) {
+                return null;
+            }
+
+            if ($node->isFirstClassCallable()) {
+                return null;
+            }
+
+            if (! $this->isName($node->name, 'get')) {
+                return null;
+            }
+
+            // check controller / container type?
+            if (! $this->isObjectType($node->var, new ObjectType(SymfonyClass::CONTROLLER))) {
+                return null;
+            }
+
+            $args = $node->getArgs();
+            if (count($args) !== 1) {
+                return null;
+            }
+
+            $firstArg = $args[0];
+            if (! $firstArg->value instanceof ClassConstFetch) {
+                return null;
+            }
+
+            // must be class const fetch
+            if (! $this->isName($firstArg->value->name, 'class')) {
+                return null;
+            }
+
+            $className = $this->getName($firstArg->value->class);
+            if (! is_string($className)) {
+                return null;
+            }
+
+            $propertyMetadata = new PropertyMetadata($className, new FullyQualifiedObjectType($className));
+
+            $propertyMetadatas[] = $propertyMetadata;
+            return $this->nodeFactory->createPropertyFetch('this', $propertyMetadata->getName());
+        });
+
+        if ($propertyMetadatas === []) {
+            return null;
+        }
+
+        foreach ($propertyMetadatas as $propertyMetadata) {
+            $this->classDependencyManipulator->addConstructorDependency($class, $propertyMetadata);
+        }
+
+        return $node;
+    }
+
+    private function shouldSkipClass(Class_ $class): bool
+    {
+        // keep it safe
+        if (! $class->isFinal()) {
+            return true;
+        }
+
+        // handled by another rule
+        if ($this->testsNodeAnalyzer->isInTestClass($class)) {
+            return true;
+        }
+
+        $scope = ScopeFetcher::fetch($class);
+        $classReflection = $scope->getClassReflection();
+        if (! $classReflection instanceof ClassReflection) {
+            return true;
+        }
+
+        return $classReflection->isSubclassOf(SymfonyClass::CONTROLLER);
+    }
+}
diff --git a/rules/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector.php b/rules/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector.php
@@ -7,18 +7,14 @@
 use PhpParser\Node;
 use PhpParser\Node\Expr\ClassConstFetch;
 use PhpParser\Node\Expr\MethodCall;
-use PhpParser\Node\Expr\StaticCall;
-use PhpParser\Node\Name;
 use PhpParser\Node\Stmt\Class_;
-use PhpParser\Node\Stmt\ClassMethod;
-use PhpParser\Node\Stmt\Expression;
 use PHPStan\Type\ObjectType;
 use Rector\NodeManipulator\ClassDependencyManipulator;
 use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer;
 use Rector\PostRector\ValueObject\PropertyMetadata;
 use Rector\Rector\AbstractRector;
+use Rector\Symfony\DependencyInjection\NodeDecorator\CommandConstructorDecorator;
 use Rector\Symfony\NodeAnalyzer\DependencyInjectionMethodCallAnalyzer;
-use Rector\ValueObject\MethodName;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -33,6 +29,7 @@ public function __construct(
         private readonly DependencyInjectionMethodCallAnalyzer $dependencyInjectionMethodCallAnalyzer,
         private readonly TestsNodeAnalyzer $testsNodeAnalyzer,
         private readonly ClassDependencyManipulator $classDependencyManipulator,
+        private readonly CommandConstructorDecorator $commandConstructorDecorator,
     ) {
     }
 
@@ -141,27 +138,8 @@ public function refactor(Node $node): ?Node
             $this->classDependencyManipulator->addConstructorDependency($class, $propertyMetadata);
         }
 
-        $this->decorateCommandConstructor($class);
+        $this->commandConstructorDecorator->decorate($class);
 
         return $node;
     }
-
-    private function decorateCommandConstructor(Class_ $class): void
-    {
-        // special case for command to keep parent constructor call
-        if (! $this->isObjectType($class, new ObjectType('Symfony\Component\Console\Command\Command'))) {
-            return;
-        }
-
-        $constuctClassMethod = $class->getMethod(MethodName::CONSTRUCT);
-        if (! $constuctClassMethod instanceof ClassMethod) {
-            return;
-        }
-
-        // empty stmts? add parent::__construct() to setup command
-        if ((array) $constuctClassMethod->stmts === []) {
-            $parentConstructStaticCall = new StaticCall(new Name('parent'), '__construct');
-            $constuctClassMethod->stmts[] = new Expression($parentConstructStaticCall);
-        }
-    }
 }
diff --git a/src/Enum/SymfonyClass.php b/src/Enum/SymfonyClass.php
@@ -6,6 +6,11 @@
 
 final class SymfonyClass
 {
+    /**
+     * @var string
+     */
+    public const CONTROLLER = 'Symfony\Bundle\FrameworkBundle\Controller\Controller';
+
     /**
      * @var string
      */

diff --git a/src/NodeAnalyzer/ServiceTypeMethodCallResolver.php b/src/NodeAnalyzer/ServiceTypeMethodCallResolver.php
@@ -28,8 +28,8 @@ public function resolve(MethodCall $methodCall): ?Type
             return new MixedType();
         }
 
-        $argument = $methodCall->getArgs()[0]
-            ->value;
+        $firstArg = $methodCall->getArgs()[0];
+        $argument = $firstArg->value;
         $serviceMap = $this->serviceMapProvider->provide();
 
         if ($argument instanceof String_) {