Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit 384c8af

Browse files
committed
feat: add option to report ignores without comments
1 parent 47d8329 commit 384c8af

File tree

11 files changed

+168
-57
lines changed

11 files changed

+168
-57
lines changed

‎conf/config.neon‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ parameters:
9696
nodesByStringCountMax: 256
9797
reportUnmatchedIgnoredErrors: true
9898
reportIgnoresWithoutIdentifiers: false
99+
reportIgnoresWithoutComments: false
99100
typeAliases: []
100101
universalObjectCratesClasses:
101102
- stdClass
@@ -200,6 +201,7 @@ parameters:
200201
- [parameters, ignoreErrors]
201202
- [parameters, reportUnmatchedIgnoredErrors]
202203
- [parameters, reportIgnoresWithoutIdentifiers]
204+
- [parameters, reportIgnoresWithoutComments]
203205
- [parameters, tipsOfTheDay]
204206
- [parameters, parallel]
205207
- [parameters, internalErrorsCountLimit]
@@ -464,6 +466,7 @@ services:
464466
arguments:
465467
reportUnmatchedIgnoredErrors: %reportUnmatchedIgnoredErrors%
466468
reportIgnoresWithoutIdentifiers: %reportIgnoresWithoutIdentifiers%
469+
reportIgnoresWithoutComments: %reportIgnoresWithoutComments%
467470

468471
-
469472
class: PHPStan\Analyser\FileAnalyser

‎conf/parametersSchema.neon‎

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

‎src/Analyser/AnalyserResultFinalizer.php‎

Lines changed: 59 additions & 9 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->addIgnoresWithoutCommentsErrors($this->addIgnoresWithoutIdentifiersErrors($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->addIgnoresWithoutCommentsErrors($this->addIgnoresWithoutIdentifiersErrors(new AnalyserResult(
135133
array_merge($errors, $analyserResult->getFilteredPhpErrors()),
136134
[],
137135
$analyserResult->getAllPhpErrors(),
@@ -144,7 +142,7 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $
144142
$analyserResult->getExportedNodes(),
145143
$analyserResult->hasReachedInternalErrorsCountLimit(),
146144
$analyserResult->getPeakMemoryUsageBytes(),
147-
)), $collectorErrors, $locallyIgnoredCollectorErrors);
145+
))), $collectorErrors, $locallyIgnoredCollectorErrors);
148146
}
149147

150148
private function mergeFilteredPhpErrors(AnalyserResult $analyserResult): AnalyserResult
@@ -200,7 +198,7 @@ private function addUnmatchedIgnoredErrors(
200198

201199
foreach ($identifiers as $identifier) {
202200
$errors[] = (new Error(
203-
sprintf('No error with identifier %s is reported on line %d.', $identifier, $line),
201+
sprintf('No error with identifier %s is reported on line %d.', $identifier['name'], $line),
204202
$file,
205203
$line,
206204
false,
@@ -276,4 +274,56 @@ private function addIgnoresWithoutIdentifiersErrors(AnalyserResult $analyserResu
276274
);
277275
}
278276

277+
private function addIgnoresWithoutCommentsErrors(AnalyserResult $analyserResult): AnalyserResult
278+
{
279+
if (!$this->reportIgnoresWithoutComments) {
280+
return $analyserResult;
281+
}
282+
283+
$errors = $analyserResult->getUnorderedErrors();
284+
foreach ($analyserResult->getLinesToIgnore() as $file => $data) {
285+
foreach ($data as $ignoredFile => $lines) {
286+
if ($ignoredFile !== $file) {
287+
continue;
288+
}
289+
290+
foreach ($lines as $line => $identifiers) {
291+
if ($identifiers === null) {
292+
continue;
293+
}
294+
295+
foreach ($identifiers as $identifier) {
296+
['name' => $name, 'comment' => $comment] = $identifier;
297+
if ($comment !== null) {
298+
continue;
299+
}
300+
301+
$errors[] = (new Error(
302+
sprintf('Ignore with identifier %s has no comment.', $name),
303+
$file,
304+
$line,
305+
false,
306+
$file,
307+
))->withIdentifier('ignore.noComment');
308+
}
309+
}
310+
}
311+
}
312+
313+
return new AnalyserResult(
314+
$errors,
315+
$analyserResult->getFilteredPhpErrors(),
316+
$analyserResult->getAllPhpErrors(),
317+
$analyserResult->getLocallyIgnoredErrors(),
318+
$analyserResult->getLinesToIgnore(),
319+
$analyserResult->getUnmatchedLineIgnores(),
320+
$analyserResult->getInternalErrors(),
321+
$analyserResult->getCollectedData(),
322+
$analyserResult->getDependencies(),
323+
$analyserResult->getExportedNodes(),
324+
$analyserResult->hasReachedInternalErrorsCountLimit(),
325+
$analyserResult->getPeakMemoryUsageBytes(),
326+
);
327+
}
328+
279329
}

‎src/Analyser/FileAnalyser.php‎

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

4040
/**
41+
* @phpstan-import-type Identifier from FileAnalyserResult
4142
* @phpstan-import-type CollectorData from CollectedData
4243
*/
4344
final class FileAnalyser
@@ -306,15 +307,15 @@ public function analyseFile(
306307

307308
/**
308309
* @param Node[] $nodes
309-
* @return array<int, non-empty-list<string>|null>
310+
* @return array<int, non-empty-list<Identifier>|null>
310311
*/
311312
private function getLinesToIgnoreFromTokens(array $nodes): array
312313
{
313314
if (!isset($nodes[0])) {
314315
return [];
315316
}
316317

317-
/** @var array<int, non-empty-list<string>|null> */
318+
/** @var array<int, non-empty-list<Identifier>|null> */
318319
return $nodes[0]->getAttribute('linesToIgnore', []);
319320
}
320321

‎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/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

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)

AltStyle によって変換されたページ (->オリジナル) /