Skip to content

Commit 8e4e5f8

Browse files
calebdwcalebdw
committed
feat: add option to report ignores without comments
1 parent 6a9f0bb commit 8e4e5f8

12 files changed

+143
-67
lines changed

‎conf/config.neon

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ parameters:
9797
nodesByStringCountMax: 256
9898
reportUnmatchedIgnoredErrors: true
9999
reportIgnoresWithoutIdentifiers: false
100+
reportIgnoresWithoutComments: false
100101
typeAliases: []
101102
universalObjectCratesClasses:
102103
- stdClass
@@ -201,6 +202,7 @@ parameters:
201202
- [parameters, ignoreErrors]
202203
- [parameters, reportUnmatchedIgnoredErrors]
203204
- [parameters, reportIgnoresWithoutIdentifiers]
205+
- [parameters, reportIgnoresWithoutComments]
204206
- [parameters, tipsOfTheDay]
205207
- [parameters, parallel]
206208
- [parameters, internalErrorsCountLimit]
@@ -466,6 +468,7 @@ services:
466468
arguments:
467469
reportUnmatchedIgnoredErrors: %reportUnmatchedIgnoredErrors%
468470
reportIgnoresWithoutIdentifiers: %reportIgnoresWithoutIdentifiers%
471+
reportIgnoresWithoutComments: %reportIgnoresWithoutComments%
469472

470473
-
471474
class: PHPStan\Analyser\FileAnalyser

‎conf/parametersSchema.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ parametersSchema:
145145
])
146146
reportUnmatchedIgnoredErrors: bool()
147147
reportIgnoresWithoutIdentifiers: bool()
148+
reportIgnoresWithoutComments: bool()
148149
typeAliases: arrayOf(string())
149150
universalObjectCratesClasses: listOf(string())
150151
stubFiles: listOf(string())

‎src/Analyser/AnalyserResultFinalizer.php

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,17 @@ public function __construct(
2525
private LocalIgnoresProcessor $localIgnoresProcessor,
2626
private bool $reportUnmatchedIgnoredErrors,
2727
private bool $reportIgnoresWithoutIdentifiers,
28+
private bool $reportIgnoresWithoutComments,
2829
)
2930
{
3031
}
3132

3233
public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $debug): FinalizerResult
3334
{
34-
if (count($analyserResult->getCollectedData()) === 0) {
35-
return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutIdentifiersErrors($this->mergeFilteredPhpErrors($analyserResult)), [], []);
36-
}
37-
35+
$hasCollectedData = count($analyserResult->getCollectedData()) > 0;
3836
$hasInternalErrors = count($analyserResult->getInternalErrors()) > 0 || $analyserResult->hasReachedInternalErrorsCountLimit();
39-
if ($hasInternalErrors) {
40-
return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutIdentifiersErrors($this->mergeFilteredPhpErrors($analyserResult)), [], []);
37+
if (! $hasCollectedData || $hasInternalErrors) {
38+
return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutCommentsOrIdentifiersErrors($this->mergeFilteredPhpErrors($analyserResult)), [], []);
4139
}
4240

4341
$nodeType = CollectedDataNode::class;
@@ -131,7 +129,7 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $
131129
$allUnmatchedLineIgnores[$file] = $localIgnoresProcessorResult->getUnmatchedLineIgnores();
132130
}
133131

134-
return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutIdentifiersErrors(new AnalyserResult(
132+
return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutCommentsOrIdentifiersErrors(new AnalyserResult(
135133
array_merge($errors, $analyserResult->getFilteredPhpErrors()),
136134
[],
137135
$analyserResult->getAllPhpErrors(),
@@ -202,7 +200,7 @@ private function addUnmatchedIgnoredErrors(
202200

203201
foreach ($identifiers as $identifier) {
204202
$errors[] = (new Error(
205-
sprintf('No error with identifier %s is reported on line %d.', $identifier, $line),
203+
sprintf('No error with identifier %s is reported on line %d.', $identifier['name'], $line),
206204
$file,
207205
$line,
208206
false,
@@ -234,9 +232,9 @@ private function addUnmatchedIgnoredErrors(
234232
);
235233
}
236234

237-
private function addIgnoresWithoutIdentifiersErrors(AnalyserResult $analyserResult): AnalyserResult
235+
private function addIgnoresWithoutCommentsOrIdentifiersErrors(AnalyserResult $analyserResult): AnalyserResult
238236
{
239-
if (!$this->reportIgnoresWithoutIdentifiers) {
237+
if (!$this->reportIgnoresWithoutComments && !$this->reportIgnoresWithoutIdentifiers) {
240238
return $analyserResult;
241239
}
242240

@@ -248,17 +246,34 @@ private function addIgnoresWithoutIdentifiersErrors(AnalyserResult $analyserResu
248246
}
249247

250248
foreach ($lines as $line => $identifiers) {
251-
if ($identifiers !== null) {
249+
if ($identifiers === null) {
250+
if (!$this->reportIgnoresWithoutIdentifiers) {
251+
continue;
252+
}
253+
$errors[] = (new Error(
254+
sprintf('Error is ignored with no identifiers on line %d.', $line),
255+
$file,
256+
$line,
257+
false,
258+
$file,
259+
))->withIdentifier('ignore.noIdentifier');
252260
continue;
253261
}
254262

255-
$errors[] = (new Error(
256-
sprintf('Error is ignored with no identifiers on line %d.', $line),
257-
$file,
258-
$line,
259-
false,
260-
$file,
261-
))->withIdentifier('ignore.noIdentifier');
263+
foreach ($identifiers as $identifier) {
264+
['name' => $name, 'comment' => $comment] = $identifier;
265+
if ($comment !== null || !$this->reportIgnoresWithoutComments) {
266+
continue;
267+
}
268+
269+
$errors[] = (new Error(
270+
sprintf('Ignore with identifier %s has no comment.', $name),
271+
$file,
272+
$line,
273+
false,
274+
$file,
275+
))->withIdentifier('ignore.noComment');
276+
}
262277
}
263278
}
264279
}

‎src/Analyser/FileAnalyser.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
use const E_WARNING;
4040

4141
/**
42+
* @phpstan-import-type Identifier from FileAnalyserResult
4243
* @phpstan-import-type CollectorData from CollectedData
4344
*/
4445
final class FileAnalyser
@@ -318,15 +319,15 @@ public function analyseFile(
318319

319320
/**
320321
* @param Node[] $nodes
321-
* @return array<int, non-empty-list<string>|null>
322+
* @return array<int, non-empty-list<Identifier>|null>
322323
*/
323324
private function getLinesToIgnoreFromTokens(array $nodes): array
324325
{
325326
if (!isset($nodes[0])) {
326327
return [];
327328
}
328329

329-
/** @var array<int, non-empty-list<string>|null> */
330+
/** @var array<int, non-empty-list<Identifier>|null> */
330331
return $nodes[0]->getAttribute('linesToIgnore', []);
331332
}
332333

‎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
@@ -47,7 +47,7 @@ public function process(
4747
}
4848

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

@@ -66,7 +66,7 @@ public function process(
6666
$unmatchedIgnoredIdentifiers = $unmatchedLineIgnores[$tmpFileError->getFile()][$line];
6767
if (is_array($unmatchedIgnoredIdentifiers)) {
6868
foreach ($unmatchedIgnoredIdentifiers as $j => $unmatchedIgnoredIdentifier) {
69-
if ($ignoredIdentifier !== $unmatchedIgnoredIdentifier) {
69+
if ($ignoredIdentifier['name'] !== $unmatchedIgnoredIdentifier['name']) {
7070
continue;
7171
}
7272

‎src/Command/FixerWorkerCommand.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ function (array $errors, array $locallyIgnoredErrors, array $analysedFiles) use
310310
if ($error->getIdentifier() === null) {
311311
continue;
312312
}
313-
if (!in_array($error->getIdentifier(), ['ignore.count', 'ignore.unmatched', 'ignore.unmatchedLine', 'ignore.unmatchedIdentifier', 'ignore.noIdentifier'], true)) {
313+
if (!in_array($error->getIdentifier(), ['ignore.count', 'ignore.unmatched', 'ignore.unmatchedLine', 'ignore.unmatchedIdentifier', 'ignore.noIdentifier', 'ignore.noComment'], true)) {
314314
continue;
315315
}
316316
$ignoreFileErrors[] = $error;

‎src/Parser/RichParser.php

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,16 @@
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;
19+
use function array_values;
1720
use function count;
1821
use function implode;
1922
use function in_array;
@@ -30,6 +33,9 @@
3033
use const T_DOC_COMMENT;
3134
use const T_WHITESPACE;
3235

36+
/**
37+
* @phpstan-import-type Identifier from FileAnalyserResult
38+
*/
3339
final class RichParser implements Parser
3440
{
3541

@@ -111,7 +117,7 @@ public function parseString(string $sourceCode): array
111117

112118
/**
113119
* @param Token[] $tokens
114-
* @return array{lines: array<int, non-empty-list<string>|null>, errors: array<int, non-empty-list<string>>}
120+
* @return array{lines: array<int, non-empty-list<Identifier>|null>, errors: array<int, non-empty-list<string>>}
115121
*/
116122
private function getLinesToIgnore(array $tokens): array
117123
{
@@ -268,33 +274,29 @@ private function getLinesToIgnoreForTokenByIgnoreComment(
268274
}
269275

270276
/**
271-
* @return non-empty-list<string>
277+
* @return non-empty-list<Identifier>
272278
* @throws IgnoreParseException
273279
*/
274280
private function parseIdentifiers(string $text, int $ignorePos): array
275281
{
276282
$text = substr($text, $ignorePos + strlen('@phpstan-ignore'));
277-
$originalTokens = $this->ignoreLexer->tokenize($text);
278-
$tokens = [];
279-
280-
foreach ($originalTokens as $originalToken) {
281-
if ($originalToken[IgnoreLexer::TYPE_OFFSET] === IgnoreLexer::TOKEN_WHITESPACE) {
282-
continue;
283-
}
284-
$tokens[] = $originalToken;
285-
}
283+
$tokens = $this->ignoreLexer->tokenize($text);
286284

287285
$c = count($tokens);
288286

289287
$identifiers = [];
288+
$comment = null;
290289
$openParenthesisCount = 0;
291290
$expected = [IgnoreLexer::TOKEN_IDENTIFIER];
291+
$lastTokenTypeLabel = '@phpstan-ignore';
292292

293293
for ($i = 0; $i < $c; $i++) {
294-
$lastTokenTypeLabel = isset($tokenType) ? $this->ignoreLexer->getLabel($tokenType) : '@phpstan-ignore';
294+
if (isset($tokenType) && $tokenType !== IgnoreLexer::TOKEN_WHITESPACE) {
295+
$lastTokenTypeLabel = $this->ignoreLexer->getLabel($tokenType);
296+
}
295297
[IgnoreLexer::VALUE_OFFSET => $content, IgnoreLexer::TYPE_OFFSET => $tokenType, IgnoreLexer::LINE_OFFSET => $tokenLine] = $tokens[$i];
296298

297-
if ($expected !== null && !in_array($tokenType, $expected, true)) {
299+
if ($expected !== null && !in_array($tokenType, [...$expected, IgnoreLexer::TOKEN_WHITESPACE], true)) {
298300
$tokenTypeLabel = $this->ignoreLexer->getLabel($tokenType);
299301
$otherTokenContent = $tokenType === IgnoreLexer::TOKEN_OTHER ? sprintf(" '%s'", $content) : '';
300302
$expectedLabels = implode(' or ', array_map(fn ($token) => $this->ignoreLexer->getLabel($token), $expected));
@@ -303,6 +305,9 @@ private function parseIdentifiers(string $text, int $ignorePos): array
303305
}
304306

305307
if ($tokenType === IgnoreLexer::TOKEN_OPEN_PARENTHESIS) {
308+
if ($openParenthesisCount > 0) {
309+
$comment .= $content;
310+
}
306311
$openParenthesisCount++;
307312
$expected = null;
308313
continue;
@@ -311,17 +316,25 @@ private function parseIdentifiers(string $text, int $ignorePos): array
311316
if ($tokenType === IgnoreLexer::TOKEN_CLOSE_PARENTHESIS) {
312317
$openParenthesisCount--;
313318
if ($openParenthesisCount === 0) {
319+
$key = array_key_last($identifiers);
320+
if ($key !== null) {
321+
$identifiers[$key]['comment'] = $comment;
322+
$comment = null;
323+
}
314324
$expected = [IgnoreLexer::TOKEN_COMMA, IgnoreLexer::TOKEN_END];
325+
} else {
326+
$comment .= $content;
315327
}
316328
continue;
317329
}
318330

319331
if ($openParenthesisCount > 0) {
332+
$comment .= $content;
320333
continue; // waiting for comment end
321334
}
322335

323336
if ($tokenType === IgnoreLexer::TOKEN_IDENTIFIER) {
324-
$identifiers[] = $content;
337+
$identifiers[] = ['name' => $content, 'comment' => null];
325338
$expected = [IgnoreLexer::TOKEN_COMMA, IgnoreLexer::TOKEN_END, IgnoreLexer::TOKEN_OPEN_PARENTHESIS];
326339
continue;
327340
}
@@ -340,7 +353,7 @@ private function parseIdentifiers(string $text, int $ignorePos): array
340353
throw new IgnoreParseException('Missing identifier', 1);
341354
}
342355

343-
return $identifiers;
356+
return array_values($identifiers);
344357
}
345358

346359
}

‎src/Testing/RuleTestCase.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ private function gatherAnalyserErrorsWithDelayedErrors(array $files): array
242242
new LocalIgnoresProcessor(),
243243
true,
244244
false,
245+
false,
245246
);
246247

247248
return [

‎tests/PHPStan/Analyser/AnalyserTest.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,27 @@ public function testIgnoresWithoutIdentifiersReported(): void
573573
}
574574
}
575575

576+
public function testIgnoresWithoutCommentsReported(): void
577+
{
578+
$expects = [
579+
[9, 'variable.undefined'],
580+
[12, 'variable.undefined'],
581+
[12, 'wrong.id'],
582+
[13, 'wrong.id'],
583+
[14, 'variable.undefined'],
584+
];
585+
$result = $this->runAnalyser([], false, [
586+
__DIR__ . '/data/ignore-no-comments.php',
587+
], true, false, true);
588+
$this->assertCount(5, $result);
589+
foreach ($expects as $i => $expect) {
590+
$this->assertArrayHasKey($i, $result);
591+
$this->assertInstanceOf(Error::class, $result[$i]);
592+
$this->assertStringContainsString(sprintf('Ignore with identifier %s has no comment.', $expect[1]), $result[$i]->getMessage());
593+
$this->assertSame($expect[0], $result[$i]->getLine());
594+
}
595+
}
596+
576597
/**
577598
* @dataProvider dataTrueAndFalse
578599
*/
@@ -660,6 +681,7 @@ private function runAnalyser(
660681
$filePaths,
661682
bool $onlyFiles,
662683
bool $reportIgnoresWithoutIdentifiers = false,
684+
bool $reportIgnoresWithoutComments = false,
663685
): array
664686
{
665687
$analyser = $this->createAnalyser();
@@ -693,6 +715,7 @@ private function runAnalyser(
693715
new LocalIgnoresProcessor(),
694716
$reportUnmatchedIgnoredErrors,
695717
$reportIgnoresWithoutIdentifiers,
718+
$reportIgnoresWithoutComments,
696719
);
697720
$analyserResult = $finalizer->finalize($analyserResult, $onlyFiles, false)->getAnalyserResult();
698721

‎tests/PHPStan/Analyser/data/ignore-no-comments.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
namespace IgnoreNoComments;
4+
5+
class Foo
6+
{
7+
public function doFoo(): void
8+
{
9+
echo $foo; // @phpstan-ignore variable.undefined
10+
echo $foo; // @phpstan-ignore wrong.id (comment)
11+
12+
echo $foo, $bar; // @phpstan-ignore variable.undefined, wrong.id
13+
echo $foo, $bar; // @phpstan-ignore variable.undefined (comment), wrong.id
14+
echo $foo, $bar; // @phpstan-ignore variable.undefined, wrong.id (comment)
15+
echo $foo, $bar; // @phpstan-ignore variable.undefined (comment), wrong.id (comment)
16+
}
17+
18+
}

0 commit comments

Comments
 (0)