[PHP 8.0] Fix used variable rename in propperty promotion (#4841)

* [PHP 8.0] Add test case for used variable

* [ci-review] Rector Rectify

Co-authored-by: rector-bot <tomas@getrector.org>

phpstan.neon

@@ -10,6 +10,16 @@ includes:
     - vendor/symplify/phpstan-rules/packages/cognitive-complexity/config/cognitive-complexity-services.neon
 
 services:
+    -
+        class: Symplify\PHPStanRules\Rules\ForbiddenMethodCallOnTypeRule
+        tags: [phpstan.rules.rule]
+        arguments:
+            forbiddenMethodNamesByTypes:
+                PhpParser\Node:
+                    - 'getDocComment'
+                    - 'getComments'
+                    - 'setDocComment'
+
     -
         class: Symplify\PHPStanRules\Rules\OnlyOneClassMethodRule
         tags: [phpstan.rules.rule]
@@ -780,9 +790,10 @@ parameters:
         - '#Only EntityManager\* type as dependency is allowed#'
 
         -
-            message: '#"(getComments|getDocComment)\(\)" call on "PhpParser\\Node" type is not allowed#'
+            message: '#"(getComments|getDocComment|setDocComment)\(\)" call on "PhpParser\\Node" type is not allowed#'
             paths:
                 # merging comments
+                - packages/better-php-doc-parser/tests/PhpDocInfo/PhpDocInfo/PhpDocInfoTest.php
                 - src/Rector/AbstractRector.php
                 - src/PhpParser/NodeTransformer.php
                 # playing around with doc block format
@@ -791,11 +802,13 @@ parameters:
                 - rules/phpstan/src/Rector/Node/RemoveNonExistingVarAnnotationRector.php
                 - rules/dead-code/src/Rector/Expression/RemoveDeadStmtRector.php
                 - rules/code-quality/src/Rector/FuncCall/ChangeArrayPushToArrayAssignRector.php
+                - rules/php-spec-to-phpunit/src/Rector/MethodCall/PhpSpecMocksToPHPUnitMocksRector.php
                 - packages/better-php-doc-parser/src/Comment/CommentsMerger.php
                 - packages/node-type-resolver/src/PhpDoc/NodeAnalyzer/DocBlockManipulator.php
-                - packages/better-php-doc-parser/src/PhpDocInfo/PhpDocInfoFactory.php
                 - rules/coding-style/src/Node/DocAliasResolver.php
+                - packages/better-php-doc-parser/src/PhpDocInfo/PhpDocInfoFactory.php
                 - packages/better-php-doc-parser/tests/PhpDocParser/AbstractPhpDocInfoTest.php
+                - packages/better-php-doc-parser/tests/PhpDocInfo/PhpDocInfoPrinter/AbstractPhpDocInfoPrinterTest.php
 
         - '#"@simplexml_load_string\(\$fileContents\)" is forbidden to use#'
         - '#@ intentionally#'

rules/naming/src/Rector/Assign/RenameVariableToMatchMethodCallReturnTypeRector.php

@@ -184,7 +184,6 @@ CODE_SAMPLE
 
     private function renameVariable(VariableAndCallAssign $variableAndCallAssign, string $expectedName): void
     {
-        // TODO: Remove in next PR, implemented in VariableRenamer::renameVariableIfMatchesName()
         $this->varTagValueNodeRenamer->renameAssignVarTagVariableName(
             $variableAndCallAssign->getAssign(),
             $variableAndCallAssign->getVariableName(),

rules/php80/src/NodeResolver/PromotedPropertyResolver.php

@@ -0,0 +1,115 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Php80\NodeResolver;
+
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Param;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Expression;
+use PhpParser\Node\Stmt\Property;
+use Rector\Core\PhpParser\Printer\BetterStandardPrinter;
+use Rector\Core\ValueObject\MethodName;
+use Rector\NodeNameResolver\NodeNameResolver;
+use Rector\Php80\ValueObject\PropertyPromotionCandidate;
+
+final class PromotedPropertyResolver
+{
+    /**
+     * @var NodeNameResolver
+     */
+    private $nodeNameResolver;
+
+    /**
+     * @var BetterStandardPrinter
+     */
+    private $betterStandardPrinter;
+
+    public function __construct(NodeNameResolver $nodeNameResolver, BetterStandardPrinter $betterStandardPrinter)
+    {
+        $this->nodeNameResolver = $nodeNameResolver;
+        $this->betterStandardPrinter = $betterStandardPrinter;
+    }
+
+    /**
+     * @return PropertyPromotionCandidate[]
+     */
+    public function resolveFromClass(Class_ $class): array
+    {
+        $constructClassMethod = $class->getMethod(MethodName::CONSTRUCT);
+        if ($constructClassMethod === null) {
+            return [];
+        }
+
+        $propertyPromotionCandidates = [];
+        foreach ($class->getProperties() as $property) {
+            if (count((array) $property->props) !== 1) {
+                continue;
+            }
+
+            $propertyPromotionCandidate = $this->matchPropertyPromotionCandidate($property, $constructClassMethod);
+            if ($propertyPromotionCandidate === null) {
+                continue;
+            }
+
+            $propertyPromotionCandidates[] = $propertyPromotionCandidate;
+        }
+
+        return $propertyPromotionCandidates;
+    }
+
+    private function matchPropertyPromotionCandidate(
+        Property $property,
+        ClassMethod $constructClassMethod
+    ): ?PropertyPromotionCandidate {
+        $onlyProperty = $property->props[0];
+        $propertyName = $this->nodeNameResolver->getName($onlyProperty);
+
+        // match property name to assign in constructor
+        foreach ((array) $constructClassMethod->stmts as $stmt) {
+            if ($stmt instanceof Expression) {
+                $stmt = $stmt->expr;
+            }
+
+            if (! $stmt instanceof Assign) {
+                continue;
+            }
+
+            $assign = $stmt;
+            if (! $this->nodeNameResolver->isLocalPropertyFetchNamed($assign->var, $propertyName)) {
+                continue;
+            }
+
+            // 1. is param
+            // @todo 2. is default value
+            $assignedExpr = $assign->expr;
+
+            $matchedParam = $this->matchClassMethodParamByAssignedVariable($constructClassMethod, $assignedExpr);
+            if ($matchedParam === null) {
+                continue;
+            }
+
+            return new PropertyPromotionCandidate($property, $assign, $matchedParam);
+        }
+
+        return null;
+    }
+
+    private function matchClassMethodParamByAssignedVariable(
+        ClassMethod $classMethod,
+        Expr $assignedExpr
+    ): ?Param {
+        foreach ($classMethod->params as $param) {
+            if (! $this->betterStandardPrinter->areNodesEqual($assignedExpr, $param->var)) {
+                continue;
+            }
+
+            return $param;
+        }
+
+        return null;
+    }
+}

rules/php80/src/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php

@@ -5,18 +5,16 @@ declare(strict_types=1);
 namespace Rector\Php80\Rector\Class_;
 
 use PhpParser\Node;
-use PhpParser\Node\Expr;
-use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassMethod;
-use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Property;
 use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\ValueObject\MethodName;
+use Rector\Naming\VariableRenamer;
 use Rector\NodeTypeResolver\Node\AttributeKey;
-use Rector\Php80\ValueObject\PromotionCandidate;
+use Rector\Php80\NodeResolver\PromotedPropertyResolver;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -29,9 +27,20 @@ use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 final class ClassPropertyAssignToConstructorPromotionRector extends AbstractRector
 {
     /**
-     * @var PromotionCandidate[]
+     * @var PromotedPropertyResolver
      */
-    private $promotionCandidates = [];
+    private $promotedPropertyResolver;
+
+    /**
+     * @var VariableRenamer
+     */
+    private $variableRenamer;
+
+    public function __construct(PromotedPropertyResolver $promotedPropertyResolver, VariableRenamer $variableRenamer)
+    {
+        $this->promotedPropertyResolver = $promotedPropertyResolver;
+        $this->variableRenamer = $variableRenamer;
+    }
 
     public function getRuleDefinition(): RuleDefinition
     {
@@ -42,18 +51,11 @@ final class ClassPropertyAssignToConstructorPromotionRector extends AbstractRect
                     <<<'CODE_SAMPLE'
 class SomeClass
 {
-    public float $x;
-    public float $y;
-    public float $z;
+    public float $someVariable;
 
-    public function __construct(
-        float $x = 0.0,
-        float $y = 0.0,
-        float $z = 0.0
-    ) {
-        $this->x = $x;
-        $this->y = $y;
-        $this->z = $z;
+    public function __construct(float $someVariable = 0.0)
+    {
+        $this->someVariable = $someVariable;
     }
 }
 CODE_SAMPLE
@@ -61,15 +63,12 @@ CODE_SAMPLE
                     <<<'CODE_SAMPLE'
 class SomeClass
 {
-    public function __construct(
-        public float $x = 0.0,
-        public float $y = 0.0,
-        public float $z = 0.0,
-    ) {}
+    public function __construct(private float $someVariable = 0.0)
+    {
+    }
 }
 CODE_SAMPLE
                 ),
-
             ]);
     }
 
@@ -86,17 +85,19 @@ CODE_SAMPLE
      */
     public function refactor(Node $node): ?Node
     {
-        $promotionCandidates = $this->collectPromotionCandidatesFromClass($node);
+        $promotionCandidates = $this->promotedPropertyResolver->resolveFromClass($node);
         if ($promotionCandidates === []) {
             return null;
         }
 
-        foreach ($this->promotionCandidates as $promotionCandidate) {
+        /** @var ClassMethod $constructClassMethod */
+        $constructClassMethod = $node->getMethod(MethodName::CONSTRUCT);
+
+        foreach ($promotionCandidates as $promotionCandidate) {
             // does property have some useful annotations?
             $property = $promotionCandidate->getProperty();
 
             $this->removeNode($property);
-
             $this->removeNode($promotionCandidate->getAssign());
 
             $property = $promotionCandidate->getProperty();
@@ -104,39 +105,23 @@ CODE_SAMPLE
 
             $this->decorateParamWithPropertyPhpDocInfo($property, $param);
 
+            /** @var string $oldName */
+            $oldName = $this->getName($param->var);
+
             // property name has higher priority
             $param->var->name = $property->props[0]->name;
-
-            // @todo add visibility - needs https://github.com/nikic/PHP-Parser/pull/667
             $param->flags = $property->flags;
+
+            // rename also following calls
+            $propertyName = $this->getName($property->props[0]);
+            $this->variableRenamer->renameVariableInFunctionLike($constructClassMethod, null, $oldName, $propertyName);
+
+            $this->removeClassMethodParam($constructClassMethod, $oldName);
         }
 
         return $node;
     }
 
-    /**
-     * @return PromotionCandidate[]
-     */
-    private function collectPromotionCandidatesFromClass(Class_ $class): array
-    {
-        $constructClassMethod = $class->getMethod(MethodName::CONSTRUCT);
-        if ($constructClassMethod === null) {
-            return [];
-        }
-
-        $this->promotionCandidates = [];
-
-        foreach ($class->getProperties() as $property) {
-            if (count((array) $property->props) !== 1) {
-                continue;
-            }
-
-            $this->collectPromotionCandidate($property, $constructClassMethod);
-        }
-
-        return $this->promotionCandidates;
-    }
-
     private function decorateParamWithPropertyPhpDocInfo(Property $property, Param $param): void
     {
         $propertyPhpDocInfo = $property->getAttribute(AttributeKey::PHP_DOC_INFO);
@@ -148,52 +133,18 @@ CODE_SAMPLE
         $param->setAttribute(AttributeKey::PHP_DOC_INFO, $propertyPhpDocInfo);
     }
 
-    private function collectPromotionCandidate(Property $property, ClassMethod $constructClassMethod): void
+    private function removeClassMethodParam(ClassMethod $classMethod, string $paramName): void
     {
-        $onlyProperty = $property->props[0];
-        $propertyName = $this->getName($onlyProperty);
-
-        // match property name to assign in constructor
-        foreach ((array) $constructClassMethod->stmts as $stmt) {
-            if ($stmt instanceof Expression) {
-                $stmt = $stmt->expr;
-            }
-
-            if (! $stmt instanceof Assign) {
-                continue;
-            }
-
-            $assign = $stmt;
-            if (! $this->isLocalPropertyFetchNamed($assign->var, $propertyName)) {
-                continue;
-            }
-
-            // 1. is param
-            // @todo 2. is default value
-
-            $assignedExpr = $assign->expr;
-
-            $matchedParam = $this->matchClassMethodParamByAssignedVariable($constructClassMethod, $assignedExpr);
-            if ($matchedParam === null) {
-                continue;
-            }
-
-            $this->promotionCandidates[] = new PromotionCandidate($property, $assign, $matchedParam);
-        }
-    }
-
-    private function matchClassMethodParamByAssignedVariable(
-        ClassMethod $classMethod,
-        Expr $assignedExpr
-    ): ?Param {
-        foreach ($classMethod->params as $param) {
-            if (! $this->areNodesEqual($assignedExpr, $param->var)) {
-                continue;
-            }
-
-            return $param;
+        $phpDocInfo = $classMethod->getAttribute(AttributeKey::PHP_DOC_INFO);
+        if (! $phpDocInfo instanceof PhpDocInfo) {
+            return;
         }
 
-        return null;
+        $attributeAwareParamTagValueNode = $phpDocInfo->getParamTagValueByName($paramName);
+        if ($attributeAwareParamTagValueNode === null) {
+            return;
+        }
+
+        $phpDocInfo->removeTagValueNodeFromNode($attributeAwareParamTagValueNode);
     }
 }

rules/php80/src/ValueObject/PropertyPromotionCandidate.php

@@ -8,7 +8,7 @@ use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\Property;
 
-final class PromotionCandidate
+final class PropertyPromotionCandidate
 {
     /**
      * @var Property

rules/php80/tests/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/different_variable_name_used.php.inc

@@ -0,0 +1,42 @@
+<?php
+
+namespace Rector\Php80\Tests\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector\Fixture;
+
+class DifferentVariableNameUsed
+{
+    /**
+     * @var int
+     */
+    private $ID;
+
+    /**
+     * @param int $id
+     */
+    public function __construct($id)
+    {
+        if (! $this->isExists($id)) {
+        }
+
+        $this->ID = $id;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Php80\Tests\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector\Fixture;
+
+class DifferentVariableNameUsed
+{
+    public function __construct(/**
+     * @var int
+     */
+    private $ID)
+    {
+        if (! $this->isExists($ID)) {
+        }
+    }
+}
+
+?>