From 2463982033b5cfd5194c96cc12a719b251cfbdfe Mon Sep 17 00:00:00 2001 From: Konrad Michalik Date: Thu, 23 Oct 2025 13:51:42 +0200 Subject: [PATCH 1/2] fix: enhance annotation parsing to support multiple values per tag --- src/Rules/DocBlockHeaderFixer.php | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/Rules/DocBlockHeaderFixer.php b/src/Rules/DocBlockHeaderFixer.php index 5fb6545..d3619c5 100644 --- a/src/Rules/DocBlockHeaderFixer.php +++ b/src/Rules/DocBlockHeaderFixer.php @@ -327,7 +327,7 @@ private function findInsertPosition(Tokens $tokens, int $structureIndex): int } /** - * @return array + * @return array> */ private function parseExistingAnnotations(string $docBlockContent): array { @@ -339,7 +339,19 @@ private function parseExistingAnnotations(string $docBlockContent): array if (preg_match('/^@(\w+)(?:\s+(.*))?$/', $line, $matches)) { $tag = $matches[1]; $value = $matches[2] ?? ''; - $annotations[$tag] = $value; + + // If this tag already exists, convert to array or append to existing array + if (isset($annotations[$tag])) { + // Convert existing single value to array + if (!is_array($annotations[$tag])) { + $annotations[$tag] = [$annotations[$tag]]; + } + // Append new value to array + $annotations[$tag][] = $value; + } else { + // First occurrence, store as string + $annotations[$tag] = $value; + } } } @@ -347,7 +359,7 @@ private function parseExistingAnnotations(string $docBlockContent): array } /** - * @param array $existing + * @param array> $existing * @param array> $new * * @return array> From 6c5abc6a56b40af2d9e5bf5996fa7f1e966a152f Mon Sep 17 00:00:00 2001 From: Konrad Michalik Date: Thu, 23 Oct 2025 14:14:58 +0200 Subject: [PATCH 2/2] test: add tests for parsing and merging duplicate annotations --- tests/src/Rules/DocBlockHeaderFixerTest.php | 114 ++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/tests/src/Rules/DocBlockHeaderFixerTest.php b/tests/src/Rules/DocBlockHeaderFixerTest.php index ca57d44..b967c91 100644 --- a/tests/src/Rules/DocBlockHeaderFixerTest.php +++ b/tests/src/Rules/DocBlockHeaderFixerTest.php @@ -196,6 +196,54 @@ public function testParseExistingAnnotations(): void self::assertSame($expected, $result); } + public function testParseExistingAnnotationsWithDuplicates(): void + { + $method = new ReflectionMethod($this->fixer, 'parseExistingAnnotations'); + + $docBlock = "/**\n * @implements ArrayAccess\n * @implements IteratorAggregate\n */"; + $result = $method->invoke($this->fixer, $docBlock); + + self::assertIsArray($result); + self::assertArrayHasKey('implements', $result); + self::assertIsArray($result['implements']); + self::assertCount(2, $result['implements']); + self::assertSame('ArrayAccess', $result['implements'][0]); + self::assertSame('IteratorAggregate', $result['implements'][1]); + } + + public function testParseExistingAnnotationsWithMultipleDuplicateTags(): void + { + $method = new ReflectionMethod($this->fixer, 'parseExistingAnnotations'); + + $docBlock = "/**\n * @author First Author\n * @license MIT\n * @author Second Author\n * @author Third Author\n */"; + $result = $method->invoke($this->fixer, $docBlock); + + self::assertIsArray($result); + self::assertArrayHasKey('author', $result); + self::assertIsArray($result['author']); + self::assertCount(3, $result['author']); + self::assertSame('First Author', $result['author'][0]); + self::assertSame('Second Author', $result['author'][1]); + self::assertSame('Third Author', $result['author'][2]); + self::assertSame('MIT', $result['license']); + } + + public function testParseExistingAnnotationsWithDuplicateEmptyValues(): void + { + $method = new ReflectionMethod($this->fixer, 'parseExistingAnnotations'); + + $docBlock = "/**\n * @internal\n * @api\n * @internal\n */"; + $result = $method->invoke($this->fixer, $docBlock); + + self::assertIsArray($result); + self::assertArrayHasKey('internal', $result); + self::assertIsArray($result['internal']); + self::assertCount(2, $result['internal']); + self::assertSame('', $result['internal'][0]); + self::assertSame('', $result['internal'][1]); + self::assertSame('', $result['api']); + } + public function testMergeAnnotations(): void { $method = new ReflectionMethod($this->fixer, 'mergeAnnotations'); @@ -994,4 +1042,70 @@ public function testSkipsAnonymousClassWithReadonlyModifier(): void // Anonymous class with readonly modifier should NOT have DocBlock added self::assertSame($code, $tokens->generateCode()); } + + public function testFullRoundTripWithDuplicateImplementsAnnotations(): void + { + $parseMethod = new ReflectionMethod($this->fixer, 'parseExistingAnnotations'); + $buildMethod = new ReflectionMethod($this->fixer, 'buildDocBlock'); + + // Original DocBlock with duplicate @implements + $originalDocBlock = "/**\n * @author Konrad Michalik\n * @license GPL-3.0\n * @implements ArrayAccess\n * @implements IteratorAggregate\n */"; + + // Parse existing annotations + $parsed = $parseMethod->invoke($this->fixer, $originalDocBlock); + + // Verify parsing preserved both @implements + self::assertIsArray($parsed['implements']); + self::assertCount(2, $parsed['implements']); + + // Build DocBlock from parsed annotations + $rebuilt = $buildMethod->invoke($this->fixer, $parsed, ''); + + // Verify both @implements are present in rebuilt DocBlock + self::assertStringContainsString('@author Konrad Michalik', $rebuilt); + self::assertStringContainsString('@license GPL-3.0', $rebuilt); + self::assertStringContainsString('@implements ArrayAccess', $rebuilt); + self::assertStringContainsString('@implements IteratorAggregate', $rebuilt); + } + + public function testMergeWithExistingDocBlockPreservesDuplicateImplements(): void + { + $code = "\n * @implements IteratorAggregate\n */ class TestClass {}"; + $tokens = Tokens::fromCode($code); + $annotations = ['author' => 'John Doe']; + + $method = new ReflectionMethod($this->fixer, 'mergeWithExistingDocBlock'); + + $this->fixer->configure(['preserve_existing' => true]); + $method->invoke($this->fixer, $tokens, 1, $annotations, 'TestClass'); + + $result = $tokens->generateCode(); + + // Both @implements should be preserved + self::assertStringContainsString('@implements ArrayAccess', $result); + self::assertStringContainsString('@implements IteratorAggregate', $result); + self::assertStringContainsString('@author John Doe', $result); + } + + public function testApplyFixPreservesDuplicateImplementsInExistingDocBlock(): void + { + $code = "\n * @implements IteratorAggregate\n */\nclass TestClass {}"; + $tokens = Tokens::fromCode($code); + $file = new SplFileInfo(__FILE__); + + $method = new ReflectionMethod($this->fixer, 'applyFix'); + + $this->fixer->configure([ + 'annotations' => ['author' => 'John Doe'], + 'preserve_existing' => true, + ]); + $method->invoke($this->fixer, $file, $tokens); + + $result = $tokens->generateCode(); + + // Verify both @implements annotations are preserved after merge + self::assertStringContainsString('@implements ArrayAccess', $result); + self::assertStringContainsString('@implements IteratorAggregate', $result); + self::assertStringContainsString('@author John Doe', $result); + } }