Add unit test for reindex stmt_key before refactor() called (#6613)

* Add unit test for reindex stmt_key before refactor() called * clean up * cs fix
rectorphp · Dec 19, 2024 · 0c2185f · 0c2185f
1 parent 3e414f3
commit 0c2185f
Show file tree

Hide file tree

Showing 8 changed files with 188 additions and 46 deletions.
diff --git a/src/Application/NodeAttributeReIndexer.php b/src/Application/NodeAttributeReIndexer.php
@@ -26,35 +26,46 @@
 
 final class NodeAttributeReIndexer
 {
-    public static function reIndexNodeAttributes(Node $node): ?Node
+    public static function reIndexStmtKeyNodeAttributes(Node $node): ?Node
     {
-        if (($node instanceof StmtsAwareInterface || $node instanceof ClassLike || $node instanceof Declare_) && $node->stmts !== null) {
-            $node->stmts = array_values($node->stmts);
+        if (! $node instanceof StmtsAwareInterface && ! $node instanceof ClassLike && ! $node instanceof Declare_) {
+            return null;
+        }
 
-            // re-index stmt key under current node
-            foreach ($node->stmts as $key => $childStmt) {
-                $childStmt->setAttribute(AttributeKey::STMT_KEY, $key);
-            }
+        if ($node->stmts === null) {
+            return null;
+        }
 
-            if ($node instanceof If_) {
-                $node->elseifs = array_values($node->elseifs);
-                return $node;
-            }
+        $node->stmts = array_values($node->stmts);
 
-            if ($node instanceof TryCatch) {
-                $node->catches = array_values($node->catches);
-                return $node;
-            }
+        // re-index stmt key under current node
+        foreach ($node->stmts as $key => $childStmt) {
+            $childStmt->setAttribute(AttributeKey::STMT_KEY, $key);
+        }
+
+        return $node;
+    }
 
-            if ($node instanceof FunctionLike) {
-                /** @var ClassMethod|Function_|Closure $node */
-                $node->params = array_values($node->params);
+    public static function reIndexNodeAttributes(Node $node): ?Node
+    {
+        self::reIndexStmtKeyNodeAttributes($node);
+
+        if ($node instanceof If_) {
+            $node->elseifs = array_values($node->elseifs);
+            return $node;
+        }
+
+        if ($node instanceof TryCatch) {
+            $node->catches = array_values($node->catches);
+            return $node;
+        }
 
-                if ($node instanceof Closure) {
-                    $node->uses = array_values($node->uses);
-                }
+        if ($node instanceof FunctionLike) {
+            /** @var ClassMethod|Function_|Closure $node */
+            $node->params = array_values($node->params);
 
-                return $node;
+            if ($node instanceof Closure) {
+                $node->uses = array_values($node->uses);
             }
 
             return $node;

diff --git a/src/NodeManipulator/StmtsManipulator.php b/src/NodeManipulator/StmtsManipulator.php
@@ -95,10 +95,7 @@ public function isVariableUsedInNextStmt(
         }
 
         if ($stmtsAware instanceof TryCatch) {
-            $stmts = array_merge(
-                $stmts,
-                $stmtsAware->catches
-            );
+            $stmts = array_merge($stmts, $stmtsAware->catches);
 
             if ($stmtsAware->finally instanceof Finally_) {
                 $stmts = array_merge($stmts, $stmtsAware->finally->stmts);

diff --git a/src/NodeTypeResolver/PHPStan/Scope/NodeVisitor/StmtKeyNodeVisitor.php b/src/NodeTypeResolver/PHPStan/Scope/NodeVisitor/StmtKeyNodeVisitor.php
@@ -5,32 +5,14 @@
 namespace Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor;
 
 use PhpParser\Node;
-use PhpParser\Node\Stmt\ClassLike;
-use PhpParser\Node\Stmt\Declare_;
 use PhpParser\NodeVisitorAbstract;
-use Rector\Contract\PhpParser\Node\StmtsAwareInterface;
-use Rector\NodeTypeResolver\Node\AttributeKey;
+use Rector\Application\NodeAttributeReIndexer;
 use Rector\NodeTypeResolver\PHPStan\Scope\Contract\NodeVisitor\ScopeResolverNodeVisitorInterface;
 
 final class StmtKeyNodeVisitor extends NodeVisitorAbstract implements ScopeResolverNodeVisitorInterface
 {
     public function enterNode(Node $node): ?Node
     {
-        if (! $node instanceof StmtsAwareInterface && ! $node instanceof ClassLike && ! $node instanceof Declare_) {
-            return null;
-        }
-
-        if ($node->stmts === null) {
-            return null;
-        }
-
-        $node->stmts = array_values($node->stmts);
-
-        // re-index stmt key under current node
-        foreach ($node->stmts as $key => $childStmt) {
-            $childStmt->setAttribute(AttributeKey::STMT_KEY, $key);
-        }
-
-        return null;
+        return NodeAttributeReIndexer::reIndexStmtKeyNodeAttributes($node);
     }
 }
diff --git a/tests/Issues/IndexedStmt/Fixture/fixture.php.inc b/tests/Issues/IndexedStmt/Fixture/fixture.php.inc
@@ -0,0 +1,30 @@
+<?php
+
+namespace Rector\Tests\Issues\IndexedStmt\Fixture;
+
+final class Fixture
+{
+    public function run()
+    {
+        'with index 0';
+        'with index 1';
+        'with index 2';
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Issues\IndexedStmt\Fixture;
+
+final class Fixture
+{
+    public function run()
+    {
+        'with index 0';
+        'final index';
+    }
+}
+
+?>
diff --git a/tests/Issues/IndexedStmt/IndexedStmtTest.php b/tests/Issues/IndexedStmt/IndexedStmtTest.php
@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Issues\IndexedStmt;
+
+use Iterator;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class IndexedStmtTest extends AbstractRectorTestCase
+{
+    #[DataProvider('provideData')]
+    public function test(string $filePath): void
+    {
+        $this->doTestFile($filePath);
+    }
+
+    public static function provideData(): Iterator
+    {
+        return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
+    }
+
+    public function provideConfigFilePath(): string
+    {
+        return __DIR__ . '/config/configured_rule.php';
+    }
+}
diff --git a/tests/Issues/IndexedStmt/Source/ChangeLastIndex1Rector.php b/tests/Issues/IndexedStmt/Source/ChangeLastIndex1Rector.php
@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Issues\IndexedStmt\Source;
+
+use PhpParser\Node;
+use PhpParser\Node\Scalar\String_;
+use PhpParser\Node\Stmt\Expression;
+use PhpParser\NodeVisitor;
+use Rector\Contract\PhpParser\Node\StmtsAwareInterface;
+use Rector\NodeTypeResolver\Node\AttributeKey;
+use Rector\Rector\AbstractRector;
+use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+
+final class ChangeLastIndex1Rector extends AbstractRector
+{
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition('',   []);
+    }
+
+    public function getNodeTypes(): array
+    {
+        return [StmtsAwareInterface::class];
+    }
+
+    /**
+     * @param StmtsAwareInterface $node
+     */
+    public function refactor(Node $node)
+    {
+        if ($node->stmts === null) {
+            return null;
+        }
+
+        foreach ($node->stmts as $stmt) {
+            if ($stmt->getAttribute(AttributeKey::STMT_KEY) === 1 && $stmt instanceof Expression && $stmt->expr instanceof String_ && $stmt->expr->value === 'with index 2') {
+                $stmt->expr->value = 'final index';
+                return $node;
+            }
+        }
+
+        return null;
+    }
+}
diff --git a/tests/Issues/IndexedStmt/Source/RemoveIndex1Rector.php b/tests/Issues/IndexedStmt/Source/RemoveIndex1Rector.php
@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Issues\IndexedStmt\Source;
+
+use PhpParser\Node;
+use PhpParser\Node\Scalar\String_;
+use PhpParser\Node\Stmt\Expression;
+use PhpParser\NodeVisitor;
+use Rector\Rector\AbstractRector;
+use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+
+final class RemoveIndex1Rector extends AbstractRector
+{
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition('',   []);
+    }
+
+    public function getNodeTypes(): array
+    {
+        return [Expression::class];
+    }
+
+    /**
+     * @param Expression $node
+     */
+    public function refactor(Node $node)
+    {
+        if ($node->expr instanceof String_ && $node->expr->value === 'with index 1') {
+            return NodeVisitor::REMOVE_NODE;
+        }
+
+        return null;
+    }
+}
diff --git a/tests/Issues/IndexedStmt/config/configured_rule.php b/tests/Issues/IndexedStmt/config/configured_rule.php
@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\Tests\Issues\IndexedStmt\Source\ChangeLastIndex1Rector;
+use Rector\Tests\Issues\IndexedStmt\Source\RemoveIndex1Rector;
+
+return static function (RectorConfig $rectorConfig): void {
+    $rectorConfig->rules([RemoveIndex1Rector::class, ChangeLastIndex1Rector::class]);
+};