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 2a4b464

Browse files
committed
feat: add option to report ignores without comments
1 parent 4d9ed53 commit 2a4b464

File tree

12 files changed

+143
-67
lines changed

12 files changed

+143
-67
lines changed

‎conf/config.neon‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ parameters:
9898
nodesByStringCountMax: 256
9999
reportUnmatchedIgnoredErrors: true
100100
reportIgnoresWithoutIdentifiers: false
101+
reportIgnoresWithoutComments: false
101102
typeAliases: []
102103
universalObjectCratesClasses:
103104
- stdClass
@@ -202,6 +203,7 @@ parameters:
202203
- [parameters, ignoreErrors]
203204
- [parameters, reportUnmatchedIgnoredErrors]
204205
- [parameters, reportIgnoresWithoutIdentifiers]
206+
- [parameters, reportIgnoresWithoutComments]
205207
- [parameters, tipsOfTheDay]
206208
- [parameters, parallel]
207209
- [parameters, internalErrorsCountLimit]
@@ -283,6 +285,7 @@ services:
283285
arguments:
284286
reportUnmatchedIgnoredErrors: %reportUnmatchedIgnoredErrors%
285287
reportIgnoresWithoutIdentifiers: %reportIgnoresWithoutIdentifiers%
288+
reportIgnoresWithoutComments: %reportIgnoresWithoutComments%
286289

287290
-
288291
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
@@ -319,15 +320,15 @@ public function analyseFile(
319320

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

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

‎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
@@ -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
@@ -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: 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

0 commit comments

Comments
(0)

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