[DX] Make ClassAnnotationMatcher differentiate between known and unkn…

…own classes (#2319) Co-authored-by: Abdul Malik Ikhsan <samsonasik@gmail.com>
rectorphp · Jun 6, 2022 · df5bd5f · df5bd5f
1 parent 40d9102
commit df5bd5f
Show file tree

Hide file tree

Showing 11 changed files with 266 additions and 9 deletions.
diff --git a/...erPhpDocParser/PhpDocParser/ClassAnnotationMatcher/Fixture/ExistingClass/SiblingClass.php b/...erPhpDocParser/PhpDocParser/ClassAnnotationMatcher/Fixture/ExistingClass/SiblingClass.php
@@ -0,0 +1,8 @@
+<?php
+
+namespace Rector\Tests\BetterPhpDocParser\PhpDocParser\ClassAnnotationMatcher\Fixture\ExistingClass;
+
+class SiblingClass
+{
+
+}
diff --git a/...terPhpDocParser/PhpDocParser/ClassAnnotationMatcher/Fixture/ExistingClass/fixture.php.inc b/...terPhpDocParser/PhpDocParser/ClassAnnotationMatcher/Fixture/ExistingClass/fixture.php.inc
@@ -0,0 +1,23 @@
+<?php
+
+namespace Rector\Tests\BetterPhpDocParser\PhpDocParser\ClassAnnotationMatcher\Fixture\ExistingClass;
+
+use Rector\Tests\BetterPhpDocParser\PhpDocParser\ClassAnnotationMatcher\Source\KnownClass;
+use Rector\Tests\BetterPhpDocParser\PhpDocParser\ClassAnnotationMatcher\Source\KnownClass as AliasedKnownClass;
+use SplHeap as AliasedSplHeap;
+
+class MyClass
+{
+    /** @var KnownClass */
+    private $knownClass;
+    /** @var \Rector\Tests\BetterPhpDocParser\PhpDocParser\ClassAnnotationMatcher\Source\KnownClass */
+    private $knownInlinedClass;
+    /** @var SiblingClass */
+    private $knownSiblingClass;
+    /** @var AliasedKnownClass */
+    private $knownAliasedClass;
+    /** @var \SplHeap */
+    private $knownGlobalClass;
+    /** @var AliasedSplHeap */
+    private $knownAliasedGlobalClass;
+}
diff --git a/...PhpDocParser/PhpDocParser/ClassAnnotationMatcher/Fixture/NonExistingClass/fixture.php.inc b/...PhpDocParser/PhpDocParser/ClassAnnotationMatcher/Fixture/NonExistingClass/fixture.php.inc
@@ -0,0 +1,20 @@
+<?php
+
+namespace Rector\Tests\BetterPhpDocParser\PhpDocParser\ClassAnnotationMatcher\Fixture\NonExistingClass;
+
+use My\Namespace\OtherUnknownClass;
+use My\Namespace\OtherUnknownClass as AliasedUnknownClass;
+
+class MyClass
+{
+    /** @var UnknownClass */
+    private $unknownClass;
+    /** @var \My\Namespace\UnknownClass */
+    private $unknownNamespacedClass;
+    /** @var OtherUnknownClass */
+    private $unknownUsedClass;
+    /** @var AliasedUnknownClass */
+    private $unknownAliasedUsedClass;
+    /** @var \UnknownGlobalClass */
+    private $unknownGlobalClass;
+}
diff --git a/...DocParser/PhpDocParser/ClassAnnotationMatcher/ResolveTagToKnownFullyQualifiedNameTest.php b/...DocParser/PhpDocParser/ClassAnnotationMatcher/ResolveTagToKnownFullyQualifiedNameTest.php
@@ -0,0 +1,81 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\BetterPhpDocParser\PhpDocParser\ClassAnnotationMatcher;
+
+use Iterator;
+use PhpParser\Node\Stmt\Property;
+use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode;
+use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
+use Rector\BetterPhpDocParser\PhpDocParser\ClassAnnotationMatcher;
+use Rector\Core\Exception\ShouldNotHappenException;
+use Rector\Core\PhpParser\Node\BetterNodeFinder;
+use Rector\NodeNameResolver\NodeNameResolver;
+use Rector\Testing\PHPUnit\AbstractTestCase;
+use Rector\Testing\TestingParser\TestingParser;
+use Symplify\EasyTesting\DataProvider\StaticFixtureFinder;
+use Symplify\SmartFileSystem\SmartFileInfo;
+
+final class ResolveTagToKnownFullyQualifiedNameTest extends AbstractTestCase
+{
+    private ClassAnnotationMatcher $classAnnotationMatcher;
+
+    private TestingParser $testingParser;
+
+    private BetterNodeFinder $nodeFinder;
+
+    private PhpDocInfoFactory $phpDocInfoFactory;
+
+    private NodeNameResolver $nodeNameResolver;
+
+    protected function setUp(): void
+    {
+        $this->boot();
+
+        $this->classAnnotationMatcher = $this->getService(ClassAnnotationMatcher::class);
+        $this->testingParser = $this->getService(TestingParser::class);
+        $this->nodeFinder = $this->getService(BetterNodeFinder::class);
+        $this->phpDocInfoFactory = $this->getService(PhpDocInfoFactory::class);
+        $this->nodeNameResolver = $this->getService(NodeNameResolver::class);
+    }
+
+    /**
+     * @dataProvider provideData()
+     */
+    public function testResolvesClass(SmartFileInfo $file): void
+    {
+        $nodes = $this->testingParser->parseFileToDecoratedNodes($file->getRelativeFilePath());
+        $properties = $this->nodeFinder->findInstancesOf($nodes, [Property::class]);
+
+        foreach ($properties as $property) {
+            /** @var Property $property */
+            $phpDoc = $this->phpDocInfoFactory->createFromNodeOrEmpty($property);
+            /** @var VarTagValueNode $varTag */
+            $varTag = $phpDoc->getByType(VarTagValueNode::class)[0];
+            $value = $varTag->type->__toString();
+            $propertyName = strtolower($this->nodeNameResolver->getName($property));
+
+            $result = $this->classAnnotationMatcher->resolveTagToKnownFullyQualifiedName($value, $property);
+            if (str_starts_with($propertyName, 'unknown')) {
+                $this->assertNull($result);
+            } elseif (str_contains($propertyName, 'aliased')) {
+                $unaliasedClass = str_replace('Aliased', '', $value);
+                $this->assertStringEndsWith($unaliasedClass, $result ?? '');
+            } elseif (str_starts_with($propertyName, 'known')) {
+                $this->assertStringEndsWith($value, $result ?? '');
+            } else {
+                throw new ShouldNotHappenException('All Variables should start with "known" or "unknown"!');
+            }
+        }
+    }
+
+    /**
+     * @return Iterator<SmartFileInfo>
+     */
+    public function provideData(): Iterator
+    {
+        $directory = __DIR__ . '/Fixture';
+        return StaticFixtureFinder::yieldDirectory($directory);
+    }
+}
diff --git a/packages-tests/BetterPhpDocParser/PhpDocParser/ClassAnnotationMatcher/Source/KnownClass.php b/packages-tests/BetterPhpDocParser/PhpDocParser/ClassAnnotationMatcher/Source/KnownClass.php
@@ -0,0 +1,8 @@
+<?php
+
+namespace Rector\Tests\BetterPhpDocParser\PhpDocParser\ClassAnnotationMatcher\Source;
+
+class KnownClass
+{
+
+}
diff --git a/packages/BetterPhpDocParser/PhpDocParser/ClassAnnotationMatcher.php b/packages/BetterPhpDocParser/PhpDocParser/ClassAnnotationMatcher.php
@@ -31,8 +31,21 @@ public function __construct(
     ) {
     }
 
-    public function resolveTagFullyQualifiedName(string $tag, Node $node): string
+    public function resolveTagToKnownFullyQualifiedName(string $tag, Node $node): ?string
     {
+        return $this->_resolveTagFullyQualifiedName($tag, $node, true);
+    }
+
+    public function resolveTagFullyQualifiedName(string $tag, Node $node): ?string
+    {
+        return $this->_resolveTagFullyQualifiedName($tag, $node, false);
+    }
+
+    private function _resolveTagFullyQualifiedName(
+        string $tag,
+        Node $node,
+        bool $returnNullOnUnknownClass
+    ): ?string {
         $uniqueHash = $tag . spl_object_hash($node);
         if (isset($this->fullyQualifiedNameByHash[$uniqueHash])) {
             return $this->fullyQualifiedNameByHash[$uniqueHash];
@@ -41,7 +54,15 @@ public function resolveTagFullyQualifiedName(string $tag, Node $node): string
         $tag = ltrim($tag, '@');
 
         $uses = $this->useImportsResolver->resolveForNode($node);
-        $fullyQualifiedClass = $this->resolveFullyQualifiedClass($uses, $node, $tag);
+        $fullyQualifiedClass = $this->resolveFullyQualifiedClass($uses, $node, $tag, $returnNullOnUnknownClass);
+
+        if ($fullyQualifiedClass === null) {
+            if ($returnNullOnUnknownClass) {
+                return null;
+            }
+
+            $fullyQualifiedClass = $tag;
+        }
 
         $this->fullyQualifiedNameByHash[$uniqueHash] = $fullyQualifiedClass;
 
@@ -51,7 +72,7 @@ public function resolveTagFullyQualifiedName(string $tag, Node $node): string
     /**
      * @param Use_[]|GroupUse[] $uses
      */
-    private function resolveFullyQualifiedClass(array $uses, Node $node, string $tag): string
+    private function resolveFullyQualifiedClass(array $uses, Node $node, string $tag, bool $returnNullOnUnknownClass): ?string
     {
         $scope = $node->getAttribute(AttributeKey::SCOPE);
 
@@ -64,18 +85,24 @@ private function resolveFullyQualifiedClass(array $uses, Node $node, string $tag
                 }
 
                 if (! str_contains($tag, '\\')) {
-                    return $this->resolveAsAliased($uses, $tag);
+                    return $this->resolveAsAliased($uses, $tag, $returnNullOnUnknownClass);
+                }
+
+                if (str_starts_with($tag, '\\') && $this->reflectionProvider->hasClass($tag)) {
+                    // Global or absolute Class
+                    return $tag;
                 }
             }
         }
 
-        return $this->useImportNameMatcher->matchNameWithUses($tag, $uses) ?? $tag;
+        $class = $this->useImportNameMatcher->matchNameWithUses($tag, $uses);
+        return $this->resolveClass($class, $returnNullOnUnknownClass);
     }
 
     /**
      * @param Use_[]|GroupUse[] $uses
      */
-    private function resolveAsAliased(array $uses, string $tag): string
+    private function resolveAsAliased(array $uses, string $tag, bool $returnNullOnUnknownClass): ?string
     {
         foreach ($uses as $use) {
             $prefix = $use instanceof GroupUse
@@ -88,11 +115,22 @@ private function resolveAsAliased(array $uses, string $tag): string
                 }
 
                 if ($useUse->alias->toString() === $tag) {
-                    return $prefix . $useUse->name->toString();
+                    $class = $prefix . $useUse->name->toString();
+                    return $this->resolveClass($class, $returnNullOnUnknownClass);
                 }
             }
         }
 
-        return $this->useImportNameMatcher->matchNameWithUses($tag, $uses) ?? $tag;
+        $class = $this->useImportNameMatcher->matchNameWithUses($tag, $uses);
+        return $this->resolveClass($class, $returnNullOnUnknownClass);
+    }
+
+    private function resolveClass(?string $class, bool $returnNullOnUnknownClass): ?string
+    {
+        if (null === $class) {
+            return null;
+        }
+        $resolvedClass = $this->reflectionProvider->hasClass($class) ? $class : null;
+        return $returnNullOnUnknownClass ? $resolvedClass : $class;
     }
 }
diff --git a/packages/BetterPhpDocParser/PhpDocParser/DoctrineAnnotationDecorator.php b/packages/BetterPhpDocParser/PhpDocParser/DoctrineAnnotationDecorator.php
@@ -175,7 +175,7 @@ private function transformGenericTagValueNodesToDoctrineAnnotationTagValueNodes(
             }
 
             // known doc tag to annotation class
-            $fullyQualifiedAnnotationClass = $this->classAnnotationMatcher->resolveTagFullyQualifiedName(
+            $fullyQualifiedAnnotationClass = (string) $this->classAnnotationMatcher->resolveTagFullyQualifiedName(
                 $phpDocChildNode->name,
                 $currentPhpNode
             );

diff --git a/tests/Issues/DoNotReplaceUnknownClasses/DoNotReplaceUnknownClassesTest.php b/tests/Issues/DoNotReplaceUnknownClasses/DoNotReplaceUnknownClassesTest.php
@@ -0,0 +1,31 @@
+<?php
+
+namespace Rector\Core\Tests\Issues\DoNotReplaceUnknownClasses;
+
+use Iterator;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+use Symplify\SmartFileSystem\SmartFileInfo;
+
+class DoNotReplaceUnknownClassesTest extends AbstractRectorTestCase
+{
+    /**
+     * @dataProvider provideData()
+     */
+    public function test(SmartFileInfo $fileInfo): void
+    {
+        $this->doTestFileInfo($fileInfo);
+    }
+
+    /**
+     * @return Iterator<SmartFileInfo>
+     */
+    public function provideData(): Iterator
+    {
+        return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture');
+    }
+
+    public function provideConfigFilePath(): string
+    {
+        return __DIR__ . '/config/configured_rule.php';
+    }
+}
diff --git a/tests/Issues/DoNotReplaceUnknownClasses/Fixture/skip_unknown_class_in_var.php.inc b/tests/Issues/DoNotReplaceUnknownClasses/Fixture/skip_unknown_class_in_var.php.inc
@@ -0,0 +1,19 @@
+<?php
+
+use My\Namespace\UnknownClass;
+
+final class SkipUnknownClassInVar
+{
+    private ?\My\Namespace\UnknownClass $otherClass = null;
+
+    public function getOtherClass(): ?\My\Namespace\UnknownClass
+    {
+        return $this->otherClass;
+    }
+
+    public function setOtherClass($otherClass): void
+    {
+        /** @var UnknownClass $otherClass */
+        $this->otherClass = $otherClass;
+    }
+}
diff --git a/tests/Issues/DoNotReplaceUnknownClasses/Fixture/skip_unknown_target_entity.php.inc b/tests/Issues/DoNotReplaceUnknownClasses/Fixture/skip_unknown_target_entity.php.inc
@@ -0,0 +1,15 @@
+<?php
+
+namespace Rector\Core\Tests\Issues\DoNotReplaceUnknownClasses\Fixture;
+
+use Doctrine\Common\Collections\Collection;
+use Doctrine\ORM\Mapping as ORM;
+use My\Namespace\UsedUnknownClass;
+
+final class SkipUnknownTargetEntity
+{
+    /**
+     * @ORM\ManyToOne(targetEntity=\My\Namespace\UsedUnknownClass::class)
+     */
+    private readonly ?Collection $items;
+}
diff --git a/tests/Issues/DoNotReplaceUnknownClasses/config/configured_rule.php b/tests/Issues/DoNotReplaceUnknownClasses/config/configured_rule.php
@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\Doctrine\Rector\Property\DoctrineTargetEntityStringToClassConstantRector;
+use Rector\TypeDeclaration\Rector\FunctionLike\ReturnTypeDeclarationRector;
+use Rector\TypeDeclaration\Rector\Property\TypedPropertyFromAssignsRector;
+
+return static function (RectorConfig $rectorConfig): void {
+    $rectorConfig->rule(DoctrineTargetEntityStringToClassConstantRector::class);
+    $rectorConfig->rule(TypedPropertyFromAssignsRector::class);
+    $rectorConfig->rule(ReturnTypeDeclarationRector::class);
+};