Skip to content

Commit 9a0ee0c

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 7a3c1e3 commit 9a0ee0c

5 files changed

Lines changed: 522 additions & 95 deletions

File tree

‎src/Files/File.php‎

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -801,7 +801,7 @@ public function addFixableWarning(
801801
protected function addMessage($error, $message, $line, $column, $code, $data, $severity, $fixable)
802802
{
803803
// Check if this line is ignoring all message codes.
804-
if (isset($this->tokenizer->ignoredLines[$line]['.all']) === true) {
804+
if (isset($this->tokenizer->ignoredLines[$line]) === true && $this->tokenizer->ignoredLines[$line]->isAll() === true) {
805805
return false;
806806
}
807807

@@ -835,32 +835,9 @@ protected function addMessage($error, $message, $line, $column, $code, $data, $s
835835
];
836836
}//end if
837837

838-
if (isset($this->tokenizer->ignoredLines[$line]) === true) {
839-
// Check if this line is ignoring this specific message.
840-
$ignored = false;
841-
foreach ($checkCodes as $checkCode) {
842-
if (isset($this->tokenizer->ignoredLines[$line][$checkCode]) === true) {
843-
$ignored = true;
844-
break;
845-
}
846-
}
847-
848-
// If it is ignored, make sure there is no exception in place.
849-
if ($ignored === true
850-
&& isset($this->tokenizer->ignoredLines[$line]['.except']) === true
851-
) {
852-
foreach ($checkCodes as $checkCode) {
853-
if (isset($this->tokenizer->ignoredLines[$line]['.except'][$checkCode]) === true) {
854-
$ignored = false;
855-
break;
856-
}
857-
}
858-
}
859-
860-
if ($ignored === true) {
861-
return false;
862-
}
863-
}//end if
838+
if (isset($this->tokenizer->ignoredLines[$line]) === true && $this->tokenizer->ignoredLines[$line]->check($sniffCode) === true) {
839+
return false;
840+
}
864841

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

‎src/Tokenizers/Tokenizer.php‎

Lines changed: 33 additions & 68 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::ignoringAll();
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,28 @@ 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+
if ($ignoring === null) {
387+
$ignoring = IgnoreList::ignoringNone();
388+
} else {
389+
$ignoring = clone $ignoring;
390+
}
391+
388392
$parts = explode(',', $additionalText);
389393
foreach ($parts as $sniffCode) {
390394
$sniffCode = trim($sniffCode);
391395
$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
396+
$ignoring->set($sniffCode, true);
397+
}
398+
}
411399

412400
$this->tokens[$i]['code'] = T_PHPCS_DISABLE;
413401
$this->tokens[$i]['type'] = 'T_PHPCS_DISABLE';
@@ -420,49 +408,22 @@ private function createPositionMap()
420408
if (empty($additionalText) === true) {
421409
$ignoring = null;
422410
} else {
423-
$parts = explode(',', $additionalText);
411+
$ignoring = clone $ignoring;
412+
$parts = explode(',', $additionalText);
424413
foreach ($parts as $sniffCode) {
425414
$sniffCode = trim($sniffCode);
426415
$enabledSniffs[$sniffCode] = true;
416+
$ignoring->set($sniffCode, false);
417+
}
427418

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) {
419+
if ($ignoring->isEmpty() === true) {
453420
$ignoring = null;
454-
} else {
455-
if (isset($ignoring['.except']) === true) {
456-
$ignoring['.except'] += $enabledSniffs;
457-
} else {
458-
$ignoring['.except'] = $enabledSniffs;
459-
}
460421
}
461-
}//end if
422+
}
462423

463424
if ($lineHasOtherContent === false) {
464425
// Completely ignore the comment line.
465-
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
426+
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll;
466427
} else {
467428
// The comment is on the same line as the code it is ignoring,
468429
// so respect the new ignore rules.
@@ -479,31 +440,35 @@ private function createPositionMap()
479440

480441
$additionalText = substr($commentText, 13);
481442
if (empty($additionalText) === true) {
482-
$ignoreRules = ['.all' => true];
443+
$ignoreRules = ['.all' => true];
444+
$lineIgnoring = $ignoreAll;
483445
} else {
484446
$parts = explode(',', $additionalText);
447+
if ($ignoring === null) {
448+
$lineIgnoring = IgnoreList::ignoringNone();
449+
} else {
450+
$lineIgnoring = clone $ignoring;
451+
}
452+
485453
foreach ($parts as $sniffCode) {
486454
$ignoreRules[trim($sniffCode)] = true;
455+
$lineIgnoring->set($sniffCode, true);
487456
}
488457
}
489458

490459
$this->tokens[$i]['code'] = T_PHPCS_IGNORE;
491460
$this->tokens[$i]['type'] = 'T_PHPCS_IGNORE';
492461
$this->tokens[$i]['sniffCodes'] = $ignoreRules;
493462

494-
if ($ignoring !== null) {
495-
$ignoreRules += $ignoring;
496-
}
497-
498463
if ($lineHasOtherContent === false) {
499464
// Completely ignore the comment line, and set the following
500465
// 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;
466+
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll;
467+
$this->ignoredLines[($this->tokens[$i]['line'] + 1)] = $lineIgnoring;
503468
} else {
504469
// The comment is on the same line as the code it is ignoring,
505470
// so respect the ignore rules it set.
506-
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoreRules;
471+
$this->ignoredLines[$this->tokens[$i]['line']] = $lineIgnoring;
507472
}
508473
}//end if
509474
}//end if

‎src/Util/IgnoreList.php‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
<?php
2+
/**
3+
* Class to manage a list of sniffs to ignore.
4+
*
5+
* @author Brad Jorsch <brad.jorsch@automattic.com>
6+
* @copyright 2023 Brad Jorsch
7+
* @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
8+
*/
9+
10+
namespace PHP_CodeSniffer\Util;
11+
12+
class IgnoreList
13+
{
14+
15+
/**
16+
* Ignore data.
17+
*
18+
* Data is a tree, standard → category → sniff → code.
19+
* Each level may be a boolean indicating that everything underneath the branch is or is not ignored, or
20+
* may have a `.default' key indicating the default status for any branches not in the tree.
21+
*
22+
* @var array|boolean
23+
*/
24+
private $data = [ '.default' => false ];
25+
26+
27+
/**
28+
* Get an instance set to ignore nothing.
29+
*
30+
* @return static
31+
*/
32+
public static function ignoringNone()
33+
{
34+
return new self();
35+
36+
}//end ignoringNone()
37+
38+
39+
/**
40+
* Get an instance set to ignore everything.
41+
*
42+
* @return static
43+
*/
44+
public static function ignoringAll()
45+
{
46+
$ret = new self();
47+
$ret->data['.default'] = true;
48+
return $ret;
49+
50+
}//end ignoringAll()
51+
52+
53+
/**
54+
* Check whether a sniff code is ignored.
55+
*
56+
* @param string $code Partial or complete sniff code.
57+
*
58+
* @return bool
59+
*/
60+
public function check($code)
61+
{
62+
$data = $this->data;
63+
$ret = $data['.default'];
64+
foreach (explode('.', $code) as $part) {
65+
if (isset($data[$part]) === false) {
66+
break;
67+
}
68+
69+
$data = $data[$part];
70+
if (is_bool($data) === true) {
71+
$ret = $data;
72+
break;
73+
}
74+
75+
if (isset($data['.default']) === true) {
76+
$ret = $data['.default'];
77+
}
78+
}
79+
80+
return $ret;
81+
82+
}//end check()
83+
84+
85+
/**
86+
* Set the ignore status for a sniff.
87+
*
88+
* @param string $code Partial or complete sniff code.
89+
* @param bool $ignore Whether the specified sniff should be ignored.
90+
*
91+
* @return this
92+
*/
93+
public function set($code, $ignore)
94+
{
95+
$data = &$this->data;
96+
$parts = explode('.', $code);
97+
while (count($parts) > 1) {
98+
$part = array_shift($parts);
99+
if (isset($data[$part]) === false) {
100+
$data[$part] = [];
101+
} else if (is_bool($data[$part]) === true) {
102+
$data[$part] = [ '.default' => $data[$part] ];
103+
}
104+
105+
$data = &$data[$part];
106+
}
107+
108+
$part = array_shift($parts);
109+
$data[$part] = (bool) $ignore;
110+
111+
return $this;
112+
113+
}//end set()
114+
115+
116+
/**
117+
* Check if the list is empty.
118+
*
119+
* @return bool
120+
*/
121+
public function isEmpty()
122+
{
123+
$arrs = [ $this->data ];
124+
while ($arrs !== []) {
125+
$arr = array_pop($arrs);
126+
foreach ($arr as $v) {
127+
if ($v === true) {
128+
return false;
129+
}
130+
131+
if (is_array($v) === true) {
132+
$arrs[] = $v;
133+
}
134+
}
135+
}
136+
137+
return true;
138+
139+
}//end isEmpty()
140+
141+
142+
/**
143+
* Check if the list ignores everything.
144+
*
145+
* @return bool
146+
*/
147+
public function isAll()
148+
{
149+
$arrs = [ $this->data ];
150+
while ($arrs !== []) {
151+
$arr = array_pop($arrs);
152+
foreach ($arr as $v) {
153+
if ($v === false) {
154+
return false;
155+
}
156+
157+
if (is_array($v) === true) {
158+
$arrs[] = $v;
159+
}
160+
}
161+
}
162+
163+
return true;
164+
165+
}//end isAll()
166+
167+
168+
}//end class

0 commit comments

Comments
 (0)