Skip to content

Commit 5b3d2c8

Browse files
anomiexanomiex
committed
Apply suggestions from code review
1 parent 9a0ee0c commit 5b3d2c8

9 files changed

Lines changed: 323 additions & 181 deletions

‎src/Files/File.php‎

Lines changed: 2 additions & 2 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]) === true && $this->tokenizer->ignoredLines[$line]->isAll() === true) {
804+
if (isset($this->tokenizer->ignoredLines[$line]) === true && $this->tokenizer->ignoredLines[$line]->ignoresEverything() === true) {
805805
return false;
806806
}
807807

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

838-
if (isset($this->tokenizer->ignoredLines[$line]) === true && $this->tokenizer->ignoredLines[$line]->check($sniffCode) === true) {
838+
if (isset($this->tokenizer->ignoredLines[$line]) === true && $this->tokenizer->ignoredLines[$line]->isIgnored($sniffCode) === true) {
839839
return false;
840840
}
841841

‎src/Tokenizers/Tokenizer.php‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ private function createPositionMap()
176176
$lineNumber = 1;
177177
$eolLen = strlen($this->eolChar);
178178
$ignoring = null;
179-
$ignoreAll = IgnoreList::ignoringAll();
179+
$ignoreAll = IgnoreList::getInstanceIgnoringAll();
180180
$inTests = defined('PHP_CODESNIFFER_IN_TESTS');
181181

182182
$checkEncoding = false;
@@ -383,11 +383,7 @@ private function createPositionMap()
383383
if (empty($additionalText) === true) {
384384
$ignoring = $ignoreAll;
385385
} else {
386-
if ($ignoring === null) {
387-
$ignoring = IgnoreList::ignoringNone();
388-
} else {
389-
$ignoring = clone $ignoring;
390-
}
386+
$ignoring = IgnoreList::getNewInstanceFrom($ignoring);
391387

392388
$parts = explode(',', $additionalText);
393389
foreach ($parts as $sniffCode) {
@@ -408,15 +404,15 @@ private function createPositionMap()
408404
if (empty($additionalText) === true) {
409405
$ignoring = null;
410406
} else {
411-
$ignoring = clone $ignoring;
407+
$ignoring = IgnoreList::getNewInstanceFrom($ignoring);
412408
$parts = explode(',', $additionalText);
413409
foreach ($parts as $sniffCode) {
414410
$sniffCode = trim($sniffCode);
415411
$enabledSniffs[$sniffCode] = true;
416412
$ignoring->set($sniffCode, false);
417413
}
418414

419-
if ($ignoring->isEmpty() === true) {
415+
if ($ignoring->ignoresNothing() === true) {
420416
$ignoring = null;
421417
}
422418
}
@@ -443,12 +439,8 @@ private function createPositionMap()
443439
$ignoreRules = ['.all' => true];
444440
$lineIgnoring = $ignoreAll;
445441
} else {
446-
$parts = explode(',', $additionalText);
447-
if ($ignoring === null) {
448-
$lineIgnoring = IgnoreList::ignoringNone();
449-
} else {
450-
$lineIgnoring = clone $ignoring;
451-
}
442+
$parts = explode(',', $additionalText);
443+
$lineIgnoring = IgnoreList::getNewInstanceFrom($ignoring);
452444

453445
foreach ($parts as $sniffCode) {
454446
$ignoreRules[trim($sniffCode)] = true;

‎src/Util/IgnoreList.php‎

Lines changed: 70 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,23 @@
22
/**
33
* Class to manage a list of sniffs to ignore.
44
*
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
5+
* @copyright 2025 PHPCSStandards and contributors
6+
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
87
*/
98

109
namespace PHP_CodeSniffer\Util;
1110

12-
class IgnoreList
11+
/**
12+
* Class to manage a list of sniffs to ignore.
13+
*
14+
* ---------------------------------------------------------------------------------------------
15+
* This class is intended for internal use only and is not part of the public API.
16+
* This also means that it has no promise of backward compatibility. Use at your own risk.
17+
* ---------------------------------------------------------------------------------------------
18+
*
19+
* @internal
20+
*/
21+
final class IgnoreList
1322
{
1423

1524
/**
@@ -19,35 +28,55 @@ class IgnoreList
1928
* Each level may be a boolean indicating that everything underneath the branch is or is not ignored, or
2029
* may have a `.default' key indicating the default status for any branches not in the tree.
2130
*
22-
* @var array|boolean
31+
* @var array<string, bool|array<string, bool|array<string, bool|array<string, bool>>>>
2332
*/
2433
private $data = [ '.default' => false ];
2534

2635

2736
/**
2837
* Get an instance set to ignore nothing.
2938
*
30-
* @return static
39+
* @return IgnoreList
3140
*/
32-
public static function ignoringNone()
41+
public static function getInstanceIgnoringNothing()
3342
{
3443
return new self();
3544

36-
}//end ignoringNone()
45+
}//end getInstanceIgnoringNothing()
3746

3847

3948
/**
4049
* Get an instance set to ignore everything.
4150
*
42-
* @return static
51+
* @return IgnoreList
52+
*/
53+
public static function getInstanceIgnoringAll()
54+
{
55+
$instance = new self();
56+
$instance->data['.default'] = true;
57+
return $instance;
58+
59+
}//end getInstanceIgnoringAll()
60+
61+
62+
/**
63+
* Get a new instance based on an existing instance.
64+
*
65+
* If passed null, creates a new instance that ignores nothing.
66+
*
67+
* @param IgnoreList|null $ignoreList List to clone.
68+
*
69+
* @return IgnoreList
4370
*/
44-
public static function ignoringAll()
71+
public static function getNewInstanceFrom(?IgnoreList $ignoreList)
4572
{
46-
$ret = new self();
47-
$ret->data['.default'] = true;
48-
return $ret;
73+
if ($ignoreList === null) {
74+
return self::getInstanceIgnoringNothing();
75+
}
76+
77+
return clone $ignoreList;
4978

50-
}//end ignoringAll()
79+
}//end getNewInstanceFrom()
5180

5281

5382
/**
@@ -57,29 +86,29 @@ public static function ignoringAll()
5786
*
5887
* @return bool
5988
*/
60-
public function check($code)
89+
public function isIgnored($code)
6190
{
62-
$data = $this->data;
63-
$ret = $data['.default'];
91+
$data = $this->data;
92+
$returnValue = $data['.default'];
6493
foreach (explode('.', $code) as $part) {
6594
if (isset($data[$part]) === false) {
6695
break;
6796
}
6897

6998
$data = $data[$part];
7099
if (is_bool($data) === true) {
71-
$ret = $data;
100+
$returnValue = $data;
72101
break;
73102
}
74103

75104
if (isset($data['.default']) === true) {
76-
$ret = $data['.default'];
105+
$returnValue = $data['.default'];
77106
}
78107
}
79108

80-
return $ret;
109+
return $returnValue;
81110

82-
}//end check()
111+
}//end isIgnored()
83112

84113

85114
/**
@@ -88,7 +117,7 @@ public function check($code)
88117
* @param string $code Partial or complete sniff code.
89118
* @param bool $ignore Whether the specified sniff should be ignored.
90119
*
91-
* @return this
120+
* @return IgnoreList $this for chaining.
92121
*/
93122
public function set($code, $ignore)
94123
{
@@ -114,55 +143,55 @@ public function set($code, $ignore)
114143

115144

116145
/**
117-
* Check if the list is empty.
146+
* Check if the list ignores nothing.
118147
*
119148
* @return bool
120149
*/
121-
public function isEmpty()
150+
public function ignoresNothing()
122151
{
123-
$arrs = [ $this->data ];
124-
while ($arrs !== []) {
125-
$arr = array_pop($arrs);
126-
foreach ($arr as $v) {
127-
if ($v === true) {
152+
$arraysToProcess = [ $this->data ];
153+
while ($arraysToProcess !== []) {
154+
$arrayBeingProcessed = array_pop($arraysToProcess);
155+
foreach ($arrayBeingProcessed as $valueBeingProcessed) {
156+
if ($valueBeingProcessed === true) {
128157
return false;
129158
}
130159

131-
if (is_array($v) === true) {
132-
$arrs[] = $v;
160+
if (is_array($valueBeingProcessed) === true) {
161+
$arraysToProcess[] = $valueBeingProcessed;
133162
}
134163
}
135164
}
136165

137166
return true;
138167

139-
}//end isEmpty()
168+
}//end ignoresNothing()
140169

141170

142171
/**
143172
* Check if the list ignores everything.
144173
*
145174
* @return bool
146175
*/
147-
public function isAll()
176+
public function ignoresEverything()
148177
{
149-
$arrs = [ $this->data ];
150-
while ($arrs !== []) {
151-
$arr = array_pop($arrs);
152-
foreach ($arr as $v) {
153-
if ($v === false) {
178+
$arraysToProcess = [ $this->data ];
179+
while ($arraysToProcess !== []) {
180+
$arrayBeingProcessed = array_pop($arraysToProcess);
181+
foreach ($arrayBeingProcessed as $valueBeingProcessed) {
182+
if ($valueBeingProcessed === false) {
154183
return false;
155184
}
156185

157-
if (is_array($v) === true) {
158-
$arrs[] = $v;
186+
if (is_array($valueBeingProcessed) === true) {
187+
$arraysToProcess[] = $valueBeingProcessed;
159188
}
160189
}
161190
}
162191

163192
return true;
164193

165-
}//end isAll()
194+
}//end ignoresEverything()
166195

167196

168197
}//end class

‎tests/Core/ErrorSuppressionTest.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,6 +1057,16 @@ public static function dataEnableSelected()
10571057
'expectedErrors' => 0,
10581058
'expectedWarnings' => 0,
10591059
],
1060+
'disable: everything; enable: sniff' => [
1061+
'code' => '
1062+
// phpcs:disable
1063+
// phpcs:enable Generic.PHP.LowerCaseConstant
1064+
//TODO: write some code
1065+
$var = TRUE;
1066+
',
1067+
'expectedErrors' => 1,
1068+
'expectedWarnings' => 0,
1069+
],
10601070
];
10611071

10621072
}//end dataEnableSelected()

0 commit comments

Comments
 (0)