Skip to content

Commit

Permalink
Bleeding Edge - PhpDocParser: add config for lines in its AST & enabl…
Browse files Browse the repository at this point in the history
…e ignoring errors within phpdocs
  • Loading branch information
janedbal authored Jan 3, 2024
1 parent 5d67f0c commit c5ca230
Show file tree
Hide file tree
Showing 13 changed files with 205 additions and 48 deletions.
2 changes: 2 additions & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ parameters:
checkUnresolvableParameterTypes: true
readOnlyByPhpDoc: true
phpDocParserRequireWhitespaceBeforeDescription: true
phpDocParserIncludeLines: true
enableIgnoreErrorsWithinPhpDocs: true
runtimeReflectionRules: true
notAnalysedTrait: true
curlSetOptTypes: true
Expand Down
5 changes: 5 additions & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ parameters:
checkUnresolvableParameterTypes: false
readOnlyByPhpDoc: false
phpDocParserRequireWhitespaceBeforeDescription: false
phpDocParserIncludeLines: false
enableIgnoreErrorsWithinPhpDocs: false
runtimeReflectionRules: false
notAnalysedTrait: false
curlSetOptTypes: false
Expand Down Expand Up @@ -389,6 +391,8 @@ services:
arguments:
requireWhitespaceBeforeDescription: %featureToggles.phpDocParserRequireWhitespaceBeforeDescription%
preserveTypeAliasesWithInvalidTypes: true
usedAttributes:
lines: %featureToggles.phpDocParserIncludeLines%

-
class: PHPStan\PhpDoc\ConstExprParserFactory
Expand Down Expand Up @@ -1824,6 +1828,7 @@ services:
arguments:
parser: @currentPhpVersionPhpParser
lexer: @currentPhpVersionLexer
enableIgnoreErrorsWithinPhpDocs: %featureToggles.enableIgnoreErrorsWithinPhpDocs%
autowired: no

currentPhpVersionSimpleParser:
Expand Down
2 changes: 2 additions & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ parametersSchema:
checkUnresolvableParameterTypes: bool()
readOnlyByPhpDoc: bool()
phpDocParserRequireWhitespaceBeforeDescription: bool()
phpDocParserIncludeLines: bool()
enableIgnoreErrorsWithinPhpDocs: bool()
runtimeReflectionRules: bool()
notAnalysedTrait: bool()
curlSetOptTypes: bool()
Expand Down
48 changes: 42 additions & 6 deletions src/Parser/RichParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
use PHPStan\ShouldNotHappenException;
use function array_filter;
use function is_string;
use function str_contains;
use function strpos;
use function substr;
use function substr_count;
use const ARRAY_FILTER_USE_KEY;
use const T_COMMENT;
Expand All @@ -28,6 +29,7 @@ public function __construct(
private Lexer $lexer,
private NameResolver $nameResolver,
private Container $container,
private bool $enableIgnoreErrorsWithinPhpDocs,
)
{
}
Expand Down Expand Up @@ -103,14 +105,48 @@ private function getLinesToIgnore(array $tokens): array

$text = $token[1];
$line = $token[2];
if (str_contains($text, '@phpstan-ignore-next-line')) {
$line++;
} elseif (!str_contains($text, '@phpstan-ignore-line')) {
continue;

if ($this->enableIgnoreErrorsWithinPhpDocs) {
$lines = $lines +
$this->getLinesToIgnoreForTokenByIgnoreComment($text, $line, '@phpstan-ignore-next-line', true) +
$this->getLinesToIgnoreForTokenByIgnoreComment($text, $line, '@phpstan-ignore-line');

} else {
if (strpos($text, '@phpstan-ignore-next-line') !== false) {
$line++;
} elseif (strpos($text, '@phpstan-ignore-line') === false) {
continue;
}

$line += substr_count($token[1], "\n");
$lines[$line] = null;
}
}

return $lines;
}

$line += substr_count($token[1], "\n");
/**
* @return array<int, null>
*/
private function getLinesToIgnoreForTokenByIgnoreComment(
string $tokenText,
int $tokenLine,
string $ignoreComment,
bool $ignoreNextLine = false,
): array
{
$lines = [];
$positionsOfIgnoreComment = [];
$offset = 0;

while (($pos = strpos($tokenText, $ignoreComment, $offset)) !== false) {
$positionsOfIgnoreComment[] = $pos;
$offset = $pos + 1;
}

foreach ($positionsOfIgnoreComment as $pos) {
$line = $tokenLine + substr_count(substr($tokenText, 0, $pos), "\n") + ($ignoreNextLine ? 1 : 0);
$lines[$line] = null;
}

Expand Down
4 changes: 3 additions & 1 deletion src/Rules/PhpDoc/InvalidPHPStanDocTagRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ public function processNode(Node $node, Scope $scope): array
$errors[] = RuleErrorBuilder::message(sprintf(
'Unknown PHPDoc tag: %s',
$phpDocTag->name,
))->build();
))
->line(PhpDocLineHelper::detectLine($node, $phpDocTag))
->build();
}

return $errors;
Expand Down
8 changes: 6 additions & 2 deletions src/Rules/PhpDoc/InvalidPhpDocTagValueRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ public function processNode(Node $node, Scope $scope): array
$phpDocTag->name,
$phpDocTag->value->alias,
$this->trimExceptionMessage($phpDocTag->value->type->getException()->getMessage()),
))->build();
))
->line(PhpDocLineHelper::detectLine($node, $phpDocTag))
->build();

continue;
} elseif (!($phpDocTag->value instanceof InvalidTagValueNode)) {
Expand All @@ -102,7 +104,9 @@ public function processNode(Node $node, Scope $scope): array
$phpDocTag->name,
$phpDocTag->value->value,
$this->trimExceptionMessage($phpDocTag->value->exception->getMessage()),
))->build();
))
->line(PhpDocLineHelper::detectLine($node, $phpDocTag))
->build();
}

return $errors;
Expand Down
28 changes: 28 additions & 0 deletions src/Rules/PhpDoc/PhpDocLineHelper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\PhpDoc;

use PhpParser\Node as PhpParserNode;
use PHPStan\PhpDocParser\Ast\Node as PhpDocNode;

class PhpDocLineHelper
{

/**
* This method returns exact line of e.g. `@param` tag in PHPDoc so that it can be used for precise error reporting
* - exact position is available only when bleedingEdge is enabled
* - otherwise, it falls back to given node start line
*/
public static function detectLine(PhpParserNode $node, PhpDocNode $phpDocNode): int
{
$phpDocTagLine = $phpDocNode->getAttribute('startLine');
$phpDoc = $node->getDocComment();

if ($phpDocTagLine === null || $phpDoc === null) {
return $node->getLine();
}

return $phpDoc->getStartLine() + $phpDocTagLine - 1;
}

}
24 changes: 19 additions & 5 deletions tests/PHPStan/Analyser/AnalyserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ public function testIgnoreNextLine(bool $reportUnmatchedIgnoredErrors): void
__DIR__ . '/data/ignore-next-line.php',
], true);
$this->assertCount($reportUnmatchedIgnoredErrors ? 4 : 3, $result);
foreach ([10, 30, 34] as $i => $line) {
foreach ([10, 20, 24] as $i => $line) {
$this->assertArrayHasKey($i, $result);
$this->assertInstanceOf(Error::class, $result[$i]);
$this->assertSame('Fail.', $result[$i]->getMessage());
Expand All @@ -487,8 +487,20 @@ public function testIgnoreNextLine(bool $reportUnmatchedIgnoredErrors): void

$this->assertArrayHasKey(3, $result);
$this->assertInstanceOf(Error::class, $result[3]);
$this->assertSame('No error to ignore is reported on line 38.', $result[3]->getMessage());
$this->assertSame(38, $result[3]->getLine());
$this->assertSame('No error to ignore is reported on line 28.', $result[3]->getMessage());
$this->assertSame(28, $result[3]->getLine());
}

public function testIgnoreNextLineLegacyBehaviour(): void
{
$result = $this->runAnalyser([], false, [__DIR__ . '/data/ignore-next-line-legacy.php'], true, false);

foreach ([10, 32, 36] as $i => $line) {
$this->assertArrayHasKey($i, $result);
$this->assertInstanceOf(Error::class, $result[$i]);
$this->assertSame('Fail.', $result[$i]->getMessage());
$this->assertSame($line, $result[$i]->getLine());
}
}

/**
Expand Down Expand Up @@ -577,9 +589,10 @@ private function runAnalyser(
bool $reportUnmatchedIgnoredErrors,
$filePaths,
bool $onlyFiles,
bool $enableIgnoreErrorsWithinPhpDocs = true,
): array
{
$analyser = $this->createAnalyser($reportUnmatchedIgnoredErrors);
$analyser = $this->createAnalyser($reportUnmatchedIgnoredErrors, $enableIgnoreErrorsWithinPhpDocs);

if (is_string($filePaths)) {
$filePaths = [$filePaths];
Expand Down Expand Up @@ -610,7 +623,7 @@ private function runAnalyser(
);
}

private function createAnalyser(bool $reportUnmatchedIgnoredErrors): Analyser
private function createAnalyser(bool $reportUnmatchedIgnoredErrors, bool $enableIgnoreErrorsWithinPhpDocs): Analyser
{
$ruleRegistry = new DirectRuleRegistry([
new AlwaysFailRule(),
Expand Down Expand Up @@ -658,6 +671,7 @@ private function createAnalyser(bool $reportUnmatchedIgnoredErrors): Analyser
$lexer,
new NameResolver(),
self::getContainer(),
$enableIgnoreErrorsWithinPhpDocs,
),
new DependencyResolver($fileHelper, $reflectionProvider, new ExportedNodeResolver($fileTypeMapper, new ExprPrinter(new Printer())), $fileTypeMapper),
new RuleErrorTransformer(),
Expand Down
40 changes: 40 additions & 0 deletions tests/PHPStan/Analyser/data/ignore-next-line-legacy.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

namespace IgnoreNextLineLegacy;

class Foo
{

public function doFoo(): void
{
fail(); // reported

// @phpstan-ignore-next-line
fail();

/* @phpstan-ignore-next-line */
fail();

/** @phpstan-ignore-next-line */
fail();

/*
* @phpstan-ignore-next-line
*/
fail();

/**
* @phpstan-ignore-next-line
*
* This is the legacy behaviour, the next line is meant as next-non-comment-line
*/
fail();
fail(); // reported

// @phpstan-ignore-next-line
if (fail()) {
fail(); // reported
}
}

}
10 changes: 0 additions & 10 deletions tests/PHPStan/Analyser/data/ignore-next-line.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,6 @@ public function doFoo(): void

/** @phpstan-ignore-next-line */
fail();

/*
* @phpstan-ignore-next-line
*/
fail();

/**
* @phpstan-ignore-next-line
*/
fail();
fail(); // reported

// @phpstan-ignore-next-line
Expand Down
10 changes: 5 additions & 5 deletions tests/PHPStan/Rules/PhpDoc/InvalidPHPStanDocTagRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,26 @@ public function dataRule(): iterable
$errors = [
[
'Unknown PHPDoc tag: @phpstan-extens',
7,
6,
],
[
'Unknown PHPDoc tag: @phpstan-pararm',
14,
11,
],
[
'Unknown PHPDoc tag: @phpstan-varr',
44,
43,
],
[
'Unknown PHPDoc tag: @phpstan-varr',
47,
46,
],
];
yield [false, $errors];
yield [true, array_merge($errors, [
[
'Unknown PHPDoc tag: @phpstan-varr',
57,
56,
],
])];
}
Expand Down
Loading

0 comments on commit c5ca230

Please sign in to comment.