Skip to content

Commit 514ee6a

Browse files
calebdwstaabm
calebdw
authored and
staabm
committed
feat: add option to report ignores without comments
1 parent 8717c5d commit 514ee6a

File tree

12 files changed

+177
-58
lines changed

12 files changed

+177
-58
lines changed

‎conf/config.neon‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ parameters:
118118
resolvedPhpDocBlockCacheCountMax: 2048
119119
nameScopeMapMemoryCacheCountMax: 2048
120120
reportUnmatchedIgnoredErrors: true
121+
reportIgnoresWithoutComments: false
121122
typeAliases: []
122123
universalObjectCratesClasses:
123124
- stdClass
@@ -226,6 +227,7 @@ parameters:
226227
- [parameters, errorFormat]
227228
- [parameters, ignoreErrors]
228229
- [parameters, reportUnmatchedIgnoredErrors]
230+
- [parameters, reportIgnoresWithoutComments]
229231
- [parameters, tipsOfTheDay]
230232
- [parameters, parallel]
231233
- [parameters, internalErrorsCountLimit]

‎conf/parametersSchema.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ parametersSchema:
143143
nameScopeMapMemoryCacheCountMax: int()
144144
])
145145
reportUnmatchedIgnoredErrors: bool()
146+
reportIgnoresWithoutComments: bool()
146147
typeAliases: arrayOf(string())
147148
universalObjectCratesClasses: listOf(string())
148149
stubFiles: listOf(string())

‎src/Analyser/AnalyserResultFinalizer.php‎

Lines changed: 63 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,18 @@ public function __construct(
2828
private LocalIgnoresProcessor $localIgnoresProcessor,
2929
#[AutowiredParameter]
3030
private bool $reportUnmatchedIgnoredErrors,
31+
#[AutowiredParameter]
32+
private bool $reportIgnoresWithoutComments,
3133
)
3234
{
3335
}
3436

3537
public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $debug): FinalizerResult
3638
{
37-
if (count($analyserResult->getCollectedData()) === 0) {
38-
return $this->addUnmatchedIgnoredErrors($this->mergeFilteredPhpErrors($analyserResult), [], []);
39-
}
40-
39+
$hasCollectedData = count($analyserResult->getCollectedData()) > 0;
4140
$hasInternalErrors = count($analyserResult->getInternalErrors()) > 0 || $analyserResult->hasReachedInternalErrorsCountLimit();
42-
if ($hasInternalErrors) {
43-
return $this->addUnmatchedIgnoredErrors($this->mergeFilteredPhpErrors($analyserResult), [], []);
41+
if (! $hasCollectedData || $hasInternalErrors) {
42+
return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutCommentErrors($this->mergeFilteredPhpErrors($analyserResult)), [], []);
4443
}
4544

4645
$nodeType = CollectedDataNode::class;
@@ -134,7 +133,7 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $
134133
$allUnmatchedLineIgnores[$file] = $localIgnoresProcessorResult->getUnmatchedLineIgnores();
135134
}
136135

137-
return $this->addUnmatchedIgnoredErrors(new AnalyserResult(
136+
return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutCommentErrors(new AnalyserResult(
138137
unorderedErrors: array_merge($errors, $analyserResult->getFilteredPhpErrors()),
139138
filteredPhpErrors: [],
140139
allPhpErrors: $analyserResult->getAllPhpErrors(),
@@ -148,7 +147,7 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $
148147
exportedNodes: $analyserResult->getExportedNodes(),
149148
reachedInternalErrorsCountLimit: $analyserResult->hasReachedInternalErrorsCountLimit(),
150149
peakMemoryUsageBytes: $analyserResult->getPeakMemoryUsageBytes(),
151-
), $collectorErrors, $locallyIgnoredCollectorErrors);
150+
)), $collectorErrors, $locallyIgnoredCollectorErrors);
152151
}
153152

154153
private function mergeFilteredPhpErrors(AnalyserResult $analyserResult): AnalyserResult
@@ -205,7 +204,7 @@ private function addUnmatchedIgnoredErrors(
205204

206205
foreach ($identifiers as $identifier) {
207206
$errors[] = (new Error(
208-
sprintf('No error with identifier %s is reported on line %d.', $identifier, $line),
207+
sprintf('No error with identifier %s is reported on line %d.', $identifier['name'], $line),
209208
$file,
210209
$line,
211210
false,
@@ -237,4 +236,59 @@ private function addUnmatchedIgnoredErrors(
237236
);
238237
}
239238

239+
private function addIgnoresWithoutCommentErrors(AnalyserResult $analyserResult): AnalyserResult
240+
{
241+
if (!$this->reportIgnoresWithoutComments) {
242+
return $analyserResult;
243+
}
244+
245+
$errors = $analyserResult->getUnorderedErrors();
246+
foreach ($analyserResult->getLinesToIgnore() as $file => $data) {
247+
foreach ($data as $ignoredFile => $lines) {
248+
if ($ignoredFile !== $file) {
249+
continue;
250+
}
251+
252+
foreach ($lines as $line => $identifiers) {
253+
if ($identifiers === null) {
254+
continue;
255+
}
256+
257+
foreach ($identifiers as $identifier) {
258+
['name' => $name, 'comment' => $comment] = $identifier;
259+
if ($comment !== null) {
260+
continue;
261+
}
262+
263+
$errors[] = (new Error(
264+
sprintf('Ignore with identifier %s has no comment.', $name),
265+
$file,
266+
$line,
267+
false,
268+
$file,
269+
null,
270+
'Explain why this ignore is necessary in parentheses after the identifier.',
271+
))->withIdentifier('ignore.noComment');
272+
}
273+
}
274+
}
275+
}
276+
277+
return new AnalyserResult(
278+
$errors,
279+
$analyserResult->getFilteredPhpErrors(),
280+
$analyserResult->getAllPhpErrors(),
281+
$analyserResult->getLocallyIgnoredErrors(),
282+
$analyserResult->getLinesToIgnore(),
283+
$analyserResult->getUnmatchedLineIgnores(),
284+
$analyserResult->getInternalErrors(),
285+
$analyserResult->getCollectedData(),
286+
$analyserResult->getDependencies(),
287+
$analyserResult->getUsedTraitDependencies(),
288+
$analyserResult->getExportedNodes(),
289+
$analyserResult->hasReachedInternalErrorsCountLimit(),
290+
$analyserResult->getPeakMemoryUsageBytes(),
291+
);
292+
}
293+
240294
}

‎src/Analyser/FileAnalyserCallback.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
/**
2323
* @phpstan-import-type CollectorData from CollectedData
24+
* @phpstan-import-type Identifier from FileAnalyserResult
2425
* @phpstan-import-type LinesToIgnore from FileAnalyserResult
2526
*/
2627
final class FileAnalyserCallback
@@ -231,15 +232,15 @@ public function __invoke(Node $node, Scope $scope): void
231232

232233
/**
233234
* @param Node[] $nodes
234-
* @return array<int, non-empty-list<string>|null>
235+
* @return array<int, non-empty-list<Identifier>|null>
235236
*/
236237
private function getLinesToIgnoreFromTokens(array $nodes): array
237238
{
238239
if (!isset($nodes[0])) {
239240
return [];
240241
}
241242

242-
/** @var array<int, non-empty-list<string>|null> */
243+
/** @var array<int, non-empty-list<Identifier>|null> */
243244
return $nodes[0]->getAttribute('linesToIgnore', []);
244245
}
245246

‎src/Analyser/FileAnalyserResult.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
use PHPStan\Dependency\RootExportedNode;
77

88
/**
9-
* @phpstan-type LinesToIgnore = array<string, array<int, non-empty-list<string>|null>>
9+
* @phpstan-type Identifier = array{name: string, comment: string|null}
10+
* @phpstan-type LinesToIgnore = array<string, array<int, non-empty-list<Identifier>|null>>
1011
* @phpstan-import-type CollectorData from CollectedData
1112
*/
1213
final class FileAnalyserResult

‎src/Analyser/LocalIgnoresProcessor.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function process(
4949
}
5050

5151
foreach ($identifiers as $i => $ignoredIdentifier) {
52-
if ($ignoredIdentifier !== $tmpFileError->getIdentifier()) {
52+
if ($ignoredIdentifier['name'] !== $tmpFileError->getIdentifier()) {
5353
continue;
5454
}
5555

@@ -68,7 +68,7 @@ public function process(
6868
$unmatchedIgnoredIdentifiers = $unmatchedLineIgnores[$tmpFileError->getFile()][$line];
6969
if (is_array($unmatchedIgnoredIdentifiers)) {
7070
foreach ($unmatchedIgnoredIdentifiers as $j => $unmatchedIgnoredIdentifier) {
71-
if ($ignoredIdentifier !== $unmatchedIgnoredIdentifier) {
71+
if ($ignoredIdentifier['name'] !== $unmatchedIgnoredIdentifier['name']) {
7272
continue;
7373
}
7474

‎src/Command/FixerWorkerCommand.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ function (array $errors, array $locallyIgnoredErrors, array $analysedFiles) use
313313
if ($error->getIdentifier() === null) {
314314
continue;
315315
}
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)) {
317317
continue;
318318
}
319319
$ignoreFileErrors[] = $error;

‎src/Parser/RichParser.php‎

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@
77
use PhpParser\NodeTraverser;
88
use PhpParser\NodeVisitor\NameResolver;
99
use PhpParser\Token;
10+
use PHPStan\Analyser\FileAnalyserResult;
1011
use PHPStan\Analyser\Ignore\IgnoreLexer;
1112
use PHPStan\Analyser\Ignore\IgnoreParseException;
1213
use PHPStan\DependencyInjection\Container;
1314
use PHPStan\File\FileReader;
1415
use PHPStan\ShouldNotHappenException;
1516
use function array_filter;
17+
use function array_key_last;
1618
use function array_map;
1719
use function count;
1820
use function implode;
@@ -30,6 +32,9 @@
3032
use const T_DOC_COMMENT;
3133
use const T_WHITESPACE;
3234

35+
/**
36+
* @phpstan-import-type Identifier from FileAnalyserResult
37+
*/
3338
final class RichParser implements Parser
3439
{
3540

@@ -120,7 +125,7 @@ public function parseString(string $sourceCode): array
120125

121126
/**
122127
* @param Token[] $tokens
123-
* @return array{lines: array<int, non-empty-list<string>|null>, errors: array<int, non-empty-list<string>>}
128+
* @return array{lines: array<int, non-empty-list<Identifier>|null>, errors: array<int, non-empty-list<string>>}
124129
*/
125130
private function getLinesToIgnore(array $tokens): array
126131
{
@@ -277,33 +282,29 @@ private function getLinesToIgnoreForTokenByIgnoreComment(
277282
}
278283

279284
/**
280-
* @return non-empty-list<string>
285+
* @return non-empty-list<Identifier>
281286
* @throws IgnoreParseException
282287
*/
283288
private function parseIdentifiers(string $text, int $ignorePos): array
284289
{
285290
$text = substr($text, $ignorePos + strlen('@phpstan-ignore'));
286-
$originalTokens = $this->ignoreLexer->tokenize($text);
287-
$tokens = [];
288-
289-
foreach ($originalTokens as $originalToken) {
290-
if ($originalToken[IgnoreLexer::TYPE_OFFSET] === IgnoreLexer::TOKEN_WHITESPACE) {
291-
continue;
292-
}
293-
$tokens[] = $originalToken;
294-
}
291+
$tokens = $this->ignoreLexer->tokenize($text);
295292

296293
$c = count($tokens);
297294

298295
$identifiers = [];
296+
$comment = null;
299297
$openParenthesisCount = 0;
300298
$expected = [IgnoreLexer::TOKEN_IDENTIFIER];
299+
$lastTokenTypeLabel = '@phpstan-ignore';
301300

302301
for ($i = 0; $i < $c; $i++) {
303-
$lastTokenTypeLabel = isset($tokenType) ? $this->ignoreLexer->getLabel($tokenType) : '@phpstan-ignore';
302+
if (isset($tokenType) && $tokenType !== IgnoreLexer::TOKEN_WHITESPACE) {
303+
$lastTokenTypeLabel = $this->ignoreLexer->getLabel($tokenType);
304+
}
304305
[IgnoreLexer::VALUE_OFFSET => $content, IgnoreLexer::TYPE_OFFSET => $tokenType, IgnoreLexer::LINE_OFFSET => $tokenLine] = $tokens[$i];
305306

306-
if ($expected !== null && !in_array($tokenType, $expected, true)) {
307+
if ($expected !== null && !in_array($tokenType, [...$expected, IgnoreLexer::TOKEN_WHITESPACE], true)) {
307308
$tokenTypeLabel = $this->ignoreLexer->getLabel($tokenType);
308309
$otherTokenContent = $tokenType === IgnoreLexer::TOKEN_OTHER ? sprintf(" '%s'", $content) : '';
309310
$expectedLabels = implode(' or ', array_map(fn ($token) => $this->ignoreLexer->getLabel($token), $expected));
@@ -312,6 +313,9 @@ private function parseIdentifiers(string $text, int $ignorePos): array
312313
}
313314

314315
if ($tokenType === IgnoreLexer::TOKEN_OPEN_PARENTHESIS) {
316+
if ($openParenthesisCount > 0) {
317+
$comment .= $content;
318+
}
315319
$openParenthesisCount++;
316320
$expected = null;
317321
continue;
@@ -320,17 +324,25 @@ private function parseIdentifiers(string $text, int $ignorePos): array
320324
if ($tokenType === IgnoreLexer::TOKEN_CLOSE_PARENTHESIS) {
321325
$openParenthesisCount--;
322326
if ($openParenthesisCount === 0) {
327+
$key = array_key_last($identifiers);
328+
if ($key !== null) {
329+
$identifiers[$key]['comment'] = $comment;
330+
$comment = null;
331+
}
323332
$expected = [IgnoreLexer::TOKEN_COMMA, IgnoreLexer::TOKEN_END];
333+
} else {
334+
$comment .= $content;
324335
}
325336
continue;
326337
}
327338

328339
if ($openParenthesisCount > 0) {
340+
$comment .= $content;
329341
continue; // waiting for comment end
330342
}
331343

332344
if ($tokenType === IgnoreLexer::TOKEN_IDENTIFIER) {
333-
$identifiers[] = $content;
345+
$identifiers[] = ['name' => $content, 'comment' => null];
334346
$expected = [IgnoreLexer::TOKEN_COMMA, IgnoreLexer::TOKEN_END, IgnoreLexer::TOKEN_OPEN_PARENTHESIS];
335347
continue;
336348
}
@@ -349,6 +361,7 @@ private function parseIdentifiers(string $text, int $ignorePos): array
349361
throw new IgnoreParseException('Missing identifier', 1);
350362
}
351363

364+
/** @phpstan-ignore return.type (return type is correct, not sure why it's being changed from array shape to key-value shape) */
352365
return $identifiers;
353366
}
354367

‎src/Testing/RuleTestCase.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ private function gatherAnalyserErrorsWithDelayedErrors(array $files): array
327327
self::createScopeFactory($reflectionProvider, self::getContainer()->getService('typeSpecifier')),
328328
new LocalIgnoresProcessor(),
329329
true,
330+
false,
330331
);
331332

332333
return [

‎tests/PHPStan/Analyser/AnalyserTest.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,28 @@ public function testIgnoreNextLineUnmatched(): void
638638
}
639639
}
640640

641+
#[DataProvider('dataTrueAndFalse')]
642+
public function testIgnoresWithoutCommentsReported(): void
643+
{
644+
$expects = [
645+
[9, 'variable.undefined'],
646+
[12, 'variable.undefined'],
647+
[12, 'wrong.id'],
648+
[13, 'wrong.id'],
649+
[14, 'variable.undefined'],
650+
];
651+
$result = $this->runAnalyser([], false, [
652+
__DIR__ . '/data/ignore-no-comments.php',
653+
], true, true);
654+
$this->assertCount(5, $result);
655+
foreach ($expects as $i => $expect) {
656+
$this->assertArrayHasKey($i, $result);
657+
$this->assertInstanceOf(Error::class, $result[$i]);
658+
$this->assertStringContainsString(sprintf('Ignore with identifier %s has no comment.', $expect[1]), $result[$i]->getMessage());
659+
$this->assertSame($expect[0], $result[$i]->getLine());
660+
}
661+
}
662+
641663
#[DataProvider('dataTrueAndFalse')]
642664
public function testIgnoreLine(bool $reportUnmatchedIgnoredErrors): void
643665
{
@@ -747,6 +769,7 @@ private function runAnalyser(
747769
bool $reportUnmatchedIgnoredErrors,
748770
$filePaths,
749771
bool $onlyFiles,
772+
bool $reportIgnoresWithoutComments = false,
750773
): array
751774
{
752775
$analyser = $this->createAnalyser();
@@ -779,6 +802,7 @@ private function runAnalyser(
779802
),
780803
new LocalIgnoresProcessor(),
781804
$reportUnmatchedIgnoredErrors,
805+
$reportIgnoresWithoutComments,
782806
);
783807
$analyserResult = $finalizer->finalize($analyserResult, $onlyFiles, false)->getAnalyserResult();
784808

0 commit comments

Comments
 (0)