[Transform] Do not create a new node if the existing node is already …

…the correct one on AttributeKeyToClassConstFetchRector (#5328)

* Write testcase to show existing attributes are replaced by new nodes

* Do not replace existing constant nodes if they are already the correct ones

---------

Co-authored-by: SerethiX <[email protected]>

phpstan.neon

@@ -524,7 +524,7 @@ parameters:
 
         -
             path: rules/Transform/Rector/Attribute/AttributeKeyToClassConstFetchRector.php
-            message: "#Method \"processToClassConstFetch\\(\\)\" returns bool type, so the name should start with is/has/was#"
+            message: "#Method \"process(ToClassConstFetch|Arg)\\(\\)\" returns bool type, so the name should start with is/has/was#"
 
         # optional as changes behavior, should be used explicitly outside PHP upgrade
         - '#Register "Rector\\Php73\\Rector\\FuncCall\\JsonThrowOnErrorRector" service to "php73\.php" config set#'

rules-tests/Transform/Rector/Attribute/AttributeKeyToClassConstFetchRector/Fixture/not_replacing_existing_constant.php.inc

@@ -0,0 +1,15 @@
+<?php
+
+namespace Rector\Tests\Transform\Rector\Attribute\AttributeKeyToClassConstFetchRector\Fixture;
+
+use Doctrine\ORM\Mapping\Column;
+use Rector\Tests\Transform\Rector\Attribute\AttributeKeyToClassConstFetchRector\Source\Constant;
+use Rector\Tests\Transform\Rector\Attribute\AttributeKeyToClassConstFetchRector\Source\TestAttribute;
+
+class SomeClass
+{
+    #[TestAttribute(type: Constant::VALUE)]
+    public string $name;
+}
+
+?>

rules-tests/Transform/Rector/Attribute/AttributeKeyToClassConstFetchRector/Fixture/skip_already.php.inc

@@ -0,0 +1,15 @@
+<?php
+
+namespace Rector\Tests\Transform\Rector\Attribute\AttributeKeyToClassConstFetchRector\Fixture;
+
+use Doctrine\ORM\Mapping\Column;
+
+use Doctrine\DBAL\Types\Types;
+
+class SkipAlready
+{
+    #[Column(type: Types::STRING)]
+    public $name;
+}
+
+?>

rules-tests/Transform/Rector/Attribute/AttributeKeyToClassConstFetchRector/Source/Constant.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Transform\Rector\Attribute\AttributeKeyToClassConstFetchRector\Source;
+
+class Constant
+{
+    public const VALUE = 'value';
+}

rules-tests/Transform/Rector/Attribute/AttributeKeyToClassConstFetchRector/Source/TestAttribute.php

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Transform\Rector\Attribute\AttributeKeyToClassConstFetchRector\Source;
+
+use Attribute;
+
+#[Attribute(Attribute::TARGET_PROPERTY)]
+class TestAttribute
+{
+    public function __construct(
+        public readonly string $type,
+    ) {
+    }
+}

rules-tests/Transform/Rector/Attribute/AttributeKeyToClassConstFetchRector/config/configured_rule.php

@@ -12,5 +12,8 @@
             new AttributeKeyToClassConstFetch('Doctrine\ORM\Mapping\Column', 'type', 'Doctrine\DBAL\Types\Types', [
                 'string' => 'STRING',
             ]),
+            new AttributeKeyToClassConstFetch('Rector\Tests\Transform\Rector\Attribute\AttributeKeyToClassConstFetchRector\Source\TestAttribute', 'type', 'Rector\Tests\Transform\Rector\Attribute\AttributeKeyToClassConstFetchRector\Source\Constant', [
+                'value' => 'VALUE',
+            ]),
         ]);
 };

rules/Transform/Rector/Attribute/AttributeKeyToClassConstFetchRector.php

@@ -132,6 +132,7 @@ private function processToClassConstFetch(
         AttributeKeyToClassConstFetch $attributeKeyToClassConstFetch
     ): bool {
         $hasChanged = false;
+
         foreach ($attributeGroup->attrs as $attribute) {
             if (! $this->isName($attribute->name, $attributeKeyToClassConstFetch->getAttributeClass())) {
                 continue;
@@ -147,23 +148,38 @@ private function processToClassConstFetch(
                     continue;
                 }
 
-                $value = $this->valueResolver->getValue($arg->value);
-
-                $constName = $attributeKeyToClassConstFetch->getValuesToConstantsMap()[$value] ?? null;
-                if ($constName === null) {
-                    continue;
+                if ($this->processArg($arg, $attributeKeyToClassConstFetch)) {
+                    $hasChanged = true;
                 }
-
-                $arg->value = $this->nodeFactory->createClassConstFetch(
-                    $attributeKeyToClassConstFetch->getConstantClass(),
-                    $constName
-                );
-
-                $hasChanged = true;
-                continue 2;
             }
         }
 
         return $hasChanged;
     }
+
+    private function processArg(Node\Arg $arg, AttributeKeyToClassConstFetch $attributeKeyToClassConstFetch): bool
+    {
+        $value = $this->valueResolver->getValue($arg->value);
+
+        $constName = $attributeKeyToClassConstFetch->getValuesToConstantsMap()[$value] ?? null;
+        if ($constName === null) {
+            return false;
+        }
+
+        $newValue = $this->nodeFactory->createClassConstFetch(
+            $attributeKeyToClassConstFetch->getConstantClass(),
+            $constName
+        );
+
+        if (
+            $arg->value instanceof Node\Expr\ClassConstFetch
+            && $this->getName($arg->value) === $this->getName($newValue)
+        ) {
+            return false;
+        }
+
+        $arg->value = $newValue;
+
+        return true;
+    }
 }