feat: add option to report ignores without comments

calebdw · calebdw · commit a29ed6af00a1 · 2026-02-23T09:16:32.000-06:00
diff --git a/conf/config.neon b/conf/config.neon
@@ -118,6 +118,7 @@ parameters:
 		resolvedPhpDocBlockCacheCountMax: 2048
 		nameScopeMapMemoryCacheCountMax: 2048
 	reportUnmatchedIgnoredErrors: true
+	reportIgnoresWithoutComments: false
 	typeAliases: []
 	universalObjectCratesClasses:
 		- stdClass
@@ -226,6 +227,7 @@ parameters:
 		- [parameters, errorFormat]
 		- [parameters, ignoreErrors]
 		- [parameters, reportUnmatchedIgnoredErrors]
+		- [parameters, reportIgnoresWithoutComments]
 		- [parameters, tipsOfTheDay]
 		- [parameters, parallel]
 		- [parameters, internalErrorsCountLimit]
diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon
@@ -143,6 +143,7 @@ parametersSchema:
 		nameScopeMapMemoryCacheCountMax: int()
 	])
 	reportUnmatchedIgnoredErrors: bool()
+	reportIgnoresWithoutComments: bool()
 	typeAliases: arrayOf(string())
 	universalObjectCratesClasses: listOf(string())
 	stubFiles: listOf(string())
diff --git a/src/Analyser/AnalyserResultFinalizer.php b/src/Analyser/AnalyserResultFinalizer.php
@@ -205,7 +205,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,
diff --git a/src/Analyser/FileAnalyser.php b/src/Analyser/FileAnalyser.php
@@ -60,6 +60,8 @@ public function __construct(
 		private IgnoreErrorExtensionProvider $ignoreErrorExtensionProvider,
 		private RuleErrorTransformer $ruleErrorTransformer,
 		private LocalIgnoresProcessor $localIgnoresProcessor,
+		#[AutowiredParameter]
+		private bool $reportIgnoresWithoutComments,
 	)
 	{
 	}
@@ -129,6 +131,32 @@ public function analyseFile(
 				$temporaryFileErrors = $nodeCallback->getTemporaryFileErrors();
 				$processedFiles = $nodeCallback->getProcessedFiles();
 
+				if ($this->reportIgnoresWithoutComments) {
+					foreach ($linesToIgnore as $ignoredFile => $lines) {
+						foreach ($lines as $line => $identifiers) {
+							if ($identifiers === null) {
+								continue;
+							}
+
+							foreach ($identifiers as $identifier) {
+								if ($identifier['comment'] !== null) {
+									continue;
+								}
+
+								$fileErrors[] = (new Error(
+									sprintf('Ignore with identifier %s has no comment.', $identifier['name']),
+									$ignoredFile,
+									$line,
+									false,
+									$ignoredFile,
+									null,
+									'Explain why this ignore is necessary in parentheses after the identifier.',
+								))->withIdentifier('ignore.noComment');
+							}
+						}
+					}
+				}
+
 				$localIgnoresProcessorResult = $this->localIgnoresProcessor->process(
 					$temporaryFileErrors,
 					$linesToIgnore,
diff --git a/src/Analyser/FileAnalyserCallback.php b/src/Analyser/FileAnalyserCallback.php
@@ -21,6 +21,7 @@
 
 /**
  * @phpstan-import-type CollectorData from CollectedData
+ * @phpstan-import-type Identifier from FileAnalyserResult
  * @phpstan-import-type LinesToIgnore from FileAnalyserResult
  */
 final class FileAnalyserCallback
@@ -231,15 +232,15 @@ public function __invoke(Node $node, Scope $scope): void
 
 	/**
 	 * @param Node[] $nodes
-	 * @return array<int, non-empty-list<string>|null>
+	 * @return array<int, non-empty-list<Identifier>|null>
 	 */
 	private function getLinesToIgnoreFromTokens(array $nodes): array
 	{
 		if (!isset($nodes[0])) {
 			return [];
 		}
 
-		/** @var array<int, non-empty-list<string>|null> */
+		/** @var array<int, non-empty-list<Identifier>|null> */
 		return $nodes[0]->getAttribute('linesToIgnore', []);
 	}
 
diff --git a/src/Analyser/FileAnalyserResult.php b/src/Analyser/FileAnalyserResult.php
@@ -6,7 +6,8 @@
 use PHPStan\Dependency\RootExportedNode;
 
 /**
- * @phpstan-type LinesToIgnore = array<string, array<int, non-empty-list<string>|null>>
+ * @phpstan-type Identifier = array{name: string, comment: string|null}
+ * @phpstan-type LinesToIgnore = array<string, array<int, non-empty-list<Identifier>|null>>
  * @phpstan-import-type CollectorData from CollectedData
  */
 final class FileAnalyserResult
diff --git a/src/Analyser/LocalIgnoresProcessor.php b/src/Analyser/LocalIgnoresProcessor.php
@@ -49,7 +49,7 @@ public function process(
 				}
 
 				foreach ($identifiers as $i => $ignoredIdentifier) {
-					if ($ignoredIdentifier !== $tmpFileError->getIdentifier()) {
+					if ($ignoredIdentifier['name'] !== $tmpFileError->getIdentifier()) {
 						continue;
 					}
 
@@ -68,7 +68,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
@@ -313,7 +313,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.noComment'], true)) {
 						continue;
 					}
 					$ignoreFileErrors[] = $error;
diff --git a/src/Parser/RichParser.php b/src/Parser/RichParser.php
@@ -7,12 +7,14 @@
 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 count;
 use function implode;
@@ -30,6 +32,9 @@
 use const T_DOC_COMMENT;
 use const T_WHITESPACE;
 
+/**
+ * @phpstan-import-type Identifier from FileAnalyserResult
+ */
 final class RichParser implements Parser
 {
 
@@ -120,7 +125,7 @@ public function parseString(string $sourceCode): array
 
 	/**
 	 * @param Token[] $tokens
-	 * @return array{lines: array<int, non-empty-list<string>|null>, errors: array<int, non-empty-list<string>>}
+	 * @return array{lines: array<int, non-empty-list<Identifier>|null>, errors: array<int, non-empty-list<string>>}
 	 */
 	private function getLinesToIgnore(array $tokens): array
 	{
@@ -277,33 +282,29 @@ private function getLinesToIgnoreForTokenByIgnoreComment(
 	}
 
 	/**
-	 * @return non-empty-list<string>
+	 * @return non-empty-list<Identifier>
 	 * @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));
@@ -312,6 +313,9 @@ private function parseIdentifiers(string $text, int $ignorePos): array
 			}
 
 			if ($tokenType === IgnoreLexer::TOKEN_OPEN_PARENTHESIS) {
+				if ($openParenthesisCount > 0) {
+					$comment .= $content;
+				}
 				$openParenthesisCount++;
 				$expected = null;
 				continue;
@@ -320,17 +324,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;
 			}
@@ -349,6 +361,7 @@ private function parseIdentifiers(string $text, int $ignorePos): array
 			throw new IgnoreParseException('Missing identifier', 1);
 		}
 
+		/** @phpstan-ignore return.type (return type is correct, not sure why it's being changed from array shape to key-value shape) */
 		return $identifiers;
 	}
 
diff --git a/src/Testing/RuleTestCase.php b/src/Testing/RuleTestCase.php
@@ -140,6 +140,7 @@ private function getAnalyser(DirectRuleRegistry $ruleRegistry): Analyser
 				new IgnoreErrorExtensionProvider(self::getContainer()),
 				self::getContainer()->getByType(RuleErrorTransformer::class),
 				new LocalIgnoresProcessor(),
+				false,
 			);
 			$this->analyser = new Analyser(
 				$fileAnalyser,
diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php
@@ -850,6 +850,7 @@ private function createAnalyser(): Analyser
 			new IgnoreErrorExtensionProvider(new NetteContainer(new Container([]))),
 			$container->getByType(RuleErrorTransformer::class),
 			new LocalIgnoresProcessor(),
+			false,
 		);
 
 		return new Analyser(
diff --git a/tests/PHPStan/Analyser/ReportIgnoresWithoutCommentsIntegrationTest.php b/tests/PHPStan/Analyser/ReportIgnoresWithoutCommentsIntegrationTest.php
@@ -0,0 +1,81 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Analyser;
+
+use PHPStan\Testing\PHPStanTestCase;
+use PHPUnit\Framework\Attributes\CoversNothing;
+use function array_unique;
+use function count;
+use function sprintf;
+
+#[CoversNothing]
+class ReportIgnoresWithoutCommentsIntegrationTest extends PHPStanTestCase
+{
+
+	public function testReportIgnoresWithoutComments(): void
+	{
+		[$errors, $allErrors] = $this->runAnalyse(__DIR__ . '/data/ignore-no-comments.php');
+
+		$expectedErrors = [
+			[9, 'variable.undefined'],
+			[12, 'variable.undefined'],
+			[12, 'wrong.id'],
+			[13, 'wrong.id'],
+			[14, 'variable.undefined'],
+		];
+
+		$debugInfo = "All errors:\n";
+		foreach ($allErrors as $error) {
+			$debugInfo .= sprintf("  Line %d: %s [%s]\n", $error->getLine(), $error->getMessage(), $error->getIdentifier() ?? 'null');
+		}
+		$debugInfo .= "Filtered errors:\n";
+		foreach ($errors as $error) {
+			$debugInfo .= sprintf("  Line %d: %s [%s]\n", $error->getLine(), $error->getMessage(), $error->getIdentifier() ?? 'null');
+		}
+
+		$this->assertCount(count($expectedErrors), $errors, 'Expected ' . count($expectedErrors) . " errors\n" . $debugInfo);
+		foreach ($expectedErrors as $i => $expected) {
+			$this->assertArrayHasKey($i, $errors);
+			$this->assertInstanceOf(Error::class, $errors[$i]);
+			$this->assertStringContainsString(
+				sprintf('Ignore with identifier %s has no comment.', $expected[1]),
+				$errors[$i]->getMessage(),
+			);
+			$this->assertSame($expected[0], $errors[$i]->getLine());
+		}
+	}
+
+	/**
+	 * @return array{list<Error>, list<Error>}
+	 */
+	private function runAnalyse(string $file): array
+	{
+		$file = $this->getFileHelper()->normalizePath($file);
+
+		$analyser = self::getContainer()->getByType(Analyser::class);
+		$finalizer = self::getContainer()->getByType(AnalyserResultFinalizer::class);
+		$allErrors = $finalizer->finalize(
+			$analyser->analyse([$file], null, null, true),
+			false,
+			true,
+		)->getAnalyserResult()->getErrors();
+
+		$relevantErrors = [];
+		foreach ($allErrors as $error) {
+			if ($error->getIdentifier() === 'ignore.noComment') {
+				$relevantErrors[] = $error;
+			}
+		}
+
+		return [$relevantErrors, $allErrors];
+	}
+
+	public static function getAdditionalConfigFiles(): array
+	{
+		return array_unique([
+			...parent::getAdditionalConfigFiles(),
+			__DIR__ . '/report-ignores-without-comments.neon',
+		]);
+	}
+
+}
diff --git a/tests/PHPStan/Analyser/data/ignore-no-comments.php b/tests/PHPStan/Analyser/data/ignore-no-comments.php
@@ -0,0 +1,18 @@
+<?php
+
+namespace IgnoreNoComments;
+
+class Foo
+{
+	public function doFoo(): void
+	{
+		echo $foo; // @phpstan-ignore variable.undefined
+		echo $foo; // @phpstan-ignore wrong.id (comment)
+
+		echo $foo, $bar;  // @phpstan-ignore variable.undefined, wrong.id
+		echo $foo, $bar;  // @phpstan-ignore variable.undefined (comment), wrong.id
+		echo $foo, $bar;  // @phpstan-ignore variable.undefined, wrong.id (comment)
+		echo $foo, $bar;  // @phpstan-ignore variable.undefined (comment), wrong.id (comment)
+	}
+
+}
diff --git a/tests/PHPStan/Analyser/report-ignores-without-comments.neon b/tests/PHPStan/Analyser/report-ignores-without-comments.neon
@@ -0,0 +1,2 @@
+parameters:
+	reportIgnoresWithoutComments: true
diff --git a/tests/PHPStan/Parser/RichParserTest.php b/tests/PHPStan/Parser/RichParserTest.php

Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ public function process(`
`49`	`49`	`}`
`50`	`50`
`51`	`51`	`foreach ($identifiers as $i => $ignoredIdentifier) {`
`52`		`- if ($ignoredIdentifier !== $tmpFileError->getIdentifier()) {`
	`52`	`+ if ($ignoredIdentifier['name'] !== $tmpFileError->getIdentifier()) {`
`53`	`53`	`continue;`
`54`	`54`	`}`
`55`	`55`
`@@ -68,7 +68,7 @@ public function process(`
`68`	`68`	`$unmatchedIgnoredIdentifiers = $unmatchedLineIgnores[$tmpFileError->getFile()][$line];`
`69`	`69`	`if (is_array($unmatchedIgnoredIdentifiers)) {`
`70`	`70`	`foreach ($unmatchedIgnoredIdentifiers as $j => $unmatchedIgnoredIdentifier) {`
`71`		`- if ($ignoredIdentifier !== $unmatchedIgnoredIdentifier) {`
	`71`	`+ if ($ignoredIdentifier['name'] !== $unmatchedIgnoredIdentifier['name']) {`
`72`	`72`	`continue;`
`73`	`73`	`}`
`74`	`74`
Original file line number	Diff line number	Diff line change
`@@ -313,7 +313,7 @@ function (array $errors, array $locallyIgnoredErrors, array $analysedFiles) use`
`313`	`313`	`if ($error->getIdentifier() === null) {`
`314`	`314`	`continue;`
`315`	`315`	`}`
`316`		`- if (!in_array($error->getIdentifier(), ['ignore.count', 'ignore.unmatched', 'ignore.unmatchedLine', 'ignore.unmatchedIdentifier'], true)) {`
	`316`	`+ if (!in_array($error->getIdentifier(), ['ignore.count', 'ignore.unmatched', 'ignore.unmatchedLine', 'ignore.unmatchedIdentifier', 'ignore.noComment'], true)) {`
`317`	`317`	`continue;`
`318`	`318`	`}`
`319`	`319`	`$ignoreFileErrors[] = $error;`