Skip to content

Commit ade94ce

Browse files
anomiexanomiex
committed
Improve handling of disable/enable/ignore directives
The current method, listing codes to disable and a list of exceptions to that list, still has trouble with some cases. For example, disabling a standard, re-enabling a category within that standard, then ignoring or disabling a sniff within that category cannot be handled. We'd need a list of exceptions to the exceptions, and possibly a list of exceptions to that list too, and figuring out how to keep those lists up to date as new directives are encountered could prove to be confusing. Since the standard→category→sniff→code hierarchy is supposed to be thought of as a tree, let's store the ignore list that way instead. Manipulating the branches of the tree is straightforward no matter what directives are encountered. In this implementation I've favored speed over space: there are cases where we could prune a subtree that would evaluate to "ignore" or "don't ignore" for any possible input, but detecting that doesn't seem worth the time when it's not likely there will be so many enable or disable directives that the wasted space will be a problem. Fixes #111
1 parent c248a92 commit ade94ce

File tree

5 files changed

+525
-98
lines changed

5 files changed

+525
-98
lines changed

‎src/Files/File.php‎

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,7 @@ public function addFixableWarning(
858858
protected function addMessage($error, $message, $line, $column, $code, $data, $severity, $fixable)
859859
{
860860
// Check if this line is ignoring all message codes.
861-
if (isset($this->tokenizer->ignoredLines[$line]['.all']) === true) {
861+
if (isset($this->tokenizer->ignoredLines[$line]) === true && $this->tokenizer->ignoredLines[$line]->isAll() === true) {
862862
return false;
863863
}
864864

@@ -888,32 +888,9 @@ protected function addMessage($error, $message, $line, $column, $code, $data, $s
888888
];
889889
}//end if
890890

891-
if (isset($this->tokenizer->ignoredLines[$line]) === true) {
892-
// Check if this line is ignoring this specific message.
893-
$ignored = false;
894-
foreach ($checkCodes as $checkCode) {
895-
if (isset($this->tokenizer->ignoredLines[$line][$checkCode]) === true) {
896-
$ignored = true;
897-
break;
898-
}
899-
}
900-
901-
// If it is ignored, make sure it's not whitelisted.
902-
if ($ignored === true
903-
&& isset($this->tokenizer->ignoredLines[$line]['.except']) === true
904-
) {
905-
foreach ($checkCodes as $checkCode) {
906-
if (isset($this->tokenizer->ignoredLines[$line]['.except'][$checkCode]) === true) {
907-
$ignored = false;
908-
break;
909-
}
910-
}
911-
}
912-
913-
if ($ignored === true) {
914-
return false;
915-
}
916-
}//end if
891+
if (isset($this->tokenizer->ignoredLines[$line]) === true && $this->tokenizer->ignoredLines[$line]->check($sniffCode) === true) {
892+
return false;
893+
}
917894

918895
$includeAll = true;
919896
if ($this->configCache['cache'] === false

‎src/Tokenizers/Tokenizer.php‎

Lines changed: 36 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
use PHP_CodeSniffer\Exceptions\TokenizerException;
1313
use PHP_CodeSniffer\Util;
14+
use PHP_CodeSniffer\Util\IgnoreList;
1415

1516
abstract class Tokenizer
1617
{
@@ -173,6 +174,7 @@ private function createPositionMap()
173174
$lineNumber = 1;
174175
$eolLen = strlen($this->eolChar);
175176
$ignoring = null;
177+
$ignoreAll = IgnoreList::ignoringAll();
176178
$inTests = defined('PHP_CODESNIFFER_IN_TESTS');
177179

178180
$checkEncoding = false;
@@ -277,15 +279,15 @@ private function createPositionMap()
277279
if ($ignoring === null
278280
&& strpos($commentText, '@codingStandardsIgnoreStart') !== false
279281
) {
280-
$ignoring = ['.all' => true];
282+
$ignoring = $ignoreAll;
281283
if ($ownLine === true) {
282284
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoring;
283285
}
284286
} else if ($ignoring !== null
285287
&& strpos($commentText, '@codingStandardsIgnoreEnd') !== false
286288
) {
287289
if ($ownLine === true) {
288-
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
290+
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll;
289291
} else {
290292
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoring;
291293
}
@@ -294,7 +296,7 @@ private function createPositionMap()
294296
} else if ($ignoring === null
295297
&& strpos($commentText, '@codingStandardsIgnoreLine') !== false
296298
) {
297-
$ignoring = ['.all' => true];
299+
$ignoring = $ignoreAll;
298300
if ($ownLine === true) {
299301
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoring;
300302
$this->ignoredLines[($this->tokens[$i]['line'] + 1)] = $ignoring;
@@ -393,7 +395,7 @@ private function createPositionMap()
393395
if (substr($commentTextLower, 0, 9) === 'phpcs:set') {
394396
// Ignore standards for complete lines that change sniff settings.
395397
if ($lineHasOtherTokens === false) {
396-
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
398+
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll;
397399
}
398400

399401
// Need to maintain case here, to get the correct sniff code.
@@ -416,42 +418,28 @@ private function createPositionMap()
416418
} else if (substr($commentTextLower, 0, 13) === 'phpcs:disable') {
417419
if ($lineHasOtherContent === false) {
418420
// Completely ignore the comment line.
419-
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
420-
}
421-
422-
if ($ignoring === null) {
423-
$ignoring = [];
421+
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll;
424422
}
425423

426424
$disabledSniffs = [];
427425

428426
$additionalText = substr($commentText, 14);
429427
if (empty($additionalText) === true) {
430-
$ignoring = ['.all' => true];
428+
$ignoring = $ignoreAll;
431429
} else {
430+
if ($ignoring === null) {
431+
$ignoring = IgnoreList::ignoringNone();
432+
} else {
433+
$ignoring = clone $ignoring;
434+
}
435+
432436
$parts = explode(',', $additionalText);
433437
foreach ($parts as $sniffCode) {
434438
$sniffCode = trim($sniffCode);
435439
$disabledSniffs[$sniffCode] = true;
436-
$ignoring[$sniffCode] = true;
437-
438-
// This newly disabled sniff might be disabling an existing
439-
// enabled exception that we are tracking.
440-
if (isset($ignoring['.except']) === true) {
441-
foreach (array_keys($ignoring['.except']) as $ignoredSniffCode) {
442-
if ($ignoredSniffCode === $sniffCode
443-
|| strpos($ignoredSniffCode, $sniffCode.'.') === 0
444-
) {
445-
unset($ignoring['.except'][$ignoredSniffCode]);
446-
}
447-
}
448-
449-
if (empty($ignoring['.except']) === true) {
450-
unset($ignoring['.except']);
451-
}
452-
}
453-
}//end foreach
454-
}//end if
440+
$ignoring->set($sniffCode, true);
441+
}
442+
}
455443

456444
$this->tokens[$i]['code'] = T_PHPCS_DISABLE;
457445
$this->tokens[$i]['type'] = 'T_PHPCS_DISABLE';
@@ -464,49 +452,22 @@ private function createPositionMap()
464452
if (empty($additionalText) === true) {
465453
$ignoring = null;
466454
} else {
467-
$parts = explode(',', $additionalText);
455+
$ignoring = clone $ignoring;
456+
$parts = explode(',', $additionalText);
468457
foreach ($parts as $sniffCode) {
469458
$sniffCode = trim($sniffCode);
470459
$enabledSniffs[$sniffCode] = true;
460+
$ignoring->set($sniffCode, false);
461+
}
471462

472-
// This new enabled sniff might remove previously disabled
473-
// sniffs if it is actually a standard or category of sniffs.
474-
foreach (array_keys($ignoring) as $ignoredSniffCode) {
475-
if ($ignoredSniffCode === $sniffCode
476-
|| strpos($ignoredSniffCode, $sniffCode.'.') === 0
477-
) {
478-
unset($ignoring[$ignoredSniffCode]);
479-
}
480-
}
481-
482-
// This new enabled sniff might be able to clear up
483-
// previously enabled sniffs if it is actually a standard or
484-
// category of sniffs.
485-
if (isset($ignoring['.except']) === true) {
486-
foreach (array_keys($ignoring['.except']) as $ignoredSniffCode) {
487-
if ($ignoredSniffCode === $sniffCode
488-
|| strpos($ignoredSniffCode, $sniffCode.'.') === 0
489-
) {
490-
unset($ignoring['.except'][$ignoredSniffCode]);
491-
}
492-
}
493-
}
494-
}//end foreach
495-
496-
if (empty($ignoring) === true) {
463+
if ($ignoring->isEmpty() === true) {
497464
$ignoring = null;
498-
} else {
499-
if (isset($ignoring['.except']) === true) {
500-
$ignoring['.except'] += $enabledSniffs;
501-
} else {
502-
$ignoring['.except'] = $enabledSniffs;
503-
}
504465
}
505-
}//end if
466+
}
506467

507468
if ($lineHasOtherContent === false) {
508469
// Completely ignore the comment line.
509-
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
470+
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll;
510471
} else {
511472
// The comment is on the same line as the code it is ignoring,
512473
// so respect the new ignore rules.
@@ -523,31 +484,35 @@ private function createPositionMap()
523484

524485
$additionalText = substr($commentText, 13);
525486
if (empty($additionalText) === true) {
526-
$ignoreRules = ['.all' => true];
487+
$ignoreRules = ['.all' => true];
488+
$lineIgnoring = $ignoreAll;
527489
} else {
528490
$parts = explode(',', $additionalText);
491+
if ($ignoring === null) {
492+
$lineIgnoring = IgnoreList::ignoringNone();
493+
} else {
494+
$lineIgnoring = clone $ignoring;
495+
}
496+
529497
foreach ($parts as $sniffCode) {
530498
$ignoreRules[trim($sniffCode)] = true;
499+
$lineIgnoring->set($sniffCode, true);
531500
}
532501
}
533502

534503
$this->tokens[$i]['code'] = T_PHPCS_IGNORE;
535504
$this->tokens[$i]['type'] = 'T_PHPCS_IGNORE';
536505
$this->tokens[$i]['sniffCodes'] = $ignoreRules;
537506

538-
if ($ignoring !== null) {
539-
$ignoreRules += $ignoring;
540-
}
541-
542507
if ($lineHasOtherContent === false) {
543508
// Completely ignore the comment line, and set the following
544509
// line to include the ignore rules we've set.
545-
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
546-
$this->ignoredLines[($this->tokens[$i]['line'] + 1)] = $ignoreRules;
510+
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll;
511+
$this->ignoredLines[($this->tokens[$i]['line'] + 1)] = $lineIgnoring;
547512
} else {
548513
// The comment is on the same line as the code it is ignoring,
549514
// so respect the ignore rules it set.
550-
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreRules;
515+
$this->ignoredLines[$this->tokens[$i]['line']] = $lineIgnoring;
551516
}
552517
}//end if
553518
}//end if

0 commit comments

Comments
 (0)