Merge pull request #1169 from kukulich/ast-failure

Improved `ParseToAstFailure` message
Roave · Aug 24, 2022 · 6531391 · 6531391
2 parents 71fa7c8 + f84fc33
commit 6531391
Show file tree

Hide file tree

Showing 2 changed files with 212 additions and 11 deletions.
diff --git a/src/SourceLocator/Ast/Exception/ParseToAstFailure.php b/src/SourceLocator/Ast/Exception/ParseToAstFailure.php
@@ -4,12 +4,18 @@
 
 namespace Roave\BetterReflection\SourceLocator\Ast\Exception;
 
+use PhpParser\Error;
 use Roave\BetterReflection\SourceLocator\Located\LocatedSource;
 use RuntimeException;
 use Throwable;
 
+use function array_slice;
+use function count;
+use function explode;
+use function implode;
+use function max;
+use function min;
 use function sprintf;
-use function substr;
 
 class ParseToAstFailure extends RuntimeException
 {
@@ -20,11 +26,32 @@ public static function fromLocatedSource(LocatedSource $locatedSource, Throwable
         $fileName = $locatedSource->getFileName();
 
         if ($fileName !== null) {
-            $additionalInformation = sprintf(' (in %s)', $fileName);
+            $additionalInformation .= sprintf(' in file %s', $fileName);
         }
 
-        if ($additionalInformation === '') {
-            $additionalInformation = sprintf(' (first 20 characters: %s)', substr($locatedSource->getSource(), 0, 20));
+        if ($previous instanceof Error) {
+            $errorStartLine = $previous->getStartLine();
+
+            $source = null;
+
+            if ($errorStartLine !== -1) {
+                $additionalInformation .= sprintf(' (line %d)', $errorStartLine);
+
+                $lines = explode("\n", $locatedSource->getSource());
+
+                $minLine = max(1, $errorStartLine - 5);
+                $maxLine = min(count($lines), $errorStartLine + 5);
+
+                $source = implode("\n", array_slice($lines, $minLine - 1, $maxLine - $minLine + 1));
+            }
+
+            $additionalInformation .= sprintf(': %s', $previous->getRawMessage());
+
+            if ($source !== null) {
+                $additionalInformation .= sprintf("\n\n%s", $source);
+            }
+        } else {
+            $additionalInformation .= sprintf(': %s', $previous->getMessage());
         }
 
         return new self(sprintf(

diff --git a/test/unit/SourceLocator/Ast/Exception/ParseToAstFailureTest.php b/test/unit/SourceLocator/Ast/Exception/ParseToAstFailureTest.php
@@ -5,6 +5,7 @@
 namespace Roave\BetterReflection\SourceLocator\Ast\Exception;
 
 use Exception;
+use PhpParser\Error;
 use PHPUnit\Framework\TestCase;
 use ReflectionProperty;
 use Roave\BetterReflection\SourceLocator\Located\LocatedSource;
@@ -14,33 +15,206 @@
  */
 class ParseToAstFailureTest extends TestCase
 {
-    public function testFromLocatedSourceWithoutFilename(): void
+    public function testErrorInTheMiddleOfSource(): void
     {
-        $locatedSource = new LocatedSource('<?php class SomeClass {}', 'Whatever');
+        $locatedSource = new LocatedSource(
+            <<<'PHP'
+            <?php
+            /**
+             * Some
+             * very
+             * long
+             * comment
+             */
+            class SomeClass
+            {
+                public function __construct(foo)
+                {
+                    $this->foo = $foo;
+                    $this->boo = 'boo';
 
-        $previous = new Exception();
+                    // More code
+                    // More code
+                    // More code
+                }
+            }
+            PHP,
+            'Whatever',
+        );
+
+        $previous = new Error('Error message', ['startLine' => 10]);
+
+        $exception = ParseToAstFailure::fromLocatedSource($locatedSource, $previous);
+
+        self::assertInstanceOf(ParseToAstFailure::class, $exception);
+        self::assertSame(
+            <<<'ERROR'
+            AST failed to parse in located source (line 10): Error message
+
+             * long
+             * comment
+             */
+            class SomeClass
+            {
+                public function __construct(foo)
+                {
+                    $this->foo = $foo;
+                    $this->boo = 'boo';
+
+                    // More code
+            ERROR,
+            $exception->getMessage(),
+        );
+        self::assertSame($previous, $exception->getPrevious());
+    }
+
+    public function testErrorAtTheBeginningOfSource(): void
+    {
+        $locatedSource = new LocatedSource(
+            <<<'PHP'
+            <?php
+            class SomeClass
+            {
+                // Code
+                // Code
+                // Code
+                // Code
+                // Code
+            }
+            PHP,
+            'Whatever',
+        );
+
+        $previous = new Error('Error message', ['startLine' => 2]);
+
+        $exception = ParseToAstFailure::fromLocatedSource($locatedSource, $previous);
+
+        self::assertInstanceOf(ParseToAstFailure::class, $exception);
+        self::assertSame(
+            <<<'ERROR'
+            AST failed to parse in located source (line 2): Error message
+
+            <?php
+            class SomeClass
+            {
+                // Code
+                // Code
+                // Code
+                // Code
+            ERROR,
+            $exception->getMessage(),
+        );
+        self::assertSame($previous, $exception->getPrevious());
+    }
+
+    public function testErrorAtTheEndOfSource(): void
+    {
+        $locatedSource = new LocatedSource(
+            <<<'PHP'
+            <?php
+
+            // Comment
+            // Comment
+            // Comment
+            // Comment
+            // Comment
+
+            class SomeClass
+            {
+            }
+            PHP,
+            'Whatever',
+        );
+
+        $previous = new Error('Error message', ['startLine' => 10]);
+
+        $exception = ParseToAstFailure::fromLocatedSource($locatedSource, $previous);
+
+        self::assertInstanceOf(ParseToAstFailure::class, $exception);
+        self::assertSame(
+            <<<'ERROR'
+            AST failed to parse in located source (line 10): Error message
+
+            // Comment
+            // Comment
+            // Comment
+
+            class SomeClass
+            {
+            }
+            ERROR,
+            $exception->getMessage(),
+        );
+        self::assertSame($previous, $exception->getPrevious());
+    }
+
+    public function testLocatedSourceWithoutFilename(): void
+    {
+        $locatedSource = new LocatedSource('<?php abc', 'Whatever');
+
+        $previous = new Error('Error message', ['startLine' => 1]);
 
         $exception = ParseToAstFailure::fromLocatedSource($locatedSource, $previous);
 
         self::assertInstanceOf(ParseToAstFailure::class, $exception);
-        self::assertSame('AST failed to parse in located source (first 20 characters: <?php class SomeClas)', $exception->getMessage());
+        self::assertSame(
+            <<<'ERROR'
+            AST failed to parse in located source (line 1): Error message
+
+            <?php abc
+            ERROR,
+            $exception->getMessage(),
+        );
         self::assertSame($previous, $exception->getPrevious());
     }
 
-    public function testFromLocatedSourceWithFilename(): void
+    public function testLocatedSourceWithFilename(): void
     {
         $locatedSource = new LocatedSource('<?php abc', 'Whatever');
 
         $filenameProperty = new ReflectionProperty($locatedSource, 'filename');
         $filenameProperty->setAccessible(true);
         $filenameProperty->setValue($locatedSource, '/foo/bar');
 
-        $previous = new Exception();
+        $previous = new Error('Some error message', ['startLine' => 1]);
+
+        $exception = ParseToAstFailure::fromLocatedSource($locatedSource, $previous);
+
+        self::assertInstanceOf(ParseToAstFailure::class, $exception);
+        self::assertSame(
+            <<<'ERROR'
+            AST failed to parse in located source in file /foo/bar (line 1): Some error message
+
+            <?php abc
+            ERROR,
+            $exception->getMessage(),
+        );
+        self::assertSame($previous, $exception->getPrevious());
+    }
+
+    public function testErrorWithoutLine(): void
+    {
+        $locatedSource = new LocatedSource('<?php abc', 'Whatever');
+
+        $previous = new Error('No line');
+
+        $exception = ParseToAstFailure::fromLocatedSource($locatedSource, $previous);
+
+        self::assertInstanceOf(ParseToAstFailure::class, $exception);
+        self::assertSame('AST failed to parse in located source: No line', $exception->getMessage());
+        self::assertSame($previous, $exception->getPrevious());
+    }
+
+    public function testUnknownError(): void
+    {
+        $locatedSource = new LocatedSource('<?php abc', 'Whatever');
+
+        $previous = new Exception('Unknown error');
 
         $exception = ParseToAstFailure::fromLocatedSource($locatedSource, $previous);
 
         self::assertInstanceOf(ParseToAstFailure::class, $exception);
-        self::assertSame('AST failed to parse in located source (in /foo/bar)', $exception->getMessage());
+        self::assertSame('AST failed to parse in located source: Unknown error', $exception->getMessage());
         self::assertSame($previous, $exception->getPrevious());
     }
 }