From 08c0b366d7dd79f0158d0963f59de08530bf3b61 Mon Sep 17 00:00:00 2001 From: Jaapio Date: Wed, 2 Sep 2020 22:46:14 +0200 Subject: [PATCH 1/3] Improve test coverage --- Makefile | 2 +- src/DocBlock/Tags/Example.php | 6 +- tests/unit/DocBlock/Tags/AuthorTest.php | 36 ++++- tests/unit/DocBlock/Tags/ExampleTest.php | 163 ++++++++++++++++++++--- 4 files changed, 177 insertions(+), 30 deletions(-) diff --git a/Makefile b/Makefile index 5aff03e..ca5f31b 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ psalm: .PHONY: test test: docker run -it --rm -v${CURDIR}:/github/workspace phpdoc/phpunit-ga - docker run -it --rm -v${CURDIR}:/data -w /data php:7.2 -f ./tests/coverage-checker.php 89 + docker run -it --rm -v${CURDIR}:/data -w /data php:7.2 -f ./tests/coverage-checker.php 90 .PHONY: pre-commit-test pre-commit-test: test phpcs phpstan psalm diff --git a/src/DocBlock/Tags/Example.php b/src/DocBlock/Tags/Example.php index 8ccb4fd..3673ffb 100644 --- a/src/DocBlock/Tags/Example.php +++ b/src/DocBlock/Tags/Example.php @@ -48,7 +48,7 @@ final class Example implements Tag, Factory\StaticMethod public function __construct(string $filePath, bool $isURI, int $startingLine, int $lineCount, ?string $content) { Assert::notEmpty($filePath); - Assert::greaterThanEq($startingLine, 0); + Assert::greaterThanEq($startingLine, 1); Assert::greaterThanEq($lineCount, 0); $this->filePath = $filePath; @@ -63,7 +63,7 @@ final class Example implements Tag, Factory\StaticMethod public function getContent() : string { - if ($this->content === null) { + if ($this->content === null || $this->content === '') { $filePath = '"' . $this->filePath . '"'; if ($this->isURI) { $filePath = $this->isUriRelative($this->filePath) @@ -107,7 +107,7 @@ final class Example implements Tag, Factory\StaticMethod // Starting line / Number of lines / Description if (preg_match('/^([1-9]\d*)(?:\s+((?1))\s*)?(.*)$/sux', $matches[3], $contentMatches)) { $startingLine = (int) $contentMatches[1]; - if (isset($contentMatches[2]) && $contentMatches[2] !== '') { + if (isset($contentMatches[2])) { $lineCount = (int) $contentMatches[2]; } diff --git a/tests/unit/DocBlock/Tags/AuthorTest.php b/tests/unit/DocBlock/Tags/AuthorTest.php index 59e4cda..cc06727 100644 --- a/tests/unit/DocBlock/Tags/AuthorTest.php +++ b/tests/unit/DocBlock/Tags/AuthorTest.php @@ -133,14 +133,40 @@ class AuthorTest extends TestCase * @uses \phpDocumentor\Reflection\DocBlock\Tags\Author:: * * @covers ::create + * @dataProvider authorTagProvider */ - public function testFactoryMethod() : void + public function testFactoryMethod(string $input, string $output, string $name, string $email) : void { - $fixture = Author::create('Mike van Riel '); + $fixture = Author::create($input); - $this->assertSame('Mike van Riel ', (string) $fixture); - $this->assertSame('Mike van Riel', $fixture->getAuthorName()); - $this->assertSame('mike@phpdoc.org', $fixture->getEmail()); + $this->assertSame($output, (string) $fixture); + $this->assertSame($name, $fixture->getAuthorName()); + $this->assertSame($email, $fixture->getEmail()); + } + + /** @return mixed[][] */ + public function authorTagProvider() : array + { + return [ + [ + 'Mike van Riel ', + 'Mike van Riel ', + 'Mike van Riel', + 'mike@phpdoc.org', + ], + [ + 'Mike van Riel < mike@phpdoc.org >', + 'Mike van Riel ', + 'Mike van Riel', + 'mike@phpdoc.org', + ], + [ + 'Mike van Riel', + 'Mike van Riel', + 'Mike van Riel', + '', + ], + ]; } /** diff --git a/tests/unit/DocBlock/Tags/ExampleTest.php b/tests/unit/DocBlock/Tags/ExampleTest.php index 77809d4..1d6d159 100644 --- a/tests/unit/DocBlock/Tags/ExampleTest.php +++ b/tests/unit/DocBlock/Tags/ExampleTest.php @@ -4,25 +4,18 @@ declare(strict_types=1); namespace DocBlock\Tags; -use Mockery as m; +use InvalidArgumentException; use phpDocumentor\Reflection\DocBlock\Tags\Example; use PHPUnit\Framework\TestCase; /** * @coversDefaultClass \phpDocumentor\Reflection\DocBlock\Tags\Example + * @covers :: */ class ExampleTest extends TestCase { /** - * Call Mockery::close after each test. - */ - public function tearDown() : void - { - m::close(); - } - - /** - * @uses phpDocumentor\Reflection\DocBlock\Tags\BaseTag + * @uses \phpDocumentor\Reflection\DocBlock\Tags\BaseTag * * @covers ::create * @covers ::__construct @@ -37,7 +30,7 @@ class ExampleTest extends TestCase } /** - * @uses phpDocumentor\Reflection\DocBlock\Tags\BaseTag + * @uses \phpDocumentor\Reflection\DocBlock\Tags\BaseTag * * @covers ::create * @covers ::__construct @@ -52,7 +45,7 @@ class ExampleTest extends TestCase } /** - * @uses phpDocumentor\Reflection\DocBlock\Tags\BaseTag + * @uses \phpDocumentor\Reflection\DocBlock\Tags\BaseTag * * @covers ::create * @covers ::__construct @@ -67,7 +60,7 @@ class ExampleTest extends TestCase } /** - * @uses phpDocumentor\Reflection\DocBlock\Tags\BaseTag + * @uses \phpDocumentor\Reflection\DocBlock\Tags\BaseTag * * @covers ::create * @covers ::__construct @@ -84,7 +77,7 @@ class ExampleTest extends TestCase } /** - * @uses phpDocumentor\Reflection\DocBlock\Tags\BaseTag + * @uses \phpDocumentor\Reflection\DocBlock\Tags\BaseTag * * @covers ::create * @covers ::__construct @@ -101,21 +94,149 @@ class ExampleTest extends TestCase } /** - * @uses phpDocumentor\Reflection\DocBlock\Tags\BaseTag + * @uses \phpDocumentor\Reflection\DocBlock\Tags\BaseTag * + * @dataProvider tagContentProvider * @covers ::create * @covers ::__construct * @covers ::getFilePath * @covers ::getStartingLine * @covers ::getLineCount * @covers ::getDescription + * @covers ::getContent */ - public function testFullExample() : void + public function testFactoryMethod( + string $input, + string $filePath, + int $startLine, + int $lineCount, + ?string $description, + string $content + ) : void { + $tag = Example::create($input); + $this->assertSame($filePath, $tag->getFilePath()); + $this->assertSame($startLine, $tag->getStartingLine()); + $this->assertSame($lineCount, $tag->getLineCount()); + $this->assertSame($description, $tag->getDescription()); + $this->assertSame($content, $tag->getContent()); + } + + /** @return mixed[][] */ + public function tagContentProvider() : array { - $tag = Example::create('"example1.php" 10 5 test text'); - $this->assertEquals('example1.php', $tag->getFilePath()); - $this->assertEquals(10, $tag->getStartingLine()); - $this->assertEquals(5, $tag->getLineCount()); - $this->assertEquals('test text', $tag->getDescription()); + return [ + [ + '"example1.php" 10 5 test text ', + 'example1.php', + 10, + 5, + 'test text', + 'test text', + ], + [ + 'example1.php 10 5 test text', + 'example1.php', + 10, + 5, + 'test text', + 'test text', + ], + [ + 'example1.php 1 10 test text', + 'example1.php', + 1, + 10, + 'test text', + 'test text', + ], + [ + 'example1.php', + 'example1.php', + 1, + 0, + null, + 'example1.php', + ], + [ + 'file://example1.php ', + 'file://example1.php', + 1, + 0, + '', + 'file://example1.php', + ], + [ + '/example1.php', + '/example1.php', + 1, + 0, + null, + '/example1.php', + ], + ]; + } + + /** + * @dataProvider invalidExampleProvider + * @covers ::__construct + */ + public function testValidatesArguments( + string $filePath, + bool $isUrl, + int $startLine, + int $lineCount, + string $description + ) : void { + $this->expectException(InvalidArgumentException::class); + + new Example( + $filePath, + $isUrl, + $startLine, + $lineCount, + $description + ); + } + + /** @return mixed[][] */ + public function invalidExampleProvider() : array + { + return [ + 'invalid start' => [ + '/some/path', + false, + -1, + 0, + 'text', + ], + 'invalid start 2' => [ + '/some/path', + false, + -10, + 0, + 'text', + ], + 'invalid length' => [ + '/some/path', + false, + 1, + -1, + 'text', + ], + 'invalid length 2' => [ + '/some/path', + false, + 1, + -10, + 'text', + ], + 'empty filepath' => [ + '', + false, + 1, + 0, + 'text', + ], + ]; } } From 73650dde9181b643ca9d12cf1143c61ca4901f75 Mon Sep 17 00:00:00 2001 From: Jaapio Date: Thu, 18 Jun 2020 23:48:32 +0200 Subject: [PATCH 2/3] Fix FQSEN resolving on see,covers,uses The See, Covers and Use tags can reference also methods, properties and constants. Which means that the FqsenResolver cannot handle those properly. This patch fixes that issue. --- src/DocBlock/Tags/Covers.php | 17 ++++++++- src/DocBlock/Tags/See.php | 19 +++++++++- src/DocBlock/Tags/Uses.php | 17 ++++++++- .../DocblockSeeTagResolvingTest.php | 38 +++++++++++++++++++ tests/unit/DocBlock/Tags/SeeTest.php | 32 ++++++++++++++++ 5 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 tests/integration/DocblockSeeTagResolvingTest.php diff --git a/src/DocBlock/Tags/Covers.php b/src/DocBlock/Tags/Covers.php index 820d595..582be6c 100644 --- a/src/DocBlock/Tags/Covers.php +++ b/src/DocBlock/Tags/Covers.php @@ -20,6 +20,8 @@ use phpDocumentor\Reflection\FqsenResolver; use phpDocumentor\Reflection\Types\Context as TypeContext; use phpDocumentor\Reflection\Utils; use Webmozart\Assert\Assert; +use function array_key_exists; +use function explode; /** * Reflection class for a @covers tag in a Docblock. @@ -54,11 +56,24 @@ final class Covers extends BaseTag implements Factory\StaticMethod $parts = Utils::pregSplit('/\s+/Su', $body, 2); return new static( - $resolver->resolve($parts[0], $context), + self::resolveFqsen($parts[0], $resolver, $context), $descriptionFactory->create($parts[1] ?? '', $context) ); } + private static function resolveFqsen(string $parts, ?FqsenResolver $fqsenResolver, ?TypeContext $context) : Fqsen + { + Assert::notNull($fqsenResolver); + $fqsenParts = explode('::', $parts); + $resolved = $fqsenResolver->resolve($fqsenParts[0], $context); + + if (!array_key_exists(1, $fqsenParts)) { + return $resolved; + } + + return new Fqsen($resolved . '::' . $fqsenParts[1]); + } + /** * Returns the structural element this tag refers to. */ diff --git a/src/DocBlock/Tags/See.php b/src/DocBlock/Tags/See.php index e71401d..2c77f86 100644 --- a/src/DocBlock/Tags/See.php +++ b/src/DocBlock/Tags/See.php @@ -18,10 +18,13 @@ use phpDocumentor\Reflection\DocBlock\DescriptionFactory; use phpDocumentor\Reflection\DocBlock\Tags\Reference\Fqsen as FqsenRef; use phpDocumentor\Reflection\DocBlock\Tags\Reference\Reference; use phpDocumentor\Reflection\DocBlock\Tags\Reference\Url; +use phpDocumentor\Reflection\Fqsen; use phpDocumentor\Reflection\FqsenResolver; use phpDocumentor\Reflection\Types\Context as TypeContext; use phpDocumentor\Reflection\Utils; use Webmozart\Assert\Assert; +use function array_key_exists; +use function explode; use function preg_match; /** @@ -50,7 +53,6 @@ final class See extends BaseTag implements Factory\StaticMethod ?DescriptionFactory $descriptionFactory = null, ?TypeContext $context = null ) : self { - Assert::notNull($typeResolver); Assert::notNull($descriptionFactory); $parts = Utils::pregSplit('/\s+/Su', $body, 2); @@ -61,7 +63,20 @@ final class See extends BaseTag implements Factory\StaticMethod return new static(new Url($parts[0]), $description); } - return new static(new FqsenRef($typeResolver->resolve($parts[0], $context)), $description); + return new static(new FqsenRef(self::resolveFqsen($parts[0], $typeResolver, $context)), $description); + } + + private static function resolveFqsen(string $parts, ?FqsenResolver $fqsenResolver, ?TypeContext $context) : Fqsen + { + Assert::notNull($fqsenResolver); + $fqsenParts = explode('::', $parts); + $resolved = $fqsenResolver->resolve($fqsenParts[0], $context); + + if (!array_key_exists(1, $fqsenParts)) { + return $resolved; + } + + return new Fqsen($resolved . '::' . $fqsenParts[1]); } /** diff --git a/src/DocBlock/Tags/Uses.php b/src/DocBlock/Tags/Uses.php index 57fb290..2f6e8f1 100644 --- a/src/DocBlock/Tags/Uses.php +++ b/src/DocBlock/Tags/Uses.php @@ -20,6 +20,8 @@ use phpDocumentor\Reflection\FqsenResolver; use phpDocumentor\Reflection\Types\Context as TypeContext; use phpDocumentor\Reflection\Utils; use Webmozart\Assert\Assert; +use function array_key_exists; +use function explode; /** * Reflection class for a {@}uses tag in a Docblock. @@ -53,11 +55,24 @@ final class Uses extends BaseTag implements Factory\StaticMethod $parts = Utils::pregSplit('/\s+/Su', $body, 2); return new static( - $resolver->resolve($parts[0], $context), + self::resolveFqsen($parts[0], $resolver, $context), $descriptionFactory->create($parts[1] ?? '', $context) ); } + private static function resolveFqsen(string $parts, ?FqsenResolver $fqsenResolver, ?TypeContext $context) : Fqsen + { + Assert::notNull($fqsenResolver); + $fqsenParts = explode('::', $parts); + $resolved = $fqsenResolver->resolve($fqsenParts[0], $context); + + if (!array_key_exists(1, $fqsenParts)) { + return $resolved; + } + + return new Fqsen($resolved . '::' . $fqsenParts[1]); + } + /** * Returns the structural element this tag refers to. */ diff --git a/tests/integration/DocblockSeeTagResolvingTest.php b/tests/integration/DocblockSeeTagResolvingTest.php new file mode 100644 index 0000000..f98a888 --- /dev/null +++ b/tests/integration/DocblockSeeTagResolvingTest.php @@ -0,0 +1,38 @@ + '\Project\Other\Level\Issue2425B', 'Aliased' => 'Project\Other\Level\Issue2425C']); + $docblockString = <<create($docblockString, $context); + + /** @var See $see1 */ + $see1 = $docblock->getDescription()->getTags()[0]; + + $this->assertSame('\Project\Other\Level\Issue2425B::bar()', (string)$see1->getReference()); + } +} diff --git a/tests/unit/DocBlock/Tags/SeeTest.php b/tests/unit/DocBlock/Tags/SeeTest.php index f9ed04c..c8e5642 100644 --- a/tests/unit/DocBlock/Tags/SeeTest.php +++ b/tests/unit/DocBlock/Tags/SeeTest.php @@ -167,6 +167,38 @@ class SeeTest extends TestCase $this->assertSame($description, $fixture->getDescription()); } + /** + * @uses \phpDocumentor\Reflection\DocBlock\Tags\See:: + * @uses \phpDocumentor\Reflection\DocBlock\DescriptionFactory + * @uses \phpDocumentor\Reflection\FqsenResolver + * @uses \phpDocumentor\Reflection\DocBlock\Description + * @uses \phpDocumentor\Reflection\DocBlock\Tags\Reference\Fqsen + * @uses \phpDocumentor\Reflection\Fqsen + * @uses \phpDocumentor\Reflection\Types\Context + * + * @covers ::create + */ + public function testFactoryMethodWithNonClassFQSEN() : void + { + $descriptionFactory = m::mock(DescriptionFactory::class); + $resolver = m::mock(FqsenResolver::class); + $context = new Context(''); + + $fqsen = new Fqsen('\DateTime'); + $description = new Description('My Description'); + + $descriptionFactory + ->shouldReceive('create')->with('My Description', $context)->andReturn($description); + $resolver->shouldReceive('resolve')->with('DateTime', $context)->andReturn($fqsen); + + $fixture = See::create('DateTime::createFromFormat() My Description', $resolver, $descriptionFactory, $context); + + $this->assertSame('\DateTime::createFromFormat() My Description', (string) $fixture); + $this->assertInstanceOf(FqsenRef::class, $fixture->getReference()); + $this->assertSame('\DateTime::createFromFormat()', (string) $fixture->getReference()); + $this->assertSame($description, $fixture->getDescription()); + } + /** * @uses \phpDocumentor\Reflection\DocBlock\Tags\See:: * @uses \phpDocumentor\Reflection\DocBlock\DescriptionFactory From f6075926e937828b180e02964e2d2062af8a9537 Mon Sep 17 00:00:00 2001 From: Jaapio Date: Wed, 2 Sep 2020 23:38:01 +0200 Subject: [PATCH 3/3] Bump mockery --- composer.lock | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/composer.lock b/composer.lock index 2195244..8ac3a86 100644 --- a/composer.lock +++ b/composer.lock @@ -160,20 +160,6 @@ "polyfill", "portable" ], - "funding": [ - { - "url": "https://symfony.com/sponsor", - "type": "custom" - }, - { - "url": "https://github.com/fabpot", - "type": "github" - }, - { - "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", - "type": "tidelift" - } - ], "time": "2020-06-06T08:46:27+00:00" }, {