Merge pull request #697 from DanielBadura/handle-code-locations-missing

Handle gracefully code locations missing
Roave · Nov 23, 2022 · 8c6e978 · 8c6e978
2 parents 5e31508 + b4d41cf
commit 8c6e978
Show file tree

Hide file tree

Showing 10 changed files with 77 additions and 25 deletions.
diff --git a/src/Change.php b/src/Change.php
@@ -74,7 +74,7 @@ public function isSkipped(): bool
     /** @internal */
     public function withFilePositionsIfNotAlreadySet(
         string|null $file,
-        int $line,
+        int|null $line,
         int|null $column,
     ): self {
         $instance = clone $this;

diff --git a/src/DetectChanges/BCBreak/ClassBased/MultipleChecksOnAClass.php b/src/DetectChanges/BCBreak/ClassBased/MultipleChecksOnAClass.php
@@ -6,7 +6,7 @@
 
 use Roave\BackwardCompatibility\Change;
 use Roave\BackwardCompatibility\Changes;
-use Roave\BackwardCompatibility\Formatter\SymbolStartColumn;
+use Roave\BackwardCompatibility\Formatter\SymbolStart;
 use Roave\BetterReflection\Reflection\ReflectionClass;
 
 final class MultipleChecksOnAClass implements ClassBased
@@ -29,7 +29,7 @@ private function multipleChecks(ReflectionClass $fromClass, ReflectionClass $toC
     {
         $toFile   = $toClass->getFileName();
         $toLine   = $toClass->getStartLine();
-        $toColumn = SymbolStartColumn::get($toClass);
+        $toColumn = SymbolStart::getColumn($toClass);
 
         foreach ($this->checks as $check) {
             foreach ($check($fromClass, $toClass) as $change) {

diff --git a/src/DetectChanges/BCBreak/ClassConstantBased/MultipleChecksOnAClassConstant.php b/src/DetectChanges/BCBreak/ClassConstantBased/MultipleChecksOnAClassConstant.php
@@ -6,7 +6,7 @@
 
 use Roave\BackwardCompatibility\Change;
 use Roave\BackwardCompatibility\Changes;
-use Roave\BackwardCompatibility\Formatter\SymbolStartColumn;
+use Roave\BackwardCompatibility\Formatter\SymbolStart;
 use Roave\BetterReflection\Reflection\ReflectionClassConstant;
 
 final class MultipleChecksOnAClassConstant implements ClassConstantBased
@@ -28,7 +28,7 @@ public function __invoke(ReflectionClassConstant $fromConstant, ReflectionClassC
     private function multipleChecks(ReflectionClassConstant $fromConstant, ReflectionClassConstant $toConstant): iterable
     {
         $toLine   = $toConstant->getStartLine();
-        $toColumn = SymbolStartColumn::get($toConstant);
+        $toColumn = SymbolStart::getColumn($toConstant);
         $toFile   = $toConstant->getDeclaringClass()
             ->getFileName();
 

diff --git a/src/DetectChanges/BCBreak/FunctionBased/MultipleChecksOnAFunction.php b/src/DetectChanges/BCBreak/FunctionBased/MultipleChecksOnAFunction.php
@@ -6,7 +6,7 @@
 
 use Roave\BackwardCompatibility\Change;
 use Roave\BackwardCompatibility\Changes;
-use Roave\BackwardCompatibility\Formatter\SymbolStartColumn;
+use Roave\BackwardCompatibility\Formatter\SymbolStart;
 use Roave\BetterReflection\Reflection\ReflectionFunction;
 use Roave\BetterReflection\Reflection\ReflectionMethod;
 
@@ -40,8 +40,8 @@ private function multipleChecks(
         ReflectionMethod|ReflectionFunction $toFunction,
     ): iterable {
         $toFile   = $toFunction->getFileName();
-        $toLine   = $toFunction->getStartLine();
-        $toColumn = SymbolStartColumn::get($toFunction);
+        $toLine   = SymbolStart::getLine($toFunction);
+        $toColumn = SymbolStart::getColumn($toFunction);
 
         foreach ($this->checks as $check) {
             foreach ($check($fromFunction, $toFunction) as $change) {

diff --git a/src/DetectChanges/BCBreak/InterfaceBased/MultipleChecksOnAnInterface.php b/src/DetectChanges/BCBreak/InterfaceBased/MultipleChecksOnAnInterface.php
@@ -6,7 +6,7 @@
 
 use Roave\BackwardCompatibility\Change;
 use Roave\BackwardCompatibility\Changes;
-use Roave\BackwardCompatibility\Formatter\SymbolStartColumn;
+use Roave\BackwardCompatibility\Formatter\SymbolStart;
 use Roave\BetterReflection\Reflection\ReflectionClass;
 
 final class MultipleChecksOnAnInterface implements InterfaceBased
@@ -29,7 +29,7 @@ private function multipleChecks(ReflectionClass $fromInterface, ReflectionClass
     {
         $toFile   = $toInterface->getFileName();
         $toLine   = $toInterface->getStartLine();
-        $toColumn = SymbolStartColumn::get($toInterface);
+        $toColumn = SymbolStart::getColumn($toInterface);
 
         foreach ($this->checks as $check) {
             foreach ($check($fromInterface, $toInterface) as $change) {

diff --git a/src/DetectChanges/BCBreak/MethodBased/MultipleChecksOnAMethod.php b/src/DetectChanges/BCBreak/MethodBased/MultipleChecksOnAMethod.php
@@ -6,7 +6,7 @@
 
 use Roave\BackwardCompatibility\Change;
 use Roave\BackwardCompatibility\Changes;
-use Roave\BackwardCompatibility\Formatter\SymbolStartColumn;
+use Roave\BackwardCompatibility\Formatter\SymbolStart;
 use Roave\BetterReflection\Reflection\ReflectionMethod;
 
 final class MultipleChecksOnAMethod implements MethodBased
@@ -28,8 +28,8 @@ public function __invoke(ReflectionMethod $fromMethod, ReflectionMethod $toMetho
     private function multipleChecks(ReflectionMethod $fromMethod, ReflectionMethod $toMethod): iterable
     {
         $toFile   = $toMethod->getFileName();
-        $toLine   = $toMethod->getStartLine();
-        $toColumn = SymbolStartColumn::get($toMethod);
+        $toLine   = SymbolStart::getLine($toMethod);
+        $toColumn = SymbolStart::getColumn($toMethod);
 
         foreach ($this->checks as $check) {
             foreach ($check($fromMethod, $toMethod) as $change) {

diff --git a/src/DetectChanges/BCBreak/PropertyBased/MultipleChecksOnAProperty.php b/src/DetectChanges/BCBreak/PropertyBased/MultipleChecksOnAProperty.php
@@ -6,7 +6,7 @@
 
 use Roave\BackwardCompatibility\Change;
 use Roave\BackwardCompatibility\Changes;
-use Roave\BackwardCompatibility\Formatter\SymbolStartColumn;
+use Roave\BackwardCompatibility\Formatter\SymbolStart;
 use Roave\BetterReflection\Reflection\ReflectionProperty;
 
 final class MultipleChecksOnAProperty implements PropertyBased
@@ -27,8 +27,8 @@ public function __invoke(ReflectionProperty $fromProperty, ReflectionProperty $t
     /** @return iterable<int, Change> */
     private function multipleChecks(ReflectionProperty $fromProperty, ReflectionProperty $toProperty): iterable
     {
-        $toLine   = $toProperty->getStartLine();
-        $toColumn = SymbolStartColumn::get($toProperty);
+        $toLine   = SymbolStart::getLine($toProperty);
+        $toColumn = SymbolStart::getColumn($toProperty);
         $toFile   = $toProperty->getImplementingClass()
             ->getFileName();
 

diff --git a/src/DetectChanges/BCBreak/TraitBased/MultipleChecksOnATrait.php b/src/DetectChanges/BCBreak/TraitBased/MultipleChecksOnATrait.php
@@ -6,7 +6,7 @@
 
 use Roave\BackwardCompatibility\Change;
 use Roave\BackwardCompatibility\Changes;
-use Roave\BackwardCompatibility\Formatter\SymbolStartColumn;
+use Roave\BackwardCompatibility\Formatter\SymbolStart;
 use Roave\BetterReflection\Reflection\ReflectionClass;
 
 final class MultipleChecksOnATrait implements TraitBased
@@ -29,7 +29,7 @@ private function multipleChecks(ReflectionClass $fromTrait, ReflectionClass $toT
     {
         $toFile   = $toTrait->getFileName();
         $toLine   = $toTrait->getStartLine();
-        $toColumn = SymbolStartColumn::get($toTrait);
+        $toColumn = SymbolStart::getColumn($toTrait);
 
         foreach ($this->checks as $check) {
             foreach ($check($fromTrait, $toTrait) as $change) {

diff --git a/src/Formatter/SymbolStartColumn.php → src/Formatter/SymbolStart.php b/src/Formatter/SymbolStartColumn.php → src/Formatter/SymbolStart.php
@@ -13,7 +13,7 @@
 use RuntimeException;
 
 /** @internal */
-final class SymbolStartColumn
+final class SymbolStart
 {
     /**
      * Determines the column at which a given symbol starts, if able to do so.
@@ -24,7 +24,7 @@ final class SymbolStartColumn
      * declared via custom AST added by `roave/better-reflection` post-parsing (such as `__toString()`
      * in {@see \Stringable::__toString()}).
      */
-    public static function get(
+    public static function getColumn(
         ReflectionFunction|ReflectionMethod|ReflectionProperty|ReflectionClass|ReflectionClassConstant|ReflectionConstant $symbol,
     ): int|null {
         try {
@@ -33,4 +33,23 @@ public static function get(
             return null;
         }
     }
+
+    /**
+     * Determines the line at which a given symbol starts, if able to do so.
+     *
+     * Mostly exists because the reflection logic may be configured to skip parsing/collecting
+     * AST node exact positions, and because some sources (especially if stubbed from PHP core)
+     * could have trouble identifying the position of a symbol, given that some of them are
+     * declared via custom AST added by `roave/better-reflection` post-parsing (such as `__toString()`
+     * in {@see \Stringable::__toString()}).
+     */
+    public static function getLine(
+        ReflectionFunction|ReflectionMethod|ReflectionProperty|ReflectionClass|ReflectionClassConstant|ReflectionConstant $symbol,
+    ): int|null {
+        try {
+            return $symbol->getStartLine();
+        } catch (RuntimeException) {
+            return null;
+        }
+    }
 }
diff --git a/.../unit/Formatter/SymbolStartColumnTest.php → test/unit/Formatter/SymbolStartTest.php b/.../unit/Formatter/SymbolStartColumnTest.php → test/unit/Formatter/SymbolStartTest.php
@@ -7,7 +7,7 @@
 use BadMethodCallException;
 use PhpParser\Node\Stmt\Function_;
 use PHPUnit\Framework\TestCase;
-use Roave\BackwardCompatibility\Formatter\SymbolStartColumn;
+use Roave\BackwardCompatibility\Formatter\SymbolStart;
 use Roave\BetterReflection\BetterReflection;
 use Roave\BetterReflection\Identifier\Identifier;
 use Roave\BetterReflection\Identifier\IdentifierType;
@@ -19,8 +19,8 @@
 use Roave\BetterReflection\SourceLocator\Type\SourceLocator;
 use Roave\BetterReflection\SourceLocator\Type\StringSourceLocator;
 
-/** @covers \Roave\BackwardCompatibility\Formatter\SymbolStartColumn */
-final class SymbolStartColumnTest extends TestCase
+/** @covers \Roave\BackwardCompatibility\Formatter\SymbolStart */
+final class SymbolStartTest extends TestCase
 {
     public function testCanGetStartColumnForSimpleSymbol(): void
     {
@@ -29,7 +29,7 @@ public function testCanGetStartColumnForSimpleSymbol(): void
             (new BetterReflection())->astLocator(),
         ));
 
-        self::assertSame(32, SymbolStartColumn::get($reflector->reflectClass('A')));
+        self::assertSame(32, SymbolStart::getColumn($reflector->reflectClass('A')));
     }
 
     public function testCannotGetStartColumnWhenAstHasBeenParsedWithoutColumnLocations(): void
@@ -52,6 +52,39 @@ public function locateIdentifiersByType(Reflector $reflector, IdentifierType $id
             }
         });
 
-        self::assertNull(SymbolStartColumn::get($reflector->reflectFunction('foo')));
+        self::assertNull(SymbolStart::getColumn($reflector->reflectFunction('foo')));
+    }
+
+    public function testCanGetStartLineForSimpleSymbol(): void
+    {
+        $reflector = new DefaultReflector(new StringSourceLocator(
+            '<?php /* spacing on purpose */ class A {}',
+            (new BetterReflection())->astLocator(),
+        ));
+
+        self::assertSame(1, SymbolStart::getLine($reflector->reflectClass('A')));
+    }
+
+    public function testCannotGetStartLineWhenAstHasBeenParsedWithoutLineLocations(): void
+    {
+        $reflector = new DefaultReflector(new class implements SourceLocator {
+            /** Retrieves function `foo`, but without sources (invalid position) */
+            public function locateIdentifier(Reflector $reflector, Identifier $identifier): Reflection|null
+            {
+                return ReflectionFunction::createFromNode(
+                    (new BetterReflection())->reflector(),
+                    new Function_('foo'),
+                    new LocatedSource('', null),
+                );
+            }
+
+            /** {@inheritDoc} */
+            public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array
+            {
+                throw new BadMethodCallException('Unused');
+            }
+        });
+
+        self::assertNull(SymbolStart::getLine($reflector->reflectFunction('foo')));
     }
 }