Merge pull request #198 from phpDocumentor/fix/invalid-tag-usage-fix

Fix issue when processing invalid tags

Author: jaapio

src/DocBlock/DescriptionFactory.php

@@ -69,10 +69,7 @@ public function create(string $contents, ?TypeContext $context = null) : Descrip
         $tags     = [];
 
         for ($i = 1; $i < $count; $i += 2) {
-            $tag = $this->tagFactory->create($tokens[$i], $context);
-            if ($tag !== null) {
-                $tags[] = $tag;
-            }
+            $tags[]     = $this->tagFactory->create($tokens[$i], $context);
             $tokens[$i] = '%' . ++$tagCount . '$s';
         }
 

src/DocBlock/StandardTagFactory.php

@@ -19,6 +19,7 @@
 use phpDocumentor\Reflection\DocBlock\Tags\Deprecated;
 use phpDocumentor\Reflection\DocBlock\Tags\Factory\StaticMethod;
 use phpDocumentor\Reflection\DocBlock\Tags\Generic;
+use phpDocumentor\Reflection\DocBlock\Tags\InvalidTag;
 use phpDocumentor\Reflection\DocBlock\Tags\Link as LinkTag;
 use phpDocumentor\Reflection\DocBlock\Tags\Method;
 use phpDocumentor\Reflection\DocBlock\Tags\Param;
@@ -138,7 +139,7 @@ public function __construct(FqsenResolver $fqsenResolver, ?array $tagHandlers =
     /**
      * {@inheritDoc}
      */
-    public function create(string $tagLine, ?TypeContext $context = null) : ?Tag
+    public function create(string $tagLine, ?TypeContext $context = null) : Tag
     {
         if (!$context) {
             $context = new TypeContext('');
@@ -215,7 +216,7 @@ private function extractTagParts(string $tagLine) : array
      * Creates a new tag object with the given name and body or returns null if the tag name was recognized but the
      * body was invalid.
      */
-    private function createTag(string $body, string $name, TypeContext $context) : ?Tag
+    private function createTag(string $body, string $name, TypeContext $context) : Tag
     {
         $handlerClassName = $this->findHandlerClassName($name, $context);
         $arguments        = $this->getArgumentsForParametersFromWiring(
@@ -228,7 +229,7 @@ private function createTag(string $body, string $name, TypeContext $context) : ?
             $callable = [$handlerClassName, 'create'];
             return call_user_func_array($callable, $arguments);
         } catch (InvalidArgumentException $e) {
-            return null;
+            return InvalidTag::create($body, $name)->withError($e);
         }
     }
 

src/DocBlock/TagFactory.php

@@ -49,7 +49,7 @@ public function addParameter(string $name, $value) : void;
      *
      * @throws InvalidArgumentException If an invalid tag line was presented.
      */
-    public function create(string $tagLine, ?TypeContext $context = null) : ?Tag;
+    public function create(string $tagLine, ?TypeContext $context = null) : Tag;
 
     /**
      * Registers a service with the Service Locator using the FQCN of the class or the alias, if provided.

src/DocBlock/Tags/InvalidTag.php

@@ -0,0 +1,78 @@
+<?php
+
+declare(strict_types=1);
+
+namespace phpDocumentor\Reflection\DocBlock\Tags;
+
+use phpDocumentor\Reflection\DocBlock\Tag;
+use Throwable;
+
+/**
+ * This class represents an exception during the tag creation
+ *
+ * Since the internals of the library are relaying on the correct syntax of a docblock
+ * we cannot simply throw exceptions at all time because the exceptions will break the creation of a
+ * docklock. Just silently ignore the exceptions is not an option because the user as an issue to fix.
+ *
+ * This tag holds that error information until a using application is able to display it. The object wil just behave
+ * like any normal tag. So the normal application flow will not break.
+ */
+final class InvalidTag implements Tag
+{
+    /** @var string */
+    private $name;
+
+    /** @var string */
+    private $body;
+
+    /** @var Throwable|null */
+    private $throwable;
+
+    private function __construct(string $name, string $body)
+    {
+        $this->name = $name;
+        $this->body = $body;
+    }
+
+    public function getException() : ?Throwable
+    {
+        return $this->throwable;
+    }
+
+    public function getName() : string
+    {
+        return $this->name;
+    }
+
+    /**
+     * @return self
+     *
+     * @inheritDoc
+     */
+    public static function create(string $body, string $name = '')
+    {
+        return new self($name, $body);
+    }
+
+    public function withError(Throwable $exception) : self
+    {
+        $tag            = new self($this->name, $this->body);
+        $tag->throwable = $exception;
+
+        return $tag;
+    }
+
+    public function render(?Formatter $formatter = null) : string
+    {
+        if ($formatter === null) {
+            $formatter = new Formatter\PassthroughFormatter();
+        }
+
+        return $formatter->format($this);
+    }
+
+    public function __toString() : string
+    {
+        return $this->body;
+    }
+}

src/DocBlockFactory.php

@@ -17,7 +17,6 @@
 use LogicException;
 use phpDocumentor\Reflection\DocBlock\DescriptionFactory;
 use phpDocumentor\Reflection\DocBlock\StandardTagFactory;
-use phpDocumentor\Reflection\DocBlock\Tag;
 use phpDocumentor\Reflection\DocBlock\TagFactory;
 use Webmozart\Assert\Assert;
 use function array_shift;
@@ -236,12 +235,7 @@ private function parseTagBlock(string $tags, Types\Context $context) : array
         $result = [];
         $lines  = $this->splitTagBlockIntoTagLines($tags);
         foreach ($lines as $key => $tagLine) {
-            $tag = $this->tagFactory->create(trim($tagLine), $context);
-            if (!($tag instanceof Tag)) {
-                continue;
-            }
-
-            $result[$key] = $tag;
+            $result[$key] = $this->tagFactory->create(trim($tagLine), $context);
         }
 
         return $result;

tests/unit/DocBlock/DescriptionFactoryTest.php

@@ -13,7 +13,9 @@
 
 namespace phpDocumentor\Reflection\DocBlock;
 
+use Exception;
 use Mockery as m;
+use phpDocumentor\Reflection\DocBlock\Tags\InvalidTag;
 use phpDocumentor\Reflection\DocBlock\Tags\Link as LinkTag;
 use phpDocumentor\Reflection\Types\Context;
 use PHPUnit\Framework\TestCase;
@@ -162,6 +164,31 @@ public function testIfSuperfluousStartingSpacesAreRemoved() : void
         $this->assertSame($expectedDescription, $description->render());
     }
 
+    /**
+     * @uses \phpDocumentor\Reflection\DocBlock\Description
+     * @uses \phpDocumentor\Reflection\DocBlock\Tags\InvalidTag
+     * @uses \phpDocumentor\Reflection\DocBlock\Tags\Formatter\PassthroughFormatter
+     * @uses \phpDocumentor\Reflection\Types\Context
+     *
+     * @covers ::__construct
+     * @covers ::create
+     */
+    public function testDescriptionWithBrokenInlineTags() : void
+    {
+        $contents   = 'This {@see $name} is a broken use case, but used in real life.';
+        $context    = new Context('');
+        $tagFactory = m::mock(TagFactory::class);
+        $tagFactory->shouldReceive('create')
+            ->once()
+            ->with('@see $name', $context)
+            ->andReturn(InvalidTag::create('$name', 'see', new Exception()));
+
+        $factory     = new DescriptionFactory($tagFactory);
+        $description = $factory->create($contents, $context);
+
+        $this->assertSame($contents, $description->render());
+    }
+
     /**
      * Provides a series of example strings that the parser should correctly interpret and return.
      *

tests/unit/DocBlock/StandardTagFactoryTest.php

@@ -18,6 +18,7 @@
 use phpDocumentor\Reflection\DocBlock\Tags\Formatter;
 use phpDocumentor\Reflection\DocBlock\Tags\Formatter\PassthroughFormatter;
 use phpDocumentor\Reflection\DocBlock\Tags\Generic;
+use phpDocumentor\Reflection\DocBlock\Tags\InvalidTag;
 use phpDocumentor\Reflection\DocBlock\Tags\Return_;
 use phpDocumentor\Reflection\DocBlock\Tags\See;
 use phpDocumentor\Reflection\Fqsen;
@@ -330,4 +331,14 @@ public function testReturnTagIsMappedCorrectly() : void
         $this->assertInstanceOf(Return_::class, $tag);
         $this->assertSame('return', $tag->getName());
     }
+
+    public function testInvalidTagIsReturnedOnFailure() : void
+    {
+        $tagFactory = new StandardTagFactory(m::mock(FqsenResolver::class));
+
+        /** @var InvalidTag $tag */
+        $tag = $tagFactory->create('@see $name some invalid tag');
+
+        $this->assertInstanceOf(InvalidTag::class, $tag);
+    }
 }

tests/unit/DocBlock/Tags/InvalidTagTest.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace phpDocumentor\Reflection\DocBlock\Tags;
+
+use Exception;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @coversDefaultClass \phpDocumentor\Reflection\DocBlock\Tags\InvalidTag
+ * @covers ::<private>
+ * @covers ::getName
+ * @covers ::render
+ * @covers ::getException
+ * @covers ::create
+ */
+final class InvalidTagTest extends TestCase
+{
+    public function testCreationWithoutError() : void
+    {
+        $tag = InvalidTag::create('Body', 'name');
+
+        self::assertSame('name', $tag->getName());
+        self::assertSame('@name Body', $tag->render());
+        self::assertNull($tag->getException());
+    }
+
+    /**
+     * @covers ::withError
+     */
+    public function testCreationWithError() : void
+    {
+        $exception = new Exception();
+        $tag       = InvalidTag::create('Body', 'name')->withError($exception);
+
+        self::assertSame('name', $tag->getName());
+        self::assertSame('@name Body', $tag->render());
+        self::assertSame($exception, $tag->getException());
+    }
+}

tests/unit/DocBlockFactoryTest.php

@@ -279,37 +279,4 @@ public function testTagsWithContextNamespace() : void
 
         $this->assertInstanceOf(DocBlock::class, $docblock);
     }
-
-    /**
-     * @uses \phpDocumentor\Reflection\DocBlock\DescriptionFactory
-     * @uses \phpDocumentor\Reflection\DocBlock\Description
-     *
-     * @covers ::__construct
-     * @covers ::create
-     */
-    public function testTagsAreFilteredForNullValues() : void
-    {
-        $tagString = <<<TAG
-@author Mike van Riel <[email protected]> This is with
-  multiline description.
-TAG;
-
-        $tagFactory = m::mock(TagFactory::class);
-        $tagFactory->shouldReceive('create')->with($tagString, m::any())->andReturn(null);
-
-        $fixture = new DocBlockFactory(new DescriptionFactory($tagFactory), $tagFactory);
-
-        $given = <<<DOCBLOCK
-/**
- * This is a summary.
- *
- * @author Mike van Riel <[email protected]> This is with
- *   multiline description.
- */
-DOCBLOCK;
-
-        $docblock = $fixture->create($given, new Context(''));
-
-        $this->assertEquals([], $docblock->getTags());
-    }
 }