Use autowired method if exists in adding new dependency (#6652)

* Use autowired method if exists in adding new dependency * Add constructor fixture, just to verify alter behavior
rectorphp · Jan 6, 2025 · fe99f21 · fe99f21
1 parent 8bc80b4
commit fe99f21
Show file tree

Hide file tree

Showing 8 changed files with 198 additions and 8 deletions.
diff --git a/rules-tests/Transform/Rector/FuncCall/FuncCallToMethodCallRector/config/configured_rule.php b/rules-tests/Transform/Rector/FuncCall/FuncCallToMethodCallRector/config/configured_rule.php
@@ -2,11 +2,11 @@
 
 declare(strict_types=1);
 
-use Rector\ValueObject\PhpVersion;
 use Rector\Config\RectorConfig;
 use Rector\Tests\Transform\Rector\FuncCall\FuncCallToMethodCallRector\Source\SomeTranslator;
 use Rector\Transform\Rector\FuncCall\FuncCallToMethodCallRector;
 use Rector\Transform\ValueObject\FuncCallToMethodCall;
+use Rector\ValueObject\PhpVersion;
 
 return static function (RectorConfig $rectorConfig): void {
     $rectorConfig->phpVersion(PhpVersion::PHP_80);

diff --git a/rules/TypeDeclaration/NodeAnalyzer/AutowiredClassMethodOrPropertyAnalyzer.php b/rules/TypeDeclaration/NodeAnalyzer/AutowiredClassMethodOrPropertyAnalyzer.php
@@ -5,6 +5,7 @@
 namespace Rector\TypeDeclaration\NodeAnalyzer;
 
 use PhpParser\Node\Param;
+use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Property;
 use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
@@ -18,6 +19,27 @@ public function __construct(
     ) {
     }
 
+    public function matchAutowiredMethodInClass(Class_ $class): ?ClassMethod
+    {
+        foreach ($class->getMethods() as $classMethod) {
+            if (! $classMethod->isPublic()) {
+                continue;
+            }
+
+            if ($classMethod->isMagic()) {
+                continue;
+            }
+
+            if (! $this->detect($classMethod)) {
+                continue;
+            }
+
+            return $classMethod;
+        }
+
+        return null;
+    }
+
     public function detect(ClassMethod | Param | Property $node): bool
     {
         $nodePhpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);

diff --git a/src/NodeManipulator/ClassDependencyManipulator.php b/src/NodeManipulator/ClassDependencyManipulator.php
@@ -39,36 +39,58 @@ public function __construct(
         private PropertyPresenceChecker $propertyPresenceChecker,
         private NodeNameResolver $nodeNameResolver,
         private AutowiredClassMethodOrPropertyAnalyzer $autowiredClassMethodOrPropertyAnalyzer,
-        private ReflectionResolver $reflectionResolver
+        private ReflectionResolver $reflectionResolver,
     ) {
     }
 
     public function addConstructorDependency(Class_ $class, PropertyMetadata $propertyMetadata): void
     {
+        // already has property as dependency? skip it
         if ($this->hasClassPropertyAndDependency($class, $propertyMetadata)) {
             return;
         }
 
-        if (! $this->phpVersionProvider->isAtLeastPhpVersion(PhpVersionFeature::PROPERTY_PROMOTION)) {
+        // special case for Symfony @required
+        $autowireClassMethod = $this->autowiredClassMethodOrPropertyAnalyzer->matchAutowiredMethodInClass($class);
+
+        if (! $this->phpVersionProvider->isAtLeastPhpVersion(
+            PhpVersionFeature::PROPERTY_PROMOTION
+        ) || $autowireClassMethod instanceof ClassMethod) {
             $this->classInsertManipulator->addPropertyToClass(
                 $class,
                 $propertyMetadata->getName(),
                 $propertyMetadata->getType()
             );
         }
 
-        if ($this->shouldAddPromotedProperty($class, $propertyMetadata)) {
-            $this->addPromotedProperty($class, $propertyMetadata);
-        } else {
+        // in case of existing autowire method, re-use it
+        if ($autowireClassMethod instanceof ClassMethod) {
             $assign = $this->nodeFactory->createPropertyAssignment($propertyMetadata->getName());
 
-            $this->addConstructorDependencyWithCustomAssign(
-                $class,
+            $this->classMethodAssignManipulator->addParameterAndAssignToMethod(
+                $autowireClassMethod,
                 $propertyMetadata->getName(),
                 $propertyMetadata->getType(),
                 $assign
             );
+            return;
+
+        }
+
+        // add PHP 8.0 promoted property
+        if ($this->shouldAddPromotedProperty($class, $propertyMetadata)) {
+            $this->addPromotedProperty($class, $propertyMetadata);
+            return;
         }
+
+        $assign = $this->nodeFactory->createPropertyAssignment($propertyMetadata->getName());
+
+        $this->addConstructorDependencyWithCustomAssign(
+            $class,
+            $propertyMetadata->getName(),
+            $propertyMetadata->getType(),
+            $assign
+        );
     }
 
     /**

diff --git a/tests/Issues/AddClassDependency/AddClassDependencyTest.php b/tests/Issues/AddClassDependency/AddClassDependencyTest.php
@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Issues\AddClassDependency;
+
+use Iterator;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class AddClassDependencyTest 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/tests/Issues/AddClassDependency/Fixture/fixture.php.inc b/tests/Issues/AddClassDependency/Fixture/fixture.php.inc
@@ -0,0 +1,57 @@
+<?php
+
+namespace Rector\Tests\Issues\AddClassDependency\Fixture;
+
+use Rector\Tests\Issues\AddClassDependency\Source\SomeAutowiredService;
+use Symfony\Bundle\FrameworkBundle\Controller\Controller;
+
+class PreferRequiredSetter extends Controller
+{
+    private SomeAutowiredService $someAutowiredService;
+
+    /**
+     * @required
+     */
+    public function autowire(
+        SomeAutowiredService $someAutowiredService,
+    ) {
+        $this->someAutowiredService = $someAutowiredService;
+    }
+
+    public function configure()
+    {
+        $someType = $this->get('validator');
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Issues\AddClassDependency\Fixture;
+
+use Rector\Tests\Issues\AddClassDependency\Source\SomeAutowiredService;
+use Symfony\Bundle\FrameworkBundle\Controller\Controller;
+
+class PreferRequiredSetter extends Controller
+{
+    private SomeAutowiredService $someAutowiredService;
+    private \Symfony\Component\Validator\Validator\ValidatorInterface $validator;
+
+    /**
+     * @required
+     */
+    public function autowire(
+        SomeAutowiredService $someAutowiredService, \Symfony\Component\Validator\Validator\ValidatorInterface $validator,
+    ) {
+        $this->someAutowiredService = $someAutowiredService;
+        $this->validator = $validator;
+    }
+
+    public function configure()
+    {
+        $someType = $this->validator;
+    }
+}
+
+?>
diff --git a/tests/Issues/AddClassDependency/Fixture/strick_with_constructor.php.inc b/tests/Issues/AddClassDependency/Fixture/strick_with_constructor.php.inc
@@ -0,0 +1,43 @@
+<?php
+
+namespace Rector\Tests\Issues\AddClassDependency\Fixture;
+
+use Rector\Tests\Issues\AddClassDependency\Source\SomeAutowiredService;
+use Symfony\Bundle\FrameworkBundle\Controller\Controller;
+
+final class StickWithConstructor extends Controller
+{
+    public function __construct(
+        private SomeAutowiredService $someAutowiredService,
+    ) {
+    }
+
+    public function configure()
+    {
+        $someType = $this->get('validator');
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Issues\AddClassDependency\Fixture;
+
+use Rector\Tests\Issues\AddClassDependency\Source\SomeAutowiredService;
+use Symfony\Bundle\FrameworkBundle\Controller\Controller;
+
+final class StickWithConstructor extends Controller
+{
+    public function __construct(
+        private SomeAutowiredService $someAutowiredService, private readonly \Symfony\Component\Validator\Validator\ValidatorInterface $validator,
+    ) {
+    }
+
+    public function configure()
+    {
+        $someType = $this->validator;
+    }
+}
+
+?>
diff --git a/tests/Issues/AddClassDependency/Source/SomeAutowiredService.php b/tests/Issues/AddClassDependency/Source/SomeAutowiredService.php
@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Issues\AddClassDependency\Source;
+
+final class SomeAutowiredService
+{
+}
diff --git a/tests/Issues/AddClassDependency/config/configured_rule.php b/tests/Issues/AddClassDependency/config/configured_rule.php
@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\Symfony\DependencyInjection\Rector\Class_\GetBySymfonyStringToConstructorInjectionRector;
+
+return RectorConfig::configure()
+    ->withRules([GetBySymfonyStringToConstructorInjectionRector::class]);