Merge pull request #740 from lacatoire/fix/parse-error-source-location

Include file path and line number in parse error messages

Author: jaapio

src/phpDocumentor/Reflection/Php/Factory/File.php

@@ -105,7 +105,7 @@ private function createFile(CreateCommand $command): FileElement
     {
         $file = $command->getFile();
         $code = $file->getContents();
-        $nodes = $this->nodesFactory->create($code);
+        $nodes = $this->nodesFactory->create($code, $file->path());
 
         $fileToContext = new FileToContext();
         $typeContext = $fileToContext($nodes);

src/phpDocumentor/Reflection/Php/NodesFactory.php

@@ -13,7 +13,9 @@
 
 namespace phpDocumentor\Reflection\Php;
 
+use phpDocumentor\Reflection\Exception;
 use phpDocumentor\Reflection\NodeVisitor\ElementNameResolver;
+use PhpParser\Error;
 use PhpParser\Node;
 use PhpParser\NodeTraverser;
 use PhpParser\NodeTraverserInterface;
@@ -22,6 +24,8 @@
 use PhpParser\ParserFactory;
 use Webmozart\Assert\Assert;
 
+use function sprintf;
+
 /**
  * Factory to create a array of nodes from a provided file.
  *
@@ -59,12 +63,28 @@ public static function createInstance(int $kind = 1): self
      * Will convert the provided code to nodes.
      *
      * @param string $code code to process.
+     * @param string $filePath optional source file path for error context.
      *
      * @return Node[]
+     *
+     * @throws Exception when the provided code cannot be parsed.
      */
-    public function create(string $code): array
+    public function create(string $code, string $filePath = ''): array
     {
-        $nodes = $this->parser->parse($code);
+        try {
+            $nodes = $this->parser->parse($code);
+        } catch (Error $e) {
+            $line = $e->getStartLine();
+            $location = $filePath !== '' ? sprintf(' in %s', $filePath) : '';
+            $location .= $line > 0 ? sprintf(' on line %d', $line) : '';
+
+            throw new Exception(
+                sprintf('Syntax error%s: %s', $location, $e->getRawMessage()),
+                0,
+                $e,
+            );
+        }
+
         Assert::isArray($nodes);
 
         return $this->traverser->traverse($nodes);

tests/unit/phpDocumentor/Reflection/Php/Factory/FileTest.php

@@ -79,7 +79,7 @@ public function testMatches(): void
     public function testMiddlewareIsExecuted(): void
     {
         $file = new FileElement('aa', __FILE__);
-        $this->nodesFactoryMock->create(file_get_contents(__FILE__))->willReturn([]);
+        $this->nodesFactoryMock->create(file_get_contents(__FILE__), Argument::any())->willReturn([]);
         $middleware = $this->prophesize(Middleware::class);
         $middleware->execute(Argument::any(), Argument::any())->shouldBeCalled()->willReturn($file);
         $fixture = new File(
@@ -105,7 +105,7 @@ public function testMiddlewareIsChecked(): void
     #[DataProvider('nodeProvider')]
     public function testFileGetsCommentFromFirstNode(Node $node, DocBlockDescriptor $docblock): void
     {
-        $this->nodesFactoryMock->create(file_get_contents(__FILE__))->willReturn([$node]);
+        $this->nodesFactoryMock->create(file_get_contents(__FILE__), Argument::any())->willReturn([$node]);
         $this->docBlockFactory->create('Text', Argument::any())->willReturn($docblock);
 
         $strategies = $this->prophesize(StrategyContainer::class);

tests/unit/phpDocumentor/Reflection/Php/NodesFactoryTest.php

@@ -13,7 +13,9 @@
 
 namespace phpDocumentor\Reflection\Php;
 
+use phpDocumentor\Reflection\Exception;
 use phpDocumentor\Reflection\NodeVisitor\ElementNameResolver;
+use PhpParser\Error;
 use PhpParser\NodeTraverser;
 use PhpParser\NodeTraverserInterface;
 use PhpParser\NodeVisitor\NameResolver;
@@ -57,6 +59,44 @@ public function testThatCodeGetsConvertedIntoNodes(): void
         $this->assertSame(['traversed code'], $result);
     }
 
+    public function testThatParseErrorIncludesFileAndLine(): void
+    {
+        $factory = NodesFactory::createInstance();
+
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Syntax error in /some/file.php on line 1:');
+
+        $factory->create('<?php class { }', '/some/file.php');
+    }
+
+    public function testThatParseErrorWithoutFilePathOnlyIncludesLine(): void
+    {
+        $factory = NodesFactory::createInstance();
+
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Syntax error on line 1:');
+
+        $factory->create('<?php class { }');
+    }
+
+    public function testThatParseErrorWrapsOriginalException(): void
+    {
+        $parser = $this->prophesize(Parser::class);
+        $parser->parse('bad code')->willThrow(new Error('Unexpected token', ['startLine' => 42]));
+
+        $nodeTraverser = $this->prophesize(NodeTraverserInterface::class);
+
+        $factory = new NodesFactory($parser->reveal(), $nodeTraverser->reveal());
+
+        try {
+            $factory->create('bad code', '/path/to/source.php');
+            $this->fail('Expected Exception was not thrown');
+        } catch (Exception $e) {
+            $this->assertSame('Syntax error in /path/to/source.php on line 42: Unexpected token', $e->getMessage());
+            $this->assertInstanceOf(Error::class, $e->getPrevious());
+        }
+    }
+
     private function givenTheExpectedDefaultNodesFactory(): NodesFactory
     {
         $parser = (new ParserFactory())->createForNewestSupportedVersion();