Bugfix: resolve issue with multiline descriptions

The phpstan parser is not consuming the full description when parsing
docblocks with a more complex description. For them it's mostlikely not an
issue as phpstan doesn't use the descriptions. But it will also parse
the descriptions into unexpected tags. This could be an advantage but is
not according to the phpdoc spec.

Our own tokenizer is already tokenizing the docblocks into the correct parts.
So all we needed to do is assume all remaining tokens in the phpstan ast belong
to the description. From there our own code is able to handle this as before in
v5.3.

fixes #365
This commit is contained in:
Jaapio
2024-05-08 20:49:40 +02:00
parent 2f7b34c2ee
commit 518be131c0
10 changed files with 157 additions and 11 deletions
+1
View File
@@ -44,6 +44,7 @@
<NoInterfaceProperties> <NoInterfaceProperties>
<errorLevel type="info"> <errorLevel type="info">
<file name="src/DocBlock/Tags/Factory/ParamFactory.php"/> <file name="src/DocBlock/Tags/Factory/ParamFactory.php"/>
<file name="src/DocBlock/Tags/Factory/AbstractPHPStanFactory.php"/>
</errorLevel> </errorLevel>
</NoInterfaceProperties> </NoInterfaceProperties>
@@ -23,6 +23,8 @@ use PHPStan\PhpDocParser\Parser\TokenIterator;
use PHPStan\PhpDocParser\Parser\TypeParser; use PHPStan\PhpDocParser\Parser\TypeParser;
use RuntimeException; use RuntimeException;
use function property_exists;
/** /**
* Factory class creating tags using phpstan's parser * Factory class creating tags using phpstan's parser
* *
@@ -48,8 +50,11 @@ class AbstractPHPStanFactory implements Factory
public function create(string $tagLine, ?TypeContext $context = null): Tag public function create(string $tagLine, ?TypeContext $context = null): Tag
{ {
$tokens = $this->lexer->tokenize($tagLine); $tokens = new TokenIterator($this->lexer->tokenize($tagLine));
$ast = $this->parser->parseTag(new TokenIterator($tokens)); $ast = $this->parser->parseTag($tokens);
if (property_exists($ast->value, 'description') === true) {
$ast->value->setAttribute('description', $ast->value->description . $tokens->joinUntil(Lexer::TOKEN_END));
}
if ($context === null) { if ($context === null) {
$context = new TypeContext(''); $context = new TypeContext('');
+7 -1
View File
@@ -19,6 +19,7 @@ use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
use PHPStan\PhpDocParser\Ast\Type\OffsetAccessTypeNode; use PHPStan\PhpDocParser\Ast\Type\OffsetAccessTypeNode;
use Webmozart\Assert\Assert; use Webmozart\Assert\Assert;
use function is_string;
use function sprintf; use function sprintf;
use function trim; use function trim;
@@ -68,11 +69,16 @@ final class ParamFactory implements PHPStanFactory
); );
} }
$description = $tagValue->getAttribute('description');
if (is_string($description) === false) {
$description = $tagValue->description;
}
return new Param( return new Param(
trim($tagValue->parameterName, '$'), trim($tagValue->parameterName, '$'),
$this->typeResolver->createType($tagValue->type ?? new IdentifierTypeNode('mixed'), $context), $this->typeResolver->createType($tagValue->type ?? new IdentifierTypeNode('mixed'), $context),
$tagValue->isVariadic, $tagValue->isVariadic,
$this->descriptionFactory->create($tagValue->description, $context), $this->descriptionFactory->create($description, $context),
$tagValue->isReference $tagValue->isReference
); );
} }
@@ -13,6 +13,7 @@ use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode;
use PHPStan\PhpDocParser\Ast\PhpDoc\PropertyTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\PropertyTagValueNode;
use Webmozart\Assert\Assert; use Webmozart\Assert\Assert;
use function is_string;
use function trim; use function trim;
/** /**
@@ -34,10 +35,15 @@ final class PropertyFactory implements PHPStanFactory
$tagValue = $node->value; $tagValue = $node->value;
Assert::isInstanceOf($tagValue, PropertyTagValueNode::class); Assert::isInstanceOf($tagValue, PropertyTagValueNode::class);
$description = $tagValue->getAttribute('description');
if (is_string($description) === false) {
$description = $tagValue->description;
}
return new Property( return new Property(
trim($tagValue->propertyName, '$'), trim($tagValue->propertyName, '$'),
$this->typeResolver->createType($tagValue->type, $context), $this->typeResolver->createType($tagValue->type, $context),
$this->descriptionFactory->create($tagValue->description, $context) $this->descriptionFactory->create($description, $context)
); );
} }
@@ -13,6 +13,7 @@ use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode;
use PHPStan\PhpDocParser\Ast\PhpDoc\PropertyTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\PropertyTagValueNode;
use Webmozart\Assert\Assert; use Webmozart\Assert\Assert;
use function is_string;
use function trim; use function trim;
/** /**
@@ -34,10 +35,15 @@ final class PropertyReadFactory implements PHPStanFactory
$tagValue = $node->value; $tagValue = $node->value;
Assert::isInstanceOf($tagValue, PropertyTagValueNode::class); Assert::isInstanceOf($tagValue, PropertyTagValueNode::class);
$description = $tagValue->getAttribute('description');
if (is_string($description) === false) {
$description = $tagValue->description;
}
return new PropertyRead( return new PropertyRead(
trim($tagValue->propertyName, '$'), trim($tagValue->propertyName, '$'),
$this->typeResolver->createType($tagValue->type, $context), $this->typeResolver->createType($tagValue->type, $context),
$this->descriptionFactory->create($tagValue->description, $context) $this->descriptionFactory->create($description, $context)
); );
} }
@@ -13,6 +13,7 @@ use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode;
use PHPStan\PhpDocParser\Ast\PhpDoc\PropertyTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\PropertyTagValueNode;
use Webmozart\Assert\Assert; use Webmozart\Assert\Assert;
use function is_string;
use function trim; use function trim;
/** /**
@@ -34,10 +35,15 @@ final class PropertyWriteFactory implements PHPStanFactory
$tagValue = $node->value; $tagValue = $node->value;
Assert::isInstanceOf($tagValue, PropertyTagValueNode::class); Assert::isInstanceOf($tagValue, PropertyTagValueNode::class);
$description = $tagValue->getAttribute('description');
if (is_string($description) === false) {
$description = $tagValue->description;
}
return new PropertyWrite( return new PropertyWrite(
trim($tagValue->propertyName, '$'), trim($tagValue->propertyName, '$'),
$this->typeResolver->createType($tagValue->type, $context), $this->typeResolver->createType($tagValue->type, $context),
$this->descriptionFactory->create($tagValue->description, $context) $this->descriptionFactory->create($description, $context)
); );
} }
+8 -1
View File
@@ -13,6 +13,8 @@ use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode;
use PHPStan\PhpDocParser\Ast\PhpDoc\ReturnTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\ReturnTagValueNode;
use Webmozart\Assert\Assert; use Webmozart\Assert\Assert;
use function is_string;
/** /**
* @internal This class is not part of the BC promise of this library. * @internal This class is not part of the BC promise of this library.
*/ */
@@ -32,9 +34,14 @@ final class ReturnFactory implements PHPStanFactory
$tagValue = $node->value; $tagValue = $node->value;
Assert::isInstanceOf($tagValue, ReturnTagValueNode::class); Assert::isInstanceOf($tagValue, ReturnTagValueNode::class);
$description = $tagValue->getAttribute('description');
if (is_string($description) === false) {
$description = $tagValue->description;
}
return new Return_( return new Return_(
$this->typeResolver->createType($tagValue->type, $context), $this->typeResolver->createType($tagValue->type, $context),
$this->descriptionFactory->create($tagValue->description, $context) $this->descriptionFactory->create($description, $context)
); );
} }
+7 -1
View File
@@ -13,6 +13,7 @@ use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode;
use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode;
use Webmozart\Assert\Assert; use Webmozart\Assert\Assert;
use function is_string;
use function trim; use function trim;
/** /**
@@ -34,10 +35,15 @@ final class VarFactory implements PHPStanFactory
$tagValue = $node->value; $tagValue = $node->value;
Assert::isInstanceOf($tagValue, VarTagValueNode::class); Assert::isInstanceOf($tagValue, VarTagValueNode::class);
$description = $tagValue->getAttribute('description');
if (is_string($description) === false) {
$description = $tagValue->description;
}
return new Var_( return new Var_(
trim($tagValue->variableName, '$'), trim($tagValue->variableName, '$'),
$this->typeResolver->createType($tagValue->type, $context), $this->typeResolver->createType($tagValue->type, $context),
$this->descriptionFactory->create($tagValue->description, $context) $this->descriptionFactory->create($description, $context)
); );
} }
@@ -17,16 +17,21 @@ use Mockery as m;
use phpDocumentor\Reflection\DocBlock\Description; use phpDocumentor\Reflection\DocBlock\Description;
use phpDocumentor\Reflection\DocBlock\StandardTagFactory; use phpDocumentor\Reflection\DocBlock\StandardTagFactory;
use phpDocumentor\Reflection\DocBlock\Tag; use phpDocumentor\Reflection\DocBlock\Tag;
use phpDocumentor\Reflection\DocBlock\Tags\Generic;
use phpDocumentor\Reflection\DocBlock\Tags\InvalidTag; use phpDocumentor\Reflection\DocBlock\Tags\InvalidTag;
use phpDocumentor\Reflection\DocBlock\Tags\Method; use phpDocumentor\Reflection\DocBlock\Tags\Method;
use phpDocumentor\Reflection\DocBlock\Tags\MethodParameter; use phpDocumentor\Reflection\DocBlock\Tags\MethodParameter;
use phpDocumentor\Reflection\DocBlock\Tags\Param; use phpDocumentor\Reflection\DocBlock\Tags\Param;
use phpDocumentor\Reflection\DocBlock\Tags\Return_; use phpDocumentor\Reflection\DocBlock\Tags\Return_;
use phpDocumentor\Reflection\DocBlock\Tags\See; use phpDocumentor\Reflection\DocBlock\Tags\See;
use phpDocumentor\Reflection\DocBlock\Tags\Since;
use phpDocumentor\Reflection\PseudoTypes\ConstExpression; use phpDocumentor\Reflection\PseudoTypes\ConstExpression;
use phpDocumentor\Reflection\Types\Array_; use phpDocumentor\Reflection\Types\Array_;
use phpDocumentor\Reflection\Types\Compound;
use phpDocumentor\Reflection\Types\Context;
use phpDocumentor\Reflection\Types\Integer; use phpDocumentor\Reflection\Types\Integer;
use phpDocumentor\Reflection\Types\Mixed_; use phpDocumentor\Reflection\Types\Mixed_;
use phpDocumentor\Reflection\Types\Object_;
use phpDocumentor\Reflection\Types\Self_; use phpDocumentor\Reflection\Types\Self_;
use phpDocumentor\Reflection\Types\String_; use phpDocumentor\Reflection\Types\String_;
use phpDocumentor\Reflection\Types\Void_; use phpDocumentor\Reflection\Types\Void_;
@@ -123,8 +128,8 @@ DESCRIPTION;
$this->assertInstanceOf(See::class, $seeTags[0]); $this->assertInstanceOf(See::class, $seeTags[0]);
$seeTag = $seeTags[0]; $seeTag = $seeTags[0];
$this->assertSame('\\' . StandardTagFactory::class, (string) $seeTag->getReference()); $this->assertSame('\\' . StandardTagFactory::class, (string)$seeTag->getReference());
$this->assertSame('', (string) $seeTag->getDescription()); $this->assertSame('', (string)$seeTag->getDescription());
} }
public function testDescriptionsCanEscapeAtSignsAndClosingBraces(): void public function testDescriptionsCanEscapeAtSignsAndClosingBraces(): void
@@ -268,4 +273,95 @@ DOC;
$docblock->getTags() $docblock->getTags()
); );
} }
public function testRegressionWordpressDocblocks(): void
{
$docCommment = <<<DOC
/**
* Install a package.
*
* Copies the contents of a package from a source directory, and installs them in
* a destination directory. Optionally removes the source. It can also optionally
* clear out the destination folder if it already exists.
*
* @since 2.8.0
* @since 6.2.0 Use move_dir() instead of copy_dir() when possible.
*
* @global WP_Filesystem_Base \$wp_filesystem WordPress filesystem subclass.
* @global array \$wp_theme_directories
*
* @param array|string \$args {
* Optional. Array or string of arguments for installing a package. Default empty array.
*
* @type string \$source Required path to the package source. Default empty.
* @type string \$destination Required path to a folder to install the package in.
* Default empty.
* @type bool \$clear_destination Whether to delete any files already in the destination
* folder. Default false.
* @type bool \$clear_working Whether to delete the files from the working directory
* after copying them to the destination. Default false.
* @type bool \$abort_if_destination_exists Whether to abort the installation if
* the destination folder already exists. Default true.
* @type array \$hook_extra Extra arguments to pass to the filter hooks called by
* WP_Upgrader::install_package(). Default empty array.
* }
*
* @return array|WP_Error The result (also stored in `WP_Upgrader::\$result`), or a WP_Error on failure.
*/
DOC;
$factory = DocBlockFactory::createInstance();
$docblock = $factory->create($docCommment);
self::assertEquals(
new DocBlock(
'Install a package.',
new Description(
'Copies the contents of a package from a source directory, and installs them in' . PHP_EOL .
'a destination directory. Optionally removes the source. It can also optionally' . PHP_EOL .
'clear out the destination folder if it already exists.'
),
[
new Since('2.8.0', new Description('')),
new Since('6.2.0', new Description('Use move_dir() instead of copy_dir() when possible.')),
new Generic(
'global',
new Description('WP_Filesystem_Base $wp_filesystem WordPress filesystem subclass.')
),
new Generic(
'global',
new Description('array $wp_theme_directories')
),
new Param(
'args',
new Compound([new Array_(new Mixed_()), new String_()]),
false,
new Description(
'{' . "\n" .
'Optional. Array or string of arguments for installing a package. Default empty array.' . "\n" .
"\n" .
' @type string $source Required path to the package source. Default empty.' . "\n" .
' @type string $destination Required path to a folder to install the package in.' . "\n" .
' Default empty.' . "\n" .
' @type bool $clear_destination Whether to delete any files already in the destination' . "\n" .
' folder. Default false.' . "\n" .
' @type bool $clear_working Whether to delete the files from the working directory' . "\n" .
' after copying them to the destination. Default false.' . "\n" .
' @type bool $abort_if_destination_exists Whether to abort the installation if' . "\n" .
' the destination folder already exists. Default true.' . "\n" .
' @type array $hook_extra Extra arguments to pass to the filter hooks called by' . "\n" .
' WP_Upgrader::install_package(). Default empty array.' . "\n" .
'}'
)
),
new Return_(
new Compound([new Array_(new Mixed_()), new Object_(new Fqsen('\WP_Error'))]),
new Description('The result (also stored in `WP_Upgrader::$result`), or a WP_Error on failure.')
),
],
new Context('\\')
),
$docblock
);
}
} }
@@ -26,6 +26,8 @@ use PHPStan\PhpDocParser\Parser\TokenIterator;
use PHPStan\PhpDocParser\Parser\TypeParser; use PHPStan\PhpDocParser\Parser\TypeParser;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use function property_exists;
abstract class TagFactoryTestCase extends TestCase abstract class TagFactoryTestCase extends TestCase
{ {
public function parseTag(string $tag): PhpDocTagNode public function parseTag(string $tag): PhpDocTagNode
@@ -34,7 +36,12 @@ abstract class TagFactoryTestCase extends TestCase
$tokens = $lexer->tokenize($tag); $tokens = $lexer->tokenize($tag);
$constParser = new ConstExprParser(); $constParser = new ConstExprParser();
return (new PhpDocParser(new TypeParser($constParser), $constParser))->parseTag(new TokenIterator($tokens)); $tagNode = (new PhpDocParser(new TypeParser($constParser), $constParser))->parseTag(new TokenIterator($tokens));
if (property_exists($tagNode->value, 'description') === true) {
$tagNode->value->setAttribute('description', $tagNode->value->description);
}
return $tagNode;
} }
public function giveTypeResolver(): TypeResolver public function giveTypeResolver(): TypeResolver