diff --git a/rules-tests/Transform/Rector/FuncCall/FuncCallToMethodCallRector/config/configured_rule.php b/rules-tests/Transform/Rector/FuncCall/FuncCallToMethodCallRector/config/configured_rule.php index f65e91806ca..3acbdab1679 100644 --- 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 index 80080786a25..ab6391ecf28 100644 --- 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 index 46527c5d42b..38244f3e026 100644 --- 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 index 218a3b74a14..b6a3dfde5d4 100644 --- a/src/NodeManipulator/ClassDependencyManipulator.php +++ b/src/NodeManipulator/ClassDependencyManipulator.php @@ -39,17 +39,23 @@ 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(), @@ -57,18 +63,34 @@ public function addConstructorDependency(Class_ $class, PropertyMetadata $proper ); } - 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 index b049723948e..8cfb1e4ca7d 100644 --- 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 index cbbb0436da7..ff30a4ff5f0 100644 --- 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 new file mode 100644 index 00000000000..6cc977287ad --- /dev/null +++ b/tests/Issues/AddClassDependency/AddClassDependencyTest.php @@ -0,0 +1,28 @@ +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 new file mode 100644 index 00000000000..a57efc90db8 --- /dev/null +++ b/tests/Issues/AddClassDependency/Fixture/fixture.php.inc @@ -0,0 +1,57 @@ +someAutowiredService = $someAutowiredService; + } + + public function configure() + { + $someType = $this->get('validator'); + } +} + +?> +----- +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 new file mode 100644 index 00000000000..8c4ca1776e4 --- /dev/null +++ b/tests/Issues/AddClassDependency/Source/SomeAutowiredService.php @@ -0,0 +1,9 @@ +withRules([ + \Rector\Symfony\DependencyInjection\Rector\Class_\GetBySymfonyStringToConstructorInjectionRector::class, + ]); diff --git a/tests/NodeManipulator/ClassDependencyManipulatorTest.php b/tests/NodeManipulator/ClassDependencyManipulatorTest.php index d93d2857fc2..beec1f39651 100644 --- 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 index 4d9bd55e8cd..92cae65cfb6 100644 --- 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 index cfd440f5efe..c6e3fb6f60e 100644 --- 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 index 3893fd1d8bb..dad864579af 100644 --- 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 index 6027ac82af9..380a81130fe 100644 --- 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 index d2374712fb1..90d546ad8a6 100644 --- 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) { } }