[Core] [Printer] Ensure empty php tag init on original node not re-p…

…rinted (#3284)

* [e2e] Ensure empty php tag init on original node not re-printed

* [e2e] Ensure empty php tag init on original node not re-printed

* re-print HTML next node is not needed, only prev is needed

* add test for next stmt is html

* add before HTML

* add test for before node

* Revert re-print HTML next node is not needed, only prev is needed

This reverts commit 5d753e3.

* failing fixture add node before node

* Fixed 🎉

* Fix

* [ci-review] Rector Rectify

* [ci-review] Rector Rectify

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

e2e/plain-views/src/empty2.php

@@ -0,0 +1,5 @@
+<?php
+
+?>
+<div></div>
+<?php

src/PhpParser/Printer/BetterStandardPrinter.php

@@ -21,6 +21,7 @@
 use PhpParser\Node\Scalar\EncapsedStringPart;
 use PhpParser\Node\Scalar\LNumber;
 use PhpParser\Node\Scalar\String_;
+use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Declare_;
@@ -118,13 +119,28 @@ public function printFormatPreserving(array $stmts, array $origStmts, array $ori
             return $content;
         }
 
+        /** @var Stmt $firstStmt */
         $firstStmt = current($newStmts);
+        $isFirstStmtReprinted = $firstStmt->getAttribute(AttributeKey::ORIGINAL_NODE) === null;
+        if (! $isFirstStmtReprinted) {
+            return $content;
+        }
+
+        if (! $firstStmt instanceof InlineHTML) {
+            return $content;
+        }
+
         $lastStmt = end($newStmts);
 
-        if ($firstStmt instanceof InlineHTML && str_starts_with($content, '<?php' . $this->nl . $this->nl . '?>')) {
+        if (str_starts_with($content, '<?php' . $this->nl . $this->nl . '?>')) {
             $content = substr($content, 10);
         }
 
+        if (str_starts_with($content, '?>' . $this->nl)) {
+            $content = str_replace('<?php <?php' . $this->nl, '<?php' . $this->nl, $content);
+            $content = substr($content, 3);
+        }
+
         if ($lastStmt instanceof InlineHTML && str_ends_with($content, '<?php ' . $this->nl)) {
             return substr($content, 0, -7);
         }

src/Rector/AbstractRector.php

@@ -419,6 +419,11 @@ private function connectNodes(array $nodes, Node $node): void
         $firstNode = current($nodes);
         $firstNodePreviousNode = $firstNode->getAttribute(AttributeKey::PREVIOUS_NODE);
 
+        if ($firstNodePreviousNode instanceof InlineHTML && ! $firstNode instanceof InlineHTML) {
+            // re-print InlineHTML is safe
+            $firstNodePreviousNode->setAttribute(AttributeKey::ORIGINAL_NODE, null);
+        }
+
         if (! $firstNodePreviousNode instanceof Node && $node->hasAttribute(AttributeKey::PREVIOUS_NODE)) {
             /** @var Node $previousNode */
             $previousNode = $node->getAttribute(AttributeKey::PREVIOUS_NODE);
@@ -437,12 +442,6 @@ private function connectNodes(array $nodes, Node $node): void
         if (! $lastNodeNextNode instanceof Node && $node->hasAttribute(AttributeKey::NEXT_NODE)) {
             /** @var Node $nextNode */
             $nextNode = $node->getAttribute(AttributeKey::NEXT_NODE);
-
-            if ($nextNode instanceof InlineHTML && ! $lastNode instanceof InlineHTML) {
-                // re-print InlineHTML is safe
-                $nextNode->setAttribute(AttributeKey::ORIGINAL_NODE, null);
-            }
-
             $nodes = [...$nodes, $nextNode];
         }
 

tests/Issues/AddNodeAfterNodeStmt/Fixture/before_html.php.inc

@@ -0,0 +1,12 @@
+<div>some text</div>
+<?php
+if ($a === 1) {
+}
+?>
+-----
+<div>some text</div>
+<?php
+if ($a === 1) {
+}
+echo 'this is new stmt after if';
+?>

tests/Issues/AddNodeAfterNodeStmt/Fixture/next_html.php.inc

@@ -0,0 +1,12 @@
+<?php
+if ($a === 1) {
+}
+?>
+<div>some text</div>
+-----
+<?php
+if ($a === 1) {
+}
+echo 'this is new stmt after if';
+?>
+<div>some text</div>

tests/Issues/AddNodeBeforeNodeStmt/AddNodeBeforeNodeStmtTest.php

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

tests/Issues/AddNodeBeforeNodeStmt/Fixture/before_html.php.inc

@@ -0,0 +1,11 @@
+<div>some text</div>
+<?php
+if ($a === 1) {
+}
+?>
+-----
+<div>some text</div>
+<?php 
+echo 'this is new stmt before if';
+if ($a === 1) {
+}

tests/Issues/AddNodeBeforeNodeStmt/Fixture/fixture.php.inc

@@ -0,0 +1,32 @@
+<?php
+
+namespace Rector\Core\Tests\Issues\AddNodeAfterNodeStmt\Fixture;
+
+final class Fixture
+{
+    public function run()
+    {
+        if ($a === 1) {
+        }
+        echo 'existing next stmt after if';
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Core\Tests\Issues\AddNodeAfterNodeStmt\Fixture;
+
+final class Fixture
+{
+    public function run()
+    {
+        echo 'this is new stmt before if';
+        if ($a === 1) {
+        }
+        echo 'existing next stmt after if';
+    }
+}
+
+?>

tests/Issues/AddNodeBeforeNodeStmt/Fixture/next_html.php.inc

@@ -0,0 +1,12 @@
+<?php
+if ($a === 1) {
+}
+?>
+<div>some text next</div>
+-----
+<?php
+echo 'this is new stmt before if';
+if ($a === 1) {
+}
+?>
+<div>some text next</div>

tests/Issues/AddNodeBeforeNodeStmt/Source/AddBeforeStmtRector.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Core\Tests\Issues\AddNodeBeforeNodeStmt\Source;
+
+use PhpParser\Node;
+use PhpParser\Node\Scalar\String_;
+use PhpParser\Node\Stmt\Echo_;
+use PhpParser\Node\Stmt\If_;
+use Rector\Core\Rector\AbstractRector;
+use Rector\PostRector\Collector\NodesToAddCollector;
+use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+
+class AddBeforeStmtRector extends AbstractRector
+{
+    public function __construct(private readonly NodesToAddCollector $nodesToAddCollector)
+    {
+    }
+
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition('uff', []);
+    }
+
+    public function getNodeTypes(): array
+    {
+        return [
+            If_::class,
+        ];
+    }
+
+    public function refactor(Node $node)
+    {
+        $this->nodesToAddCollector->addNodeBeforeNode(
+            new Echo_([new String_("this is new stmt before if")]),
+            $node
+        );
+        return $node;
+    }
+}

tests/Issues/AddNodeBeforeNodeStmt/config/configured_rule.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\Core\Tests\Issues\AddNodeBeforeNodeStmt\Source\AddBeforeStmtRector;
+
+return static function (RectorConfig $rectorConfig): void {
+    $rectorConfig->rule(AddBeforeStmtRector::class);
+};