[DocBlock] Update docblock contents right in the rule (#4999)

* Make doc importing return bool based on return value * cleanup test without existing classes, make rename docblock return bool value * update RenameAnnotationRector right in the rule * update last promoted properties case * remove only failing fixture, to finalize in-rule doc block printer * misc * fix return bool at PhpDocClassRenamer * Update rules/Php80/DocBlock/PropertyPromotionDocBlockMerger.php * change param return bool * [ci-review] Rector Rectify * Fix * FIx * FIx * Fix different variable name used * eol * add back fixture tests --------- Co-authored-by: Abdul Malik Ikhsan <samsonasik@gmail.com> Co-authored-by: GitHub Action <actions@github.com>
rectorphp · Sep 11, 2023 · a2f7005 · a2f7005
1 parent 596bbc9
commit a2f7005
Show file tree

Hide file tree

Showing 24 changed files with 235 additions and 212 deletions.
diff --git a/packages/BetterPhpDocParser/PhpDocInfo/PhpDocInfo.php b/packages/BetterPhpDocParser/PhpDocInfo/PhpDocInfo.php
@@ -27,7 +27,6 @@
 use Rector\BetterPhpDocParser\PhpDoc\DoctrineAnnotationTagValueNode;
 use Rector\BetterPhpDocParser\PhpDoc\SpacelessPhpDocTagNode;
 use Rector\BetterPhpDocParser\PhpDocNodeFinder\PhpDocNodeByTypeFinder;
-use Rector\BetterPhpDocParser\PhpDocNodeVisitor\ChangedPhpDocNodeVisitor;
 use Rector\BetterPhpDocParser\ValueObject\Parser\BetterTokenIterator;
 use Rector\BetterPhpDocParser\ValueObject\PhpDocAttributeKey;
 use Rector\BetterPhpDocParser\ValueObject\Type\ShortenedIdentifierTypeNode;
@@ -54,8 +53,6 @@ final class PhpDocInfo
 
     private readonly PhpDocNode $originalPhpDocNode;
 
-    private bool $hasChanged = false;
-
     public function __construct(
         private readonly PhpDocNode $phpDocNode,
         private readonly BetterTokenIterator $betterTokenIterator,
@@ -374,30 +371,11 @@ public function getTemplateTagValueNodes(): array
      * @deprecated Change doc block and print directly in the node instead
      * @internal
      * Should be handled by attributes of phpdoc node - if stard_and_end is missing in one of nodes, it has been changed
-     * Similar to missing original node in php-aprser
+     *
+     * @api
      */
     public function markAsChanged(): void
     {
-        $this->hasChanged = true;
-    }
-
-    public function hasChanged(): bool
-    {
-        if ($this->isNewNode()) {
-            return true;
-        }
-
-        if ($this->hasChanged) {
-            return true;
-        }
-
-        // has a single node with missing start_end
-        $phpDocNodeTraverser = new PhpDocNodeTraverser();
-        $changedPhpDocNodeVisitor = new ChangedPhpDocNodeVisitor();
-        $phpDocNodeTraverser->addPhpDocNodeVisitor($changedPhpDocNodeVisitor);
-        $phpDocNodeTraverser->traverse($this->phpDocNode);
-
-        return $changedPhpDocNodeVisitor->hasChanged();
     }
 
     public function makeMultiLined(): void

diff --git a/packages/BetterPhpDocParser/PhpDocManipulator/PhpDocClassRenamer.php b/packages/BetterPhpDocParser/PhpDocManipulator/PhpDocClassRenamer.php
@@ -27,17 +27,19 @@ public function __construct(
      *
      * @param string[] $oldToNewClasses
      */
-    public function changeTypeInAnnotationTypes(Node $node, PhpDocInfo $phpDocInfo, array $oldToNewClasses): void
+    public function changeTypeInAnnotationTypes(Node $node, PhpDocInfo $phpDocInfo, array $oldToNewClasses, bool &$hasChanged): bool
     {
-        $this->processAssertChoiceTagValueNode($oldToNewClasses, $phpDocInfo);
-        $this->processDoctrineRelationTagValueNode($node, $oldToNewClasses, $phpDocInfo);
-        $this->processSerializerTypeTagValueNode($oldToNewClasses, $phpDocInfo);
+        $this->processAssertChoiceTagValueNode($oldToNewClasses, $phpDocInfo, $hasChanged);
+        $this->processDoctrineRelationTagValueNode($node, $oldToNewClasses, $phpDocInfo, $hasChanged);
+        $this->processSerializerTypeTagValueNode($oldToNewClasses, $phpDocInfo, $hasChanged);
+
+        return $hasChanged;
     }
 
     /**
      * @param array<string, string> $oldToNewClasses
      */
-    private function processAssertChoiceTagValueNode(array $oldToNewClasses, PhpDocInfo $phpDocInfo): void
+    private function processAssertChoiceTagValueNode(array $oldToNewClasses, PhpDocInfo $phpDocInfo, bool &$hasChanged): void
     {
         $assertChoiceDoctrineAnnotationTagValueNode = $phpDocInfo->findOneByAnnotationClass(
             'Symfony\Component\Validator\Constraints\Choice'
@@ -75,6 +77,7 @@ private function processAssertChoiceTagValueNode(array $oldToNewClasses, PhpDocI
 
             // trigger reprint
             $classNameArrayItemNode->setAttribute(PhpDocAttributeKey::ORIG_NODE, null);
+            $hasChanged = true;
             break;
         }
     }
@@ -85,7 +88,8 @@ private function processAssertChoiceTagValueNode(array $oldToNewClasses, PhpDocI
     private function processDoctrineRelationTagValueNode(
         Node $node,
         array $oldToNewClasses,
-        PhpDocInfo $phpDocInfo
+        PhpDocInfo $phpDocInfo,
+        bool &$hasChanged
     ): void {
         $doctrineAnnotationTagValueNode = $phpDocInfo->getByAnnotationClasses([
             'Doctrine\ORM\Mapping\OneToMany',
@@ -97,13 +101,13 @@ private function processDoctrineRelationTagValueNode(
             return;
         }
 
-        $this->processDoctrineToMany($doctrineAnnotationTagValueNode, $node, $oldToNewClasses);
+        $this->processDoctrineToMany($doctrineAnnotationTagValueNode, $node, $oldToNewClasses, $hasChanged);
     }
 
     /**
      * @param array<string, string> $oldToNewClasses
      */
-    private function processSerializerTypeTagValueNode(array $oldToNewClasses, PhpDocInfo $phpDocInfo): void
+    private function processSerializerTypeTagValueNode(array $oldToNewClasses, PhpDocInfo $phpDocInfo, bool &$hasChanged): void
     {
         $doctrineAnnotationTagValueNode = $phpDocInfo->findOneByAnnotationClass('JMS\Serializer\Annotation\Type');
         if (! $doctrineAnnotationTagValueNode instanceof DoctrineAnnotationTagValueNode) {
@@ -128,6 +132,8 @@ private function processSerializerTypeTagValueNode(array $oldToNewClasses, PhpDo
                 );
 
                 $classNameArrayItemNode->setAttribute(PhpDocAttributeKey::ORIG_NODE, null);
+
+                $hasChanged = true;
             }
 
             $currentTypeArrayItemNode = $doctrineAnnotationTagValueNode->getValue('type');
@@ -142,6 +148,8 @@ private function processSerializerTypeTagValueNode(array $oldToNewClasses, PhpDo
 
             if ($currentTypeStringNode->value === $oldClass) {
                 $currentTypeStringNode->value = $newClass;
+
+                $hasChanged = true;
             }
         }
     }
@@ -152,7 +160,8 @@ private function processSerializerTypeTagValueNode(array $oldToNewClasses, PhpDo
     private function processDoctrineToMany(
         DoctrineAnnotationTagValueNode $doctrineAnnotationTagValueNode,
         Node $node,
-        array $oldToNewClasses
+        array $oldToNewClasses,
+        bool &$hasChanged
     ): void {
         $classKey = $doctrineAnnotationTagValueNode->hasClassName(
             'Doctrine\ORM\Mapping\Embedded'
@@ -180,6 +189,8 @@ private function processDoctrineToMany(
 
             $targetEntityStringNode->value = $newClass;
             $targetEntityArrayItemNode->setAttribute(PhpDocAttributeKey::ORIG_NODE, null);
+
+            $hasChanged = true;
         }
     }
 }
diff --git a/packages/BetterPhpDocParser/PhpDocManipulator/PhpDocTypeChanger.php b/packages/BetterPhpDocParser/PhpDocManipulator/PhpDocTypeChanger.php
@@ -137,14 +137,14 @@ public function changeParamType(
         Type $newType,
         Param $param,
         string $paramName
-    ): void {
+    ): bool {
         // better skip, could crash hard
         if ($phpDocInfo->hasInvalidTag('@param')) {
-            return;
+            return false;
         }
 
         if (! $this->newPhpDocFromPHPStanTypeGuard->isLegal($newType)) {
-            return;
+            return false;
         }
 
         $phpDocTypeNode = $this->staticTypeMapper->mapPHPStanTypeToPHPStanPhpDocTypeNode($newType);
@@ -160,11 +160,11 @@ public function changeParamType(
 
             // avoid overriding better type
             if ($this->typeComparator->isSubtype($currentType, $newType)) {
-                return;
+                return false;
             }
 
             if ($this->typeComparator->areTypesEqual($currentType, $newType)) {
-                return;
+                return false;
             }
 
             $paramTagValueNode->type = $phpDocTypeNode;
@@ -174,6 +174,8 @@ public function changeParamType(
         }
 
         $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($functionLike);
+
+        return true;
     }
 
     public function isAllowed(TypeNode $typeNode): bool

diff --git a/packages/BetterPhpDocParser/PhpDocManipulator/PropertyDocBlockManipulator.php b/packages/BetterPhpDocParser/PhpDocManipulator/PropertyDocBlockManipulator.php
diff --git a/packages/Comments/NodeDocBlock/DocBlockUpdater.php b/packages/Comments/NodeDocBlock/DocBlockUpdater.php
@@ -18,27 +18,6 @@ public function __construct(
     ) {
     }
 
-    public function updateNodeWithPhpDocInfo(Node $node): void
-    {
-        // nothing to change? don't save it
-        $phpDocInfo = $this->resolveChangedPhpDocInfo($node);
-        if (! $phpDocInfo instanceof PhpDocInfo) {
-            return;
-        }
-
-        $phpDoc = $this->printPhpDocInfoToString($phpDocInfo);
-
-        // make sure, that many separated comments are not removed
-        if ($phpDoc === '') {
-            $this->setCommentsAttribute($node);
-
-            return;
-        }
-
-        // this is needed to remove duplicated // commentsAsText
-        $node->setDocComment(new Doc($phpDoc));
-    }
-
     public function updateRefactoredNodeWithPhpDocInfo(Node $node): void
     {
         // nothing to change? don't save it
@@ -63,20 +42,6 @@ private function setCommentsAttribute(Node $node): void
         $node->setAttribute(AttributeKey::COMMENTS, $comments);
     }
 
-    private function resolveChangedPhpDocInfo(Node $node): ?PhpDocInfo
-    {
-        $phpDocInfo = $node->getAttribute(AttributeKey::PHP_DOC_INFO);
-        if (! $phpDocInfo instanceof PhpDocInfo) {
-            return null;
-        }
-
-        if (! $phpDocInfo->hasChanged()) {
-            return null;
-        }
-
-        return $phpDocInfo;
-    }
-
     private function printPhpDocInfoToString(PhpDocInfo $phpDocInfo): string
     {
         if ($phpDocInfo->isNewNode()) {

diff --git a/packages/NodeTypeResolver/PhpDoc/NodeAnalyzer/DocBlockClassRenamer.php b/packages/NodeTypeResolver/PhpDoc/NodeAnalyzer/DocBlockClassRenamer.php
@@ -19,10 +19,10 @@ public function __construct(
     /**
      * @param OldToNewType[] $oldToNewTypes
      */
-    public function renamePhpDocType(PhpDocInfo $phpDocInfo, array $oldToNewTypes): void
+    public function renamePhpDocType(PhpDocInfo $phpDocInfo, array $oldToNewTypes): bool
     {
         if ($oldToNewTypes === []) {
-            return;
+            return false;
         }
 
         $phpDocNodeTraverser = new PhpDocNodeTraverser();
@@ -31,5 +31,7 @@ public function renamePhpDocType(PhpDocInfo $phpDocInfo, array $oldToNewTypes):
         $this->classRenamePhpDocNodeVisitor->setOldToNewTypes($oldToNewTypes);
 
         $phpDocNodeTraverser->traverse($phpDocInfo->getPhpDocNode());
+
+        return $this->classRenamePhpDocNodeVisitor->hasChanged();
     }
 }
diff --git a/packages/NodeTypeResolver/PhpDoc/NodeAnalyzer/DocBlockNameImporter.php b/packages/NodeTypeResolver/PhpDoc/NodeAnalyzer/DocBlockNameImporter.php
@@ -16,16 +16,18 @@ public function __construct(
     ) {
     }
 
-    public function importNames(PhpDocNode $phpDocNode, Node $node): void
+    public function importNames(PhpDocNode $phpDocNode, Node $node): bool
     {
         if ($phpDocNode->children === []) {
-            return;
+            return false;
         }
 
         $this->nameImportingPhpDocNodeVisitor->setCurrentNode($node);
 
         $phpDocNodeTraverser = new PhpDocNodeTraverser();
         $phpDocNodeTraverser->addPhpDocNodeVisitor($this->nameImportingPhpDocNodeVisitor);
         $phpDocNodeTraverser->traverse($phpDocNode);
+
+        return $this->nameImportingPhpDocNodeVisitor->hasChanged();
     }
 }
diff --git a/packages/NodeTypeResolver/PhpDocNodeVisitor/ClassRenamePhpDocNodeVisitor.php b/packages/NodeTypeResolver/PhpDocNodeVisitor/ClassRenamePhpDocNodeVisitor.php
@@ -33,6 +33,8 @@ final class ClassRenamePhpDocNodeVisitor extends AbstractPhpDocNodeVisitor
      */
     private array $oldToNewTypes = [];
 
+    private bool $hasChanged = false;
+
     public function __construct(
         private readonly StaticTypeMapper $staticTypeMapper,
         private readonly CurrentNodeProvider $currentNodeProvider,
@@ -45,6 +47,8 @@ public function beforeTraverse(Node $node): void
         if ($this->oldToNewTypes === []) {
             throw new ShouldNotHappenException('Configure "$oldToNewClasses" first');
         }
+
+        $this->hasChanged = false;
     }
 
     public function enterNode(Node $node): ?Node
@@ -90,6 +94,8 @@ public function enterNode(Node $node): ?Node
                 $newTypeNode->setAttribute(PhpDocAttributeKey::PARENT, $parentType);
             }
 
+            $this->hasChanged = true;
+
             return $newTypeNode;
         }
 
@@ -104,6 +110,11 @@ public function setOldToNewTypes(array $oldToNewTypes): void
         $this->oldToNewTypes = $oldToNewTypes;
     }
 
+    public function hasChanged(): bool
+    {
+        return $this->hasChanged;
+    }
+
     private function resolveNamespacedName(
         IdentifierTypeNode $identifierTypeNode,
         PhpParserNode $phpParserNode,

diff --git a/packages/NodeTypeResolver/PhpDocNodeVisitor/NameImportingPhpDocNodeVisitor.php b/packages/NodeTypeResolver/PhpDocNodeVisitor/NameImportingPhpDocNodeVisitor.php
@@ -29,6 +29,8 @@ final class NameImportingPhpDocNodeVisitor extends AbstractPhpDocNodeVisitor
 {
     private ?PhpParserNode $currentPhpParserNode = null;
 
+    private bool $hasChanged = false;
+
     public function __construct(
         private readonly ClassNameImportSkipper $classNameImportSkipper,
         private readonly UseNodesToAddCollector $useNodesToAddCollector,
@@ -61,7 +63,6 @@ public function enterNode(Node $node): ?Node
         }
 
         $staticType = $this->identifierTypeMapper->mapIdentifierTypeNode($node, $this->currentPhpParserNode);
-
         if (! $staticType instanceof FullyQualifiedObjectType) {
             return null;
         }
@@ -85,9 +86,15 @@ public function enterNode(Node $node): ?Node
 
     public function setCurrentNode(PhpParserNode $phpParserNode): void
     {
+        $this->hasChanged = false;
         $this->currentPhpParserNode = $phpParserNode;
     }
 
+    public function hasChanged(): bool
+    {
+        return $this->hasChanged;
+    }
+
     private function processFqnNameImport(
         PhpParserNode $phpParserNode,
         IdentifierTypeNode $identifierTypeNode,
@@ -100,6 +107,7 @@ private function processFqnNameImport(
             return null;
         }
 
+        // standardize to FQN
         if (str_starts_with($fullyQualifiedObjectType->getClassName(), '@')) {
             $fullyQualifiedObjectType = new FullyQualifiedObjectType(ltrim(
                 $fullyQualifiedObjectType->getClassName(),
@@ -125,6 +133,8 @@ private function processFqnNameImport(
 
         if ($this->shouldImport($newNode, $identifierTypeNode, $fullyQualifiedObjectType)) {
             $this->useNodesToAddCollector->addUseImport($fullyQualifiedObjectType);
+            $this->hasChanged = true;
+
             return $newNode;
         }