Skip to content

Commit 703b7f4

Browse files
anomiexanomiex
authored
Improve handling of disable/enable/ignore directives (#123)
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 d7efcc0 commit 703b7f4

File tree

9 files changed

+674
-96
lines changed

9 files changed

+674
-96
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]->ignoresEverything() === true) {
862862
return false;
863863
}
864864

@@ -892,32 +892,9 @@ protected function addMessage($error, $message, $line, $column, $code, $data, $s
892892
];
893893
}//end if
894894

895-
if (isset($this->tokenizer->ignoredLines[$line]) === true) {
896-
// Check if this line is ignoring this specific message.
897-
$ignored = false;
898-
foreach ($checkCodes as $checkCode) {
899-
if (isset($this->tokenizer->ignoredLines[$line][$checkCode]) === true) {
900-
$ignored = true;
901-
break;
902-
}
903-
}
904-
905-
// If it is ignored, make sure there is no exception in place.
906-
if ($ignored === true
907-
&& isset($this->tokenizer->ignoredLines[$line]['.except']) === true
908-
) {
909-
foreach ($checkCodes as $checkCode) {
910-
if (isset($this->tokenizer->ignoredLines[$line]['.except'][$checkCode]) === true) {
911-
$ignored = false;
912-
break;
913-
}
914-
}
915-
}
916-
917-
if ($ignored === true) {
918-
return false;
919-
}
920-
}//end if
895+
if (isset($this->tokenizer->ignoredLines[$line]) === true && $this->tokenizer->ignoredLines[$line]->isIgnored($sniffCode) === true) {
896+
return false;
897+
}
921898

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

‎src/Tokenizers/Tokenizer.php

Lines changed: 26 additions & 69 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\Common;
14+
use PHP_CodeSniffer\Util\IgnoreList;
1415
use PHP_CodeSniffer\Util\Tokens;
1516
use PHP_CodeSniffer\Util\Writers\StatusWriter;
1617

@@ -175,6 +176,7 @@ private function createPositionMap()
175176
$lineNumber = 1;
176177
$eolLen = strlen($this->eolChar);
177178
$ignoring = null;
179+
$ignoreAll = IgnoreList::getInstanceIgnoringAll();
178180
$inTests = defined('PHP_CODESNIFFER_IN_TESTS');
179181

180182
$checkEncoding = false;
@@ -349,7 +351,7 @@ private function createPositionMap()
349351
if (substr($commentTextLower, 0, 9) === 'phpcs:set') {
350352
// Ignore standards for complete lines that change sniff settings.
351353
if ($lineHasOtherTokens === false) {
352-
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
354+
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll;
353355
}
354356

355357
// Need to maintain case here, to get the correct sniff code.
@@ -372,42 +374,24 @@ private function createPositionMap()
372374
} else if (substr($commentTextLower, 0, 13) === 'phpcs:disable') {
373375
if ($lineHasOtherContent === false) {
374376
// Completely ignore the comment line.
375-
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
376-
}
377-
378-
if ($ignoring === null) {
379-
$ignoring = [];
377+
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll;
380378
}
381379

382380
$disabledSniffs = [];
383381

384382
$additionalText = substr($commentText, 14);
385383
if (empty($additionalText) === true) {
386-
$ignoring = ['.all' => true];
384+
$ignoring = $ignoreAll;
387385
} else {
386+
$ignoring = IgnoreList::getNewInstanceFrom($ignoring);
387+
388388
$parts = explode(',', $additionalText);
389389
foreach ($parts as $sniffCode) {
390390
$sniffCode = trim($sniffCode);
391391
$disabledSniffs[$sniffCode] = true;
392-
$ignoring[$sniffCode] = true;
393-
394-
// This newly disabled sniff might be disabling an existing
395-
// enabled exception that we are tracking.
396-
if (isset($ignoring['.except']) === true) {
397-
foreach (array_keys($ignoring['.except']) as $ignoredSniffCode) {
398-
if ($ignoredSniffCode === $sniffCode
399-
|| strpos($ignoredSniffCode, $sniffCode.'.') === 0
400-
) {
401-
unset($ignoring['.except'][$ignoredSniffCode]);
402-
}
403-
}
404-
405-
if (empty($ignoring['.except']) === true) {
406-
unset($ignoring['.except']);
407-
}
408-
}
409-
}//end foreach
410-
}//end if
392+
$ignoring->set($sniffCode, true);
393+
}
394+
}
411395

412396
$this->tokens[$i]['code'] = T_PHPCS_DISABLE;
413397
$this->tokens[$i]['type'] = 'T_PHPCS_DISABLE';
@@ -420,49 +404,22 @@ private function createPositionMap()
420404
if (empty($additionalText) === true) {
421405
$ignoring = null;
422406
} else {
423-
$parts = explode(',', $additionalText);
407+
$ignoring = IgnoreList::getNewInstanceFrom($ignoring);
408+
$parts = explode(',', $additionalText);
424409
foreach ($parts as $sniffCode) {
425410
$sniffCode = trim($sniffCode);
426411
$enabledSniffs[$sniffCode] = true;
412+
$ignoring->set($sniffCode, false);
413+
}
427414

428-
// This new enabled sniff might remove previously disabled
429-
// sniffs if it is actually a standard or category of sniffs.
430-
foreach (array_keys($ignoring) as $ignoredSniffCode) {
431-
if ($ignoredSniffCode === $sniffCode
432-
|| strpos($ignoredSniffCode, $sniffCode.'.') === 0
433-
) {
434-
unset($ignoring[$ignoredSniffCode]);
435-
}
436-
}
437-
438-
// This new enabled sniff might be able to clear up
439-
// previously enabled sniffs if it is actually a standard or
440-
// category of sniffs.
441-
if (isset($ignoring['.except']) === true) {
442-
foreach (array_keys($ignoring['.except']) as $ignoredSniffCode) {
443-
if ($ignoredSniffCode === $sniffCode
444-
|| strpos($ignoredSniffCode, $sniffCode.'.') === 0
445-
) {
446-
unset($ignoring['.except'][$ignoredSniffCode]);
447-
}
448-
}
449-
}
450-
}//end foreach
451-
452-
if (empty($ignoring) === true) {
415+
if ($ignoring->ignoresNothing() === true) {
453416
$ignoring = null;
454-
} else {
455-
if (isset($ignoring['.except']) === true) {
456-
$ignoring['.except'] += $enabledSniffs;
457-
} else {
458-
$ignoring['.except'] = $enabledSniffs;
459-
}
460417
}
461-
}//end if
418+
}
462419

463420
if ($lineHasOtherContent === false) {
464421
// Completely ignore the comment line.
465-
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
422+
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll;
466423
} else {
467424
// The comment is on the same line as the code it is ignoring,
468425
// so respect the new ignore rules.
@@ -479,31 +436,31 @@ private function createPositionMap()
479436

480437
$additionalText = substr($commentText, 13);
481438
if (empty($additionalText) === true) {
482-
$ignoreRules = ['.all' => true];
439+
$ignoreRules = ['.all' => true];
440+
$lineIgnoring = $ignoreAll;
483441
} else {
484-
$parts = explode(',', $additionalText);
442+
$parts = explode(',', $additionalText);
443+
$lineIgnoring = IgnoreList::getNewInstanceFrom($ignoring);
444+
485445
foreach ($parts as $sniffCode) {
486446
$ignoreRules[trim($sniffCode)] = true;
447+
$lineIgnoring->set($sniffCode, true);
487448
}
488449
}
489450

490451
$this->tokens[$i]['code'] = T_PHPCS_IGNORE;
491452
$this->tokens[$i]['type'] = 'T_PHPCS_IGNORE';
492453
$this->tokens[$i]['sniffCodes'] = $ignoreRules;
493454

494-
if ($ignoring !== null) {
495-
$ignoreRules += $ignoring;
496-
}
497-
498455
if ($lineHasOtherContent === false) {
499456
// Completely ignore the comment line, and set the following
500457
// line to include the ignore rules we've set.
501-
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
502-
$this->ignoredLines[($this->tokens[$i]['line'] + 1)] = $ignoreRules;
458+
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll;
459+
$this->ignoredLines[($this->tokens[$i]['line'] + 1)] = $lineIgnoring;
503460
} else {
504461
// The comment is on the same line as the code it is ignoring,
505462
// so respect the ignore rules it set.
506-
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreRules;
463+
$this->ignoredLines[$this->tokens[$i]['line']] = $lineIgnoring;
507464
}
508465
}//end if
509466
}//end if

0 commit comments

Comments
 (0)