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 ae45feb

Browse files
committed
feat: add option to report ignores without comments
1 parent 7de8b93 commit ae45feb

File tree

12 files changed

+144
-67
lines changed

12 files changed

+144
-67
lines changed

‎conf/config.neon‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ parameters:
9999
nodesByStringCountMax: 256
100100
reportUnmatchedIgnoredErrors: true
101101
reportIgnoresWithoutIdentifiers: false
102+
reportIgnoresWithoutComments: false
102103
typeAliases: []
103104
universalObjectCratesClasses:
104105
- stdClass
@@ -203,6 +204,7 @@ parameters:
203204
- [parameters, ignoreErrors]
204205
- [parameters, reportUnmatchedIgnoredErrors]
205206
- [parameters, reportIgnoresWithoutIdentifiers]
207+
- [parameters, reportIgnoresWithoutComments]
206208
- [parameters, tipsOfTheDay]
207209
- [parameters, parallel]
208210
- [parameters, internalErrorsCountLimit]

‎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: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,18 @@ public function __construct(
3030
private bool $reportUnmatchedIgnoredErrors,
3131
#[AutowiredParameter]
3232
private bool $reportIgnoresWithoutIdentifiers,
33+
#[AutowiredParameter]
34+
private bool $reportIgnoresWithoutComments,
3335
)
3436
{
3537
}
3638

3739
public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $debug): FinalizerResult
3840
{
39-
if (count($analyserResult->getCollectedData()) === 0) {
40-
return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutIdentifiersErrors($this->mergeFilteredPhpErrors($analyserResult)), [], []);
41-
}
42-
41+
$hasCollectedData = count($analyserResult->getCollectedData()) > 0;
4342
$hasInternalErrors = count($analyserResult->getInternalErrors()) > 0 || $analyserResult->hasReachedInternalErrorsCountLimit();
44-
if ($hasInternalErrors) {
45-
return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutIdentifiersErrors($this->mergeFilteredPhpErrors($analyserResult)), [], []);
43+
if (! $hasCollectedData || $hasInternalErrors) {
44+
return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutCommentsOrIdentifiersErrors($this->mergeFilteredPhpErrors($analyserResult)), [], []);
4645
}
4746

4847
$nodeType = CollectedDataNode::class;
@@ -136,7 +135,7 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $
136135
$allUnmatchedLineIgnores[$file] = $localIgnoresProcessorResult->getUnmatchedLineIgnores();
137136
}
138137

139-
return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutIdentifiersErrors(new AnalyserResult(
138+
return $this->addUnmatchedIgnoredErrors($this->addIgnoresWithoutCommentsOrIdentifiersErrors(new AnalyserResult(
140139
array_merge($errors, $analyserResult->getFilteredPhpErrors()),
141140
[],
142141
$analyserResult->getAllPhpErrors(),
@@ -207,7 +206,7 @@ private function addUnmatchedIgnoredErrors(
207206

208207
foreach ($identifiers as $identifier) {
209208
$errors[] = (new Error(
210-
sprintf('No error with identifier %s is reported on line %d.', $identifier, $line),
209+
sprintf('No error with identifier %s is reported on line %d.', $identifier['name'], $line),
211210
$file,
212211
$line,
213212
false,
@@ -239,9 +238,9 @@ private function addUnmatchedIgnoredErrors(
239238
);
240239
}
241240

242-
private function addIgnoresWithoutIdentifiersErrors(AnalyserResult $analyserResult): AnalyserResult
241+
private function addIgnoresWithoutCommentsOrIdentifiersErrors(AnalyserResult $analyserResult): AnalyserResult
243242
{
244-
if (!$this->reportIgnoresWithoutIdentifiers) {
243+
if (!$this->reportIgnoresWithoutComments && !$this->reportIgnoresWithoutIdentifiers) {
245244
return $analyserResult;
246245
}
247246

@@ -253,17 +252,34 @@ private function addIgnoresWithoutIdentifiersErrors(AnalyserResult $analyserResu
253252
}
254253

255254
foreach ($lines as $line => $identifiers) {
256-
if ($identifiers !== null) {
255+
if ($identifiers === null) {
256+
if (!$this->reportIgnoresWithoutIdentifiers) {
257+
continue;
258+
}
259+
$errors[] = (new Error(
260+
sprintf('Error is ignored with no identifiers on line %d.', $line),
261+
$file,
262+
$line,
263+
false,
264+
$file,
265+
))->withIdentifier('ignore.noIdentifier');
257266
continue;
258267
}
259268

260-
$errors[] = (new Error(
261-
sprintf('Error is ignored with no identifiers on line %d.', $line),
262-
$file,
263-
$line,
264-
false,
265-
$file,
266-
))->withIdentifier('ignore.noIdentifier');
269+
foreach ($identifiers as $identifier) {
270+
['name' => $name, 'comment' => $comment] = $identifier;
271+
if ($comment !== null || !$this->reportIgnoresWithoutComments) {
272+
continue;
273+
}
274+
275+
$errors[] = (new Error(
276+
sprintf('Ignore with identifier %s has no comment.', $name),
277+
$file,
278+
$line,
279+
false,
280+
$file,
281+
))->withIdentifier('ignore.noComment');
282+
}
267283
}
268284
}
269285
}

‎src/Analyser/FileAnalyser.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
use const E_WARNING;
4242

4343
/**
44+
* @phpstan-import-type Identifier from FileAnalyserResult
4445
* @phpstan-import-type CollectorData from CollectedData
4546
*/
4647
#[AutowiredService]
@@ -331,15 +332,15 @@ public function analyseFile(
331332

332333
/**
333334
* @param Node[] $nodes
334-
* @return array<int, non-empty-list<string>|null>
335+
* @return array<int, non-empty-list<Identifier>|null>
335336
*/
336337
private function getLinesToIgnoreFromTokens(array $nodes): array
337338
{
338339
if (!isset($nodes[0])) {
339340
return [];
340341
}
341342

342-
/** @var array<int, non-empty-list<string>|null> */
343+
/** @var array<int, non-empty-list<Identifier>|null> */
343344
return $nodes[0]->getAttribute('linesToIgnore', []);
344345
}
345346

‎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', 'ignore.noIdentifier'], true)) {
316+
if (!in_array($error->getIdentifier(), ['ignore.count', 'ignore.unmatched', 'ignore.unmatchedLine', 'ignore.unmatchedIdentifier', 'ignore.noIdentifier', 'ignore.noComment'], true)) {
317317
continue;
318318
}
319319
$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
@@ -269,6 +269,7 @@ private function gatherAnalyserErrorsWithDelayedErrors(array $files): array
269269
new LocalIgnoresProcessor(),
270270
true,
271271
false,
272+
false,
272273
);
273274

274275
return [

‎tests/PHPStan/Analyser/AnalyserTest.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,28 @@ public function testIgnoresWithoutIdentifiersReported(): void
563563
}
564564
}
565565

566+
#[DataProvider('dataTrueAndFalse')]
567+
public function testIgnoresWithoutCommentsReported(): void
568+
{
569+
$expects = [
570+
[9, 'variable.undefined'],
571+
[12, 'variable.undefined'],
572+
[12, 'wrong.id'],
573+
[13, 'wrong.id'],
574+
[14, 'variable.undefined'],
575+
];
576+
$result = $this->runAnalyser([], false, [
577+
__DIR__ . '/data/ignore-no-comments.php',
578+
], true, false, true);
579+
$this->assertCount(5, $result);
580+
foreach ($expects as $i => $expect) {
581+
$this->assertArrayHasKey($i, $result);
582+
$this->assertInstanceOf(Error::class, $result[$i]);
583+
$this->assertStringContainsString(sprintf('Ignore with identifier %s has no comment.', $expect[1]), $result[$i]->getMessage());
584+
$this->assertSame($expect[0], $result[$i]->getLine());
585+
}
586+
}
587+
566588
#[DataProvider('dataTrueAndFalse')]
567589
public function testIgnoreLine(bool $reportUnmatchedIgnoredErrors): void
568590
{
@@ -648,6 +670,7 @@ private function runAnalyser(
648670
$filePaths,
649671
bool $onlyFiles,
650672
bool $reportIgnoresWithoutIdentifiers = false,
673+
bool $reportIgnoresWithoutComments = false,
651674
): array
652675
{
653676
$analyser = $this->createAnalyser();
@@ -681,6 +704,7 @@ private function runAnalyser(
681704
new LocalIgnoresProcessor(),
682705
$reportUnmatchedIgnoredErrors,
683706
$reportIgnoresWithoutIdentifiers,
707+
$reportIgnoresWithoutComments,
684708
);
685709
$analyserResult = $finalizer->finalize($analyserResult, $onlyFiles, false)->getAnalyserResult();
686710

0 commit comments

Comments
(0)

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