Repository: Roave/BetterReflection (php)

Issue:
Getting last doc comment instead of first one would match PHP reflection
See PHP behaviour: https://3v4l.org/RGiqv

PoC fix: https://github.com/ondrejmirtes/BetterReflection/commit/ab41f90af9119abb2ea9223cd22bf913c777de16

Original issue: https://github.com/phpstan/phpstan/issues/3402

Pull request:
Getting the last doc comment instead of the first one matches PHP reflection
Fixes #648 

There should be no BC break because the class is marked as `@internal`.

@ondrejmirtes is too lazy to send the PR :-D


Patch:
diff --git a/src/Reflection/ReflectionClass.php b/src/Reflection/ReflectionClass.php
index 5a2563f3..62ad0acb 100644
--- a/src/Reflection/ReflectionClass.php
+++ b/src/Reflection/ReflectionClass.php
@@ -33,7 +33,7 @@ use Roave\BetterReflection\Reflector\Exception\IdentifierNotFound;
 use Roave\BetterReflection\Reflector\Reflector;
 use Roave\BetterReflection\SourceLocator\Located\LocatedSource;
 use Roave\BetterReflection\Util\CalculateReflectionColumn;
-use Roave\BetterReflection\Util\GetFirstDocComment;
+use Roave\BetterReflection\Util\GetLastDocComment;
 use Traversable;
 use function array_combine;
 use function array_filter;
@@ -826,7 +826,7 @@ class ReflectionClass implements Reflection
 
     public function getDocComment() : string
     {
-        return GetFirstDocComment::forNode($this->node);
+        return GetLastDocComment::forNode($this->node);
     }
 
     public function isAnonymous() : bool
diff --git a/src/Reflection/ReflectionClassConstant.php b/src/Reflection/ReflectionClassConstant.php
index c9a82fc5..95426e2e 100644
--- a/src/Reflection/ReflectionClassConstant.php
+++ b/src/Reflection/ReflectionClassConstant.php
@@ -11,7 +11,7 @@ use Roave\BetterReflection\NodeCompiler\CompilerContext;
 use Roave\BetterReflection\Reflection\StringCast\ReflectionClassConstantStringCast;
 use Roave\BetterReflection\Reflector\Reflector;
 use Roave\BetterReflection\Util\CalculateReflectionColumn;
-use Roave\BetterReflection\Util\GetFirstDocComment;
+use Roave\BetterReflection\Util\GetLastDocComment;
 
 class ReflectionClassConstant
 {
@@ -160,7 +160,7 @@ class ReflectionClassConstant
      */
     public function getDocComment() : string
     {
-        return GetFirstDocComment::forNode($this->node);
+        return GetLastDocComment::forNode($this->node);
     }
 
     public function __toString() : string
diff --git a/src/Reflection/ReflectionConstant.php b/src/Reflection/ReflectionConstant.php
index 22991467..f3a4423b 100644
--- a/src/Reflection/ReflectionConstant.php
+++ b/src/Reflection/ReflectionConstant.php
@@ -16,7 +16,7 @@ use Roave\BetterReflection\Reflector\Reflector;
 use Roave\BetterReflection\SourceLocator\Located\LocatedSource;
 use Roave\BetterReflection\Util\CalculateReflectionColumn;
 use Roave\BetterReflection\Util\ConstantNodeChecker;
-use Roave\BetterReflection\Util\GetFirstDocComment;
+use Roave\BetterReflection\Util\GetLastDocComment;
 use function array_slice;
 use function assert;
 use function count;
@@ -269,7 +269,7 @@ class ReflectionConstant implements Reflection
      */
     public function getDocComment() : string
     {
-        return GetFirstDocComment::forNode($this->node);
+        return GetLastDocComment::forNode($this->node);
     }
 
     public function __toString() : string
diff --git a/src/Reflection/ReflectionFunctionAbstract.php b/src/Reflection/ReflectionFunctionAbstract.php
index 46d18d68..6542ac8a 100644
--- a/src/Reflection/ReflectionFunctionAbstract.php
+++ b/src/Reflection/ReflectionFunctionAbstract.php
@@ -30,7 +30,7 @@ use Roave\BetterReflection\SourceLocator\Located\LocatedSource;
 use Roave\BetterReflection\SourceLocator\Type\ClosureSourceLocator;
 use Roave\BetterReflection\TypesFinder\FindReturnType;
 use Roave\BetterReflection\Util\CalculateReflectionColumn;
-use Roave\BetterReflection\Util\GetFirstDocComment;
+use Roave\BetterReflection\Util\GetLastDocComment;
 use Roave\BetterReflection\Util\Visitor\ReturnNodeVisitor;
 use function array_filter;
 use function assert;
@@ -229,7 +229,7 @@ abstract class ReflectionFunctionAbstract
 
     public function getDocComment() : string
     {
-        return GetFirstDocComment::forNode($this->getNode());
+        return GetLastDocComment::forNode($this->getNode());
     }
 
     public function setDocCommentFromString(string $string) : void
diff --git a/src/Reflection/ReflectionProperty.php b/src/Reflection/ReflectionProperty.php
index 9258d3a0..93418d70 100644
--- a/src/Reflection/ReflectionProperty.php
+++ b/src/Reflection/ReflectionProperty.php
@@ -26,7 +26,7 @@ use Roave\BetterReflection\Reflector\Exception\IdentifierNotFound;
 use Roave\BetterReflection\Reflector\Reflector;
 use Roave\BetterReflection\TypesFinder\FindPropertyType;
 use Roave\BetterReflection\Util\CalculateReflectionColumn;
-use Roave\BetterReflection\Util\GetFirstDocComment;
+use Roave\BetterReflection\Util\GetLastDocComment;
 use function class_exists;
 use function func_num_args;
 use function get_class;
@@ -233,7 +233,7 @@ class ReflectionProperty
 
     public function getDocComment() : string
     {
-        return GetFirstDocComment::forNode($this->node);
+        return GetLastDocComment::forNode($this->node);
     }
 
     /**
diff --git a/src/Util/GetFirstDocComment.php b/src/Util/GetFirstDocComment.php
deleted file mode 100644
index c681b59d..00000000
--- a/src/Util/GetFirstDocComment.php
+++ /dev/null
@@ -1,31 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Roave\BetterReflection\Util;
-
-use PhpParser\Comment\Doc;
-use PhpParser\NodeAbstract;
-use function assert;
-use function is_string;
-
-/**
- * @internal
- */
-final class GetFirstDocComment
-{
-    public static function forNode(NodeAbstract $node) : string
-    {
-        foreach ($node->getComments() as $comment) {
-            if ($comment instanceof Doc) {
-                $text = $comment->getReformattedText();
-
-                assert(is_string($text));
-
-                return $text;
-            }
-        }
-
-        return '';
-    }
-}
diff --git a/src/Util/GetLastDocComment.php b/src/Util/GetLastDocComment.php
new file mode 100644
index 00000000..0e6e75c1
--- /dev/null
+++ b/src/Util/GetLastDocComment.php
@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Roave\BetterReflection\Util;
+
+use PhpParser\NodeAbstract;
+
+/**
+ * @internal
+ */
+final class GetLastDocComment
+{
+    public static function forNode(NodeAbstract $node) : string
+    {
+        $docComment = $node->getDocComment();
+
+        return $docComment !== null
+            ? $docComment->getReformattedText()
+            : '';
+    }
+}


Test patch:
diff --git a/test/unit/Fixture/ExampleClass.php b/test/unit/Fixture/ExampleClass.php
index 8ee80d28..79e0fdd4 100644
--- a/test/unit/Fixture/ExampleClass.php
+++ b/test/unit/Fixture/ExampleClass.php
@@ -2,14 +2,20 @@
 
 namespace Roave\BetterReflectionTest\Fixture {
     /**
-     * Some comments here
+     * Unused class comment
+     */
+    /**
+     * This class comment should be used.
      */
     class ExampleClass
     {
         const MY_CONST_1 = 123;
 
         /**
-         * Documentation for constant
+         * Unused documentation for constant
+         */
+        /**
+         * This comment for constant should be used.
          */
         const MY_CONST_2 = 234;
         public const MY_CONST_3 = 345;
@@ -26,6 +32,7 @@ namespace Roave\BetterReflectionTest\Fixture {
          */
         protected $protectedProperty;
 
+        /** Unused property comment */
         /**
          * @var string
          */
diff --git a/test/unit/Fixture/ExampleClassExport.txt b/test/unit/Fixture/ExampleClassExport.txt
index 612be19b..5be76111 100644
--- a/test/unit/Fixture/ExampleClassExport.txt
+++ b/test/unit/Fixture/ExampleClassExport.txt
@@ -1,5 +1,5 @@
 Class [ <user> class Roave\BetterReflectionTest\Fixture\ExampleClass ] {
-  @@ %s/test/unit/Fixture/ExampleClass.php 7-43
+  @@ %s/test/unit/Fixture/ExampleClass.php 10-50
 
   - Constants [5] {
     Constant [ public integer MY_CONST_1 ] { 123 }
@@ -24,11 +24,11 @@ Class [ <user> class Roave\BetterReflectionTest\Fixture\ExampleClass ] {
 
   - Methods [2] {
     Method [ <user, ctor> public method __construct ] {
-      @@ %s/test/unit/Fixture/ExampleClass.php 36 - 38
+      @@ %s/test/unit/Fixture/ExampleClass.php 43 - 45
     }
 
     Method [ <user> public method someMethod ] {
-      @@ %s/test/unit/Fixture/ExampleClass.php 40 - 42
+      @@ %s/test/unit/Fixture/ExampleClass.php 47 - 49
     }
   }
 }
diff --git a/test/unit/Reflection/ReflectionClassConstantTest.php b/test/unit/Reflection/ReflectionClassConstantTest.php
index 9334758f..069e71d7 100644
--- a/test/unit/Reflection/ReflectionClassConstantTest.php
+++ b/test/unit/Reflection/ReflectionClassConstantTest.php
@@ -82,7 +82,7 @@ class ReflectionClassConstantTest extends TestCase
     public function testGetDocComment() : void
     {
         $const = $this->getExampleConstant('MY_CONST_2');
-        self::assertStringContainsString('Documentation for constant', $const->getDocComment());
+        self::assertStringContainsString('This comment for constant should be used.', $const->getDocComment());
     }
 
     public function testGetDocCommentReturnsEmptyStringWithNoComment() : void
diff --git a/test/unit/Reflection/ReflectionClassTest.php b/test/unit/Reflection/ReflectionClassTest.php
index 60256db2..34eaa3ee 100644
--- a/test/unit/Reflection/ReflectionClassTest.php
+++ b/test/unit/Reflection/ReflectionClassTest.php
@@ -623,7 +623,7 @@ PHP;
         $reflector = new ClassReflector($this->getComposerLocator());
         $classInfo = $reflector->reflect(ExampleClass::class);
 
-        self::assertStringContainsString('Some comments here', $classInfo->getDocComment());
+        self::assertStringContainsString('This class comment should be used.', $classInfo->getDocComment());
     }
 
     public function testGetDocCommentBetweenComments() : void
diff --git a/test/unit/Reflection/ReflectionConstantTest.php b/test/unit/Reflection/ReflectionConstantTest.php
index e5aac4ef..e11a7fae 100644
--- a/test/unit/Reflection/ReflectionConstantTest.php
+++ b/test/unit/Reflection/ReflectionConstantTest.php
@@ -203,12 +203,13 @@ class ReflectionConstantTest extends TestCase
             /**
              * @var int
              */
+            /** This constant comment should be used. */ 
             const FOO = 1;';
 
         $reflector  = new ConstantReflector(new StringSourceLocator($php, $this->astLocator), $this->classReflector);
         $reflection = $reflector->reflect('FOO');
 
-        self::assertNotNull($reflection->getDocComment());
+        self::assertStringContainsString('This constant comment should be used.', $reflection->getDocComment());
     }
 
     public function testGetDocCommentByDefine() : void
@@ -217,12 +218,13 @@ class ReflectionConstantTest extends TestCase
             /**
              * @var int
              */
+            /** This constant comment should be used. */ 
             define("FOO", 1);';
 
         $reflector  = new ConstantReflector(new StringSourceLocator($php, $this->astLocator), $this->classReflector);
         $reflection = $reflector->reflect('FOO');
 
-        self::assertNotNull($reflection->getDocComment());
+        self::assertStringContainsString('This constant comment should be used.', $reflection->getDocComment());
     }
 
     public function startEndLineProvider() : array
diff --git a/test/unit/Reflection/ReflectionFunctionAbstractTest.php b/test/unit/Reflection/ReflectionFunctionAbstractTest.php
index 959ec4a6..12a59a9e 100644
--- a/test/unit/Reflection/ReflectionFunctionAbstractTest.php
+++ b/test/unit/Reflection/ReflectionFunctionAbstractTest.php
@@ -288,15 +288,16 @@ class ReflectionFunctionAbstractTest extends TestCase
         /* --- This is a separator --------------- */
 
         /**
-         * Some function comment
+         * Unused function comment
          */
+        /** This function comment should be used. */ 
         function foo() {}
         ';
 
         $reflector    = new FunctionReflector(new StringSourceLocator($php, $this->astLocator), $this->classReflector);
         $functionInfo = $reflector->reflect('foo');
 
-        self::assertStringContainsString('Some function comment', $functionInfo->getDocComment());
+        self::assertStringContainsString('This function comment should be used.', $functionInfo->getDocComment());
     }
 
     public function testSetDocCommentFromString() : void
diff --git a/test/unit/Util/GetFirstDocCommentTest.php b/test/unit/Util/GetLastDocCommentTest.php
similarity index 70%
rename from test/unit/Util/GetFirstDocCommentTest.php
rename to test/unit/Util/GetLastDocCommentTest.php
index 59706532..9abdbf07 100644
--- a/test/unit/Util/GetFirstDocCommentTest.php
+++ b/test/unit/Util/GetLastDocCommentTest.php
@@ -8,26 +8,26 @@ use PhpParser\Comment;
 use PhpParser\Comment\Doc;
 use PhpParser\Node\Stmt\Function_;
 use PHPUnit\Framework\TestCase;
-use Roave\BetterReflection\Util\GetFirstDocComment;
+use Roave\BetterReflection\Util\GetLastDocComment;
 
 /**
- * @covers \Roave\BetterReflection\Util\GetFirstDocComment
+ * @covers \Roave\BetterReflection\Util\GetLastDocComment
  */
-class GetFirstDocCommentTest extends TestCase
+class GetLastDocCommentTest extends TestCase
 {
     public function testWithComment() : void
     {
         $comment = new Comment('/* An ordinary comment */');
         $node    = new Function_('test', [], ['comments' => [$comment]]);
 
-        self::assertSame('', GetFirstDocComment::forNode($node));
+        self::assertSame('', GetLastDocComment::forNode($node));
     }
 
     public function testWithoutComment() : void
     {
         $node = new Function_('test');
 
-        self::assertSame('', GetFirstDocComment::forNode($node));
+        self::assertSame('', GetLastDocComment::forNode($node));
     }
 
     public function testWithMixedCommentTypes() : void
@@ -36,7 +36,7 @@ class GetFirstDocCommentTest extends TestCase
         $docComment = new Doc('/** Property description */');
         $node       = new Function_('test', [], ['comments' => [$comment, $docComment]]);
 
-        self::assertSame('/** Property description */', GetFirstDocComment::forNode($node));
+        self::assertSame('/** Property description */', GetLastDocComment::forNode($node));
     }
 
     public function testWithMultipleDocComments() : void
@@ -45,6 +45,6 @@ class GetFirstDocCommentTest extends TestCase
         $comment2 = new Doc('/** Second doc comment */');
         $node     = new Function_('test', [], ['comments' => [$comment1, $comment2]]);
 
-        self::assertSame('/** First doc comment */', GetFirstDocComment::forNode($node));
+        self::assertSame('/** Second doc comment */', GetLastDocComment::forNode($node));
     }
 }