Repository: Roave/BetterReflection (php)

Issue:
Enforce `StringSourceLocator` to be built with a `non-empty-string`
Currently, a `StringSourceLocator` can be constructed with an empty string, but will crash at runtime when doing so:

https://github.com/Roave/BetterReflection/blob/f3966ba125e3b4db948a240e8b29bc28882623c1/src/SourceLocator/Type/StringSourceLocator.php#L23-L38

We should really get rid of the exception, and instead enforce `non-empty-string` as input. 

Pull request:
`StringSourceLocator` is forced to be built with a `non-empty-string`
Fixes #932

I've started with 6.0.0 cleanup.

Patch:
diff --git a/src/SourceLocator/Exception/EmptyPhpSourceCode.php b/src/SourceLocator/Exception/EmptyPhpSourceCode.php
deleted file mode 100644
index 7b789f49..00000000
--- a/src/SourceLocator/Exception/EmptyPhpSourceCode.php
+++ /dev/null
@@ -1,11 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Roave\BetterReflection\SourceLocator\Exception;
-
-use InvalidArgumentException;
-
-class EmptyPhpSourceCode extends InvalidArgumentException
-{
-}
diff --git a/src/SourceLocator/Type/StringSourceLocator.php b/src/SourceLocator/Type/StringSourceLocator.php
index 783ba62e..f1210ccd 100644
--- a/src/SourceLocator/Type/StringSourceLocator.php
+++ b/src/SourceLocator/Type/StringSourceLocator.php
@@ -7,7 +7,6 @@ namespace Roave\BetterReflection\SourceLocator\Type;
 use InvalidArgumentException;
 use Roave\BetterReflection\Identifier\Identifier;
 use Roave\BetterReflection\SourceLocator\Ast\Locator;
-use Roave\BetterReflection\SourceLocator\Exception\EmptyPhpSourceCode;
 use Roave\BetterReflection\SourceLocator\Exception\InvalidFileLocation;
 use Roave\BetterReflection\SourceLocator\Located\LocatedSource;
 
@@ -20,19 +19,10 @@ use Roave\BetterReflection\SourceLocator\Located\LocatedSource;
  */
 class StringSourceLocator extends AbstractSourceLocator
 {
-    /** @throws EmptyPhpSourceCode */
+    /** @param non-empty-string $source */
     public function __construct(private string $source, Locator $astLocator)
     {
         parent::__construct($astLocator);
-
-        if (empty($source)) {
-            // Whilst an empty string is still "valid" PHP code, there is no
-            // point in us even trying to parse it because we won't find what
-            // we are looking for, therefore this throws an exception
-            throw new EmptyPhpSourceCode(
-                'Source code string was empty',
-            );
-        }
     }
 
     /**


Test patch:
diff --git a/test/unit/Reflection/ReflectionClassConstantTest.php b/test/unit/Reflection/ReflectionClassConstantTest.php
index 2ff0bac2..abb4b9c5 100644
--- a/test/unit/Reflection/ReflectionClassConstantTest.php
+++ b/test/unit/Reflection/ReflectionClassConstantTest.php
@@ -126,7 +126,11 @@ class ReflectionClassConstantTest extends TestCase
         self::assertSame($classInfo, $const->getDeclaringClass());
     }
 
-    /** @dataProvider startEndLineProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider startEndLineProvider
+     */
     public function testStartEndLine(string $php, int $startLine, int $endLine): void
     {
         $reflector       = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
@@ -136,7 +140,7 @@ class ReflectionClassConstantTest extends TestCase
         self::assertEquals($endLine, $constReflection->getEndLine());
     }
 
-    /** @return list<array{0: string, 1: int, 2: int}> */
+    /** @return list<array{0: non-empty-string, 1: int, 2: int}> */
     public function startEndLineProvider(): array
     {
         return [
@@ -147,7 +151,7 @@ class ReflectionClassConstantTest extends TestCase
         ];
     }
 
-    /** @return list<array{0: string, 1: int, 2: int}> */
+    /** @return list<array{0: non-empty-string, 1: int, 2: int}> */
     public function columnsProvider(): array
     {
         return [
@@ -157,7 +161,11 @@ class ReflectionClassConstantTest extends TestCase
         ];
     }
 
-    /** @dataProvider columnsProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider columnsProvider
+     */
     public function testGetStartColumnAndEndColumn(string $php, int $startColumn, int $endColumn): void
     {
         $reflector          = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
diff --git a/test/unit/Reflection/ReflectionClassTest.php b/test/unit/Reflection/ReflectionClassTest.php
index 45c902dd..e47b88ed 100644
--- a/test/unit/Reflection/ReflectionClassTest.php
+++ b/test/unit/Reflection/ReflectionClassTest.php
@@ -814,7 +814,7 @@ PHP;
         self::assertSame(['Roave\\BetterReflectionTest\\Fixture\\ClassWithParent', 'Roave\\BetterReflectionTest\\Fixture\\ExampleClass'], $childReflection->getParentClassNames());
     }
 
-    /** @return list<array{0: string, 1: int, 2: int}> */
+    /** @return list<array{0: non-empty-string, 1: int, 2: int}> */
     public function startEndLineProvider(): array
     {
         return [
@@ -824,7 +824,11 @@ PHP;
         ];
     }
 
-    /** @dataProvider startEndLineProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider startEndLineProvider
+     */
     public function testStartEndLine(string $php, int $expectedStart, int $expectedEnd): void
     {
         $reflector = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
@@ -834,7 +838,7 @@ PHP;
         self::assertSame($expectedEnd, $classInfo->getEndLine());
     }
 
-    /** @return list<array{0: string, 1: int, 2: int}> */
+    /** @return list<array{0: non-empty-string, 1: int, 2: int}> */
     public function columnsProvider(): array
     {
         return [
@@ -844,7 +848,11 @@ PHP;
         ];
     }
 
-    /** @dataProvider columnsProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider columnsProvider
+     */
     public function testGetStartColumnAndEndColumn(string $php, int $startColumn, int $endColumn): void
     {
         $reflector = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
diff --git a/test/unit/Reflection/ReflectionConstantTest.php b/test/unit/Reflection/ReflectionConstantTest.php
index 7d7cc751..c93ecb37 100644
--- a/test/unit/Reflection/ReflectionConstantTest.php
+++ b/test/unit/Reflection/ReflectionConstantTest.php
@@ -255,7 +255,7 @@ class ReflectionConstantTest extends TestCase
         self::assertStringContainsString('This constant comment should be used.', $reflection->getDocComment());
     }
 
-    /** @return list<array{0: string, 1: int, 2: int}> */
+    /** @return list<array{0: non-empty-string, 1: int, 2: int}> */
     public function startEndLineProvider(): array
     {
         return [
@@ -265,7 +265,11 @@ class ReflectionConstantTest extends TestCase
         ];
     }
 
-    /** @dataProvider startEndLineProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider startEndLineProvider
+     */
     public function testStartEndLine(string $php, int $expectedStart, int $expectedEnd): void
     {
         $reflector  = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
@@ -275,7 +279,7 @@ class ReflectionConstantTest extends TestCase
         self::assertSame($expectedEnd, $reflection->getEndLine());
     }
 
-    /** @return list<array{0: string, 1: int, 2: int}> */
+    /** @return list<array{0: non-empty-string, 1: int, 2: int}> */
     public function columnsProvider(): array
     {
         return [
@@ -284,7 +288,11 @@ class ReflectionConstantTest extends TestCase
         ];
     }
 
-    /** @dataProvider columnsProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider columnsProvider
+     */
     public function testGetStartColumnAndEndColumn(string $php, int $startColumn, int $endColumn): void
     {
         $reflector  = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
diff --git a/test/unit/Reflection/ReflectionFunctionAbstractTest.php b/test/unit/Reflection/ReflectionFunctionAbstractTest.php
index bb088af3..d5d6505d 100644
--- a/test/unit/Reflection/ReflectionFunctionAbstractTest.php
+++ b/test/unit/Reflection/ReflectionFunctionAbstractTest.php
@@ -177,7 +177,7 @@ class ReflectionFunctionAbstractTest extends TestCase
         self::assertNull($function->getExtensionName());
     }
 
-    /** @return list<array{0: string, 1: bool}> */
+    /** @return list<array{0: non-empty-string, 1: bool}> */
     public function variadicProvider(): array
     {
         return [
@@ -187,7 +187,11 @@ class ReflectionFunctionAbstractTest extends TestCase
         ];
     }
 
-    /** @dataProvider variadicProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider variadicProvider
+     */
     public function testIsVariadic(string $php, bool $expectingVariadic): void
     {
         $reflector = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
@@ -202,7 +206,7 @@ class ReflectionFunctionAbstractTest extends TestCase
      *
      * @see https://github.com/nikic/PHP-Parser/blob/1.x/test/code/parser/stmt/function/generator.test
      *
-     * @return list<array{0: string, 1: bool}>
+     * @return list<array{0: non-empty-string, 1: bool}>
      */
     public function generatorProvider(): array
     {
@@ -227,7 +231,11 @@ class ReflectionFunctionAbstractTest extends TestCase
         ];
     }
 
-    /** @dataProvider generatorProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider generatorProvider
+     */
     public function testIsGenerator(string $php, bool $expectingGenerator): void
     {
         $reflector = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
@@ -236,7 +244,7 @@ class ReflectionFunctionAbstractTest extends TestCase
         self::assertSame($expectingGenerator, $function->isGenerator());
     }
 
-    /** @return list<array{0: string, 1: int, 2: int}> */
+    /** @return list<array{0: non-empty-string, 1: int, 2: int}> */
     public function startEndLineProvider(): array
     {
         return [
@@ -246,7 +254,11 @@ class ReflectionFunctionAbstractTest extends TestCase
         ];
     }
 
-    /** @dataProvider startEndLineProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider startEndLineProvider
+     */
     public function testStartEndLine(string $php, int $expectedStart, int $expectedEnd): void
     {
         $reflector = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
@@ -256,7 +268,7 @@ class ReflectionFunctionAbstractTest extends TestCase
         self::assertSame($expectedEnd, $function->getEndLine());
     }
 
-    /** @return list<array{0: string, 1: int, 2: int}> */
+    /** @return list<array{0: non-empty-string, 1: int, 2: int}> */
     public function columnsProvider(): array
     {
         return [
@@ -266,7 +278,11 @@ class ReflectionFunctionAbstractTest extends TestCase
         ];
     }
 
-    /** @dataProvider columnsProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider columnsProvider
+     */
     public function testGetStartColumnAndEndColumn(string $php, int $startColumn, int $endColumn): void
     {
         $reflector = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
@@ -276,7 +292,7 @@ class ReflectionFunctionAbstractTest extends TestCase
         self::assertSame($endColumn, $function->getEndColumn());
     }
 
-    /** @return list<array{0: string, 1: bool}> */
+    /** @return list<array{0: non-empty-string, 1: bool}> */
     public function returnsReferenceProvider(): array
     {
         return [
@@ -285,7 +301,11 @@ class ReflectionFunctionAbstractTest extends TestCase
         ];
     }
 
-    /** @dataProvider returnsReferenceProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider returnsReferenceProvider
+     */
     public function testReturnsReference(string $php, bool $expectingReturnsReference): void
     {
         $reflector = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
diff --git a/test/unit/Reflection/ReflectionParameterTest.php b/test/unit/Reflection/ReflectionParameterTest.php
index ae84125f..1f4c4011 100644
--- a/test/unit/Reflection/ReflectionParameterTest.php
+++ b/test/unit/Reflection/ReflectionParameterTest.php
@@ -687,7 +687,7 @@ class ReflectionParameterTest extends TestCase
         self::assertSame('object', $type->__toString());
     }
 
-    /** @return list<array{0: string, 1: int, 2: int}> */
+    /** @return list<array{0: non-empty-string, 1: int, 2: int}> */
     public function columnsProvider(): array
     {
         return [
@@ -698,7 +698,11 @@ class ReflectionParameterTest extends TestCase
         ];
     }
 
-    /** @dataProvider columnsProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider columnsProvider
+     */
     public function testGetStartColumnAndEndColumn(string $php, int $startColumn, int $endColumn): void
     {
         $reflector = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
diff --git a/test/unit/Reflection/ReflectionPropertyTest.php b/test/unit/Reflection/ReflectionPropertyTest.php
index af9c4020..a0235323 100644
--- a/test/unit/Reflection/ReflectionPropertyTest.php
+++ b/test/unit/Reflection/ReflectionPropertyTest.php
@@ -268,7 +268,11 @@ class ReflectionPropertyTest extends TestCase
         clone $publicProp;
     }
 
-    /** @dataProvider startEndLineProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider startEndLineProvider
+     */
     public function testStartEndLine(string $php, int $startLine, int $endLine): void
     {
         $reflector       = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
@@ -278,7 +282,7 @@ class ReflectionPropertyTest extends TestCase
         self::assertEquals($endLine, $constReflection->getEndLine());
     }
 
-    /** @return list<array{0: string, 1: int, 2: int}> */
+    /** @return list<array{0: non-empty-string, 1: int, 2: int}> */
     public function startEndLineProvider(): array
     {
         return [
@@ -289,7 +293,7 @@ class ReflectionPropertyTest extends TestCase
         ];
     }
 
-    /** @return list<array{0: string, 1: int, 2: int}> */
+    /** @return list<array{0: non-empty-string, 1: int, 2: int}> */
     public function columnsProvider(): array
     {
         return [
@@ -299,7 +303,11 @@ class ReflectionPropertyTest extends TestCase
         ];
     }
 
-    /** @dataProvider columnsProvider */
+    /**
+     * @param non-empty-string $php
+     *
+     * @dataProvider columnsProvider
+     */
     public function testGetStartColumnAndEndColumn(string $php, int $startColumn, int $endColumn): void
     {
         $reflector          = new DefaultReflector(new StringSourceLocator($php, $this->astLocator));
diff --git a/test/unit/SourceLocator/Type/StringSourceLocatorTest.php b/test/unit/SourceLocator/Type/StringSourceLocatorTest.php
index 77dabf08..abdb4816 100644
--- a/test/unit/SourceLocator/Type/StringSourceLocatorTest.php
+++ b/test/unit/SourceLocator/Type/StringSourceLocatorTest.php
@@ -10,7 +10,6 @@ use Roave\BetterReflection\Identifier\Identifier;
 use Roave\BetterReflection\Identifier\IdentifierType;
 use Roave\BetterReflection\Reflector\Reflector;
 use Roave\BetterReflection\SourceLocator\Ast\Locator;
-use Roave\BetterReflection\SourceLocator\Exception\EmptyPhpSourceCode;
 use Roave\BetterReflection\SourceLocator\Type\StringSourceLocator;
 use Roave\BetterReflectionTest\BetterReflectionSingleton;
 
@@ -62,10 +61,4 @@ class StringSourceLocatorTest extends TestCase
 
         self::assertSame('Foo', $reflectionClass->getName());
     }
-
-    public function testConstructorThrowsExceptionIfEmptyStringGiven(): void
-    {
-        $this->expectException(EmptyPhpSourceCode::class);
-        new StringSourceLocator('', $this->astLocator);
-    }
 }