From 6669abe53a5bfa81d438aafed140833634d21f13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Mirtes?= Date: Wed, 28 Oct 2026 09:22:15 +0000 Subject: [PATCH 1/2] Use PhpDocParser from phpstan/phpdoc-parser without subclass BetterPhpDocParser now calls PhpDocParser::parse() and then runs the decorators. Positions come from the START_INDEX and END_INDEX attributes. Tags after an inline tag are now parsed. Before, "@internal Use @see Foo" ended the docblock and the next tags were lost. CloningPhpDocNodeVisitor is replaced with CloningVisitor. Amp-Thread-ID: https://ampcode.com/threads/T-01a17930-0285-73c5-a957-4325c77aa1b2 --- .../Fixture/after_inline_tag.php.inc | 28 ++++ .../DoctrineAnnotationDecorator.php | 3 - src/BetterPhpDocParser/PhpDocNodeMapper.php | 5 +- .../PhpDocParser/BetterPhpDocParser.php | 125 +++++++++--------- src/Configuration/Option.php | 24 ---- .../CloningPhpDocNodeVisitor.php | 27 ---- 6 files changed, 93 insertions(+), 119 deletions(-) create mode 100644 rules-tests/DeadCode/Rector/Property/RemoveUselessVarTagRector/Fixture/after_inline_tag.php.inc delete mode 100644 src/PhpDocParser/PhpDocParser/PhpDocNodeVisitor/CloningPhpDocNodeVisitor.php diff --git a/rules-tests/DeadCode/Rector/Property/RemoveUselessVarTagRector/Fixture/after_inline_tag.php.inc b/rules-tests/DeadCode/Rector/Property/RemoveUselessVarTagRector/Fixture/after_inline_tag.php.inc new file mode 100644 index 00000000000..944f67e01fa --- /dev/null +++ b/rules-tests/DeadCode/Rector/Property/RemoveUselessVarTagRector/Fixture/after_inline_tag.php.inc @@ -0,0 +1,28 @@ + +----- + diff --git a/src/BetterPhpDocParser/NodeDecorator/DoctrineAnnotationDecorator.php b/src/BetterPhpDocParser/NodeDecorator/DoctrineAnnotationDecorator.php index 7cb5d8a707d..b7bb60e2655 100644 --- a/src/BetterPhpDocParser/NodeDecorator/DoctrineAnnotationDecorator.php +++ b/src/BetterPhpDocParser/NodeDecorator/DoctrineAnnotationDecorator.php @@ -313,9 +313,6 @@ private function transformGenericTagValueNodesToDoctrineAnnotationTagValueNodes( $this->attributeMirrorer->mirror($phpDocNode->children[$key], $spacelessPhpDocTagNode); $phpDocNode->children[$key] = $spacelessPhpDocTagNode; - // require to reprint the generic - $phpDocNode->children[$key]->value->setAttribute(PhpDocAttributeKey::ORIG_NODE, null); - array_splice($phpDocNode->children, $key + 1, 0, $spacelessPhpDocTagNodes); } } diff --git a/src/BetterPhpDocParser/PhpDocNodeMapper.php b/src/BetterPhpDocParser/PhpDocNodeMapper.php index 9ff9dd04838..a7a20adea96 100644 --- a/src/BetterPhpDocParser/PhpDocNodeMapper.php +++ b/src/BetterPhpDocParser/PhpDocNodeMapper.php @@ -5,8 +5,8 @@ namespace Rector\BetterPhpDocParser; use PHPStan\PhpDocParser\Ast\NodeTraverser; +use PHPStan\PhpDocParser\Ast\NodeVisitor\CloningVisitor; use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocNode; -use Rector\PhpDocParser\PhpDocParser\PhpDocNodeVisitor\CloningPhpDocNodeVisitor; use Rector\PhpDocParser\PhpDocParser\PhpDocNodeVisitor\ParentConnectingPhpDocNodeVisitor; use Webmozart\Assert\Assert; @@ -18,11 +18,10 @@ private NodeTraverser $phpDocNodeTraverser; public function __construct( - CloningPhpDocNodeVisitor $cloningPhpDocNodeVisitor, ParentConnectingPhpDocNodeVisitor $parentConnectingPhpDocNodeVisitor, ) { $this->phpDocNodeTraverser = new NodeTraverser([ - $cloningPhpDocNodeVisitor, + new CloningVisitor(), $parentConnectingPhpDocNodeVisitor, ]); } diff --git a/src/BetterPhpDocParser/PhpDocParser/BetterPhpDocParser.php b/src/BetterPhpDocParser/PhpDocParser/BetterPhpDocParser.php index 976b6fe2615..95f0e45102e 100644 --- a/src/BetterPhpDocParser/PhpDocParser/BetterPhpDocParser.php +++ b/src/BetterPhpDocParser/PhpDocParser/BetterPhpDocParser.php @@ -6,19 +6,16 @@ use Nette\Utils\Strings; use PhpParser\Node; +use PHPStan\PhpDocParser\Ast\Attribute; +use PHPStan\PhpDocParser\Ast\Node as PhpDocParserNode; use PHPStan\PhpDocParser\Ast\PhpDoc\GenericTagValueNode; -use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocChildNode; use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocNode; use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode; -use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTextNode; use PHPStan\PhpDocParser\Lexer\Lexer; -use PHPStan\PhpDocParser\Parser\ConstExprParser; use PHPStan\PhpDocParser\Parser\ParserException; use PHPStan\PhpDocParser\Parser\PhpDocParser; use PHPStan\PhpDocParser\Parser\TokenIterator; -use PHPStan\PhpDocParser\Parser\TypeParser; -use PHPStan\PhpDocParser\ParserConfig; use Rector\BetterPhpDocParser\Contract\PhpDocParser\PhpDocNodeDecoratorInterface; use Rector\BetterPhpDocParser\NodeDecorator\ArrayItemClassNameDecorator; use Rector\BetterPhpDocParser\NodeDecorator\ConstExprClassNameDecorator; @@ -26,32 +23,39 @@ use Rector\BetterPhpDocParser\NodeDecorator\PhpDocTagGenericUsesDecorator; use Rector\BetterPhpDocParser\ValueObject\PhpDocAttributeKey; use Rector\BetterPhpDocParser\ValueObject\StartAndEnd; -use Rector\Util\Reflection\PrivatesAccessor; /** * @see \Rector\Tests\BetterPhpDocParser\PhpDocParser\TagValueNodeReprint\TagValueNodeReprintTest */ -final class BetterPhpDocParser extends PhpDocParser +final readonly class BetterPhpDocParser { /** * @see https://regex101.com/r/JOKSmr/5 */ private const string MULTI_NEW_LINES_REGEX = '#(?\r\n|\n){2,}#'; + /** + * @var int[] + */ + private const array NO_TEXT_TOKEN_TYPES = [ + Lexer::TOKEN_OPEN_PHPDOC, + Lexer::TOKEN_CLOSE_PHPDOC, + Lexer::TOKEN_PHPDOC_EOL, + Lexer::TOKEN_HORIZONTAL_WS, + Lexer::TOKEN_END, + ]; + /** * @var PhpDocNodeDecoratorInterface[] */ - private readonly array $phpDocNodeDecorators; + private array $phpDocNodeDecorators; public function __construct( - ParserConfig $parserConfig, - TypeParser $typeParser, - ConstExprParser $constExprParser, + private PhpDocParser $phpDocParser, ConstExprClassNameDecorator $constExprClassNameDecorator, DoctrineAnnotationDecorator $doctrineAnnotationDecorator, ArrayItemClassNameDecorator $arrayItemClassNameDecorator, PhpDocTagGenericUsesDecorator $phpDocTagGenericUsesDecorator, - private readonly PrivatesAccessor $privatesAccessor, ) { // The decorator order below is significant; keep it as is. DoctrineAnnotationDecorator must // run before ArrayItemClassNameDecorator, which resolves class names inside the array items the @@ -63,37 +67,35 @@ public function __construct( $arrayItemClassNameDecorator, $phpDocTagGenericUsesDecorator, ]; - - parent::__construct( - // ParserConfig - $parserConfig, - // TypeParser - $typeParser, - // ConstExprParser - $constExprParser, - ); } public function parseWithNode(TokenIterator $tokenIterator, Node $node): PhpDocNode { - $tokenIterator->consumeTokenType(Lexer::TOKEN_OPEN_PHPDOC); - $tokenIterator->tryConsumeTokenType(Lexer::TOKEN_PHPDOC_EOL); + try { + $phpDocNode = $this->phpDocParser->parse($tokenIterator); + } catch (ParserException) { + // invalid Doctrine annotation arguments; keep the docblock as it is + return $this->createTextPhpDocNode($tokenIterator); + } - $children = []; - if (! $tokenIterator->isCurrentTokenType(Lexer::TOKEN_CLOSE_PHPDOC)) { - $children[] = $this->parseChildAndStoreItsPositions($tokenIterator); + foreach ($phpDocNode->children as $phpDocChildNode) { + $this->setStartAndEnd($phpDocChildNode); - while ($tokenIterator->tryConsumeTokenType( - Lexer::TOKEN_PHPDOC_EOL - ) && ! $tokenIterator->isCurrentTokenType(Lexer::TOKEN_CLOSE_PHPDOC)) { - $children[] = $this->parseChildAndStoreItsPositions($tokenIterator); + if (! $phpDocChildNode instanceof PhpDocTagNode) { + continue; } - } - // might be in the middle of annotations - $tokenIterator->tryConsumeTokenType(Lexer::TOKEN_CLOSE_PHPDOC); + $this->setStartAndEnd($phpDocChildNode->value); + + if ($phpDocChildNode->value instanceof GenericTagValueNode) { + $phpDocChildNode->value->value = Strings::replace( + $phpDocChildNode->value->value, + self::MULTI_NEW_LINES_REGEX, + static fn (array $match): string => (string) $match['new_line'] + ); + } + } - $phpDocNode = new PhpDocNode($children); foreach ($this->phpDocNodeDecorators as $phpDocNodeDecorator) { $phpDocNodeDecorator->decorate($phpDocNode, $node); } @@ -101,46 +103,45 @@ public function parseWithNode(TokenIterator $tokenIterator, Node $node): PhpDocN return $phpDocNode; } - public function parseTagValue(TokenIterator $tokenIterator, string $tag): PhpDocTagValueNode + /** + * The Doctrine annotation decorator moves these positions to the nodes it creates + */ + private function setStartAndEnd(PhpDocParserNode $phpDocParserNode): void { - $startPosition = $tokenIterator->currentTokenIndex(); - $phpDocTagValueNode = parent::parseTagValue($tokenIterator, $tag); - - $endPosition = $tokenIterator->currentTokenIndex(); - - $startAndEnd = new StartAndEnd($startPosition, $endPosition); - $phpDocTagValueNode->setAttribute(PhpDocAttributeKey::START_AND_END, $startAndEnd); - - if ($phpDocTagValueNode instanceof GenericTagValueNode) { - $phpDocTagValueNode->value = Strings::replace( - $phpDocTagValueNode->value, - self::MULTI_NEW_LINES_REGEX, - static fn (array $match): string => (string) $match['new_line'] - ); + $startIndex = $phpDocParserNode->getAttribute(Attribute::START_INDEX); + $endIndex = $phpDocParserNode->getAttribute(Attribute::END_INDEX); + if (! is_int($startIndex) || ! is_int($endIndex)) { + return; } - return $phpDocTagValueNode; + // an empty node has no tokens, its end index is before its start index + $phpDocParserNode->setAttribute( + PhpDocAttributeKey::START_AND_END, + new StartAndEnd($startIndex, max($startIndex, $endIndex + 1)) + ); } /** - * @return PhpDocTextNode|PhpDocTagNode + * One text node with the content between "/**" and "*\/" */ - private function parseChildAndStoreItsPositions(TokenIterator $tokenIterator): PhpDocChildNode + private function createTextPhpDocNode(TokenIterator $tokenIterator): PhpDocNode { - $startPosition = $tokenIterator->currentTokenIndex(); + $tokens = $tokenIterator->getTokens(); + $startIndex = 0; + $endIndex = count($tokens) - 1; - try { - /** @var PhpDocTextNode|PhpDocTagNode $phpDocNode */ - $phpDocNode = $this->privatesAccessor->callPrivateMethod($this, 'parseChild', [$tokenIterator]); - } catch (ParserException) { - $phpDocNode = new PhpDocTextNode(''); + while ($startIndex < $endIndex && in_array($tokens[$startIndex][Lexer::TYPE_OFFSET], self::NO_TEXT_TOKEN_TYPES, true)) { + ++$startIndex; } - $endPosition = $tokenIterator->currentTokenIndex(); + while ($endIndex > $startIndex && in_array($tokens[$endIndex][Lexer::TYPE_OFFSET], self::NO_TEXT_TOKEN_TYPES, true)) { + --$endIndex; + } - $startAndEnd = new StartAndEnd($startPosition, $endPosition); - $phpDocNode->setAttribute(PhpDocAttributeKey::START_AND_END, $startAndEnd); + $phpDocTextNode = new PhpDocTextNode($tokenIterator->getContentBetween($startIndex, $endIndex + 1)); + $phpDocTextNode->setAttribute(Attribute::START_INDEX, $startIndex); + $phpDocTextNode->setAttribute(Attribute::END_INDEX, $endIndex); - return $phpDocNode; + return new PhpDocNode([$phpDocTextNode]); } } diff --git a/src/Configuration/Option.php b/src/Configuration/Option.php index 604a5e14016..82000e10f74 100644 --- a/src/Configuration/Option.php +++ b/src/Configuration/Option.php @@ -12,7 +12,6 @@ final class Option /** * @internal Use @see \Rector\Config\RectorConfig::bootstrapFiles() instead - * @var string */ public const string BOOTSTRAP_FILES = 'bootstrap_files'; @@ -26,19 +25,16 @@ final class Option /** * @internal Use @see \Rector\Config\RectorConfig::phpVersion() instead - * @var string */ public const string PHP_VERSION_FEATURES = 'php_version_features'; /** * @internal Use @see \Rector\Config\RectorConfig::importNames() instead - * @var string */ public const string AUTO_IMPORT_NAMES = 'auto_import_names'; /** * @internal Use @see \Rector\Config\RectorConfig::polyfillPackages() instead - * @var string */ public const string POLYFILL_PACKAGES = 'polyfill_packages'; @@ -52,31 +48,26 @@ final class Option /** * @internal Use @see \Rector\Config\RectorConfig::importNames() instead - * @var string */ public const string AUTO_IMPORT_DOC_BLOCK_NAMES = 'auto_import_doc_block_names'; /** * @internal Use @see \Rector\Config\RectorConfig::importShortClasses() instead - * @var string */ public const string IMPORT_SHORT_CLASSES = 'import_short_classes'; /** * @internal Use @see \Rector\Config\RectorConfig::symfonyContainerXml() instead - * @var string */ public const string SYMFONY_CONTAINER_XML_PATH_PARAMETER = 'symfony_container_xml_path'; /** * @internal Use @see \Rector\Config\RectorConfig::symfonyContainerPhp() - * @var string */ public const string SYMFONY_CONTAINER_PHP_PATH_PARAMETER = 'symfony_container_php_path'; /** * @internal Use @see \Rector\Config\RectorConfig::newLineOnFluentCall() - * @var string */ public const string NEW_LINE_ON_FLUENT_CALL = 'new_line_on_fluent_call'; @@ -86,31 +77,26 @@ final class Option /** * @internal Use @see \Rector\Config\RectorConfig::parallel() instead - * @var string */ public const string PARALLEL = 'parallel'; /** * @internal Use @see \Rector\Config\RectorConfig::paths() instead - * @var string */ public const string PATHS = 'paths'; /** * @internal Use @see \Rector\Config\RectorConfig::autoloadPaths() instead - * @var string */ public const string AUTOLOAD_PATHS = 'autoload_paths'; /** * @internal Use @see \Rector\Config\RectorConfig::skip() instead - * @var string */ public const string SKIP = 'skip'; /** * @internal Use @see \Rector\Config\RectorConfig::reportUnusedSkips() instead - * @var string */ public const string REPORT_UNUSED_SKIPS = 'report_unused_skips'; @@ -154,7 +140,6 @@ final class Option /** * @internal Use @see \Rector\Config\RectorConfig::phpstanConfig() instead - * @var string */ public const string PHPSTAN_FOR_RECTOR_PATHS = 'phpstan_for_rector_paths'; @@ -168,19 +153,16 @@ final class Option /** * @internal Use @see \Rector\Config\RectorConfig::parallel() instead with pass int $jobSize parameter - * @var string */ public const string PARALLEL_JOB_SIZE = 'parallel-job-size'; /** * @internal Use @see \Rector\Config\RectorConfig::parallel() instead with pass int $maxNumberOfProcess parameter - * @var string */ public const string PARALLEL_MAX_NUMBER_OF_PROCESSES = 'parallel-max-number-of-processes'; /** * @internal Use @see \Rector\Config\RectorConfig::parallel() instead with pass int $seconds parameter - * @var string */ public const string PARALLEL_JOB_TIMEOUT_IN_SECONDS = 'parallel-job-timeout-in-seconds'; @@ -188,25 +170,21 @@ final class Option /** * @internal Use @see \Rector\Config\RectorConfig::indent() method - * @var string */ public const string INDENT_CHAR = 'indent-char'; /** * @internal Use @see \Rector\Config\RectorConfig::indent() method - * @var string */ public const string INDENT_SIZE = 'indent-size'; /** * @internal Use @see \Rector\Config\RectorConfig::removeUnusedImports() method - * @var string */ public const string REMOVE_UNUSED_IMPORTS = 'remove-unused-imports'; /** * @internal Use @see \Rector\Config\RectorConfig::containerCacheDirectory() method - * @var string */ public const string CONTAINER_CACHE_DIRECTORY = 'container-cache-directory'; @@ -269,7 +247,6 @@ final class Option /** * @internal Use @see \Rector\Config\RectorConfig::treatClassesAsFinal() method - * @var string */ public const string TREAT_CLASSES_AS_FINAL = 'treat_classes_as_final'; @@ -322,7 +299,6 @@ final class Option /** * @internal Use @see \Rector\Config\RectorConfig::typeGuardedClasses() instead - * @var string */ public const string TYPE_GUARDED_CLASSES = 'type_guarded_classes'; } diff --git a/src/PhpDocParser/PhpDocParser/PhpDocNodeVisitor/CloningPhpDocNodeVisitor.php b/src/PhpDocParser/PhpDocParser/PhpDocNodeVisitor/CloningPhpDocNodeVisitor.php deleted file mode 100644 index 5d3be124d05..00000000000 --- a/src/PhpDocParser/PhpDocParser/PhpDocNodeVisitor/CloningPhpDocNodeVisitor.php +++ /dev/null @@ -1,27 +0,0 @@ -hasAttribute(PhpDocAttributeKey::ORIG_NODE)) { - $clonedNode->setAttribute(PhpDocAttributeKey::ORIG_NODE, $node); - } - - return $clonedNode; - } -} From 3a9581143a46e9482efddfe4709aa9f4e7c632de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Mirtes?= Date: Wed, 28 Oct 2026 09:40:38 +0000 Subject: [PATCH 2/2] Skip incomplete Doctrine annotation with nested annotations @ORM\Table('user', indexes={...}) with a single-quoted argument gets InvalidTagValueNode. Its value ends before the nested @ORM\Index lines. NestedAnnotationToAttributeRector then threw ShouldNotHappenException. The decorator now keeps such annotation and its next children unchanged. Amp-Thread-ID: https://ampcode.com/threads/T-01a17930-0285-73c5-a957-4325c77aa1b2 --- .../skip_single_quoted_alias.php.inc | 16 ++++++++++++++++ .../DoctrineAnnotationDecorator.php | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 rules-tests/Php80/Rector/Class_/AnnotationToAttributeRector/FixturePhp81/skip_single_quoted_alias.php.inc diff --git a/rules-tests/Php80/Rector/Class_/AnnotationToAttributeRector/FixturePhp81/skip_single_quoted_alias.php.inc b/rules-tests/Php80/Rector/Class_/AnnotationToAttributeRector/FixturePhp81/skip_single_quoted_alias.php.inc new file mode 100644 index 00000000000..5ff57cc60a1 --- /dev/null +++ b/rules-tests/Php80/Rector/Class_/AnnotationToAttributeRector/FixturePhp81/skip_single_quoted_alias.php.inc @@ -0,0 +1,16 @@ +children as $key => $phpDocChildNode) { + // the next children are the rest of an incomplete annotation, e.g. its nested annotations; keep them + if ($incompleteAnnotationContent !== null) { + $incompleteAnnotationContent .= "\n" . $phpDocChildNode; + if ($this->isClosedContent($incompleteAnnotationContent)) { + $incompleteAnnotationContent = null; + } + + continue; + } + // the @\FQN use case if ($phpDocChildNode instanceof PhpDocTextNode) { $this->processTextSpacelessInTextNode($phpDocNode, $phpDocChildNode, $currentPhpNode, $key); @@ -215,6 +227,12 @@ private function transformGenericTagValueNodesToDoctrineAnnotationTagValueNodes( // single quoted got invalid tag, keep process if ($phpDocChildNode->value instanceof InvalidTagValueNode) { + // the value ends before a nested annotation on the next line + if (! $this->isClosedContent($phpDocChildNode->value->value)) { + $incompleteAnnotationContent = $phpDocChildNode->value->value; + continue; + } + $name = ltrim($phpDocChildNode->name, '@'); $values = $phpDocChildNode->value->value;