diff --git a/src/Files/File.php b/src/Files/File.php index eb2357a15e..e71ceef6ed 100644 --- a/src/Files/File.php +++ b/src/Files/File.php @@ -864,7 +864,7 @@ public function addFixableWarning( protected function addMessage($error, $message, $line, $column, $code, $data, $severity, $fixable) { // Check if this line is ignoring all message codes. - if (isset($this->tokenizer->ignoredLines[$line]['.all']) === true) { + if (isset($this->tokenizer->ignoredLines[$line]) === true && $this->tokenizer->ignoredLines[$line]->isAll() === true) { return false; } @@ -894,32 +894,9 @@ protected function addMessage($error, $message, $line, $column, $code, $data, $s ]; }//end if - if (isset($this->tokenizer->ignoredLines[$line]) === true) { - // Check if this line is ignoring this specific message. - $ignored = false; - foreach ($checkCodes as $checkCode) { - if (isset($this->tokenizer->ignoredLines[$line][$checkCode]) === true) { - $ignored = true; - break; - } - } - - // If it is ignored, make sure it's not whitelisted. - if ($ignored === true - && isset($this->tokenizer->ignoredLines[$line]['.except']) === true - ) { - foreach ($checkCodes as $checkCode) { - if (isset($this->tokenizer->ignoredLines[$line]['.except'][$checkCode]) === true) { - $ignored = false; - break; - } - } - } - - if ($ignored === true) { - return false; - } - }//end if + if (isset($this->tokenizer->ignoredLines[$line]) === true && $this->tokenizer->ignoredLines[$line]->check($sniffCode) === true) { + return false; + } $includeAll = true; if ($this->configCache['cache'] === false diff --git a/src/Tokenizers/Tokenizer.php b/src/Tokenizers/Tokenizer.php index bfe8c7a143..227f796d74 100644 --- a/src/Tokenizers/Tokenizer.php +++ b/src/Tokenizers/Tokenizer.php @@ -11,6 +11,7 @@ use PHP_CodeSniffer\Exceptions\TokenizerException; use PHP_CodeSniffer\Util; +use PHP_CodeSniffer\Util\IgnoreList; abstract class Tokenizer { @@ -173,6 +174,7 @@ private function createPositionMap() $lineNumber = 1; $eolLen = strlen($this->eolChar); $ignoring = null; + $ignoreAll = IgnoreList::ignoringAll(); $inTests = defined('PHP_CODESNIFFER_IN_TESTS'); $checkEncoding = false; @@ -277,7 +279,7 @@ private function createPositionMap() if ($ignoring === null && strpos($commentText, '@codingStandardsIgnoreStart') !== false ) { - $ignoring = ['.all' => true]; + $ignoring = $ignoreAll; if ($ownLine === true) { $this->ignoredLines[$this->tokens[$i]['line']] = $ignoring; } @@ -285,7 +287,7 @@ private function createPositionMap() && strpos($commentText, '@codingStandardsIgnoreEnd') !== false ) { if ($ownLine === true) { - $this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true]; + $this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll; } else { $this->ignoredLines[$this->tokens[$i]['line']] = $ignoring; } @@ -294,7 +296,7 @@ private function createPositionMap() } else if ($ignoring === null && strpos($commentText, '@codingStandardsIgnoreLine') !== false ) { - $ignoring = ['.all' => true]; + $ignoring = $ignoreAll; if ($ownLine === true) { $this->ignoredLines[$this->tokens[$i]['line']] = $ignoring; $this->ignoredLines[($this->tokens[$i]['line'] + 1)] = $ignoring; @@ -393,7 +395,7 @@ private function createPositionMap() if (substr($commentTextLower, 0, 9) === 'phpcs:set') { // Ignore standards for complete lines that change sniff settings. if ($lineHasOtherTokens === false) { - $this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true]; + $this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll; } // Need to maintain case here, to get the correct sniff code. @@ -416,42 +418,28 @@ private function createPositionMap() } else if (substr($commentTextLower, 0, 13) === 'phpcs:disable') { if ($lineHasOtherContent === false) { // Completely ignore the comment line. - $this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true]; - } - - if ($ignoring === null) { - $ignoring = []; + $this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll; } $disabledSniffs = []; $additionalText = substr($commentText, 14); if (empty($additionalText) === true) { - $ignoring = ['.all' => true]; + $ignoring = $ignoreAll; } else { + if ($ignoring === null) { + $ignoring = IgnoreList::ignoringNone(); + } else { + $ignoring = clone $ignoring; + } + $parts = explode(',', $additionalText); foreach ($parts as $sniffCode) { $sniffCode = trim($sniffCode); $disabledSniffs[$sniffCode] = true; - $ignoring[$sniffCode] = true; - - // This newly disabled sniff might be disabling an existing - // enabled exception that we are tracking. - if (isset($ignoring['.except']) === true) { - foreach (array_keys($ignoring['.except']) as $ignoredSniffCode) { - if ($ignoredSniffCode === $sniffCode - || strpos($ignoredSniffCode, $sniffCode.'.') === 0 - ) { - unset($ignoring['.except'][$ignoredSniffCode]); - } - } - - if (empty($ignoring['.except']) === true) { - unset($ignoring['.except']); - } - } - }//end foreach - }//end if + $ignoring->set($sniffCode, true); + } + } $this->tokens[$i]['code'] = T_PHPCS_DISABLE; $this->tokens[$i]['type'] = 'T_PHPCS_DISABLE'; @@ -464,49 +452,22 @@ private function createPositionMap() if (empty($additionalText) === true) { $ignoring = null; } else { - $parts = explode(',', $additionalText); + $ignoring = clone $ignoring; + $parts = explode(',', $additionalText); foreach ($parts as $sniffCode) { $sniffCode = trim($sniffCode); $enabledSniffs[$sniffCode] = true; + $ignoring->set($sniffCode, false); + } - // This new enabled sniff might remove previously disabled - // sniffs if it is actually a standard or category of sniffs. - foreach (array_keys($ignoring) as $ignoredSniffCode) { - if ($ignoredSniffCode === $sniffCode - || strpos($ignoredSniffCode, $sniffCode.'.') === 0 - ) { - unset($ignoring[$ignoredSniffCode]); - } - } - - // This new enabled sniff might be able to clear up - // previously enabled sniffs if it is actually a standard or - // category of sniffs. - if (isset($ignoring['.except']) === true) { - foreach (array_keys($ignoring['.except']) as $ignoredSniffCode) { - if ($ignoredSniffCode === $sniffCode - || strpos($ignoredSniffCode, $sniffCode.'.') === 0 - ) { - unset($ignoring['.except'][$ignoredSniffCode]); - } - } - } - }//end foreach - - if (empty($ignoring) === true) { + if ($ignoring->isEmpty() === true) { $ignoring = null; - } else { - if (isset($ignoring['.except']) === true) { - $ignoring['.except'] += $enabledSniffs; - } else { - $ignoring['.except'] = $enabledSniffs; - } } - }//end if + } if ($lineHasOtherContent === false) { // Completely ignore the comment line. - $this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true]; + $this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll; } else { // The comment is on the same line as the code it is ignoring, // so respect the new ignore rules. @@ -523,11 +484,19 @@ private function createPositionMap() $additionalText = substr($commentText, 13); if (empty($additionalText) === true) { - $ignoreRules = ['.all' => true]; + $ignoreRules = ['.all' => true]; + $lineIgnoring = $ignoreAll; } else { $parts = explode(',', $additionalText); + if ($ignoring === null) { + $lineIgnoring = IgnoreList::ignoringNone(); + } else { + $lineIgnoring = clone $ignoring; + } + foreach ($parts as $sniffCode) { $ignoreRules[trim($sniffCode)] = true; + $lineIgnoring->set($sniffCode, true); } } @@ -535,19 +504,15 @@ private function createPositionMap() $this->tokens[$i]['type'] = 'T_PHPCS_IGNORE'; $this->tokens[$i]['sniffCodes'] = $ignoreRules; - if ($ignoring !== null) { - $ignoreRules += $ignoring; - } - if ($lineHasOtherContent === false) { // Completely ignore the comment line, and set the following // line to include the ignore rules we've set. - $this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true]; - $this->ignoredLines[($this->tokens[$i]['line'] + 1)] = $ignoreRules; + $this->ignoredLines[$this->tokens[$i]['line']] = $ignoreAll; + $this->ignoredLines[($this->tokens[$i]['line'] + 1)] = $lineIgnoring; } else { // The comment is on the same line as the code it is ignoring, // so respect the ignore rules it set. - $this->ignoredLines[$this->tokens[$i]['line']] = $ignoreRules; + $this->ignoredLines[$this->tokens[$i]['line']] = $lineIgnoring; } }//end if }//end if diff --git a/src/Util/IgnoreList.php b/src/Util/IgnoreList.php new file mode 100644 index 0000000000..26ba8f4fbb --- /dev/null +++ b/src/Util/IgnoreList.php @@ -0,0 +1,168 @@ + + * @copyright 2023 Brad Jorsch + * @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Util; + +class IgnoreList +{ + + /** + * Ignore data. + * + * Data is a tree, standard → category → sniff → code. + * Each level may be a boolean indicating that everything underneath the branch is or is not ignored, or + * may have a `.default' key indicating the default status for any branches not in the tree. + * + * @var array|boolean + */ + private $data = [ '.default' => false ]; + + + /** + * Get an instance set to ignore nothing. + * + * @return static + */ + public static function ignoringNone() + { + return new self(); + + }//end ignoringNone() + + + /** + * Get an instance set to ignore everything. + * + * @return static + */ + public static function ignoringAll() + { + $ret = new self(); + $ret->data['.default'] = true; + return $ret; + + }//end ignoringAll() + + + /** + * Check whether a sniff code is ignored. + * + * @param string $code Partial or complete sniff code. + * + * @return bool + */ + public function check($code) + { + $data = $this->data; + $ret = $data['.default']; + foreach (explode('.', $code) as $part) { + if (isset($data[$part]) === false) { + break; + } + + $data = $data[$part]; + if (is_bool($data) === true) { + $ret = $data; + break; + } + + if (isset($data['.default']) === true) { + $ret = $data['.default']; + } + } + + return $ret; + + }//end check() + + + /** + * Set the ignore status for a sniff. + * + * @param string $code Partial or complete sniff code. + * @param bool $ignore Whether the specified sniff should be ignored. + * + * @return this + */ + public function set($code, $ignore) + { + $data = &$this->data; + $parts = explode('.', $code); + while (count($parts) > 1) { + $part = array_shift($parts); + if (isset($data[$part]) === false) { + $data[$part] = []; + } else if (is_bool($data[$part]) === true) { + $data[$part] = [ '.default' => $data[$part] ]; + } + + $data = &$data[$part]; + } + + $part = array_shift($parts); + $data[$part] = (bool) $ignore; + + return $this; + + }//end set() + + + /** + * Check if the list is empty. + * + * @return bool + */ + public function isEmpty() + { + $arrs = [ $this->data ]; + while ($arrs !== []) { + $arr = array_pop($arrs); + foreach ($arr as $v) { + if ($v === true) { + return false; + } + + if (is_array($v) === true) { + $arrs[] = $v; + } + } + } + + return true; + + }//end isEmpty() + + + /** + * Check if the list ignores everything. + * + * @return bool + */ + public function isAll() + { + $arrs = [ $this->data ]; + while ($arrs !== []) { + $arr = array_pop($arrs); + foreach ($arr as $v) { + if ($v === false) { + return false; + } + + if (is_array($v) === true) { + $arrs[] = $v; + } + } + } + + return true; + + }//end isAll() + + +}//end class diff --git a/tests/Core/ErrorSuppressionTest.php b/tests/Core/ErrorSuppressionTest.php index 7fc28d0e2e..0d8b013f93 100644 --- a/tests/Core/ErrorSuppressionTest.php +++ b/tests/Core/ErrorSuppressionTest.php @@ -1051,6 +1051,70 @@ public function dataEnableSelected() 'expectedErrors' => 1, 'expectedWarnings' => 2, ], + 'disable: two sniffs; enable: both sniffs; ignore: one of those sniffs (#3889)' => [ + 'code' => ' + // phpcs:disable Generic.PHP.LowerCaseConstant + // phpcs:disable Generic.Commenting.Todo + //TODO: write some code + $var = TRUE; + // phpcs:enable Generic.Commenting.Todo + // phpcs:enable Generic.PHP.LowerCaseConstant + + $var = FALSE; // phpcs:ignore Generic.PHP.LowerCaseConstant + ', + 'expectedErrors' => 0, + 'expectedWarnings' => 0, + ], + 'disable: two sniffs; enable: one sniff; ignore: enabled sniff' => [ + 'code' => ' + // phpcs:disable Generic.PHP.LowerCaseConstant + // phpcs:disable Generic.Commenting.Todo + //TODO: write some code + $var = TRUE; + // phpcs:enable Generic.PHP.LowerCaseConstant + + $var = FALSE; // phpcs:ignore Generic.PHP.LowerCaseConstant + ', + 'expectedErrors' => 0, + 'expectedWarnings' => 0, + ], + 'disable: two sniffs; enable: one sniff; ignore: category' => [ + 'code' => ' + // phpcs:disable Generic.PHP.LowerCaseConstant + // phpcs:disable Generic.Commenting.Todo + //TODO: write some code + $var = TRUE; + // phpcs:enable Generic.PHP.LowerCaseConstant + + $var = FALSE; // phpcs:ignore Generic.PHP + ', + 'expectedErrors' => 0, + 'expectedWarnings' => 0, + ], + 'disable: two sniffs; enable: category; ignore: sniff in category' => [ + 'code' => ' + // phpcs:disable Generic.PHP.LowerCaseConstant + // phpcs:disable Generic.Commenting.Todo + //TODO: write some code + $var = TRUE; + // phpcs:enable Generic.PHP + + $var = FALSE; // phpcs:ignore Generic.PHP.LowerCaseConstant + ', + 'expectedErrors' => 0, + 'expectedWarnings' => 0, + ], + 'disable: standard; enable: category in standard; disable: sniff in category' => [ + 'code' => ' + // phpcs:disable Generic + // phpcs:enable Generic.PHP + // phpcs:disable Generic.PHP.LowerCaseConstant + //TODO: write some code + $var = TRUE; + ', + 'expectedErrors' => 0, + 'expectedWarnings' => 0, + ], ]; }//end dataEnableSelected() diff --git a/tests/Core/IgnoreListTest.php b/tests/Core/IgnoreListTest.php new file mode 100644 index 0000000000..72352856c1 --- /dev/null +++ b/tests/Core/IgnoreListTest.php @@ -0,0 +1,253 @@ + + * @copyright 2023 Brad Jorsch + * @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Tests\Core; + +use PHP_CodeSniffer\Util\IgnoreList; +use PHPUnit\Framework\TestCase; + +class IgnoreListTest extends TestCase +{ + + + /** + * Test ignoringNone() works. + * + * @covers PHP_CodeSniffer\Util\IgnoreList::ignoringNone + * @return void + */ + public function testIgnoringNoneWorks() + { + $ignoreList = IgnoreList::ignoringNone(); + $this->assertInstanceOf(IgnoreList::class, $ignoreList); + $this->assertFalse($ignoreList->check('Anything')); + + }//end testIgnoringNoneWorks() + + + /** + * Test ignoringAll() works. + * + * @covers PHP_CodeSniffer\Util\IgnoreList::ignoringAll + * @return void + */ + public function testIgnoringAllWorks() + { + $ignoreList = IgnoreList::ignoringAll(); + $this->assertInstanceOf(IgnoreList::class, $ignoreList); + $this->assertTrue($ignoreList->check('Anything')); + + }//end testIgnoringAllWorks() + + + /** + * Test isEmpty() and isAll(). + * + * @param IgnoreList $ignoreList IgnoreList to test. + * @param bool $expectEmpty Expected return value from isEmpty(). + * @param bool $expectAll Expected return value from isAll(). + * + * @return void + * + * @dataProvider dataIsEmptyAndAll + * @covers PHP_CodeSniffer\Util\IgnoreList::isEmpty + * @covers PHP_CodeSniffer\Util\IgnoreList::isAll + */ + public function testIsEmptyAndAll($ignoreList, $expectEmpty, $expectAll) + { + $this->assertSame($expectEmpty, $ignoreList->isEmpty()); + $this->assertSame($expectAll, $ignoreList->isAll()); + + }//end testIsEmptyAndAll() + + + /** + * Data provider. + * + * @see testIsEmptyAndAll() + * + * @return array + */ + public function dataIsEmptyAndAll() + { + return [ + 'fresh list' => [ + new IgnoreList(), + true, + false, + ], + 'list from ignoringNone' => [ + IgnoreList::ignoringNone(), + true, + false, + ], + 'list from ignoringAll' => [ + IgnoreList::ignoringAll(), + false, + true, + ], + 'list from ignoringNone, something set to false' => [ + IgnoreList::ignoringNone()->set('Foo.Bar', false), + true, + false, + ], + 'list from ignoringNone, something set to true' => [ + IgnoreList::ignoringNone()->set('Foo.Bar', true), + false, + false, + ], + 'list from ignoringAll, something set to false' => [ + IgnoreList::ignoringAll()->set('Foo.Bar', false), + false, + false, + ], + 'list from ignoringAll, something set to true' => [ + IgnoreList::ignoringAll()->set('Foo.Bar', true), + false, + true, + ], + 'list from ignoringNone, something set to true then overridden' => [ + IgnoreList::ignoringNone()->set('Foo.Bar', true)->set('Foo', false), + true, + false, + ], + 'list from ignoringAll, something set to false then overridden' => [ + IgnoreList::ignoringAll()->set('Foo.Bar', false)->set('Foo', true), + false, + true, + ], + ]; + + }//end dataIsEmptyAndAll() + + + /** + * Test check() and set(). + * + * @param array $toSet Associative array of $code => $ignore to pass to set(). + * @param array $toCheck Associative array of $code => $expect to pass to check(). + * + * @return void + * + * @dataProvider dataCheckAndSet + * @covers PHP_CodeSniffer\Util\IgnoreList::check + * @covers PHP_CodeSniffer\Util\IgnoreList::set + */ + public function testCheckAndSet($toSet, $toCheck) + { + $ignoreList = new IgnoreList(); + foreach ($toSet as $code => $ignore) { + $this->assertSame($ignoreList, $ignoreList->set($code, $ignore)); + } + + foreach ($toCheck as $code => $expect) { + $this->assertSame($expect, $ignoreList->check($code)); + } + + }//end testCheckAndSet() + + + /** + * Data provider. + * + * @see testCheckAndSet() + * + * @return array + */ + public function dataCheckAndSet() + { + return [ + 'set a code' => [ + ['Standard.Category.Sniff.Code' => true], + [ + 'Standard.Category.Sniff.Code' => true, + 'Standard.Category.Sniff.OtherCode' => false, + 'Standard.Category.OtherSniff.Code' => false, + 'Standard.OtherCategory.Sniff.Code' => false, + 'OtherStandard.Category.Sniff.Code' => false, + ], + ], + 'set a sniff' => [ + ['Standard.Category.Sniff' => true], + [ + 'Standard.Category.Sniff.Code' => true, + 'Standard.Category.Sniff.OtherCode' => true, + 'Standard.Category.OtherSniff.Code' => false, + 'Standard.OtherCategory.Sniff.Code' => false, + 'OtherStandard.Category.Sniff.Code' => false, + ], + ], + 'set a category' => [ + ['Standard.Category' => true], + [ + 'Standard.Category.Sniff.Code' => true, + 'Standard.Category.Sniff.OtherCode' => true, + 'Standard.Category.OtherSniff.Code' => true, + 'Standard.OtherCategory.Sniff.Code' => false, + 'OtherStandard.Category.Sniff.Code' => false, + ], + ], + 'set a standard' => [ + ['Standard' => true], + [ + 'Standard.Category.Sniff.Code' => true, + 'Standard.Category.Sniff.OtherCode' => true, + 'Standard.Category.OtherSniff.Code' => true, + 'Standard.OtherCategory.Sniff.Code' => true, + 'OtherStandard.Category.Sniff.Code' => false, + ], + ], + 'set a standard, unignore a sniff in it' => [ + [ + 'Standard' => true, + 'Standard.Category.Sniff' => false, + ], + [ + 'Standard.Category.Sniff.Code' => false, + 'Standard.Category.Sniff.OtherCode' => false, + 'Standard.Category.OtherSniff.Code' => true, + 'Standard.OtherCategory.Sniff.Code' => true, + 'OtherStandard.Category.Sniff.Code' => false, + ], + ], + 'set a standard, unignore a category in it, ignore a sniff in that' => [ + [ + 'Standard' => true, + 'Standard.Category' => false, + 'Standard.Category.Sniff' => true, + ], + [ + 'Standard.Category.Sniff.Code' => true, + 'Standard.Category.Sniff.OtherCode' => true, + 'Standard.Category.OtherSniff.Code' => false, + 'Standard.OtherCategory.Sniff.Code' => true, + 'OtherStandard.Category.Sniff.Code' => false, + ], + ], + 'ignore some sniffs, then override some of those by unignoring the whole category' => [ + [ + 'Standard.Category1.Sniff1' => true, + 'Standard.Category1.Sniff2' => true, + 'Standard.Category2.Sniff1' => true, + 'Standard.Category2.Sniff2' => true, + 'Standard.Category1' => false, + ], + [ + 'Standard.Category1.Sniff1' => false, + 'Standard.Category1.Sniff2' => false, + 'Standard.Category2.Sniff1' => true, + 'Standard.Category2.Sniff2' => true, + ], + ], + ]; + + }//end dataCheckAndSet() + + +}//end class