Skip to content

Commit 518be13

Browse files
jaapiojaapio
committed
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
1 parent 2f7b34c commit 518be13

File tree

10 files changed

+157
-11
lines changed

10 files changed

+157
-11
lines changed

‎psalm.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
<NoInterfaceProperties>
4545
<errorLevel type="info">
4646
<file name="src/DocBlock/Tags/Factory/ParamFactory.php"/>
47+
<file name="src/DocBlock/Tags/Factory/AbstractPHPStanFactory.php"/>
4748
</errorLevel>
4849
</NoInterfaceProperties>
4950

‎src/DocBlock/Tags/Factory/AbstractPHPStanFactory.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
use PHPStan\PhpDocParser\Parser\TypeParser;
2424
use RuntimeException;
2525

26+
use function property_exists;
27+
2628
/**
2729
* Factory class creating tags using phpstan's parser
2830
*
@@ -48,8 +50,11 @@ public function __construct(PHPStanFactory ...$factories)
4850

4951
public function create(string $tagLine, ?TypeContext $context = null): Tag
5052
{
51-
$tokens = $this->lexer->tokenize($tagLine);
52-
$ast = $this->parser->parseTag(new TokenIterator($tokens));
53+
$tokens = new TokenIterator($this->lexer->tokenize($tagLine));
54+
$ast = $this->parser->parseTag($tokens);
55+
if (property_exists($ast->value, 'description') === true) {
56+
$ast->value->setAttribute('description', $ast->value->description . $tokens->joinUntil(Lexer::TOKEN_END));
57+
}
5358

5459
if ($context === null) {
5560
$context = new TypeContext('');

‎src/DocBlock/Tags/Factory/ParamFactory.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use PHPStan\PhpDocParser\Ast\Type\OffsetAccessTypeNode;
2020
use Webmozart\Assert\Assert;
2121

22+
use function is_string;
2223
use function sprintf;
2324
use function trim;
2425

@@ -68,11 +69,16 @@ public function create(PhpDocTagNode $node, Context $context): Tag
6869
);
6970
}
7071

72+
$description = $tagValue->getAttribute('description');
73+
if (is_string($description) === false) {
74+
$description = $tagValue->description;
75+
}
76+
7177
return new Param(
7278
trim($tagValue->parameterName, '$'),
7379
$this->typeResolver->createType($tagValue->type ?? new IdentifierTypeNode('mixed'), $context),
7480
$tagValue->isVariadic,
75-
$this->descriptionFactory->create($tagValue->description, $context),
81+
$this->descriptionFactory->create($description, $context),
7682
$tagValue->isReference
7783
);
7884
}

‎src/DocBlock/Tags/Factory/PropertyFactory.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use PHPStan\PhpDocParser\Ast\PhpDoc\PropertyTagValueNode;
1414
use Webmozart\Assert\Assert;
1515

16+
use function is_string;
1617
use function trim;
1718

1819
/**
@@ -34,10 +35,15 @@ public function create(PhpDocTagNode $node, Context $context): Tag
3435
$tagValue = $node->value;
3536
Assert::isInstanceOf($tagValue, PropertyTagValueNode::class);
3637

38+
$description = $tagValue->getAttribute('description');
39+
if (is_string($description) === false) {
40+
$description = $tagValue->description;
41+
}
42+
3743
return new Property(
3844
trim($tagValue->propertyName, '$'),
3945
$this->typeResolver->createType($tagValue->type, $context),
40-
$this->descriptionFactory->create($tagValue->description, $context)
46+
$this->descriptionFactory->create($description, $context)
4147
);
4248
}
4349

‎src/DocBlock/Tags/Factory/PropertyReadFactory.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use PHPStan\PhpDocParser\Ast\PhpDoc\PropertyTagValueNode;
1414
use Webmozart\Assert\Assert;
1515

16+
use function is_string;
1617
use function trim;
1718

1819
/**
@@ -34,10 +35,15 @@ public function create(PhpDocTagNode $node, Context $context): Tag
3435
$tagValue = $node->value;
3536
Assert::isInstanceOf($tagValue, PropertyTagValueNode::class);
3637

38+
$description = $tagValue->getAttribute('description');
39+
if (is_string($description) === false) {
40+
$description = $tagValue->description;
41+
}
42+
3743
return new PropertyRead(
3844
trim($tagValue->propertyName, '$'),
3945
$this->typeResolver->createType($tagValue->type, $context),
40-
$this->descriptionFactory->create($tagValue->description, $context)
46+
$this->descriptionFactory->create($description, $context)
4147
);
4248
}
4349

‎src/DocBlock/Tags/Factory/PropertyWriteFactory.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use PHPStan\PhpDocParser\Ast\PhpDoc\PropertyTagValueNode;
1414
use Webmozart\Assert\Assert;
1515

16+
use function is_string;
1617
use function trim;
1718

1819
/**
@@ -34,10 +35,15 @@ public function create(PhpDocTagNode $node, Context $context): Tag
3435
$tagValue = $node->value;
3536
Assert::isInstanceOf($tagValue, PropertyTagValueNode::class);
3637

38+
$description = $tagValue->getAttribute('description');
39+
if (is_string($description) === false) {
40+
$description = $tagValue->description;
41+
}
42+
3743
return new PropertyWrite(
3844
trim($tagValue->propertyName, '$'),
3945
$this->typeResolver->createType($tagValue->type, $context),
40-
$this->descriptionFactory->create($tagValue->description, $context)
46+
$this->descriptionFactory->create($description, $context)
4147
);
4248
}
4349

‎src/DocBlock/Tags/Factory/ReturnFactory.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
use PHPStan\PhpDocParser\Ast\PhpDoc\ReturnTagValueNode;
1414
use Webmozart\Assert\Assert;
1515

16+
use function is_string;
17+
1618
/**
1719
* @internal This class is not part of the BC promise of this library.
1820
*/
@@ -32,9 +34,14 @@ public function create(PhpDocTagNode $node, Context $context): Tag
3234
$tagValue = $node->value;
3335
Assert::isInstanceOf($tagValue, ReturnTagValueNode::class);
3436

37+
$description = $tagValue->getAttribute('description');
38+
if (is_string($description) === false) {
39+
$description = $tagValue->description;
40+
}
41+
3542
return new Return_(
3643
$this->typeResolver->createType($tagValue->type, $context),
37-
$this->descriptionFactory->create($tagValue->description, $context)
44+
$this->descriptionFactory->create($description, $context)
3845
);
3946
}
4047

‎src/DocBlock/Tags/Factory/VarFactory.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode;
1414
use Webmozart\Assert\Assert;
1515

16+
use function is_string;
1617
use function trim;
1718

1819
/**
@@ -34,10 +35,15 @@ public function create(PhpDocTagNode $node, Context $context): Tag
3435
$tagValue = $node->value;
3536
Assert::isInstanceOf($tagValue, VarTagValueNode::class);
3637

38+
$description = $tagValue->getAttribute('description');
39+
if (is_string($description) === false) {
40+
$description = $tagValue->description;
41+
}
42+
3743
return new Var_(
3844
trim($tagValue->variableName, '$'),
3945
$this->typeResolver->createType($tagValue->type, $context),
40-
$this->descriptionFactory->create($tagValue->description, $context)
46+
$this->descriptionFactory->create($description, $context)
4147
);
4248
}
4349

‎tests/integration/InterpretingDocBlocksTest.php

Lines changed: 98 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,21 @@
1717
use phpDocumentor\Reflection\DocBlock\Description;
1818
use phpDocumentor\Reflection\DocBlock\StandardTagFactory;
1919
use phpDocumentor\Reflection\DocBlock\Tag;
20+
use phpDocumentor\Reflection\DocBlock\Tags\Generic;
2021
use phpDocumentor\Reflection\DocBlock\Tags\InvalidTag;
2122
use phpDocumentor\Reflection\DocBlock\Tags\Method;
2223
use phpDocumentor\Reflection\DocBlock\Tags\MethodParameter;
2324
use phpDocumentor\Reflection\DocBlock\Tags\Param;
2425
use phpDocumentor\Reflection\DocBlock\Tags\Return_;
2526
use phpDocumentor\Reflection\DocBlock\Tags\See;
27+
use phpDocumentor\Reflection\DocBlock\Tags\Since;
2628
use phpDocumentor\Reflection\PseudoTypes\ConstExpression;
2729
use phpDocumentor\Reflection\Types\Array_;
30+
use phpDocumentor\Reflection\Types\Compound;
31+
use phpDocumentor\Reflection\Types\Context;
2832
use phpDocumentor\Reflection\Types\Integer;
2933
use phpDocumentor\Reflection\Types\Mixed_;
34+
use phpDocumentor\Reflection\Types\Object_;
3035
use phpDocumentor\Reflection\Types\Self_;
3136
use phpDocumentor\Reflection\Types\String_;
3237
use phpDocumentor\Reflection\Types\Void_;
@@ -123,8 +128,8 @@ public function testInterpretingTags(): void
123128
$this->assertInstanceOf(See::class, $seeTags[0]);
124129

125130
$seeTag = $seeTags[0];
126-
$this->assertSame('\\' . StandardTagFactory::class, (string) $seeTag->getReference());
127-
$this->assertSame('', (string) $seeTag->getDescription());
131+
$this->assertSame('\\' . StandardTagFactory::class, (string)$seeTag->getReference());
132+
$this->assertSame('', (string)$seeTag->getDescription());
128133
}
129134

130135
public function testDescriptionsCanEscapeAtSignsAndClosingBraces(): void
@@ -268,4 +273,95 @@ public function testConstantReferenceTypes(): void
268273
$docblock->getTags()
269274
);
270275
}
276+
277+
public function testRegressionWordpressDocblocks(): void
278+
{
279+
$docCommment = <<<DOC
280+
/**
281+
* Install a package.
282+
*
283+
* Copies the contents of a package from a source directory, and installs them in
284+
* a destination directory. Optionally removes the source. It can also optionally
285+
* clear out the destination folder if it already exists.
286+
*
287+
* @since 2.8.0
288+
* @since 6.2.0 Use move_dir() instead of copy_dir() when possible.
289+
*
290+
* @global WP_Filesystem_Base \$wp_filesystem WordPress filesystem subclass.
291+
* @global array \$wp_theme_directories
292+
*
293+
* @param array|string \$args {
294+
* Optional. Array or string of arguments for installing a package. Default empty array.
295+
*
296+
* @type string \$source Required path to the package source. Default empty.
297+
* @type string \$destination Required path to a folder to install the package in.
298+
* Default empty.
299+
* @type bool \$clear_destination Whether to delete any files already in the destination
300+
* folder. Default false.
301+
* @type bool \$clear_working Whether to delete the files from the working directory
302+
* after copying them to the destination. Default false.
303+
* @type bool \$abort_if_destination_exists Whether to abort the installation if
304+
* the destination folder already exists. Default true.
305+
* @type array \$hook_extra Extra arguments to pass to the filter hooks called by
306+
* WP_Upgrader::install_package(). Default empty array.
307+
* }
308+
*
309+
* @return array|WP_Error The result (also stored in `WP_Upgrader::\$result`), or a WP_Error on failure.
310+
*/
311+
DOC;
312+
313+
$factory = DocBlockFactory::createInstance();
314+
$docblock = $factory->create($docCommment);
315+
316+
self::assertEquals(
317+
new DocBlock(
318+
'Install a package.',
319+
new Description(
320+
'Copies the contents of a package from a source directory, and installs them in' . PHP_EOL .
321+
'a destination directory. Optionally removes the source. It can also optionally' . PHP_EOL .
322+
'clear out the destination folder if it already exists.'
323+
),
324+
[
325+
new Since('2.8.0', new Description('')),
326+
new Since('6.2.0', new Description('Use move_dir() instead of copy_dir() when possible.')),
327+
new Generic(
328+
'global',
329+
new Description('WP_Filesystem_Base $wp_filesystem WordPress filesystem subclass.')
330+
),
331+
new Generic(
332+
'global',
333+
new Description('array $wp_theme_directories')
334+
),
335+
new Param(
336+
'args',
337+
new Compound([new Array_(new Mixed_()), new String_()]),
338+
false,
339+
new Description(
340+
'{' . "\n" .
341+
'Optional. Array or string of arguments for installing a package. Default empty array.' . "\n" .
342+
"\n" .
343+
' @type string $source Required path to the package source. Default empty.' . "\n" .
344+
' @type string $destination Required path to a folder to install the package in.' . "\n" .
345+
' Default empty.' . "\n" .
346+
' @type bool $clear_destination Whether to delete any files already in the destination' . "\n" .
347+
' folder. Default false.' . "\n" .
348+
' @type bool $clear_working Whether to delete the files from the working directory' . "\n" .
349+
' after copying them to the destination. Default false.' . "\n" .
350+
' @type bool $abort_if_destination_exists Whether to abort the installation if' . "\n" .
351+
' the destination folder already exists. Default true.' . "\n" .
352+
' @type array $hook_extra Extra arguments to pass to the filter hooks called by' . "\n" .
353+
' WP_Upgrader::install_package(). Default empty array.' . "\n" .
354+
'}'
355+
)
356+
),
357+
new Return_(
358+
new Compound([new Array_(new Mixed_()), new Object_(new Fqsen('\WP_Error'))]),
359+
new Description('The result (also stored in `WP_Upgrader::$result`), or a WP_Error on failure.')
360+
),
361+
],
362+
new Context('\\')
363+
),
364+
$docblock
365+
);
366+
}
271367
}

‎tests/unit/DocBlock/Tags/Factory/TagFactoryTestCase.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
use PHPStan\PhpDocParser\Parser\TypeParser;
2727
use PHPUnit\Framework\TestCase;
2828

29+
use function property_exists;
30+
2931
abstract class TagFactoryTestCase extends TestCase
3032
{
3133
public function parseTag(string $tag): PhpDocTagNode
@@ -34,7 +36,12 @@ public function parseTag(string $tag): PhpDocTagNode
3436
$tokens = $lexer->tokenize($tag);
3537
$constParser = new ConstExprParser();
3638

37-
return (new PhpDocParser(new TypeParser($constParser), $constParser))->parseTag(new TokenIterator($tokens));
39+
$tagNode = (new PhpDocParser(new TypeParser($constParser), $constParser))->parseTag(new TokenIterator($tokens));
40+
if (property_exists($tagNode->value, 'description') === true) {
41+
$tagNode->value->setAttribute('description', $tagNode->value->description);
42+
}
43+
44+
return $tagNode;
3845
}
3946

4047
public function giveTypeResolver(): TypeResolver

0 commit comments

Comments
 (0)