Skip to content

Commit 9d07b3f

Browse files
jaapiojaapio
authored
Merge pull request #370 from phpDocumentor/retry-parser-docblocs
Fix docblock tag descriptions
2 parents 88a07d2 + 8c422ab commit 9d07b3f

File tree

4 files changed

+127
-6
lines changed

4 files changed

+127
-6
lines changed

‎phpstan.neon

+4
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

+6
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

+49-5
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

+68-1
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)