Use autowired method if exists in adding new dependency

rectorphp · Jan 6, 2025 · 836b37e · 836b37e
1 parent 2e82ab4
commit 836b37e
Show file tree

Hide file tree

Showing 16 changed files with 188 additions and 20 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
@@ -6,8 +6,11 @@
 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);
+
     $rectorConfig
         ->ruleWithConfiguration(FuncCallToMethodCallRector::class, [
             new FuncCallToMethodCall('view', 'Namespaced\SomeRenderer', 'render'),

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/FileSystem/FilesFinder.php b/src/FileSystem/FilesFinder.php
@@ -59,7 +59,7 @@ public function findInDirectoriesAndFiles(
         // filter files by specific suffix
         if ($hasOnlySuffix) {
             /** @var string $onlySuffix */
-            $fileWithSuffixFilter = (static fn(string $filePath): bool => str_ends_with($filePath, $onlySuffix));
+            $fileWithSuffixFilter = (static fn (string $filePath): bool => str_ends_with($filePath, $onlySuffix));
         } elseif ($suffixes !== []) {
             $fileWithSuffixFilter = static function (string $filePath) use ($suffixes): bool {
                 $filePathExtension = pathinfo($filePath, PATHINFO_EXTENSION);
@@ -88,7 +88,13 @@ function (string $file): bool {
 
         // filtering files in directories collection
         $directories = $this->fileAndDirectoryFilter->filterDirectories($filesAndDirectories);
-        $filteredFilePathsInDirectories = $this->findInDirectories($directories, $suffixes, $hasOnlySuffix, $onlySuffix, $sortByName);
+        $filteredFilePathsInDirectories = $this->findInDirectories(
+            $directories,
+            $suffixes,
+            $hasOnlySuffix,
+            $onlySuffix,
+            $sortByName
+        );
 
         $filePaths = [...$filteredFilePaths, ...$filteredFilePathsInDirectories];
         return $this->unchangedFilesFilter->filterFilePaths($filePaths);

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/src/PhpParser/Node/NodeFactory.php b/src/PhpParser/Node/NodeFactory.php
@@ -45,10 +45,12 @@
 use Rector\Exception\ShouldNotHappenException;
 use Rector\NodeDecorator\PropertyTypeDecorator;
 use Rector\NodeTypeResolver\Node\AttributeKey;
+use Rector\Php\PhpVersionProvider;
 use Rector\PhpDocParser\NodeTraverser\SimpleCallableNodeTraverser;
 use Rector\PHPStanStaticTypeMapper\Enum\TypeKind;
 use Rector\PostRector\ValueObject\PropertyMetadata;
 use Rector\StaticTypeMapper\StaticTypeMapper;
+use Rector\ValueObject\PhpVersionFeature;
 
 /**
  * @see \Rector\Tests\PhpParser\Node\NodeFactoryTest
@@ -65,7 +67,8 @@ public function __construct(
         private PhpDocInfoFactory $phpDocInfoFactory,
         private StaticTypeMapper $staticTypeMapper,
         private PropertyTypeDecorator $propertyTypeDecorator,
-        private SimpleCallableNodeTraverser $simpleCallableNodeTraverser
+        private SimpleCallableNodeTraverser $simpleCallableNodeTraverser,
+        private PhpVersionProvider $phpVersionProvider,
     ) {
     }
 
@@ -291,6 +294,11 @@ public function createPromotedPropertyParam(PropertyMetadata $propertyMetadata):
         $propertyFlags = $propertyMetadata->getFlags();
         $param->flags = $propertyFlags !== 0 ? $propertyFlags : Modifiers::PRIVATE;
 
+        // make readonly by default
+        if ($this->phpVersionProvider->isAtLeastPhpVersion(PhpVersionFeature::READONLY_PROPERTY)) {
+            $param->flags |= Modifiers::READONLY;
+        }
+
         return $param;
     }
 

diff --git a/tests/FileSystem/FilesFinder/FilesFinderTest.php b/tests/FileSystem/FilesFinder/FilesFinderTest.php
@@ -130,10 +130,7 @@ public function testFilterBySuffix(): void
         $this->assertCount(3, $foundNoFilterFiles);
 
         $foundFiles = $this->filesFinder->findInDirectoriesAndFiles(
-            [
-                __DIR__ . '/SourceWithSuffix',
-                __DIR__ . '/SourceWithSuffix/other_unrelated_file.php',
-            ],
+            [__DIR__ . '/SourceWithSuffix', __DIR__ . '/SourceWithSuffix/other_unrelated_file.php'],
             ['php'],
             true,
             'Controller'

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/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,10 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+
+return RectorConfig::configure()
+    ->withRules([
+        \Rector\Symfony\DependencyInjection\Rector\Class_\GetBySymfonyStringToConstructorInjectionRector::class,
+    ]);
diff --git a/tests/NodeManipulator/ClassDependencyManipulatorTest.php b/tests/NodeManipulator/ClassDependencyManipulatorTest.php
@@ -17,9 +17,12 @@
 use PhpParser\NodeVisitor\NameResolver;
 use PhpParser\PrettyPrinter\Standard;
 use PHPStan\Type\ObjectType;
+use Rector\Configuration\Option;
+use Rector\Configuration\Parameter\SimpleParameterProvider;
 use Rector\NodeManipulator\ClassDependencyManipulator;
 use Rector\PostRector\ValueObject\PropertyMetadata;
 use Rector\Testing\PHPUnit\AbstractLazyTestCase;
+use Rector\ValueObject\PhpVersionFeature;
 
 final class ClassDependencyManipulatorTest extends AbstractLazyTestCase
 {
@@ -30,7 +33,11 @@ final class ClassDependencyManipulatorTest extends AbstractLazyTestCase
     protected function setUp(): void
     {
         $this->classDependencyManipulator = $this->make(ClassDependencyManipulator::class);
+
         $this->printerStandard = new Standard();
+
+        // use at least readonly property
+        SimpleParameterProvider::setParameter(Option::PHP_VERSION_FEATURES, PhpVersionFeature::READONLY_PROPERTY);
     }
 
     public function testEmptyClass(): void
@@ -46,7 +53,6 @@ public function testEmptyClass(): void
     public function testSingleMethod(): void
     {
         $someClass = new Class_(new Identifier('SingleMethodClass'));
-
         $this->setNamespacedName($someClass);
 
         $someClass->stmts[] = new ClassMethod('firstMethod');

diff --git a/tests/NodeManipulator/Fixture/expected_class_const_property.php.inc b/tests/NodeManipulator/Fixture/expected_class_const_property.php.inc
@@ -5,7 +5,7 @@ class ConstantProperties
     const SOME_CONST = 'value';
     public $someProperty;
     public $anotherProperty;
-    public function __construct(private \EventDispatcherInterface $eventDispatcher)
+    public function __construct(private readonly \EventDispatcherInterface $eventDispatcher)
     {
     }
 }
diff --git a/tests/NodeManipulator/Fixture/expected_empty_class.php.inc b/tests/NodeManipulator/Fixture/expected_empty_class.php.inc
@@ -2,7 +2,7 @@
 
 class EmptyClass
 {
-    public function __construct(private \EventDispatcherInterface $eventDispatcher)
+    public function __construct(private readonly \EventDispatcherInterface $eventDispatcher)
     {
     }
 }
diff --git a/tests/NodeManipulator/Fixture/expected_method_and_property.php.inc b/tests/NodeManipulator/Fixture/expected_method_and_property.php.inc
@@ -3,7 +3,7 @@
 class ClassWithMethodAndProperty
 {
     private $someProperty;
-    public function __construct(private \EventDispatcherInterface $eventDispatcher)
+    public function __construct(private readonly \EventDispatcherInterface $eventDispatcher)
     {
     }
     function someMethod()

diff --git a/tests/NodeManipulator/Fixture/expected_single_method.php.inc b/tests/NodeManipulator/Fixture/expected_single_method.php.inc
@@ -2,7 +2,7 @@
 
 class SingleMethodClass
 {
-    public function __construct(private \EventDispatcherInterface $eventDispatcher)
+    public function __construct(private readonly \EventDispatcherInterface $eventDispatcher)
     {
     }
     function firstMethod()

diff --git a/tests/NodeManipulator/Fixture/expected_single_property.php.inc b/tests/NodeManipulator/Fixture/expected_single_property.php.inc
@@ -3,7 +3,7 @@
 class ClassWithSingleProperty
 {
     private $someProperty;
-    public function __construct(private \EventDispatcherInterface $eventDispatcher)
+    public function __construct(private readonly \EventDispatcherInterface $eventDispatcher)
     {
     }
 }