From 14488674900eee91d9b389aac4802225010c6dc5 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 29 Oct 2026 08:27:33 +0100 Subject: [PATCH 1/2] Keep blank lines between statements when a node list is emptied --- .../Printer/BetterStandardPrinter.php | 85 ++++++++++++++++++- .../Printer/BetterStandardPrinterTest.php | 50 +++++++++++ 2 files changed, 134 insertions(+), 1 deletion(-) diff --git a/src/PhpParser/Printer/BetterStandardPrinter.php b/src/PhpParser/Printer/BetterStandardPrinter.php index acfab301e97..ece34e8b844 100644 --- a/src/PhpParser/Printer/BetterStandardPrinter.php +++ b/src/PhpParser/Printer/BetterStandardPrinter.php @@ -29,8 +29,11 @@ use PhpParser\Node\InterpolatedStringPart; use PhpParser\Node\Scalar\InterpolatedString; use PhpParser\Node\Scalar\String_; +use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\Declare_; +use PhpParser\Node\Stmt\Enum_; use PhpParser\Node\Stmt\InlineHTML; +use PhpParser\Node\Stmt\Interface_; use PhpParser\Node\Stmt\Nop; use PhpParser\PrettyPrinter\Standard; use PhpParser\Token; @@ -55,6 +58,19 @@ final class BetterStandardPrinter extends Standard */ private const string EXTRA_SPACE_BEFORE_NOP_REGEX = '#^[ \t]+$#m'; + /** + * php-parser cannot emptying a node list, it falls back to a full reprint of the parent node + * and drops blank lines between its statements; these keyword tokens let us cut just the + * emptied clause instead, e.g. the "implements" of a class that loses its last interface + * + * @var array + */ + private const array EMPTIED_LIST_KEYWORD_TOKENS = [ + Class_::class . '->implements' => \T_IMPLEMENTS, + Enum_::class . '->implements' => \T_IMPLEMENTS, + Interface_::class . '->extends' => \T_EXTENDS, + ]; + public function __construct( private readonly ExprAnalyzer $exprAnalyzer, private readonly PrivatesAccessor $privatesAccessor, @@ -230,9 +246,20 @@ protected function pArray( string $subNodeName, ?int $fixup ): ?string { + $startPos = $pos; + $content = parent::pArray($nodes, $origNodes, $pos, $indentAdjustment, $parentNodeClass, $subNodeName, $fixup); if ($content === null) { - return $content; + $cutToPos = $this->resolveEmptiedNodeListEndPos($nodes, $origNodes, $startPos, $parentNodeClass, $subNodeName); + if ($cutToPos === null) { + // restore the position possibly advanced by the parent before falling back + $pos = $startPos; + return null; + } + + // drop the whole clause: whitespace + keyword + type list + $pos = $cutToPos; + return ''; } if (! $this->containsNop($nodes)) { @@ -242,6 +269,62 @@ protected function pArray( return Strings::replace($content, self::EXTRA_SPACE_BEFORE_NOP_REGEX); } + /** + * php-parser bails on emptying a node list, which forces the parent node to be fully reprinted + * and loses blank lines between statements; when the whole list is removed we can instead cut + * just the emptied clause (keyword + types) and keep the rest format-preserved + * + * Returns the token position right after the removed clause, or null when not handled. + * + * @param mixed[] $nodes + * @param mixed[] $origNodes + */ + private function resolveEmptiedNodeListEndPos( + array $nodes, + array $origNodes, + int $startPos, + string $parentNodeClass, + string $subNodeName + ): ?int { + // only the "whole list removed" case is handled here + if ($nodes !== [] || $origNodes === []) { + return null; + } + + if (! $this->origTokens instanceof TokenStream) { + return null; + } + + $keywordToken = self::EMPTIED_LIST_KEYWORD_TOKENS[$parentNodeClass . '->' . $subNodeName] ?? null; + if ($keywordToken === null) { + return null; + } + + $firstOrigNode = $origNodes[0]; + $lastOrigNode = $origNodes[count($origNodes) - 1]; + if (! $firstOrigNode instanceof Node || ! $lastOrigNode instanceof Node) { + return null; + } + + $firstStartPos = $firstOrigNode->getStartTokenPos(); + $lastEndPos = $lastOrigNode->getEndTokenPos(); + if ($firstStartPos < 0 || $lastEndPos < 0) { + return null; + } + + // the keyword must sit between the previous sub-node and the removed list + if (! $this->origTokens->haveTokenInRange($startPos, $firstStartPos, $keywordToken)) { + return null; + } + + // bail on php tags in the removed range to avoid dropping them + if ($this->origTokens->haveTagInRange($startPos, $lastEndPos + 1)) { + return null; + } + + return $lastEndPos + 1; + } + /** * Do not add "()" on Expressions * @see https://github.com/rectorphp/rector/pull/401#discussion_r181487199 diff --git a/tests/PhpParser/Printer/BetterStandardPrinterTest.php b/tests/PhpParser/Printer/BetterStandardPrinterTest.php index 582ce593519..6ba17954cba 100644 --- a/tests/PhpParser/Printer/BetterStandardPrinterTest.php +++ b/tests/PhpParser/Printer/BetterStandardPrinterTest.php @@ -11,12 +11,16 @@ use PhpParser\Node\Expr\Variable; use PhpParser\Node\Expr\Yield_; use PhpParser\Node\Scalar\String_; +use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Expression; +use PhpParser\NodeTraverser; +use PhpParser\NodeVisitor\CloningVisitor; use PHPUnit\Framework\Attributes\DataProvider; use Rector\Configuration\Option; use Rector\Configuration\Parameter\SimpleParameterProvider; use Rector\NodeTypeResolver\Node\AttributeKey; +use Rector\PhpParser\Parser\RectorParser; use Rector\PhpParser\Printer\BetterStandardPrinter; use Rector\Testing\PHPUnit\AbstractLazyTestCase; @@ -49,6 +53,32 @@ public function testAddingCommentOnSomeNodesFail(): void ); } + public function testKeepBlankLinesBetweenMethodsWhenImplementsRemoved(): void + { + $fileContent = <<<'PHP' +printWithRemovedImplements($fileContent); + + // the interface is gone... + $this->assertStringNotContainsString('implements', $printed); + + // ...but the blank line between methods is kept + $this->assertStringContainsString(" }\n\n public function second()", $printed); + } + public function testStringWithAddedComment(): void { $string = new String_('hey'); @@ -105,6 +135,26 @@ public function testNoNewlineOnFluentCallWithoutAttribute(): void $this->assertSame('$foo->bar()->baz()', $printed); } + private function printWithRemovedImplements(string $fileContent): string + { + $rectorParser = $this->make(RectorParser::class); + $stmtsAndTokens = $rectorParser->parseFileContentToStmtsAndTokens($fileContent); + + $oldStmts = $stmtsAndTokens->getStmts(); + $tokens = $stmtsAndTokens->getTokens(); + + $nodeTraverser = new NodeTraverser(new CloningVisitor()); + $newStmts = $nodeTraverser->traverse($oldStmts); + + foreach ($newStmts as $newStmt) { + if ($newStmt instanceof Class_) { + $newStmt->implements = []; + } + } + + return $this->betterStandardPrinter->printFormatPreserving($newStmts, $oldStmts, $tokens); + } + /** * @return Iterator> */ From 988c9c609f317ff0a8587adee136e4fd773004ff Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Thu, 29 Oct 2026 07:28:22 +0000 Subject: [PATCH 2/2] [ci-review] Rector Rectify --- src/PhpParser/Printer/BetterStandardPrinter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PhpParser/Printer/BetterStandardPrinter.php b/src/PhpParser/Printer/BetterStandardPrinter.php index ece34e8b844..6d551d83da4 100644 --- a/src/PhpParser/Printer/BetterStandardPrinter.php +++ b/src/PhpParser/Printer/BetterStandardPrinter.php @@ -277,7 +277,7 @@ protected function pArray( * Returns the token position right after the removed clause, or null when not handled. * * @param mixed[] $nodes - * @param mixed[] $origNodes + * @param array $origNodes */ private function resolveEmptiedNodeListEndPos( array $nodes,