Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 84 additions & 1 deletion src/PhpParser/Printer/BetterStandardPrinter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<string, int>
*/
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,
Expand Down Expand Up @@ -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)) {
Expand All @@ -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 array<int, 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
Expand Down
50 changes: 50 additions & 0 deletions tests/PhpParser/Printer/BetterStandardPrinterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -49,6 +53,32 @@ public function testAddingCommentOnSomeNodesFail(): void
);
}

public function testKeepBlankLinesBetweenMethodsWhenImplementsRemoved(): void
{
$fileContent = <<<'PHP'
<?php

class SomeClass implements \SomeInterface
{
public function first()
{
}

public function second()
{
}
}
PHP;

$printed = $this->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');
Expand Down Expand Up @@ -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<array<int, (Yield_|Expression|string)>>
*/
Expand Down
Loading