diff --git a/src/Analyser/RuleErrorTransformer.php b/src/Analyser/RuleErrorTransformer.php index ef730cac9f4..c70835be6fc 100644 --- a/src/Analyser/RuleErrorTransformer.php +++ b/src/Analyser/RuleErrorTransformer.php @@ -246,12 +246,14 @@ private function fix(string $fixingFile, array $fileNodes, array $fixes): ?Fixed return null; } - $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::hasWork($replacingVisitor->getFixedNodes())) { + $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) { diff --git a/src/Fixable/NewClassNamesVisitor.php b/src/Fixable/NewClassNamesVisitor.php index e114f107c59..1acc9897675 100644 --- a/src/Fixable/NewClassNamesVisitor.php +++ b/src/Fixable/NewClassNamesVisitor.php @@ -95,6 +95,31 @@ public function __construct( $this->phpDocParser = new PhpDocParser($config, new TypeParser($config, $constExprParser), $constExprParser); } + /** + * Whether the nodes that the fixes returned contain a class name + * or a PHPDoc that this visitor may need to write. + * + * @param Node[] $fixedNodes + */ + public static function hasWork(array $fixedNodes): bool + { + return (new NodeFinder())->findFirst($fixedNodes, static function (Node $node): bool { + $originalNode = $node->getAttribute('origNode'); + if ($node instanceof FullyQualified && $originalNode === null) { + return true; + } + + $doc = $node->getDocComment(); + if ($doc === null) { + return false; + } + + $originalDoc = $originalNode instanceof Node ? $originalNode->getDocComment() : null; + + return $originalDoc === null || $originalDoc->getText() !== $doc->getText(); + }) !== null; + } + #[Override] public function beforeTraverse(array $nodes): ?array { diff --git a/src/Fixable/ReplacingNodeVisitor.php b/src/Fixable/ReplacingNodeVisitor.php index 99081afc3b2..3466c4df16b 100644 --- a/src/Fixable/ReplacingNodeVisitor.php +++ b/src/Fixable/ReplacingNodeVisitor.php @@ -35,6 +35,9 @@ final class ReplacingNodeVisitor extends NodeVisitorAbstract private bool $failed = false; + /** @var list */ + private array $fixedNodes = []; + /** * @param non-empty-list $fixes */ @@ -115,9 +118,21 @@ public function leaveNode(Node $node): Node|int|null } } + $this->fixedNodes[] = $newNode; + return $newNode; } + /** + * Nodes returned by the fixes. A fix changes only these nodes and their children. + * + * @return list + */ + public function getFixedNodes(): array + { + return $this->fixedNodes; + } + /** * Whether every node was found and fixed. */ diff --git a/tests/PHPStan/Fixable/AddNamesRule.php b/tests/PHPStan/Fixable/AddNamesRule.php index 2f73b4571dc..76853b09eaa 100644 --- a/tests/PHPStan/Fixable/AddNamesRule.php +++ b/tests/PHPStan/Fixable/AddNamesRule.php @@ -12,13 +12,15 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use function count; +use function in_array; use function preg_match_all; use function sprintf; /** * Turns each `@add-name \Some\Class` tag into a native parameter type * and a `@param` tag, to show new names are written the same way - * in PHP code and in PHPDocs. + * in PHP code and in PHPDocs. An `@add-doc-name \Some\Class` tag becomes + * only a `@param` tag. * * @implements Rule */ @@ -37,19 +39,27 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { $docComment = $node->getDocComment(); - if ($docComment === null || preg_match_all('/@add-name\s+\\\\?(\S+)/', $docComment->getText(), $matches) === 0) { + if ($docComment === null || preg_match_all('/@add-(doc-)?name\s+\\\\?(\S+)/', $docComment->getText(), $matches) === 0) { return []; } - $names = $matches[1]; + $names = $matches[2]; + $parameterNames = []; + foreach ($matches[1] as $j => $docOnly) { + if ($docOnly !== '') { + continue; + } + + $parameterNames[] = $names[$j]; + } return [ RuleErrorBuilder::message(sprintf('Adding %d names.', count($names))) ->identifier('tests.addNames') - ->fixNode($node, function (Node\Stmt\ClassMethod $method) use ($names) { + ->fixNode($node, function (Node\Stmt\ClassMethod $method) use ($parameterNames) { $i = 0; $this->phpDocEditor->edit($method, static function (\PHPStan\PhpDocParser\Ast\Node $phpDocNode) use (&$i) { - if (!$phpDocNode instanceof PhpDocTagNode || $phpDocNode->name !== '@add-name') { + if (!$phpDocNode instanceof PhpDocTagNode || !in_array($phpDocNode->name, ['@add-name', '@add-doc-name'], true)) { return null; } @@ -64,7 +74,7 @@ public function processNode(Node $node, Scope $scope): array )); }); - foreach ($names as $j => $name) { + foreach ($parameterNames as $j => $name) { $method->params[] = new Node\Param( new Node\Expr\Variable('p' . ($j + 1)), type: new FullyQualified($name), diff --git a/tests/PHPStan/Fixable/AddNamesRuleTest.php b/tests/PHPStan/Fixable/AddNamesRuleTest.php index 2b40879f90c..798bed33a1c 100644 --- a/tests/PHPStan/Fixable/AddNamesRuleTest.php +++ b/tests/PHPStan/Fixable/AddNamesRuleTest.php @@ -37,4 +37,9 @@ public function testFixSortedUses(): void $this->fix(__DIR__ . '/data/add-names-sorted-uses.php', __DIR__ . '/data/add-names-sorted-uses.php.fixed'); } + public function testFixPhpDocOnly(): void + { + $this->fix(__DIR__ . '/data/add-names-phpdoc-only.php', __DIR__ . '/data/add-names-phpdoc-only.php.fixed'); + } + } diff --git a/tests/PHPStan/Fixable/NewClassNamesVisitorTest.php b/tests/PHPStan/Fixable/NewClassNamesVisitorTest.php new file mode 100644 index 00000000000..42394e7433a --- /dev/null +++ b/tests/PHPStan/Fixable/NewClassNamesVisitorTest.php @@ -0,0 +1,97 @@ + + */ + public static function dataHasWork(): iterable + { + yield 'unchanged' => [ + static fn (ClassMethod $method): Node => $method, + false, + ]; + + yield 'removed attribute' => [ + static function (ClassMethod $method): Node { + $method->attrGroups = []; + return $method; + }, + false, + ]; + + yield 'added name' => [ + static function (ClassMethod $method): Node { + $method->params[] = new Node\Param(new Node\Expr\Variable('qux'), type: new FullyQualified('Qux\Quux')); + return $method; + }, + true, + ]; + + yield 'changed PHPDoc' => [ + static function (ClassMethod $method): Node { + $method->setDocComment(new Doc('/** @return positive-int */')); + return $method; + }, + true, + ]; + + yield 'new node with a PHPDoc' => [ + static function (ClassMethod $method): Node { + $newMethod = new ClassMethod('doBar'); + $newMethod->setDocComment(new Doc('/** @return void */')); + return $newMethod; + }, + true, + ]; + } + + /** + * @param callable(ClassMethod): Node $fix + */ + #[DataProvider('dataHasWork')] + public function testHasWork(callable $fix, bool $expected): void + { + $stmts = (new ParserFactory())->createForHostVersion()->parse(self::CODE); + $this->assertNotNull($stmts); + + $newStmts = (new NodeTraverser(new CloningVisitor()))->traverse($stmts); + $method = (new NodeFinder())->findFirstInstanceOf($newStmts, ClassMethod::class); + $this->assertNotNull($method); + + $this->assertSame($expected, NewClassNamesVisitor::hasWork([$fix($method)])); + } + +} diff --git a/tests/PHPStan/Fixable/data/add-names-phpdoc-only.php b/tests/PHPStan/Fixable/data/add-names-phpdoc-only.php new file mode 100644 index 00000000000..5a6c2c6d3e1 --- /dev/null +++ b/tests/PHPStan/Fixable/data/add-names-phpdoc-only.php @@ -0,0 +1,18 @@ +