From b9fc4238ec69cbf6b09252e4b7198ecdfc759703 Mon Sep 17 00:00:00 2001 From: Caleb White Date: Wed, 30 Apr 2025 11:14:12 -0500 Subject: [PATCH 1/2] feat: add option to report ignores without identifiers --- conf/config.neon | 3 ++ conf/parametersSchema.neon | 1 + src/Analyser/AnalyserResultFinalizer.php | 54 +++++++++++++++++-- src/Command/FixerWorkerCommand.php | 2 +- src/Testing/RuleTestCase.php | 1 + tests/PHPStan/Analyser/AnalyserTest.php | 16 ++++++ .../Analyser/data/ignore-no-identifiers.php | 21 ++++++++ 7 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/ignore-no-identifiers.php diff --git a/conf/config.neon b/conf/config.neon index a0083d7753..ccf37c6aeb 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -95,6 +95,7 @@ parameters: cache: nodesByStringCountMax: 256 reportUnmatchedIgnoredErrors: true + reportIgnoresWithoutIdentifiers: false typeAliases: [] universalObjectCratesClasses: - stdClass @@ -198,6 +199,7 @@ parameters: - [parameters, errorFormat] - [parameters, ignoreErrors] - [parameters, reportUnmatchedIgnoredErrors] + - [parameters, reportIgnoresWithoutIdentifiers] - [parameters, tipsOfTheDay] - [parameters, parallel] - [parameters, internalErrorsCountLimit] @@ -461,6 +463,7 @@ services: class: PHPStan\Analyser\AnalyserResultFinalizer arguments: reportUnmatchedIgnoredErrors: %reportUnmatchedIgnoredErrors% + reportIgnoresWithoutIdentifiers: %reportIgnoresWithoutIdentifiers% - class: PHPStan\Analyser\FileAnalyser diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index d18df776e3..1895aa9f67 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -143,6 +143,7 @@ parametersSchema: nodesByStringCountMax: int() ]) reportUnmatchedIgnoredErrors: bool() + reportIgnoresWithoutIdentifiers: bool() typeAliases: arrayOf(string()) universalObjectCratesClasses: listOf(string()) stubFiles: listOf(string()) diff --git a/src/Analyser/AnalyserResultFinalizer.php b/src/Analyser/AnalyserResultFinalizer.php index 56fde0132e..96ac6ec632 100644 --- a/src/Analyser/AnalyserResultFinalizer.php +++ b/src/Analyser/AnalyserResultFinalizer.php @@ -24,6 +24,7 @@ public function __construct( private ScopeFactory $scopeFactory, private LocalIgnoresProcessor $localIgnoresProcessor, private bool $reportUnmatchedIgnoredErrors, + private bool $reportIgnoresWithoutIdentifiers, ) { } @@ -31,12 +32,12 @@ public function __construct( public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $debug): FinalizerResult { if (count($analyserResult->getCollectedData()) === 0) { - return $this->addUnmatchedIgnoredErrors($this->mergeFilteredPhpErrors($analyserResult), [], []); + return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutIdentifiersErrors($this->mergeFilteredPhpErrors($analyserResult)), [], []); } $hasInternalErrors = count($analyserResult->getInternalErrors()) > 0 || $analyserResult->hasReachedInternalErrorsCountLimit(); if ($hasInternalErrors) { - return $this->addUnmatchedIgnoredErrors($this->mergeFilteredPhpErrors($analyserResult), [], []); + return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutIdentifiersErrors($this->mergeFilteredPhpErrors($analyserResult)), [], []); } $nodeType = CollectedDataNode::class; @@ -130,7 +131,7 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $ $allUnmatchedLineIgnores[$file] = $localIgnoresProcessorResult->getUnmatchedLineIgnores(); } - return $this->addUnmatchedIgnoredErrors(new AnalyserResult( + return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutIdentifiersErrors(new AnalyserResult( array_merge($errors, $analyserResult->getFilteredPhpErrors()), [], $analyserResult->getAllPhpErrors(), @@ -143,7 +144,7 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $ $analyserResult->getExportedNodes(), $analyserResult->hasReachedInternalErrorsCountLimit(), $analyserResult->getPeakMemoryUsageBytes(), - ), $collectorErrors, $locallyIgnoredCollectorErrors); + )), $collectorErrors, $locallyIgnoredCollectorErrors); } private function mergeFilteredPhpErrors(AnalyserResult $analyserResult): AnalyserResult @@ -230,4 +231,49 @@ private function addUnmatchedIgnoredErrors( ); } + private function addIgnoresWithoutIdentifiersErrors(AnalyserResult $analyserResult): AnalyserResult + { + if (!$this->reportIgnoresWithoutIdentifiers) { + return $analyserResult; + } + + $errors = $analyserResult->getUnorderedErrors(); + foreach ($analyserResult->getLinesToIgnore() as $file => $data) { + foreach ($data as $ignoredFile => $lines) { + if ($ignoredFile !== $file) { + continue; + } + + foreach ($lines as $line => $identifiers) { + if ($identifiers !== null) { + continue; + } + + $errors[] = (new Error( + sprintf('Error is ignored with no identifiers on line %d.', $line), + $file, + $line, + false, + $file, + ))->withIdentifier('ignore.noIdentifier'); + } + } + } + + return new AnalyserResult( + $errors, + $analyserResult->getFilteredPhpErrors(), + $analyserResult->getAllPhpErrors(), + $analyserResult->getLocallyIgnoredErrors(), + $analyserResult->getLinesToIgnore(), + $analyserResult->getUnmatchedLineIgnores(), + $analyserResult->getInternalErrors(), + $analyserResult->getCollectedData(), + $analyserResult->getDependencies(), + $analyserResult->getExportedNodes(), + $analyserResult->hasReachedInternalErrorsCountLimit(), + $analyserResult->getPeakMemoryUsageBytes(), + ); + } + } diff --git a/src/Command/FixerWorkerCommand.php b/src/Command/FixerWorkerCommand.php index 70a7624453..1506d5c842 100644 --- a/src/Command/FixerWorkerCommand.php +++ b/src/Command/FixerWorkerCommand.php @@ -308,7 +308,7 @@ function (array $errors, array $locallyIgnoredErrors, array $analysedFiles) use if ($error->getIdentifier() === null) { continue; } - if (!in_array($error->getIdentifier(), ['ignore.count', 'ignore.unmatched', 'ignore.unmatchedLine', 'ignore.unmatchedIdentifier'], true)) { + if (!in_array($error->getIdentifier(), ['ignore.count', 'ignore.unmatched', 'ignore.unmatchedLine', 'ignore.unmatchedIdentifier', 'ignore.noIdentifier'], true)) { continue; } $ignoreFileErrors[] = $error; diff --git a/src/Testing/RuleTestCase.php b/src/Testing/RuleTestCase.php index 026069146a..63c0ef5a5a 100644 --- a/src/Testing/RuleTestCase.php +++ b/src/Testing/RuleTestCase.php @@ -241,6 +241,7 @@ private function gatherAnalyserErrorsWithDelayedErrors(array $files): array $this->createScopeFactory($reflectionProvider, $this->getTypeSpecifier()), new LocalIgnoresProcessor(), true, + false, ); return [ diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 1b41db9157..009089e17a 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -559,6 +559,20 @@ public function testIgnoreNextLineUnmatched(): void } } + public function testIgnoresWithoutIdentifiersReported(): void + { + $result = $this->runAnalyser([], false, [ + __DIR__ . '/data/ignore-no-identifiers.php', + ], true, true); + $this->assertCount(4, $result); + foreach ([10, 12, 15, 18] as $i => $line) { + $this->assertArrayHasKey($i, $result); + $this->assertInstanceOf(Error::class, $result[$i]); + $this->assertStringContainsString('Error is ignored with no identifiers on line', $result[$i]->getMessage()); + $this->assertSame($line, $result[$i]->getLine()); + } + } + /** * @dataProvider dataTrueAndFalse */ @@ -645,6 +659,7 @@ private function runAnalyser( bool $reportUnmatchedIgnoredErrors, $filePaths, bool $onlyFiles, + bool $reportIgnoresWithoutIdentifiers = false, ): array { $analyser = $this->createAnalyser(); @@ -677,6 +692,7 @@ private function runAnalyser( ), new LocalIgnoresProcessor(), $reportUnmatchedIgnoredErrors, + $reportIgnoresWithoutIdentifiers, ); $analyserResult = $finalizer->finalize($analyserResult, $onlyFiles, false)->getAnalyserResult(); diff --git a/tests/PHPStan/Analyser/data/ignore-no-identifiers.php b/tests/PHPStan/Analyser/data/ignore-no-identifiers.php new file mode 100644 index 0000000000..30f311ec5b --- /dev/null +++ b/tests/PHPStan/Analyser/data/ignore-no-identifiers.php @@ -0,0 +1,21 @@ + Date: Wed, 30 Apr 2025 13:08:56 -0500 Subject: [PATCH 2/2] feat: add option to report ignores without comments --- conf/config.neon | 3 + conf/parametersSchema.neon | 1 + src/Analyser/AnalyserResultFinalizer.php | 51 +++++++++++------ src/Analyser/FileAnalyser.php | 5 +- src/Analyser/FileAnalyserResult.php | 3 +- src/Analyser/LocalIgnoresProcessor.php | 4 +- src/Command/FixerWorkerCommand.php | 2 +- src/Parser/RichParser.php | 43 +++++++++----- src/Testing/RuleTestCase.php | 1 + tests/PHPStan/Analyser/AnalyserTest.php | 23 ++++++++ .../Analyser/data/ignore-no-comments.php | 18 ++++++ tests/PHPStan/Parser/RichParserTest.php | 56 +++++++++---------- 12 files changed, 143 insertions(+), 67 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/ignore-no-comments.php diff --git a/conf/config.neon b/conf/config.neon index ccf37c6aeb..a7d6b3d940 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -96,6 +96,7 @@ parameters: nodesByStringCountMax: 256 reportUnmatchedIgnoredErrors: true reportIgnoresWithoutIdentifiers: false + reportIgnoresWithoutComments: false typeAliases: [] universalObjectCratesClasses: - stdClass @@ -200,6 +201,7 @@ parameters: - [parameters, ignoreErrors] - [parameters, reportUnmatchedIgnoredErrors] - [parameters, reportIgnoresWithoutIdentifiers] + - [parameters, reportIgnoresWithoutComments] - [parameters, tipsOfTheDay] - [parameters, parallel] - [parameters, internalErrorsCountLimit] @@ -464,6 +466,7 @@ services: arguments: reportUnmatchedIgnoredErrors: %reportUnmatchedIgnoredErrors% reportIgnoresWithoutIdentifiers: %reportIgnoresWithoutIdentifiers% + reportIgnoresWithoutComments: %reportIgnoresWithoutComments% - class: PHPStan\Analyser\FileAnalyser diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 1895aa9f67..ec5f258dbe 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -144,6 +144,7 @@ parametersSchema: ]) reportUnmatchedIgnoredErrors: bool() reportIgnoresWithoutIdentifiers: bool() + reportIgnoresWithoutComments: bool() typeAliases: arrayOf(string()) universalObjectCratesClasses: listOf(string()) stubFiles: listOf(string()) diff --git a/src/Analyser/AnalyserResultFinalizer.php b/src/Analyser/AnalyserResultFinalizer.php index 96ac6ec632..f87e98bf19 100644 --- a/src/Analyser/AnalyserResultFinalizer.php +++ b/src/Analyser/AnalyserResultFinalizer.php @@ -25,19 +25,17 @@ public function __construct( private LocalIgnoresProcessor $localIgnoresProcessor, private bool $reportUnmatchedIgnoredErrors, private bool $reportIgnoresWithoutIdentifiers, + private bool $reportIgnoresWithoutComments, ) { } public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $debug): FinalizerResult { - if (count($analyserResult->getCollectedData()) === 0) { - return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutIdentifiersErrors($this->mergeFilteredPhpErrors($analyserResult)), [], []); - } - + $hasCollectedData = count($analyserResult->getCollectedData()) > 0; $hasInternalErrors = count($analyserResult->getInternalErrors()) > 0 || $analyserResult->hasReachedInternalErrorsCountLimit(); - if ($hasInternalErrors) { - return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutIdentifiersErrors($this->mergeFilteredPhpErrors($analyserResult)), [], []); + if (! $hasCollectedData || $hasInternalErrors) { + return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutCommentsOrIdentifiersErrors($this->mergeFilteredPhpErrors($analyserResult)), [], []); } $nodeType = CollectedDataNode::class; @@ -131,7 +129,7 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $ $allUnmatchedLineIgnores[$file] = $localIgnoresProcessorResult->getUnmatchedLineIgnores(); } - return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutIdentifiersErrors(new AnalyserResult( + return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutCommentsOrIdentifiersErrors(new AnalyserResult( array_merge($errors, $analyserResult->getFilteredPhpErrors()), [], $analyserResult->getAllPhpErrors(), @@ -200,7 +198,7 @@ private function addUnmatchedIgnoredErrors( foreach ($identifiers as $identifier) { $errors[] = (new Error( - sprintf('No error with identifier %s is reported on line %d.', $identifier, $line), + sprintf('No error with identifier %s is reported on line %d.', $identifier['name'], $line), $file, $line, false, @@ -231,9 +229,9 @@ private function addUnmatchedIgnoredErrors( ); } - private function addIgnoresWithoutIdentifiersErrors(AnalyserResult $analyserResult): AnalyserResult + private function addIgnoresWithoutCommentsOrIdentifiersErrors(AnalyserResult $analyserResult): AnalyserResult { - if (!$this->reportIgnoresWithoutIdentifiers) { + if (!$this->reportIgnoresWithoutComments && !$this->reportIgnoresWithoutIdentifiers) { return $analyserResult; } @@ -245,17 +243,34 @@ private function addIgnoresWithoutIdentifiersErrors(AnalyserResult $analyserResu } foreach ($lines as $line => $identifiers) { - if ($identifiers !== null) { + if ($identifiers === null) { + if (!$this->reportIgnoresWithoutIdentifiers) { + continue; + } + $errors[] = (new Error( + sprintf('Error is ignored with no identifiers on line %d.', $line), + $file, + $line, + false, + $file, + ))->withIdentifier('ignore.noIdentifier'); continue; } - $errors[] = (new Error( - sprintf('Error is ignored with no identifiers on line %d.', $line), - $file, - $line, - false, - $file, - ))->withIdentifier('ignore.noIdentifier'); + foreach ($identifiers as $identifier) { + ['name' => $name, 'comment' => $comment] = $identifier; + if ($comment !== null || !$this->reportIgnoresWithoutComments) { + continue; + } + + $errors[] = (new Error( + sprintf('Ignore with identifier %s has no comment.', $name), + $file, + $line, + false, + $file, + ))->withIdentifier('ignore.noComment'); + } } } } diff --git a/src/Analyser/FileAnalyser.php b/src/Analyser/FileAnalyser.php index 80724ea18a..0021f090e2 100644 --- a/src/Analyser/FileAnalyser.php +++ b/src/Analyser/FileAnalyser.php @@ -38,6 +38,7 @@ use const E_WARNING; /** + * @phpstan-import-type Identifier from FileAnalyserResult * @phpstan-import-type CollectorData from CollectedData */ final class FileAnalyser @@ -306,7 +307,7 @@ public function analyseFile( /** * @param Node[] $nodes - * @return array|null> + * @return array|null> */ private function getLinesToIgnoreFromTokens(array $nodes): array { @@ -314,7 +315,7 @@ private function getLinesToIgnoreFromTokens(array $nodes): array return []; } - /** @var array|null> */ + /** @var array|null> */ return $nodes[0]->getAttribute('linesToIgnore', []); } diff --git a/src/Analyser/FileAnalyserResult.php b/src/Analyser/FileAnalyserResult.php index 2aba60730f..20484522e6 100644 --- a/src/Analyser/FileAnalyserResult.php +++ b/src/Analyser/FileAnalyserResult.php @@ -6,7 +6,8 @@ use PHPStan\Dependency\RootExportedNode; /** - * @phpstan-type LinesToIgnore = array|null>> + * @phpstan-type Identifier = array{name: string, comment: string|null} + * @phpstan-type LinesToIgnore = array|null>> * @phpstan-import-type CollectorData from CollectedData */ final class FileAnalyserResult diff --git a/src/Analyser/LocalIgnoresProcessor.php b/src/Analyser/LocalIgnoresProcessor.php index d5c0197dd6..4f7f1bacec 100644 --- a/src/Analyser/LocalIgnoresProcessor.php +++ b/src/Analyser/LocalIgnoresProcessor.php @@ -47,7 +47,7 @@ public function process( } foreach ($identifiers as $i => $ignoredIdentifier) { - if ($ignoredIdentifier !== $tmpFileError->getIdentifier()) { + if ($ignoredIdentifier['name'] !== $tmpFileError->getIdentifier()) { continue; } @@ -66,7 +66,7 @@ public function process( $unmatchedIgnoredIdentifiers = $unmatchedLineIgnores[$tmpFileError->getFile()][$line]; if (is_array($unmatchedIgnoredIdentifiers)) { foreach ($unmatchedIgnoredIdentifiers as $j => $unmatchedIgnoredIdentifier) { - if ($ignoredIdentifier !== $unmatchedIgnoredIdentifier) { + if ($ignoredIdentifier['name'] !== $unmatchedIgnoredIdentifier['name']) { continue; } diff --git a/src/Command/FixerWorkerCommand.php b/src/Command/FixerWorkerCommand.php index 1506d5c842..0553c8d13f 100644 --- a/src/Command/FixerWorkerCommand.php +++ b/src/Command/FixerWorkerCommand.php @@ -308,7 +308,7 @@ function (array $errors, array $locallyIgnoredErrors, array $analysedFiles) use if ($error->getIdentifier() === null) { continue; } - if (!in_array($error->getIdentifier(), ['ignore.count', 'ignore.unmatched', 'ignore.unmatchedLine', 'ignore.unmatchedIdentifier', 'ignore.noIdentifier'], true)) { + if (!in_array($error->getIdentifier(), ['ignore.count', 'ignore.unmatched', 'ignore.unmatchedLine', 'ignore.unmatchedIdentifier', 'ignore.noIdentifier', 'ignore.noComment'], true)) { continue; } $ignoreFileErrors[] = $error; diff --git a/src/Parser/RichParser.php b/src/Parser/RichParser.php index a50ec45dbe..09e73eae8d 100644 --- a/src/Parser/RichParser.php +++ b/src/Parser/RichParser.php @@ -7,13 +7,16 @@ use PhpParser\NodeTraverser; use PhpParser\NodeVisitor\NameResolver; use PhpParser\Token; +use PHPStan\Analyser\FileAnalyserResult; use PHPStan\Analyser\Ignore\IgnoreLexer; use PHPStan\Analyser\Ignore\IgnoreParseException; use PHPStan\DependencyInjection\Container; use PHPStan\File\FileReader; use PHPStan\ShouldNotHappenException; use function array_filter; +use function array_key_last; use function array_map; +use function array_values; use function count; use function implode; use function in_array; @@ -30,6 +33,9 @@ use const T_DOC_COMMENT; use const T_WHITESPACE; +/** + * @phpstan-import-type Identifier from FileAnalyserResult + */ final class RichParser implements Parser { @@ -111,7 +117,7 @@ public function parseString(string $sourceCode): array /** * @param Token[] $tokens - * @return array{lines: array|null>, errors: array>} + * @return array{lines: array|null>, errors: array>} */ private function getLinesToIgnore(array $tokens): array { @@ -268,33 +274,29 @@ private function getLinesToIgnoreForTokenByIgnoreComment( } /** - * @return non-empty-list + * @return non-empty-list * @throws IgnoreParseException */ private function parseIdentifiers(string $text, int $ignorePos): array { $text = substr($text, $ignorePos + strlen('@phpstan-ignore')); - $originalTokens = $this->ignoreLexer->tokenize($text); - $tokens = []; - - foreach ($originalTokens as $originalToken) { - if ($originalToken[IgnoreLexer::TYPE_OFFSET] === IgnoreLexer::TOKEN_WHITESPACE) { - continue; - } - $tokens[] = $originalToken; - } + $tokens = $this->ignoreLexer->tokenize($text); $c = count($tokens); $identifiers = []; + $comment = null; $openParenthesisCount = 0; $expected = [IgnoreLexer::TOKEN_IDENTIFIER]; + $lastTokenTypeLabel = '@phpstan-ignore'; for ($i = 0; $i < $c; $i++) { - $lastTokenTypeLabel = isset($tokenType) ? $this->ignoreLexer->getLabel($tokenType) : '@phpstan-ignore'; + if (isset($tokenType) && $tokenType !== IgnoreLexer::TOKEN_WHITESPACE) { + $lastTokenTypeLabel = $this->ignoreLexer->getLabel($tokenType); + } [IgnoreLexer::VALUE_OFFSET => $content, IgnoreLexer::TYPE_OFFSET => $tokenType, IgnoreLexer::LINE_OFFSET => $tokenLine] = $tokens[$i]; - if ($expected !== null && !in_array($tokenType, $expected, true)) { + if ($expected !== null && !in_array($tokenType, [...$expected, IgnoreLexer::TOKEN_WHITESPACE], true)) { $tokenTypeLabel = $this->ignoreLexer->getLabel($tokenType); $otherTokenContent = $tokenType === IgnoreLexer::TOKEN_OTHER ? sprintf(" '%s'", $content) : ''; $expectedLabels = implode(' or ', array_map(fn ($token) => $this->ignoreLexer->getLabel($token), $expected)); @@ -303,6 +305,9 @@ private function parseIdentifiers(string $text, int $ignorePos): array } if ($tokenType === IgnoreLexer::TOKEN_OPEN_PARENTHESIS) { + if ($openParenthesisCount > 0) { + $comment .= $content; + } $openParenthesisCount++; $expected = null; continue; @@ -311,17 +316,25 @@ private function parseIdentifiers(string $text, int $ignorePos): array if ($tokenType === IgnoreLexer::TOKEN_CLOSE_PARENTHESIS) { $openParenthesisCount--; if ($openParenthesisCount === 0) { + $key = array_key_last($identifiers); + if ($key !== null) { + $identifiers[$key]['comment'] = $comment; + $comment = null; + } $expected = [IgnoreLexer::TOKEN_COMMA, IgnoreLexer::TOKEN_END]; + } else { + $comment .= $content; } continue; } if ($openParenthesisCount > 0) { + $comment .= $content; continue; // waiting for comment end } if ($tokenType === IgnoreLexer::TOKEN_IDENTIFIER) { - $identifiers[] = $content; + $identifiers[] = ['name' => $content, 'comment' => null]; $expected = [IgnoreLexer::TOKEN_COMMA, IgnoreLexer::TOKEN_END, IgnoreLexer::TOKEN_OPEN_PARENTHESIS]; continue; } @@ -340,7 +353,7 @@ private function parseIdentifiers(string $text, int $ignorePos): array throw new IgnoreParseException('Missing identifier', 1); } - return $identifiers; + return array_values($identifiers); } } diff --git a/src/Testing/RuleTestCase.php b/src/Testing/RuleTestCase.php index 63c0ef5a5a..24f1f87f02 100644 --- a/src/Testing/RuleTestCase.php +++ b/src/Testing/RuleTestCase.php @@ -242,6 +242,7 @@ private function gatherAnalyserErrorsWithDelayedErrors(array $files): array new LocalIgnoresProcessor(), true, false, + false, ); return [ diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 009089e17a..603d11a931 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -573,6 +573,27 @@ public function testIgnoresWithoutIdentifiersReported(): void } } + public function testIgnoresWithoutCommentsReported(): void + { + $expects = [ + [9, 'variable.undefined'], + [12, 'variable.undefined'], + [12, 'wrong.id'], + [13, 'wrong.id'], + [14, 'variable.undefined'], + ]; + $result = $this->runAnalyser([], false, [ + __DIR__ . '/data/ignore-no-comments.php', + ], true, false, true); + $this->assertCount(5, $result); + foreach ($expects as $i => $expect) { + $this->assertArrayHasKey($i, $result); + $this->assertInstanceOf(Error::class, $result[$i]); + $this->assertStringContainsString(sprintf('Ignore with identifier %s has no comment.', $expect[1]), $result[$i]->getMessage()); + $this->assertSame($expect[0], $result[$i]->getLine()); + } + } + /** * @dataProvider dataTrueAndFalse */ @@ -660,6 +681,7 @@ private function runAnalyser( $filePaths, bool $onlyFiles, bool $reportIgnoresWithoutIdentifiers = false, + bool $reportIgnoresWithoutComments = false, ): array { $analyser = $this->createAnalyser(); @@ -693,6 +715,7 @@ private function runAnalyser( new LocalIgnoresProcessor(), $reportUnmatchedIgnoredErrors, $reportIgnoresWithoutIdentifiers, + $reportIgnoresWithoutComments, ); $analyserResult = $finalizer->finalize($analyserResult, $onlyFiles, false)->getAnalyserResult(); diff --git a/tests/PHPStan/Analyser/data/ignore-no-comments.php b/tests/PHPStan/Analyser/data/ignore-no-comments.php new file mode 100644 index 0000000000..b79581a17a --- /dev/null +++ b/tests/PHPStan/Analyser/data/ignore-no-comments.php @@ -0,0 +1,18 @@ + ['test'], + 2 => [['name' => 'test', 'comment' => null]], ], ]; @@ -57,7 +57,7 @@ public function dataLinesToIgnore(): iterable ' ['return.ref'], + 2 => [['name' => 'return.ref', 'comment' => null]], ], ]; @@ -65,7 +65,7 @@ public function dataLinesToIgnore(): iterable ' ['return.ref', 'return.non'], + 2 => [['name' => 'return.ref', 'comment' => null], ['name' => 'return.non', 'comment' => null]], ], ]; @@ -73,7 +73,7 @@ public function dataLinesToIgnore(): iterable ' ['return.ref', 'return.non'], + 2 => [['name' => 'return.ref', 'comment' => null], ['name' => 'return.non', 'comment' => 'foo']], ], ]; @@ -81,7 +81,7 @@ public function dataLinesToIgnore(): iterable ' ['return.ref', 'return.non'], + 2 => [['name' => 'return.ref', 'comment' => null], ['name' => 'return.non', 'comment' => 'foo, because...']], ], ]; @@ -89,7 +89,7 @@ public function dataLinesToIgnore(): iterable ' ['test'], + 2 => [['name' => 'test', 'comment' => null]], ], ]; @@ -97,7 +97,7 @@ public function dataLinesToIgnore(): iterable ' ['test'], + 2 => [['name' => 'test', 'comment' => null]], ], ]; @@ -105,7 +105,7 @@ public function dataLinesToIgnore(): iterable ' ['test'], + 2 => [['name' => 'test', 'comment' => null]], ], ]; @@ -113,7 +113,7 @@ public function dataLinesToIgnore(): iterable ' ['test'], + 2 => [['name' => 'test', 'comment' => null]], ], ]; @@ -122,7 +122,7 @@ public function dataLinesToIgnore(): iterable '// @phpstan-ignore test' . PHP_EOL . 'test();', [ - 3 => ['test'], + 3 => [['name' => 'test', 'comment' => null]], ], ]; @@ -131,7 +131,7 @@ public function dataLinesToIgnore(): iterable '// @phpstan-ignore test' . PHP_EOL . 'test(); // @phpstan-ignore test', [ - 3 => ['test', 'test'], + 3 => [['name' => 'test', 'comment' => null], ['name' => 'test', 'comment' => null]], ], ]; @@ -140,7 +140,7 @@ public function dataLinesToIgnore(): iterable ' // @phpstan-ignore test' . PHP_EOL . 'test();', [ - 3 => ['test'], + 3 => [['name' => 'test', 'comment' => null]], ], ]; @@ -152,7 +152,7 @@ public function dataLinesToIgnore(): iterable ' */' . PHP_EOL . 'test();', [ - 6 => ['test'], + 6 => [['name' => 'test', 'comment' => null]], ], ]; @@ -162,7 +162,7 @@ public function dataLinesToIgnore(): iterable '/** @phpstan-ignore test */' . PHP_EOL . 'test();', [ - 4 => ['test'], + 4 => [['name' => 'test', 'comment' => null]], ], ]; @@ -173,7 +173,7 @@ public function dataLinesToIgnore(): iterable ' * @phpstan-ignore test' . PHP_EOL . ' */ test();', [ - 5 => ['test'], + 5 => [['name' => 'test', 'comment' => null]], ], ]; @@ -184,7 +184,7 @@ public function dataLinesToIgnore(): iterable ' * @phpstan-ignore test' . PHP_EOL . ' */', [ - 3 => ['test'], + 3 => [['name' => 'test', 'comment' => null]], ], ]; @@ -193,7 +193,7 @@ public function dataLinesToIgnore(): iterable PHP_EOL . '/** @phpstan-ignore test */' . PHP_EOL, [ - 4 => ['test'], + 4 => [['name' => 'test', 'comment' => null]], ], ]; @@ -203,7 +203,7 @@ public function dataLinesToIgnore(): iterable '/** @phpstan-ignore test */' . PHP_EOL . 'doFoo();' . PHP_EOL, [ - 4 => ['test'], + 4 => [['name' => 'test', 'comment' => null]], ], ]; @@ -212,7 +212,7 @@ public function dataLinesToIgnore(): iterable PHP_EOL . '/** @phpstan-ignore test */', [ - 3 => ['test'], + 3 => [['name' => 'test', 'comment' => null]], ], ]; @@ -220,7 +220,7 @@ public function dataLinesToIgnore(): iterable ' ['identifier', 'identifier2'], + 2 => [['name' => 'identifier', 'comment' => 'comment'], ['name' => 'identifier2', 'comment' => 'comment2']], ], ]; @@ -228,7 +228,7 @@ public function dataLinesToIgnore(): iterable ' ['identifier', 'identifier2', 'identifier3'], + 2 => [['name' => 'identifier', 'comment' => 'comment1'], ['name' => 'identifier2', 'comment' => null], ['name' => 'identifier3', 'comment' => 'comment3']], ], ]; @@ -236,7 +236,7 @@ public function dataLinesToIgnore(): iterable ' ['identifier'], + 2 => [['name' => 'identifier', 'comment' => 'comment with inner (parenthesis)']], ], ]; @@ -244,7 +244,7 @@ public function dataLinesToIgnore(): iterable ' ['identifier'], + 2 => [['name' => 'identifier', 'comment' => '(((multi!)))']], ], ]; @@ -252,7 +252,7 @@ public function dataLinesToIgnore(): iterable ' ['identifier'], + 2 => [['name' => 'identifier', 'comment' => 'var_export() is used intentionally']], ], ]; @@ -260,7 +260,7 @@ public function dataLinesToIgnore(): iterable ' ['identifier'], + 2 => [['name' => 'identifier', 'comment' => 'FileSystem::write() does not support LOCK_EX']], ], ]; @@ -268,7 +268,7 @@ public function dataLinesToIgnore(): iterable ' ['identifier'], + 2 => [['name' => 'identifier', 'comment' => 'type ensured in self::createClient()']], ], ]; @@ -286,7 +286,7 @@ public function dataLinesToIgnore(): iterable ' }' . PHP_EOL . '}', [ - 10 => ['variable.undefined'], + 10 => [['name' => 'variable.undefined', 'comment' => null]], ], ]; @@ -307,7 +307,7 @@ public function dataLinesToIgnore(): iterable ' }' . PHP_EOL . '}', [ - 13 => ['variable.undefined'], + 13 => [['name' => 'variable.undefined', 'comment' => null]], ], ]; }