diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 14f91cdd161..5405c84fb3b 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -295,6 +295,18 @@ parameters: count: 1 path: src/DependencyInjection/NeonAdapter.php + - + rawMessage: 'Call to method getIndentationBefore() of internal class PhpParser\Internal\TokenStream from outside its root namespace PhpParser.' + identifier: method.internalClass + count: 1 + path: src/Fixable/PhpPrinter.php + + - + rawMessage: 'Call to method getTokenCode() of internal class PhpParser\Internal\TokenStream from outside its root namespace PhpParser.' + identifier: method.internalClass + count: 1 + path: src/Fixable/PhpPrinter.php + - rawMessage: 'Call to method getTokenCode() of internal class PhpParser\Internal\TokenStream from outside its root namespace PhpParser.' identifier: method.internalClass diff --git a/src/Analyser/RuleErrorTransformer.php b/src/Analyser/RuleErrorTransformer.php index 6f043096dc0..43ea60731d7 100644 --- a/src/Analyser/RuleErrorTransformer.php +++ b/src/Analyser/RuleErrorTransformer.php @@ -15,9 +15,11 @@ use PHPStan\DependencyInjection\AutowiredService; use PHPStan\File\FileReader; use PHPStan\Fixable\NewClassNamesVisitor; +use PHPStan\Fixable\PathCloningVisitor; use PHPStan\Fixable\PhpPrinter; use PHPStan\Fixable\PhpPrinterIndentationDetectorVisitor; use PHPStan\Fixable\ReplacingNodeVisitor; +use PHPStan\Fixable\SharedNodeCloningVisitor; use PHPStan\Node\VirtualNode; use PHPStan\Parser\Parser as AnalysisParser; use PHPStan\Parser\ParserErrorsException; @@ -35,6 +37,7 @@ use SebastianBergmann\Diff\Differ; use SebastianBergmann\Diff\Output\UnifiedDiffOutputBuilder; use function array_key_exists; +use function array_map; use function array_shift; use function array_slice; use function count; @@ -217,50 +220,66 @@ private function fix(string $fixingFile, array $fileNodes, array $fixes): ?Fixed $parsedFile = $this->getParsedFile($fixingFile); $indentDetector = null; + $pathCloningVisitor = null; $visitors = []; + $fixedNodes = array_map(static fn (NodeFix $fix): Node => $fix->node, $fixes); if ($parsedFile['indent'] === null) { $indentDetector = new PhpPrinterIndentationDetectorVisitor(new TokenStream($parsedFile['tokens'], PhpPrinter::TAB_WIDTH)); $visitors[] = $indentDetector; + $visitors[] = new CloningVisitor(); + } elseif (PathCloningVisitor::canFind($fixedNodes)) { + $pathCloningVisitor = new PathCloningVisitor($fixedNodes); + $visitors[] = $pathCloningVisitor; + } else { + $visitors[] = new CloningVisitor(); } - $visitors[] = new CloningVisitor(); $replacingVisitor = new ReplacingNodeVisitor($fixes); $visitors[] = $replacingVisitor; - /** @var Stmt[] $newStmts */ - $newStmts = (new NodeTraverser(...$visitors))->traverse($fileNodes); + try { + /** @var Stmt[] $newStmts */ + $newStmts = (new NodeTraverser(...$visitors))->traverse($fileNodes); + + $indent = $parsedFile['indent']; + if ($indentDetector !== null) { + if (str_contains($indentDetector->indentCharacter, "\t")) { + $indent = "\t"; + } else { + $indent = str_repeat($indentDetector->indentCharacter, $indentDetector->indentSize); + } - $indent = $parsedFile['indent']; - if ($indentDetector !== null) { - if (str_contains($indentDetector->indentCharacter, "\t")) { - $indent = "\t"; - } else { - $indent = str_repeat($indentDetector->indentCharacter, $indentDetector->indentSize); + if ($indentDetector->found) { + $this->parsedFiles[$fixingFile]['indent'] = $indent; + } } - if ($indentDetector->found) { - $this->parsedFiles[$fixingFile]['indent'] = $indent; + if (!$replacingVisitor->isComplete()) { + return null; } - } - if (!$replacingVisitor->isComplete()) { - return null; - } + if (NewClassNamesVisitor::hasNewNames($replacingVisitor->getNewNodes())) { + if ($pathCloningVisitor !== null) { + // the visitors below change nodes in place, so the original nodes must not be shared + /** @var Stmt[] $newStmts */ + $newStmts = (new NodeTraverser(new SharedNodeCloningVisitor()))->traverse($newStmts); + } + $nameResolver = new NameResolver(new Collecting(), ['replaceNodes' => false]); + /** @var Stmt[] $newStmts */ + $newStmts = (new NodeTraverser( + $nameResolver, + new NewClassNamesVisitor($nameResolver->getNameContext(), $this->importNames, $this->preferSelf), + ))->traverse($newStmts); + } - if (NewClassNamesVisitor::hasNewNames($replacingVisitor->getNewNodes())) { - $nameResolver = new NameResolver(new Collecting(), ['replaceNodes' => false]); - /** @var Stmt[] $newStmts */ - $newStmts = (new NodeTraverser( - $nameResolver, - new NewClassNamesVisitor($nameResolver->getNameContext(), $this->importNames, $this->preferSelf), - ))->traverse($newStmts); - } + $newCode = $this->getPrinter($indent)->printFormatPreserving($newStmts, $fileNodes, $parsedFile['tokens']); + if ($parsedFile['code'] === $newCode) { + return null; + } - $newCode = $this->getPrinter($indent)->printFormatPreserving($newStmts, $fileNodes, $parsedFile['tokens']); - if ($parsedFile['code'] === $newCode) { - return null; + return new FixedErrorDiff($fixingFile, $parsedFile['hash'], $this->diff($parsedFile['code'], $newCode)); + } finally { + $pathCloningVisitor?->restoreOriginalNodes(); } - - return new FixedErrorDiff($fixingFile, $parsedFile['hash'], $this->diff($parsedFile['code'], $newCode)); } /** diff --git a/src/Fixable/PathCloningVisitor.php b/src/Fixable/PathCloningVisitor.php new file mode 100644 index 00000000000..901c4770550 --- /dev/null +++ b/src/Fixable/PathCloningVisitor.php @@ -0,0 +1,141 @@ + */ + private array $targetRanges; + + /** @var array */ + private array $targetKeys = []; + + private int $insideTarget = 0; + + /** @var list */ + private array $enteredTargets = []; + + /** @var list */ + private array $sharedNodes = []; + + /** + * @param non-empty-list $targets + */ + public function __construct(array $targets) + { + $ranges = []; + foreach ($targets as $target) { + $ranges[] = [$target->getStartFilePos(), $target->getEndFilePos()]; + $this->targetKeys[self::getKey($target)] = true; + } + $this->targetRanges = $ranges; + } + + /** + * @param non-empty-list $targets + */ + public static function canFind(array $targets): bool + { + foreach ($targets as $target) { + if ($target->getStartFilePos() < 0 || $target->getEndFilePos() < 0) { + return false; + } + } + + return true; + } + + #[Override] + public function enterNode(Node $node): Node|int + { + if ($this->insideTarget > 0) { + $this->enteredTargets[] = false; + return $this->cloneNode($node); + } + + if (!$this->containsTarget($node)) { + $this->enteredTargets[] = false; + $node->setAttribute('origNode', $node); + $this->sharedNodes[] = $node; + return NodeVisitor::DONT_TRAVERSE_CHILDREN; + } + + $isTarget = isset($this->targetKeys[self::getKey($node)]); + $this->enteredTargets[] = $isTarget; + if ($isTarget) { + $this->insideTarget++; + } + + return $this->cloneNode($node); + } + + #[Override] + public function leaveNode(Node $node): ?Node + { + if (array_pop($this->enteredTargets) === true) { + $this->insideTarget--; + } + + return null; + } + + /** + * Removes the origNode attribute from the original nodes again. + */ + public function restoreOriginalNodes(): void + { + foreach ($this->sharedNodes as $node) { + $attributes = $node->getAttributes(); + unset($attributes['origNode']); + $node->setAttributes($attributes); + } + $this->sharedNodes = []; + } + + private function containsTarget(Node $node): bool + { + $start = $node->getStartFilePos(); + $end = $node->getEndFilePos(); + if ($start < 0 || $end < 0) { + return true; + } + + foreach ($this->targetRanges as [$targetStart, $targetEnd]) { + if ($start <= $targetStart && $targetEnd <= $end) { + return true; + } + } + + return false; + } + + private function cloneNode(Node $node): Node + { + $clone = clone $node; + $clone->setAttribute('origNode', $node); + + return $clone; + } + + private static function getKey(Node $node): string + { + return $node::class . ':' . $node->getStartFilePos() . ':' . $node->getEndFilePos(); + } + +} diff --git a/src/Fixable/PhpPrinter.php b/src/Fixable/PhpPrinter.php index 6a3a9999aaf..7dca6b2d7f2 100644 --- a/src/Fixable/PhpPrinter.php +++ b/src/Fixable/PhpPrinter.php @@ -35,4 +35,34 @@ protected function pCommaSeparated(array $nodes): string return $result; } + /** + * A node that is its own origNode was not cloned, so a fix did not change it + * (see PathCloningVisitor). Its original code is copied as it is, without + * printing its children one by one. + */ + #[Override] + protected function p( + Node $node, + int $precedence = self::MAX_PRECEDENCE, + int $lhsPrecedence = self::MAX_PRECEDENCE, + bool $parentFormatPreserved = false, + ): string + { + if ( + $parentFormatPreserved + && $this->origTokens !== null + && $node->getAttribute('origNode') === $node + ) { + $startPos = $node->getStartTokenPos(); + $endPos = $node->getEndTokenPos(); + if ($startPos >= 0 && $endPos >= 0) { + $indentAdjustment = $this->indentLevel - $this->origTokens->getIndentationBefore($startPos); + + return $this->origTokens->getTokenCode($startPos, $endPos + 1, $indentAdjustment); + } + } + + return parent::p($node, $precedence, $lhsPrecedence, $parentFormatPreserved); + } + } diff --git a/src/Fixable/SharedNodeCloningVisitor.php b/src/Fixable/SharedNodeCloningVisitor.php new file mode 100644 index 00000000000..4c3a929e766 --- /dev/null +++ b/src/Fixable/SharedNodeCloningVisitor.php @@ -0,0 +1,54 @@ + */ + private array $enteredShared = []; + + #[Override] + public function enterNode(Node $node): Node + { + $origNode = $node->getAttribute('origNode'); + $isShared = $origNode === $node; + $this->enteredShared[] = $isShared; + if ($isShared) { + $this->insideShared++; + } + + $clone = clone $node; + if ($this->insideShared > 0) { + $clone->setAttribute('origNode', $node); + } + + return $clone; + } + + #[Override] + public function leaveNode(Node $node): ?Node + { + if (array_pop($this->enteredShared) === true) { + $this->insideShared--; + } + + return null; + } + +} diff --git a/tests/PHPStan/Fixable/RenameFunctionRule.php b/tests/PHPStan/Fixable/RenameFunctionRule.php new file mode 100644 index 00000000000..1cfa0513a3e --- /dev/null +++ b/tests/PHPStan/Fixable/RenameFunctionRule.php @@ -0,0 +1,54 @@ + + */ +final class RenameFunctionRule implements Rule +{ + + public function getNodeType(): string + { + return Function_::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if ($node->getAttribute('origNode') !== null) { + return [ + RuleErrorBuilder::message('Function has an origNode attribute.') + ->identifier('tests.origNode') + ->build(), + ]; + } + + if (str_ends_with($node->name->toString(), 'Renamed')) { + return []; + } + + return [ + RuleErrorBuilder::message('Renaming function.') + ->identifier('tests.renameFunction') + ->fixNode($node, static function (Function_ $function): Function_ { + $function->name = new Identifier($function->name->toString() . 'Renamed'); + + return $function; + }) + ->build(), + ]; + } + +} diff --git a/tests/PHPStan/Fixable/RenameFunctionRuleTest.php b/tests/PHPStan/Fixable/RenameFunctionRuleTest.php new file mode 100644 index 00000000000..27950f2fed4 --- /dev/null +++ b/tests/PHPStan/Fixable/RenameFunctionRuleTest.php @@ -0,0 +1,33 @@ + + */ +class RenameFunctionRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new RenameFunctionRule(); + } + + public function testFixesDoNotChangeTheAnalysedNodes(): void + { + $this->analyse([__DIR__ . '/data/rename-function.php'], [ + ['Renaming function.', 5], + ['Renaming function.', 10], + ['Renaming function.', 15], + ]); + } + + public function testFix(): void + { + $this->fix(__DIR__ . '/data/rename-function.php', __DIR__ . '/data/rename-function.php.fixed'); + } + +} diff --git a/tests/PHPStan/Fixable/data/rename-function.php b/tests/PHPStan/Fixable/data/rename-function.php new file mode 100644 index 00000000000..21127c79f3b --- /dev/null +++ b/tests/PHPStan/Fixable/data/rename-function.php @@ -0,0 +1,18 @@ +