[Privatization] Skip ChangeReadOnlyVariableWithDefaultValueToConstant…

…Rector when local variable never used (#1693)

* [Privatization] Skip ChangeReadOnlyVariableWithDefaultValueToConstantRector when local variable never used

* [ci-review] Rector Rectify

* Fixed 🎉

* [ci-review] Rector Rectify

* [ci-review] Rector Rectify

* phpstan

* final touch: add test for combine ChangeReadOnlyVariableWithDefaultValueToConstantRector+RemoveUnusedVariableAssignRector

* final touch: using array_filter like in other method

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

packages/NodeTypeResolver/TypeComparator/TypeComparator.php

@@ -105,7 +105,7 @@ public function arePhpParserAndPhpStanPhpDocTypesEqual(
             return false;
         }
 
-        return $this->isThisTypeInFinalClass($phpStanDocType, $phpParserNodeType, $node);
+        return $this->isThisTypeInFinalClass($phpStanDocType, $phpParserNodeType);
     }
 
     public function isSubtype(Type $checkedType, Type $mainType): bool
@@ -267,7 +267,7 @@ private function areTypesSameWithLiteralTypeInPhpDoc(
                 ->yes();
     }
 
-    private function isThisTypeInFinalClass(Type $phpStanDocType, Type $phpParserNodeType, Node $node): bool
+    private function isThisTypeInFinalClass(Type $phpStanDocType, Type $phpParserNodeType): bool
     {
 
         /**

rules-tests/Privatization/Rector/Class_/ChangeReadOnlyVariableWithDefaultValueToConstantRector/Fixture/skip_defined_never_used.php.inc

@@ -0,0 +1,14 @@
+<?php
+
+namespace Rector\Tests\Privatization\Rector\Class_\ChangeReadOnlyVariableWithDefaultValueToConstantRector\Fixture;
+
+class SkipDefinedNeverUsed
+{
+    public function run()
+    {
+        $replacements = [
+            'PHPUnit\Framework\TestCase\Notice' => 'expectNotice',
+            'PHPUnit\Framework\TestCase\Deprecated' => 'expectDeprecation',
+        ];
+    }
+}

src/NodeManipulator/ClassMethodAssignManipulator.php

@@ -24,6 +24,7 @@
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\Core\PhpParser\Node\NodeFactory;
 use Rector\Core\Reflection\ReflectionResolver;
+use Rector\DeadCode\NodeAnalyzer\ExprUsedInNextNodeAnalyzer;
 use Rector\NodeNameResolver\NodeNameResolver;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 
@@ -41,7 +42,8 @@ public function __construct(
         private readonly VariableManipulator $variableManipulator,
         private readonly NodeComparator $nodeComparator,
         private readonly ReflectionResolver $reflectionResolver,
-        private readonly ArrayDestructVariableFilter $arrayDestructVariableFilter
+        private readonly ArrayDestructVariableFilter $arrayDestructVariableFilter,
+        private readonly ExprUsedInNextNodeAnalyzer $exprUsedInNextNodeAnalyzer
     ) {
     }
 
@@ -64,6 +66,12 @@ public function collectReadyOnlyAssignScalarVariables(ClassMethod $classMethod):
         $readOnlyVariableAssigns = $this->filterOutMultiAssigns($readOnlyVariableAssigns);
         $readOnlyVariableAssigns = $this->filterOutForeachVariables($readOnlyVariableAssigns);
 
+        /**
+         * Remove unused variable assign is task of RemoveUnusedVariableAssignRector
+         * so no need to move to constant early
+         */
+        $readOnlyVariableAssigns = $this->filterOutNeverUsedNext($readOnlyVariableAssigns);
+
         return $this->variableManipulator->filterOutChangedVariables($readOnlyVariableAssigns, $classMethod);
     }
 
@@ -84,6 +92,18 @@ public function addParameterAndAssignToMethod(
         $this->alreadyAddedClassMethodNames[$classMethodHash][] = $name;
     }
 
+    /**
+     * @param Assign[] $readOnlyVariableAssigns
+     * @return Assign[]
+     */
+    private function filterOutNeverUsedNext(array $readOnlyVariableAssigns): array
+    {
+        return array_filter(
+            $readOnlyVariableAssigns,
+            fn (Assign $assign): bool => $this->exprUsedInNextNodeAnalyzer->isUsed($assign->var)
+        );
+    }
+
     /**
      * @param Assign[] $variableAssigns
      * @return Assign[]

tests/Issues/RemoveReadonlyLocalVariable/Fixture/remove_variable_assign_only.php.inc

@@ -0,0 +1,29 @@
+<?php
+
+namespace Rector\Core\Tests\Issues\RemoveReadonlyLocalVariable\Fixture;
+
+final class RemoveVariableAssignOnly
+{
+    public function run()
+    {
+        $replacements = [
+            'PHPUnit\Framework\TestCase\Notice' => 'expectNotice',
+            'PHPUnit\Framework\TestCase\Deprecated' => 'expectDeprecation',
+        ];
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Core\Tests\Issues\RemoveReadonlyLocalVariable\Fixture;
+
+final class RemoveVariableAssignOnly
+{
+    public function run()
+    {
+    }
+}
+
+?>

tests/Issues/RemoveReadonlyLocalVariable/RemoveReadonlyLocalVariableTest.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Core\Tests\Issues\RemoveReadonlyLocalVariable;
+
+use Iterator;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+use Symplify\SmartFileSystem\SmartFileInfo;
+
+final class RemoveReadonlyLocalVariableTest 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';
+    }
+}

tests/Issues/RemoveReadonlyLocalVariable/config/configured_rule.php

@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\DeadCode\Rector\Assign\RemoveUnusedVariableAssignRector;
+use Rector\Privatization\Rector\Class_\ChangeReadOnlyVariableWithDefaultValueToConstantRector;
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+
+return static function (ContainerConfigurator $containerConfigurator): void {
+    $services = $containerConfigurator->services();
+    $services->set(ChangeReadOnlyVariableWithDefaultValueToConstantRector::class);
+    $services->set(RemoveUnusedVariableAssignRector::class);
+};