Revert "feat: add option to report ignores without comments"

This reverts commit c5db948.

Author: ondrejmirtes

conf/config.neon

@@ -118,7 +118,6 @@ parameters:
 		resolvedPhpDocBlockCacheCountMax: 2048
 		nameScopeMapMemoryCacheCountMax: 2048
 	reportUnmatchedIgnoredErrors: true
-	reportIgnoresWithoutComments: false
 	typeAliases: []
 	universalObjectCratesClasses:
 		- stdClass
@@ -227,7 +226,6 @@ parameters:
 		- [parameters, errorFormat]
 		- [parameters, ignoreErrors]
 		- [parameters, reportUnmatchedIgnoredErrors]
-		- [parameters, reportIgnoresWithoutComments]
 		- [parameters, tipsOfTheDay]
 		- [parameters, parallel]
 		- [parameters, internalErrorsCountLimit]

conf/parametersSchema.neon

@@ -143,7 +143,6 @@ parametersSchema:
 		nameScopeMapMemoryCacheCountMax: int()
 	])
 	reportUnmatchedIgnoredErrors: bool()
-	reportIgnoresWithoutComments: bool()
 	typeAliases: arrayOf(string())
 	universalObjectCratesClasses: listOf(string())
 	stubFiles: listOf(string())

src/Analyser/AnalyserResultFinalizer.php

@@ -28,18 +28,19 @@ public function __construct(
 		private LocalIgnoresProcessor $localIgnoresProcessor,
 		#[AutowiredParameter]
 		private bool $reportUnmatchedIgnoredErrors,
-		#[AutowiredParameter]
-		private bool $reportIgnoresWithoutComments,
 	)
 	{
 	}
 
 	public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $debug): FinalizerResult
 	{
-		$hasCollectedData = count($analyserResult->getCollectedData()) > 0;
+		if (count($analyserResult->getCollectedData()) === 0) {
+			return $this->addUnmatchedIgnoredErrors($this->mergeFilteredPhpErrors($analyserResult), [], []);
+		}
+
 		$hasInternalErrors = count($analyserResult->getInternalErrors()) > 0 || $analyserResult->hasReachedInternalErrorsCountLimit();
-		if (! $hasCollectedData || $hasInternalErrors) {
-			return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutCommentErrors($this->mergeFilteredPhpErrors($analyserResult)), [], []);
+		if ($hasInternalErrors) {
+			return $this->addUnmatchedIgnoredErrors($this->mergeFilteredPhpErrors($analyserResult), [], []);
 		}
 
 		$nodeType = CollectedDataNode::class;
@@ -133,7 +134,7 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $
 			$allUnmatchedLineIgnores[$file] = $localIgnoresProcessorResult->getUnmatchedLineIgnores();
 		}
 
-		return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutCommentErrors(new AnalyserResult(
+		return $this->addUnmatchedIgnoredErrors(new AnalyserResult(
 			unorderedErrors: array_merge($errors, $analyserResult->getFilteredPhpErrors()),
 			filteredPhpErrors: [],
 			allPhpErrors: $analyserResult->getAllPhpErrors(),
@@ -147,7 +148,7 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $
 			exportedNodes: $analyserResult->getExportedNodes(),
 			reachedInternalErrorsCountLimit: $analyserResult->hasReachedInternalErrorsCountLimit(),
 			peakMemoryUsageBytes: $analyserResult->getPeakMemoryUsageBytes(),
-		)), $collectorErrors, $locallyIgnoredCollectorErrors);
+		), $collectorErrors, $locallyIgnoredCollectorErrors);
 	}
 
 	private function mergeFilteredPhpErrors(AnalyserResult $analyserResult): AnalyserResult
@@ -204,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['name'], $line),
+							sprintf('No error with identifier %s is reported on line %d.', $identifier, $line),
 							$file,
 							$line,
 							false,
@@ -236,59 +237,4 @@ private function addUnmatchedIgnoredErrors(
 		);
 	}
 
-	private function addIgnoresWithoutCommentErrors(AnalyserResult $analyserResult): AnalyserResult
-	{
-		if (!$this->reportIgnoresWithoutComments) {
-			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;
-					}
-
-					foreach ($identifiers as $identifier) {
-						['name' => $name, 'comment' => $comment] = $identifier;
-						if ($comment !== null) {
-							continue;
-						}
-
-						$errors[] = (new Error(
-							sprintf('Ignore with identifier %s has no comment.', $name),
-							$file,
-							$line,
-							false,
-							$file,
-							null,
-							'Explain why this ignore is necessary in parentheses after the identifier.',
-						))->withIdentifier('ignore.noComment');
-					}
-				}
-			}
-		}
-
-		return new AnalyserResult(
-			$errors,
-			$analyserResult->getFilteredPhpErrors(),
-			$analyserResult->getAllPhpErrors(),
-			$analyserResult->getLocallyIgnoredErrors(),
-			$analyserResult->getLinesToIgnore(),
-			$analyserResult->getUnmatchedLineIgnores(),
-			$analyserResult->getInternalErrors(),
-			$analyserResult->getCollectedData(),
-			$analyserResult->getDependencies(),
-			$analyserResult->getUsedTraitDependencies(),
-			$analyserResult->getExportedNodes(),
-			$analyserResult->hasReachedInternalErrorsCountLimit(),
-			$analyserResult->getPeakMemoryUsageBytes(),
-		);
-	}
-
 }

src/Analyser/FileAnalyserCallback.php

@@ -21,7 +21,6 @@
 
 /**
  * @phpstan-import-type CollectorData from CollectedData
- * @phpstan-import-type Identifier from FileAnalyserResult
  * @phpstan-import-type LinesToIgnore from FileAnalyserResult
  */
 final class FileAnalyserCallback
@@ -232,15 +231,15 @@ public function __invoke(Node $node, Scope $scope): void
 
 	/**
 	 * @param Node[] $nodes
-	 * @return array<int, non-empty-list<Identifier>|null>
+	 * @return array<int, non-empty-list<string>|null>
 	 */
 	private function getLinesToIgnoreFromTokens(array $nodes): array
 	{
 		if (!isset($nodes[0])) {
 			return [];
 		}
 
-		/** @var array<int, non-empty-list<Identifier>|null> */
+		/** @var array<int, non-empty-list<string>|null> */
 		return $nodes[0]->getAttribute('linesToIgnore', []);
 	}
 

src/Analyser/FileAnalyserResult.php

@@ -6,8 +6,7 @@
 use PHPStan\Dependency\RootExportedNode;
 
 /**
- * @phpstan-type Identifier = array{name: string, comment: string|null}
- * @phpstan-type LinesToIgnore = array<string, array<int, non-empty-list<Identifier>|null>>
+ * @phpstan-type LinesToIgnore = array<string, array<int, non-empty-list<string>|null>>
  * @phpstan-import-type CollectorData from CollectedData
  */
 final class FileAnalyserResult

src/Analyser/LocalIgnoresProcessor.php

@@ -49,7 +49,7 @@ public function process(
 				}
 
 				foreach ($identifiers as $i => $ignoredIdentifier) {
-					if ($ignoredIdentifier['name'] !== $tmpFileError->getIdentifier()) {
+					if ($ignoredIdentifier !== $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['name'] !== $unmatchedIgnoredIdentifier['name']) {
+								if ($ignoredIdentifier !== $unmatchedIgnoredIdentifier) {
 									continue;
 								}
 

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', 'ignore.noComment'], true)) {
+					if (!in_array($error->getIdentifier(), ['ignore.count', 'ignore.unmatched', 'ignore.unmatchedLine', 'ignore.unmatchedIdentifier'], true)) {
 						continue;
 					}
 					$ignoreFileErrors[] = $error;

src/Parser/RichParser.php

@@ -7,14 +7,12 @@
 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;
@@ -32,9 +30,6 @@
 use const T_DOC_COMMENT;
 use const T_WHITESPACE;
 
-/**
- * @phpstan-import-type Identifier from FileAnalyserResult
- */
 final class RichParser implements Parser
 {
 
@@ -125,7 +120,7 @@ public function parseString(string $sourceCode): array
 
 	/**
 	 * @param Token[] $tokens
-	 * @return array{lines: array<int, non-empty-list<Identifier>|null>, errors: array<int, non-empty-list<string>>}
+	 * @return array{lines: array<int, non-empty-list<string>|null>, errors: array<int, non-empty-list<string>>}
 	 */
 	private function getLinesToIgnore(array $tokens): array
 	{
@@ -282,29 +277,33 @@ private function getLinesToIgnoreForTokenByIgnoreComment(
 	}
 
 	/**
-	 * @return non-empty-list<Identifier>
+	 * @return non-empty-list<string>
 	 * @throws IgnoreParseException
 	 */
 	private function parseIdentifiers(string $text, int $ignorePos): array
 	{
 		$text = substr($text, $ignorePos + strlen('@phpstan-ignore'));
-		$tokens = $this->ignoreLexer->tokenize($text);
+		$originalTokens = $this->ignoreLexer->tokenize($text);
+		$tokens = [];
+
+		foreach ($originalTokens as $originalToken) {
+			if ($originalToken[IgnoreLexer::TYPE_OFFSET] === IgnoreLexer::TOKEN_WHITESPACE) {
+				continue;
+			}
+			$tokens[] = $originalToken;
+		}
 
 		$c = count($tokens);
 
 		$identifiers = [];
-		$comment = null;
 		$openParenthesisCount = 0;
 		$expected = [IgnoreLexer::TOKEN_IDENTIFIER];
-		$lastTokenTypeLabel = '@phpstan-ignore';
 
 		for ($i = 0; $i < $c; $i++) {
-			if (isset($tokenType) && $tokenType !== IgnoreLexer::TOKEN_WHITESPACE) {
-				$lastTokenTypeLabel = $this->ignoreLexer->getLabel($tokenType);
-			}
+			$lastTokenTypeLabel = isset($tokenType) ? $this->ignoreLexer->getLabel($tokenType) : '@phpstan-ignore';
 			[IgnoreLexer::VALUE_OFFSET => $content, IgnoreLexer::TYPE_OFFSET => $tokenType, IgnoreLexer::LINE_OFFSET => $tokenLine] = $tokens[$i];
 
-			if ($expected !== null && !in_array($tokenType, [...$expected, IgnoreLexer::TOKEN_WHITESPACE], true)) {
+			if ($expected !== null && !in_array($tokenType, $expected, 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));
@@ -313,9 +312,6 @@ private function parseIdentifiers(string $text, int $ignorePos): array
 			}
 
 			if ($tokenType === IgnoreLexer::TOKEN_OPEN_PARENTHESIS) {
-				if ($openParenthesisCount > 0) {
-					$comment .= $content;
-				}
 				$openParenthesisCount++;
 				$expected = null;
 				continue;
@@ -324,25 +320,17 @@ 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[] = ['name' => $content, 'comment' => null];
+				$identifiers[] = $content;
 				$expected = [IgnoreLexer::TOKEN_COMMA, IgnoreLexer::TOKEN_END, IgnoreLexer::TOKEN_OPEN_PARENTHESIS];
 				continue;
 			}
@@ -361,7 +349,6 @@ 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;
 	}
 

src/Testing/RuleTestCase.php

@@ -327,7 +327,6 @@ private function gatherAnalyserErrorsWithDelayedErrors(array $files): array
 			self::createScopeFactory($reflectionProvider, self::getContainer()->getService('typeSpecifier')),
 			new LocalIgnoresProcessor(),
 			true,
-			false,
 		);
 
 		return [

tests/PHPStan/Analyser/AnalyserTest.php

@@ -638,28 +638,6 @@ public function testIgnoreNextLineUnmatched(): void
 		}
 	}
 
-	#[DataProvider('dataTrueAndFalse')]
-	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, 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')]
 	public function testIgnoreLine(bool $reportUnmatchedIgnoredErrors): void
 	{
@@ -769,7 +747,6 @@ private function runAnalyser(
 		bool $reportUnmatchedIgnoredErrors,
 		$filePaths,
 		bool $onlyFiles,
-		bool $reportIgnoresWithoutComments = false,
 	): array
 	{
 		$analyser = $this->createAnalyser();
@@ -802,7 +779,6 @@ private function runAnalyser(
 			),
 			new LocalIgnoresProcessor(),
 			$reportUnmatchedIgnoredErrors,
-			$reportIgnoresWithoutComments,
 		);
 		$analyserResult = $finalizer->finalize($analyserResult, $onlyFiles, false)->getAnalyserResult();