Revert "[Printer] Remove AlwaysRememberedExpr check on BetterStandardPrinter"

[samsonasik]

Reverts #6484

Ref rectorphp/rector#8815 (comment)

[ondrejmirtes]

This bug will return for sure if you don't do what I tell you:

Class Rector\PhpParser\Printer\BetterStandardPrinter should extended PHPStan's Printer which has this method.

This is PHPStan's Printer class: https://github.com/phpstan/phpstan-src/blob/2.0.x/src/Node/Printer/Printer.php

[samsonasik]

@ondrejmirtes, @TomasVotruba don't want to extends PHPStan printer, see discussion of it #4305 (review)

[samsonasik]

Also, as PHPStan printer now final, so can't extends, so we can't do special node printing handling base on attribute kind that node have after node changed if we use PHPStan printer.

[ondrejmirtes]

I could un-final it in PHPStan. I want to be free to add more virtual nodes in PHPStan. Rector will inevitably break in the future because of this.

[samsonasik]

I will let @TomasVotruba decide, we have node visitor which unwrap AlwaysRememberedExpr (this node only)

rector-src/src/PHPStan/NodeVisitor/WrappedNodeRestoringNodeVisitor.php

Lines 15 to 29 in 8d33d50

	final class WrappedNodeRestoringNodeVisitor extends NodeVisitorAbstract
	{
	public function enterNode(Node $node): ?Node
	{
	if (! $node instanceof AlwaysRememberedExpr) {
	return null;
	}
	$expr = $node;
	while ($expr instanceof AlwaysRememberedExpr) {
	$expr = $expr->getExpr();
	}
	return $expr;
	}

that called after processNodes()

rector-src/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php

Lines 297 to 306 in 8d33d50

	$nodeTraverser = new NodeTraverser();
	$nodeTraverser->addVisitor(new WrappedNodeRestoringNodeVisitor());
	if ($hasUnreachableStatementNode) {
	$nodeTraverser->addVisitor(new UnreachableStatementNodeVisitor($this, $filePath, $scope));
	}
	$nodeTraverser->traverse($stmts);
	return $stmts;

this works well, until on specific circumtance, it somehow overlapped with what PHPStan apply during scan process, so this failsafe check instanceof AlwaysRememberedExpris needed in the BetterStandardPrinter:

rector-src/src/PhpParser/Printer/BetterStandardPrinter.php

Lines 117 to 119 in 8d33d50

	while ($node instanceof AlwaysRememberedExpr) {
	$node = $node->getExpr();
	}

that's can't be tested with unit test unfortunatelly.

[TomasVotruba]

We can give it a try :) we'll choose whichever option causes less maintenance

[ondrejmirtes]

Done phpstan/phpstan-src@1abeeb7

[samsonasik]

@ondrejmirtes I tried it, it seems after $this->nodeScopeResolver->processNodes(), it seems still needs unwrap, otherwise, it will still got error:

Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterRequiredRector\OptionalParametersAfterRequiredRectorTest::test with data set #7 ('/Users/samsonasik/www/rector-...hp.inc')
assert($subStartPos >= 0 && $subEndPos >= 0)

/Users/samsonasik/www/rector-src/vendor/nikic/php-parser/lib/PhpParser/PrettyPrinterAbstract.php:688
/Users/samsonasik/www/rector-src/src/PhpParser/Printer/BetterStandardPrinter.php:116

see PR:

• [Printer] Try extends PHPStan Printer #6518

[ondrejmirtes]

Sorry for mystifying here, the Printer class from PHPStan isn't usable for you. Making it final again.