[Printer] Handle return array of nodes with InlineHTML (#3304)

* [Printer] Handle return array on InlineHTML * add node before with inlinehtml has open close php tag * udpate fixtue * fix reprint add node before node * Fixed * comment * [ci-review] Rector Rectify * [ci-review] Rector Rectify * tweak cache * phpstan * Fix * rename fixture * add failing fixture return array * reuse already called * previousNode param * ensure token check * decorate * Fix * service comment * rectify * loop next nodes inlinehtml * clean up * [ci-review] Rector Rectify * clean up * Final touch: move down check to ensure all next nodes InlineHTML will be re-printed when inserted in between * final touch: pass File object * comment * Final touch: remove unused count() usage * inline count() is cheap * phpstan cache * Fixture source cache Co-authored-by: GitHub Action <action@github.com>
rectorphp · Jan 25, 2023 · 828ec4e · 828ec4e
1 parent 2c35322
commit 828ec4e
Show file tree

Hide file tree

Showing 20 changed files with 331 additions and 20 deletions.
diff --git a/packages/PostRector/Rector/NodeAddingPostRector.php b/packages/PostRector/Rector/NodeAddingPostRector.php
@@ -6,6 +6,7 @@
 
 use PhpParser\Node;
 use Rector\Core\NodeDecorator\MixPhpHtmlDecorator;
+use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\PostRector\Collector\NodesToAddCollector;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -54,7 +55,8 @@ public function leaveNode(Node $node): array | Node
             $this->nodesToAddCollector->clearNodesToAddBefore($node);
             $newNodes = array_merge($nodesToAddBefore, $newNodes);
 
-            $this->mixPhpHtmlDecorator->decorateBefore($node);
+            $previousNode = $node->getAttribute(AttributeKey::PREVIOUS_NODE);
+            $this->mixPhpHtmlDecorator->decorateBefore($node, $previousNode);
         }
 
         if ($newNodes === [$node]) {

diff --git a/...ypeDeclaration/Rector/ClassMethod/ReturnNeverTypeRector/Fixture/skip_nested_value.php.inc b/...ypeDeclaration/Rector/ClassMethod/ReturnNeverTypeRector/Fixture/skip_nested_value.php.inc
@@ -1,5 +1,7 @@
 <?php
 
+namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnNeverTypeRector\Fixture;
+
 function run($key)
 {
     if ($key) {

diff --git a/...claration/Rector/ClassMethod/ReturnNeverTypeRector/FixturePHP72/skip_nested_value.php.inc b/...claration/Rector/ClassMethod/ReturnNeverTypeRector/FixturePHP72/skip_nested_value.php.inc
@@ -1,5 +1,7 @@
 <?php
 
+namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnNeverTypeRector\FixturePHP72;
+
 function run($key)
 {
     if ($key) {

diff --git a/src/NodeDecorator/MixPhpHtmlDecorator.php b/src/NodeDecorator/MixPhpHtmlDecorator.php
@@ -10,9 +10,14 @@
 use PhpParser\Node\Stmt\InlineHTML;
 use PhpParser\Node\Stmt\Nop;
 use Rector\Core\PhpParser\Comparing\NodeComparator;
+use Rector\Core\ValueObject\Application\File;
 use Rector\NodeRemoval\NodeRemover;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 
+/**
+ * Mix PHP+HTML decorator, which require reprint the InlineHTML
+ * which is the safe way to make next/prev Node has open and close php tag
+ */
 final class MixPhpHtmlDecorator
 {
     public function __construct(
@@ -21,12 +26,44 @@ public function __construct(
     ) {
     }
 
-    public function decorateBefore(Node $node): void
+    /**
+     * @param Node[] $nodes
+     */
+    public function decorateNextNodesInlineHTML(File $file, array $nodes): void
+    {
+        $oldTokens = $file->getOldTokens();
+
+        foreach ($nodes as $key => $subNode) {
+            if ($subNode instanceof InlineHTML) {
+                continue;
+            }
+
+            $endTokenPost = $subNode->getEndTokenPos();
+            if (isset($oldTokens[$endTokenPost])) {
+                continue;
+            }
+
+            if (! isset($nodes[$key + 1])) {
+                // already last one, nothing to do
+                return;
+            }
+
+            if ($nodes[$key + 1] instanceof InlineHTML) {
+                // No token end? Just added
+                $nodes[$key + 1]->setAttribute(AttributeKey::ORIGINAL_NODE, null);
+            }
+        }
+    }
+
+    public function decorateBefore(Node $node, Node $previousNode = null): void
     {
-        $firstNodePreviousNode = $node->getAttribute(AttributeKey::PREVIOUS_NODE);
-        if ($firstNodePreviousNode instanceof InlineHTML && ! $node instanceof InlineHTML) {
-            // re-print InlineHTML is safe
-            $firstNodePreviousNode->setAttribute(AttributeKey::ORIGINAL_NODE, null);
+        if ($previousNode instanceof InlineHTML && ! $node instanceof InlineHTML) {
+            $previousNode->setAttribute(AttributeKey::ORIGINAL_NODE, null);
+            return;
+        }
+
+        if ($node instanceof InlineHTML && ! $previousNode instanceof Node) {
+            $node->setAttribute(AttributeKey::ORIGINAL_NODE, null);
         }
     }
 
@@ -77,7 +114,6 @@ public function decorateAfter(Node $node, array $nodes): void
 
         $stmt->setAttribute(AttributeKey::COMMENTS, $nodeComments);
 
-        // re-print InlineHTML is safe
         $firstNodeAfterNode->setAttribute(AttributeKey::ORIGINAL_NODE, null);
 
         // remove Nop is marked  as comment of Next Node
@@ -90,7 +126,7 @@ public function decorateAfter(Node $node, array $nodes): void
     private function resolveAppendAfterNode(Nop $nop, array $nodes): ?Node
     {
         foreach ($nodes as $key => $subNode) {
-            if (! $this->nodeComparator->areNodesEqual($subNode, $nop)) {
+            if (! $this->nodeComparator->areSameNode($subNode, $nop)) {
                 continue;
             }
 

diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php
@@ -31,6 +31,7 @@
 use Rector\Core\NodeDecorator\MixPhpHtmlDecorator;
 use Rector\Core\PhpParser\Comparing\NodeComparator;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
+use Rector\Core\PhpParser\Node\CustomNode\FileWithoutNamespace;
 use Rector\Core\PhpParser\Node\NodeFactory;
 use Rector\Core\PhpParser\Node\Value\ValueResolver;
 use Rector\Core\ProcessAnalyzer\RectifiedAnalyzer;
@@ -427,7 +428,7 @@ private function connectNodes(array $nodes, Node $node): void
             /** @var Node $previousNode */
             $previousNode = $node->getAttribute(AttributeKey::PREVIOUS_NODE);
 
-            $this->mixPhpHtmlDecorator->decorateBefore($node);
+            $this->mixPhpHtmlDecorator->decorateBefore($node, $previousNode);
 
             $nodes = [$previousNode, ...$nodes];
         }
@@ -444,6 +445,12 @@ private function connectNodes(array $nodes, Node $node): void
             $nodes = [...$nodes, $nextNode];
         }
 
+        if (count($nodes) > 1) {
+            $this->mixPhpHtmlDecorator->decorateNextNodesInlineHTML($this->file, $nodes);
+        } elseif ($firstNode instanceof FileWithoutNamespace) {
+            $this->mixPhpHtmlDecorator->decorateNextNodesInlineHTML($this->file, $firstNode->stmts);
+        }
+
         $nodeTraverser = new NodeTraverser();
         $nodeTraverser->addVisitor(new NodeConnectingVisitor());
         $nodeTraverser->traverse($nodes);

diff --git a/tests/Issues/AddNodeAfterNodeStmt/Source/AddNextNopRector.php b/tests/Issues/AddNodeAfterNodeStmt/Source/AddNextNopRector.php
@@ -4,21 +4,20 @@
 
 namespace Rector\Core\Tests\Issues\AddNodeAfterNodeStmt\Source;
 
-use PhpParser\Comment;
 use PhpParser\Node;
 use PhpParser\Node\Scalar\String_;
 use PhpParser\Node\Stmt\Echo_;
-use PhpParser\Node\Stmt\If_;
 use PhpParser\Node\Stmt\Nop;
 use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode;
 use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
 use Rector\Core\Rector\AbstractRector;
-use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\PostRector\Collector\NodesToAddCollector;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
 class AddNextNopRector extends AbstractRector
 {
+    private array $justAdded = [];
+
     public function __construct(private readonly NodesToAddCollector $nodesToAddCollector)
     {
     }
@@ -37,6 +36,10 @@ public function getNodeTypes(): array
 
     public function refactor(Node $node)
     {
+        if (isset($this->justAdded[$this->file->getFilePath()])) {
+            return null;
+        }
+
         $echo = new Echo_([new String_("this is new stmt after Nop")]);
 
         $phpDocInfo = $this->phpDocInfoFactory->createEmpty($echo);
@@ -53,6 +56,8 @@ public function refactor(Node $node)
             $node
         );
 
+        $this->justAdded[$this->file->getFilePath()] = true;
+
         return $node;
     }
 }
diff --git a/tests/Issues/AddNodeAfterNodeStmt/Source/AddNextStmtRector.php b/tests/Issues/AddNodeAfterNodeStmt/Source/AddNextStmtRector.php
@@ -14,6 +14,8 @@
 
 class AddNextStmtRector extends AbstractRector
 {
+    private array $justAdded = [];
+
     public function __construct(private readonly NodesToAddCollector $nodesToAddCollector)
     {
     }
@@ -32,10 +34,17 @@ public function getNodeTypes(): array
 
     public function refactor(Node $node)
     {
+        if (isset($this->justAdded[$this->file->getFilePath()])) {
+            return null;
+        }
+
         $this->nodesToAddCollector->addNodeAfterNode(
             new Echo_([new String_("this is new stmt after if")]),
             $node
         );
+
+        $this->justAdded[$this->file->getFilePath()] = true;
+
         return $node;
     }
 }
diff --git a/tests/Issues/AddNodeBeforeNodeStmt/FixtureBeforeInlineHTML/fixture2.php.inc b/tests/Issues/AddNodeBeforeNodeStmt/FixtureBeforeInlineHTML/fixture2.php.inc
@@ -0,0 +1,10 @@
+<div><?php echo $this->filter ;?></div>
+-----
+<?php
+
+$test = 'test';
+$test2 = 'test2';
+?>
+<div><?php 
+echo $this->filter ;
+?></div>
diff --git a/tests/Issues/AddNodeBeforeNodeStmt/Source/AddBeforeInlineHTMLRector.php b/tests/Issues/AddNodeBeforeNodeStmt/Source/AddBeforeInlineHTMLRector.php
@@ -9,15 +9,14 @@
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Scalar\String_;
 use PhpParser\Node\Stmt\Expression;
-use PhpParser\Node\Stmt\InlineHTML;
 use Rector\Core\PhpParser\Node\CustomNode\FileWithoutNamespace;
 use Rector\Core\Rector\AbstractRector;
 use Rector\PostRector\Collector\NodesToAddCollector;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
 class AddBeforeInlineHTMLRector extends AbstractRector
 {
-    private bool $justAdded = false;
+    private array $justAdded = [];
 
     public function __construct(private readonly NodesToAddCollector $nodesToAddCollector)
     {
@@ -40,11 +39,11 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node)
     {
-        if ($this->justAdded) {
+        if (isset($this->justAdded[$this->file->getFilePath()])) {
             return null;
         }
 
-        $firstStmt = $node->stmts[0];
+        $firstStmt = current($node->stmts);
 
         $this->nodesToAddCollector->addNodeBeforeNode(
             new Expression(
@@ -62,7 +61,9 @@ public function refactor(Node $node)
             ),
             $firstStmt
         );
-        $this->justAdded = true;
+
+        $this->justAdded[$this->file->getFilePath()] = true;
+
         return $node;
     }
 }
diff --git a/tests/Issues/AddNodeBeforeNodeStmt/Source/AddBeforeStmtRector.php b/tests/Issues/AddNodeBeforeNodeStmt/Source/AddBeforeStmtRector.php
@@ -14,6 +14,8 @@
 
 class AddBeforeStmtRector extends AbstractRector
 {
+    private array $justAdded = [];
+
     public function __construct(private readonly NodesToAddCollector $nodesToAddCollector)
     {
     }
@@ -32,10 +34,17 @@ public function getNodeTypes(): array
 
     public function refactor(Node $node)
     {
+        if (isset($this->justAdded[$this->file->getFilePath()])) {
+            return null;
+        }
+
         $this->nodesToAddCollector->addNodeBeforeNode(
             new Echo_([new String_("this is new stmt before if")]),
             $node
         );
+
+        $this->justAdded[$this->file->getFilePath()] = true;
+
         return $node;
     }
 }
diff --git a/tests/Issues/InsertFirstBeforeInlineHTML/Fixture/fixture.php.inc b/tests/Issues/InsertFirstBeforeInlineHTML/Fixture/fixture.php.inc
@@ -0,0 +1,9 @@
+<div><?php echo $this->filter ;?></div>
+-----
+<?php
+
+echo 'this is new stmt before InlineHTML';
+?>
+<div><?php 
+echo $this->filter ;
+?></div>
diff --git a/tests/Issues/InsertFirstBeforeInlineHTML/InsertFirstBeforeInlineHTMLTest.php b/tests/Issues/InsertFirstBeforeInlineHTML/InsertFirstBeforeInlineHTMLTest.php
@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Core\Tests\Issues\InsertFirstBeforeInlineHTML;
+
+use Iterator;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class InsertFirstBeforeInlineHTMLTest extends AbstractRectorTestCase
+{
+    /**
+     * @dataProvider provideData()
+     */
+    public function test(string $filePath): void
+    {
+        $this->doTestFile($filePath);
+    }
+
+    /**
+     * @return Iterator<array<string>>
+     */
+    public function provideData(): Iterator
+    {
+        return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture');
+    }
+
+    public function provideConfigFilePath(): string
+    {
+        return __DIR__ . '/config/configured_rule.php';
+    }
+}
diff --git a/tests/Issues/InsertFirstBeforeInlineHTML/Source/InsertBeforeInlineHTMLRector.php b/tests/Issues/InsertFirstBeforeInlineHTML/Source/InsertBeforeInlineHTMLRector.php
@@ -0,0 +1,47 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Core\Tests\Issues\InsertFirstBeforeInlineHTML\Source;
+
+use PhpParser\Node;
+use PhpParser\Node\Scalar\String_;
+use PhpParser\Node\Stmt\Echo_;
+use Rector\Core\PhpParser\Node\CustomNode\FileWithoutNamespace;
+use Rector\Core\Rector\AbstractRector;
+use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+
+class InsertBeforeInlineHTMLRector extends AbstractRector
+{
+    private array $justAdded = [];
+
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition('uff', []);
+    }
+
+    public function getNodeTypes(): array
+    {
+        return [
+            FileWithoutNamespace::class,
+        ];
+    }
+
+    public function refactor(Node $node)
+    {
+        if (isset($this->justAdded[$this->file->getFilePath()])) {
+            return null;
+        }
+
+        $echo = new Echo_([new String_('this is new stmt before InlineHTML')]);
+
+        $node->stmts = array_merge(
+            [$echo],
+            $node->stmts
+        );
+
+        $this->justAdded[$this->file->getFilePath()] = true;
+
+        return $node;
+    }
+}
diff --git a/tests/Issues/InsertFirstBeforeInlineHTML/config/configured_rule.php b/tests/Issues/InsertFirstBeforeInlineHTML/config/configured_rule.php
@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\Core\Tests\Issues\InsertFirstBeforeInlineHTML\Source\InsertBeforeInlineHTMLRector;
+
+return static function (RectorConfig $rectorConfig): void {
+    $rectorConfig->rule(InsertBeforeInlineHTMLRector::class);
+};