Fix RenamePropertyToMatchTypeRector for non-docblocks properties (#3189)

Co-authored-by: GitHub Action <action@github.com>

packages-tests/BetterPhpDocParser/PhpDocParser/ClassAnnotationMatcher/ResolveTagToKnownFullyQualifiedNameTest.php

@@ -22,7 +22,7 @@ final class ResolveTagToKnownFullyQualifiedNameTest extends AbstractTestCase
 
     private TestingParser $testingParser;
 
-    private BetterNodeFinder $nodeFinder;
+    private BetterNodeFinder $betterNodeFinder;
 
     private PhpDocInfoFactory $phpDocInfoFactory;
 
@@ -34,7 +34,7 @@ protected function setUp(): void
 
         $this->classAnnotationMatcher = $this->getService(ClassAnnotationMatcher::class);
         $this->testingParser = $this->getService(TestingParser::class);
-        $this->nodeFinder = $this->getService(BetterNodeFinder::class);
+        $this->betterNodeFinder = $this->getService(BetterNodeFinder::class);
         $this->phpDocInfoFactory = $this->getService(PhpDocInfoFactory::class);
         $this->nodeNameResolver = $this->getService(NodeNameResolver::class);
     }
@@ -45,7 +45,7 @@ protected function setUp(): void
     public function testResolvesClass(string $filePath): void
     {
         $nodes = $this->testingParser->parseFileToDecoratedNodes($filePath);
-        $properties = $this->nodeFinder->findInstancesOf($nodes, [Property::class]);
+        $properties = $this->betterNodeFinder->findInstancesOf($nodes, [Property::class]);
 
         foreach ($properties as $property) {
             /** @var Property $property */

packages/BetterPhpDocParser/PhpDocInfo/PhpDocInfoFactory.php

@@ -66,8 +66,9 @@ public function createFromNode(Node $node): ?PhpDocInfo
         $this->currentNodeProvider->setNode($node);
 
         $docComment = $node->getDocComment();
+
         if (! $docComment instanceof Doc) {
-            if ($node->getComments() !== []) {
+            if ($node->getComments() === []) {
                 return null;
             }
 

rules-tests/Naming/Naming/UseImportsResolver/UseImportsResolverTest.php

@@ -21,14 +21,14 @@ final class UseImportsResolverTest extends AbstractTestCase
 
     private TestingParser $testingParser;
 
-    private BetterNodeFinder $nodeFinder;
+    private BetterNodeFinder $betterNodeFinder;
 
     protected function setUp(): void
     {
         $this->boot();
         $this->useImportsResolver = $this->getService(UseImportsResolver::class);
         $this->testingParser = $this->getService(TestingParser::class);
-        $this->nodeFinder = $this->getService(BetterNodeFinder::class);
+        $this->betterNodeFinder = $this->getService(BetterNodeFinder::class);
     }
 
     /**
@@ -38,7 +38,7 @@ public function testUsesFromProperty(string $filePath): void
     {
         $nodes = $this->testingParser->parseFileToDecoratedNodes($filePath);
 
-        $firstProperty = $this->nodeFinder->findFirstInstanceOf($nodes, Property::class);
+        $firstProperty = $this->betterNodeFinder->findFirstInstanceOf($nodes, Property::class);
         $resolvedUses = $this->useImportsResolver->resolveForNode($firstProperty);
 
         $stringUses = [];

rules-tests/Naming/Rector/Class_/RenamePropertyToMatchTypeRector/Fixture/change_bare_property.php.inc

@@ -0,0 +1,21 @@
+<?php
+
+namespace Rector\Tests\Naming\Rector\Class_\RenamePropertyToMatchTypeRector\Fixture;
+
+final class ChangeBareProperty
+{
+    private ?\PDO $_pdo;
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Naming\Rector\Class_\RenamePropertyToMatchTypeRector\Fixture;
+
+final class ChangeBareProperty
+{
+    private ?\PDO $pdo;
+}
+
+?>

rules-tests/Naming/Rector/Class_/RenamePropertyToMatchTypeRector/Fixture/property_type_over_docblock.php.inc

@@ -0,0 +1,33 @@
+<?php
+
+namespace Rector\Tests\Naming\Rector\Class_\RenamePropertyToMatchTypeRector\Fixture;
+
+use PhpParser\ParserFactory as NikicParserFactory;
+use Rector\Tests\Naming\Rector\Class_\RenamePropertyToMatchTypeRector\Source\GitWrapper;
+
+final class PropertyTypeOverDocblock
+{
+    /**
+     * @var NikicParserFactory
+     */
+    private GitWrapper $whatever;
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Naming\Rector\Class_\RenamePropertyToMatchTypeRector\Fixture;
+
+use PhpParser\ParserFactory as NikicParserFactory;
+use Rector\Tests\Naming\Rector\Class_\RenamePropertyToMatchTypeRector\Source\GitWrapper;
+
+final class PropertyTypeOverDocblock
+{
+    /**
+     * @var NikicParserFactory
+     */
+    private GitWrapper $gitWrapper;
+}
+
+?>

rules-tests/Naming/ValueObjectFactory/PropertyRenameFactory/Fixture/skip_some_class.php.inc

@@ -6,7 +6,7 @@ namespace Rector\Tests\Naming\ValueObjectFactory\PropertyRenameFactory\Fixture;
 
 use Rector\Tests\Naming\ValueObjectFactory\PropertyRenameFactory\Source\EliteManager;
 
-class SkipSomeClass
+final class SkipSomeClass
 {
     /**
      * @var EliteManager

rules-tests/Naming/ValueObjectFactory/PropertyRenameFactory/PropertyRenameFactoryTest.php

@@ -5,8 +5,7 @@
 namespace Rector\Tests\Naming\ValueObjectFactory\PropertyRenameFactory;
 
 use Iterator;
-use PhpParser\Node\Stmt\Property;
-use Rector\Core\Exception\ShouldNotHappenException;
+use PhpParser\Node\Stmt\Class_;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\FileSystemRector\Parser\FileInfoParser;
 use Rector\Naming\ExpectedNameResolver\MatchPropertyTypeExpectedNameResolver;
@@ -42,9 +41,14 @@ protected function setUp(): void
      */
     public function test(string $filePathWithProperty, string $expectedName, string $currentName): void
     {
-        $property = $this->getPropertyFromFilePath($filePathWithProperty);
+        $nodes = $this->fileInfoParser->parseFileInfoToNodesAndDecorate($filePathWithProperty);
 
-        $expectedPropertyName = $this->matchPropertyTypeExpectedNameResolver->resolve($property);
+        /** @var Class_ $class */
+        $class = $this->betterNodeFinder->findFirstInstanceOf($nodes, Class_::class);
+
+        $property = $class->getProperty($currentName);
+
+        $expectedPropertyName = $this->matchPropertyTypeExpectedNameResolver->resolve($property, $class);
         if ($expectedPropertyName === null) {
             return;
         }
@@ -62,16 +66,4 @@ public function provideData(): Iterator
     {
         yield [__DIR__ . '/Fixture/skip_some_class.php.inc', 'eliteManager', 'eventManager'];
     }
-
-    private function getPropertyFromFilePath(string $filePath): Property
-    {
-        $nodes = $this->fileInfoParser->parseFileInfoToNodesAndDecorate($filePath);
-
-        $property = $this->betterNodeFinder->findFirstInstanceOf($nodes, Property::class);
-        if (! $property instanceof Property) {
-            throw new ShouldNotHappenException();
-        }
-
-        return $property;
-    }
 }

rules/Naming/ExpectedNameResolver/MatchPropertyTypeExpectedNameResolver.php

@@ -5,37 +5,37 @@
 namespace Rector\Naming\ExpectedNameResolver;
 
 use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\Property;
 use PHPStan\Reflection\ClassReflection;
+use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
 use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
 use Rector\Core\NodeManipulator\PropertyManipulator;
-use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\Core\Reflection\ReflectionResolver;
 use Rector\Naming\Naming\PropertyNaming;
 use Rector\Naming\ValueObject\ExpectedName;
 use Rector\NodeNameResolver\NodeNameResolver;
+use Rector\StaticTypeMapper\StaticTypeMapper;
 
 final class MatchPropertyTypeExpectedNameResolver
 {
     public function __construct(
         private readonly PropertyNaming $propertyNaming,
         private readonly PhpDocInfoFactory $phpDocInfoFactory,
         private readonly NodeNameResolver $nodeNameResolver,
-        private readonly BetterNodeFinder $betterNodeFinder,
         private readonly PropertyManipulator $propertyManipulator,
-        private readonly ReflectionResolver $reflectionResolver
+        private readonly ReflectionResolver $reflectionResolver,
+        private readonly StaticTypeMapper $staticTypeMapper
     ) {
     }
 
-    public function resolve(Property $property): ?string
+    public function resolve(Property $property, ClassLike $classLike): ?string
     {
-        $class = $this->betterNodeFinder->findParentType($property, Class_::class);
-        if (! $class instanceof Class_) {
+        if (! $classLike instanceof Class_) {
             return null;
         }
 
         $classReflection = $this->reflectionResolver->resolveClassReflection($property);
-
         if (! $classReflection instanceof ClassReflection) {
             return null;
         }
@@ -45,9 +45,7 @@ public function resolve(Property $property): ?string
             return null;
         }
 
-        $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($property);
-
-        $expectedName = $this->propertyNaming->getExpectedNameFromType($phpDocInfo->getVarType());
+        $expectedName = $this->resolveExpectedName($property);
         if (! $expectedName instanceof ExpectedName) {
             return null;
         }
@@ -60,4 +58,21 @@ public function resolve(Property $property): ?string
 
         return $expectedName->getName();
     }
+
+    private function resolveExpectedName(Property $property): ?ExpectedName
+    {
+        // property type first
+        if ($property->type instanceof \PhpParser\Node) {
+            $propertyType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($property->type);
+            return $this->propertyNaming->getExpectedNameFromType($propertyType);
+        }
+
+        // fallback to docblock
+        $phpDocInfo = $this->phpDocInfoFactory->createFromNode($property);
+        if ($phpDocInfo instanceof PhpDocInfo) {
+            return $this->propertyNaming->getExpectedNameFromType($phpDocInfo->getVarType());
+        }
+
+        return null;
+    }
 }

rules/Naming/Guard/PropertyConflictingNameGuard/MatchPropertyTypeConflictingNameGuard.php

@@ -32,7 +32,7 @@ public function resolve(ClassLike $classLike): array
     {
         $expectedNames = [];
         foreach ($classLike->getProperties() as $property) {
-            $expectedName = $this->matchPropertyTypeExpectedNameResolver->resolve($property);
+            $expectedName = $this->matchPropertyTypeExpectedNameResolver->resolve($property, $classLike);
             if ($expectedName === null) {
                 // fallback to existing name
                 $expectedName = $this->nodeNameResolver->getName($property);

rules/Naming/Rector/Class_/RenamePropertyToMatchTypeRector.php

@@ -103,7 +103,7 @@ public function refactor(Node $node): ?Node
     private function refactorClassProperties(ClassLike $classLike): void
     {
         foreach ($classLike->getProperties() as $property) {
-            $expectedPropertyName = $this->matchPropertyTypeExpectedNameResolver->resolve($property);
+            $expectedPropertyName = $this->matchPropertyTypeExpectedNameResolver->resolve($property, $classLike);
             if ($expectedPropertyName === null) {
                 continue;
             }

src/Reflection/ReflectionResolver.php

@@ -79,7 +79,6 @@ public function resolveClassReflection(?Node $node): ?ClassReflection
         }
 
         $scope = $node->getAttribute(AttributeKey::SCOPE);
-
         if (! $scope instanceof Scope) {
             return null;
         }

tests/Validation/Collector/EmptyConfigurableRectorCollector/ConfigurableArrayMissingTest.php

@@ -12,17 +12,17 @@
  */
 final class ConfigurableArrayMissingTest extends AbstractTestCase
 {
-    private EmptyConfigurableRectorCollector $collector;
+    private EmptyConfigurableRectorCollector $emptyConfigurableRectorCollector;
 
     protected function setUp(): void
     {
         $this->bootFromConfigFiles([__DIR__ . '/config/configurable_array_missing.php']);
-        $this->collector = $this->getService(EmptyConfigurableRectorCollector::class);
+        $this->emptyConfigurableRectorCollector = $this->getService(EmptyConfigurableRectorCollector::class);
     }
 
     public function test(): void
     {
-        $emptyConfigurableRectors = $this->collector->resolveEmptyConfigurableRectorClasses();
+        $emptyConfigurableRectors = $this->emptyConfigurableRectorCollector->resolveEmptyConfigurableRectorClasses();
         $this->assertCount(1, $emptyConfigurableRectors);
     }
 }