Clean up mix HTML+PHP first stmt check (#3285)

* Clean up mix HTML+PHP first stmt check * more failing fixture * more failing fixture * Fix * use newStmts count * [ci-review] Rector Rectify * Fix * Fix * first and last node must be different * Fix * more fixture * [ci-review] Rector Rectify * phpstan * rename method * [ci-review] Rector Rectify Co-authored-by: GitHub Action <action@github.com>
rectorphp · Jan 14, 2023 · 4b6a559 · 4b6a559
1 parent 410e939
commit 4b6a559
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 18 deletions.
diff --git a/...CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/mix_html_php2.php.inc b/...CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/mix_html_php2.php.inc
@@ -0,0 +1,23 @@
+<?php
+echo 'test';
+?>
+<div>
+    <?php
+        for ($i = 5; $i <= count($items); $i++) {
+            echo $items[$i];
+        }
+    ?>
+</div>
+-----
+<?php
+
+echo 'test';
+?>
+<div>
+    <?php 
+$itemsCount = count($items);
+for ($i = 5; $i <= $itemsCount; $i++) {
+            echo $items[$i];
+        }
+?>
+</div>
diff --git a/...CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/mix_html_php3.php.inc b/...CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/mix_html_php3.php.inc
@@ -0,0 +1,28 @@
+<?php
+echo 'test';
+?>
+<div>
+    <?php
+        for ($i = 5; $i <= count($items); $i++) {
+            echo $items[$i];
+        }
+    ?>
+</div>
+<?php
+echo 'test 2';
+?>
+-----
+<?php
+
+echo 'test';
+?>
+<div>
+    <?php 
+$itemsCount = count($items);
+for ($i = 5; $i <= $itemsCount; $i++) {
+            echo $items[$i];
+        }
+?>
+</div>
+<?php 
+echo 'test 2';
diff --git a/src/PhpParser/Printer/BetterStandardPrinter.php b/src/PhpParser/Printer/BetterStandardPrinter.php
@@ -21,7 +21,6 @@
 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_;
@@ -115,37 +114,32 @@ public function printFormatPreserving(array $stmts, array $origStmts, array $ori
             $content .= $this->nl;
         }
 
-        if ($newStmts === []) {
+        if (count($newStmts) <= 1) {
             return $content;
         }
 
-        /** @var Stmt $firstStmt */
         $firstStmt = current($newStmts);
-        $isFirstStmtReprinted = $firstStmt->getAttribute(AttributeKey::ORIGINAL_NODE) === null;
-        if (! $isFirstStmtReprinted) {
-            return $content;
-        }
+        $lastStmt = end($newStmts);
 
-        if (! $firstStmt instanceof InlineHTML) {
+        if (! $firstStmt instanceof InlineHTML && ! $lastStmt instanceof InlineHTML) {
             return $content;
         }
 
-        $lastStmt = end($newStmts);
-
-        if (str_starts_with($content, '<?php' . $this->nl . $this->nl . '?>')) {
-            $content = substr($content, 10);
+        if ($lastStmt instanceof InlineHTML && str_ends_with($content, '<?php ' . $this->nl)) {
+            $content = substr($content, 0, -7);
         }
 
-        if (str_starts_with($content, '?>' . $this->nl)) {
-            $content = str_replace('<?php <?php' . $this->nl, '<?php' . $this->nl, $content);
-            $content = substr($content, 3);
+        /** @var Node $firstStmt */
+        $isFirstStmtReprinted = $firstStmt->getAttribute(AttributeKey::ORIGINAL_NODE) === null;
+        if (! $isFirstStmtReprinted) {
+            return $content;
         }
 
-        if ($lastStmt instanceof InlineHTML && str_ends_with($content, '<?php ' . $this->nl)) {
-            return substr($content, 0, -7);
+        if (! $firstStmt instanceof InlineHTML) {
+            return $content;
         }
 
-        return $content;
+        return $this->cleanSurplusTag($content);
     }
 
     /**
@@ -517,6 +511,20 @@ protected function pParam(Param $param): string
             . ($param->default instanceof Expr ? ' = ' . $this->p($param->default) : '');
     }
 
+    private function cleanSurplusTag(string $content): string
+    {
+        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);
+        }
+
+        return $content;
+    }
+
     private function shouldPrintNewRawValue(LNumber|DNumber $lNumber): bool
     {
         return $lNumber->getAttribute(AttributeKey::REPRINT_RAW_VALUE) === true;