Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PhpDocParser: add config for lines in its AST & enable ignoring errors within phpdocs #2807

Merged
merged 9 commits into from
Jan 3, 2024
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
janedbal marked this conversation as resolved.
Show resolved Hide resolved
*/
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
Loading