Skip to content

Commit 8c422ab

Browse files
jaapiojaapio
committed
Fix docblock tag descriptions
Newlines and whitespaces were not handled as before. This caused issues for some users because our indent-recuction was broken. The cause seems to be an upstream issue in phpstan parser which is not resolved yet. But this work around post processing the tokens helps us to make it work as before.
1 parent 88a07d2 commit 8c422ab

File tree

4 files changed

+127
-6
lines changed

4 files changed

+127
-6
lines changed

‎phpstan.neon‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,9 @@ parameters:
44
- '#Method phpDocumentor\\Reflection\\DocBlock\\StandardTagFactory::createTag\(\) should return phpDocumentor\\Reflection\\DocBlock\\Tag but returns mixed#'
55
- "#Strict comparison using !== between array{'name', 'type'} and array{'name', 'type'} will always evaluate to false#"
66
- '#Call to static method Webmozart\\Assert\\Assert::implementsInterface\(\) with class-string#'
7+
- '#Class PHPStan\\PhpDocParser\\Lexer\\Lexer does not have a constructor and must be instantiated without any parameters\.#'
8+
- '#Class PHPStan\\PhpDocParser\\Parser\\ConstExprParser constructor invoked with 3 parameters, 0\-1 required\.#'
9+
- '#Class PHPStan\\PhpDocParser\\Parser\\PhpDocParser constructor invoked with 6 parameters, 2\-3 required\.#'
10+
- '#Class PHPStan\\PhpDocParser\\Parser\\TypeParser constructor invoked with 3 parameters\, 0\-1 required\.#'
711
paths:
812
- src

‎psalm.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@
4848
</errorLevel>
4949
</NoInterfaceProperties>
5050

51+
<TooManyArguments>
52+
<errorLevel type="info">
53+
<file name="src/DocBlock/Tags/Factory/AbstractPHPStanFactory.php"/>
54+
</errorLevel>
55+
</TooManyArguments>
56+
5157
<RedundantConditionGivenDocblockType>
5258
<errorLevel type="info">
5359
<!-- Psalm manage to infer a more precise type than PHPStan. notNull assert is needed for PHPStan but

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

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

26+
use function ltrim;
2627
use function property_exists;
28+
use function rtrim;
2729

2830
/**
2931
* Factory class creating tags using phpstan's parser
@@ -42,18 +44,28 @@ class AbstractPHPStanFactory implements Factory
4244

4345
public function __construct(PHPStanFactory ...$factories)
4446
{
45-
$this->lexer = new Lexer();
46-
$constParser = new ConstExprParser();
47-
$this->parser = new PhpDocParser(new TypeParser($constParser), $constParser);
47+
$this->lexer = new Lexer(true);
48+
$constParser = new ConstExprParser(true, true, ['lines' => true, 'indexes' => true]);
49+
$this->parser = new PhpDocParser(
50+
new TypeParser($constParser, true, ['lines' => true, 'indexes' => true]),
51+
$constParser,
52+
true,
53+
true,
54+
['lines' => true, 'indexes' => true],
55+
true
56+
);
4857
$this->factories = $factories;
4958
}
5059

5160
public function create(string $tagLine, ?TypeContext $context = null): Tag
5261
{
53-
$tokens = new TokenIterator($this->lexer->tokenize($tagLine));
62+
$tokens = $this->tokenizeLine($tagLine);
5463
$ast = $this->parser->parseTag($tokens);
5564
if (property_exists($ast->value, 'description') === true) {
56-
$ast->value->setAttribute('description', $ast->value->description . $tokens->joinUntil(Lexer::TOKEN_END));
65+
$ast->value->setAttribute(
66+
'description',
67+
$ast->value->description . $tokens->joinUntil(Lexer::TOKEN_END)
68+
);
5769
}
5870

5971
if ($context === null) {
@@ -75,4 +87,36 @@ public function create(string $tagLine, ?TypeContext $context = null): Tag
7587
$ast->name
7688
);
7789
}
90+
91+
/**
92+
* Solve the issue with the lexer not tokenizing the line correctly
93+
*
94+
* This method is a workaround for the lexer that includes newline tokens with spaces. For
95+
* phpstan this isn't an issue, as it doesn't do a lot of things with the indentation of descriptions.
96+
* But for us is important to keep the indentation of the descriptions, so we need to fix the lexer output.
97+
*/
98+
private function tokenizeLine(string $tagLine): TokenIterator
99+
{
100+
$tokens = $this->lexer->tokenize($tagLine);
101+
$fixed = [];
102+
foreach ($tokens as $token) {
103+
if (($token[1] === Lexer::TOKEN_PHPDOC_EOL) && rtrim($token[0], " \t") !== $token[0]) {
104+
$fixed[] = [
105+
rtrim($token[Lexer::VALUE_OFFSET], " \t"),
106+
Lexer::TOKEN_PHPDOC_EOL,
107+
$token[2] ?? null,
108+
];
109+
$fixed[] = [
110+
ltrim($token[Lexer::VALUE_OFFSET], "\n\r"),
111+
Lexer::TOKEN_HORIZONTAL_WS,
112+
($token[2] ?? null) + 1,
113+
];
114+
continue;
115+
}
116+
117+
$fixed[] = $token;
118+
}
119+
120+
return new TokenIterator($fixed);
121+
}
78122
}

‎tests/integration/InterpretingDocBlocksTest.php‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ public function testRegressionWordpressDocblocks(): void
338338
false,
339339
new Description(
340340
'{' . "\n" .
341-
'Optional. Array or string of arguments for installing a package. Default empty array.' . "\n" .
341+
' Optional. Array or string of arguments for installing a package. Default empty array.' . "\n" .
342342
"\n" .
343343
' @type string $source Required path to the package source. Default empty.' . "\n" .
344344
' @type string $destination Required path to a folder to install the package in.' . "\n" .
@@ -364,4 +364,71 @@ public function testRegressionWordpressDocblocks(): void
364364
$docblock
365365
);
366366
}
367+
368+
public function testIndentationIsKept(): void
369+
{
370+
$docComment = <<<DOC
371+
/**
372+
* Registers the script module if no script module with that script module
373+
* identifier has already been registered.
374+
*
375+
* @since 6.5.0
376+
*
377+
* @param array \$deps {
378+
* Optional. List of dependencies.
379+
*
380+
* @type string|array ...$0 {
381+
* An array of script module identifiers of the dependencies of this script
382+
* module. The dependencies can be strings or arrays. If they are arrays,
383+
* they need an `id` key with the script module identifier, and can contain
384+
* an `import` key with either `static` or `dynamic`. By default,
385+
* dependencies that don't contain an `import` key are considered static.
386+
*
387+
* @type string \$id The script module identifier.
388+
* @type string \$import Optional. Import type. May be either `static` or
389+
* `dynamic`. Defaults to `static`.
390+
* }
391+
* }
392+
*/
393+
DOC;
394+
395+
$factory = DocBlockFactory::createInstance();
396+
$docblock = $factory->create($docComment);
397+
398+
self::assertEquals(
399+
new DocBlock(
400+
'Registers the script module if no script module with that script module
401+
identifier has already been registered.',
402+
new Description(
403+
''
404+
),
405+
[
406+
new Since('6.5.0', new Description('')),
407+
new Param(
408+
'deps',
409+
new Array_(new Mixed_()),
410+
false,
411+
new Description("{
412+
Optional. List of dependencies.
413+
414+
@type string|array ...$0 {
415+
An array of script module identifiers of the dependencies of this script
416+
module. The dependencies can be strings or arrays. If they are arrays,
417+
they need an `id` key with the script module identifier, and can contain
418+
an `import` key with either `static` or `dynamic`. By default,
419+
dependencies that don't contain an `import` key are considered static.
420+
421+
@type string \$id The script module identifier.
422+
@type string \$import Optional. Import type. May be either `static` or
423+
`dynamic`. Defaults to `static`.
424+
}
425+
}"
426+
)
427+
),
428+
],
429+
new Context('\\')
430+
),
431+
$docblock
432+
);
433+
}
367434
}

0 commit comments

Comments
 (0)