From 119d4cfada6a1940e21e2b38f5ab1c5da264e7cc Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 31 Oct 2023 19:07:33 +0100 Subject: [PATCH 1/2] ErrorSuppressionTest: prevent potential `Internal` errors ... about a mismatch in line endings when the code "template" is defined using a heredoc with Linux line endings, while the code snippets being inserted into the code "template" were using line endings matching the OS on which the tests were being run. --- tests/Core/ErrorSuppressionTest.php | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/Core/ErrorSuppressionTest.php b/tests/Core/ErrorSuppressionTest.php index 22581565c7..7d8c8366b0 100644 --- a/tests/Core/ErrorSuppressionTest.php +++ b/tests/Core/ErrorSuppressionTest.php @@ -510,15 +510,15 @@ public static function dataNestedSuppressLine() // Process with line suppression nested within disable/enable suppression. 'disable/enable: slash comment, next line nested single line suppression' => [ - 'before' => '// phpcs:disable'.PHP_EOL.'// phpcs:ignore', + 'before' => "// phpcs:disable\n// phpcs:ignore", 'after' => '// phpcs:enable', ], 'disable/enable: slash comment, with @, next line nested single line suppression' => [ - 'before' => '// @phpcs:disable'.PHP_EOL.'// @phpcs:ignore', + 'before' => "// @phpcs:disable\n// @phpcs:ignore", 'after' => '// @phpcs:enable', ], 'disable/enable: hash comment, next line nested single line suppression' => [ - 'before' => '# @phpcs:disable'.PHP_EOL.'# @phpcs:ignore', + 'before' => "# @phpcs:disable\n# @phpcs:ignore", 'after' => '# @phpcs:enable', ], ]; @@ -689,7 +689,7 @@ public static function dataSuppressFile() 'before' => '/* phpcs:ignoreFile */', ], 'ignoreFile: start of file, multi-line star comment' => [ - 'before' => '/*'.PHP_EOL.' phpcs:ignoreFile'.PHP_EOL.' */', + 'before' => "/*\n phpcs:ignoreFile\n */", ], 'ignoreFile: start of file, single-line docblock comment' => [ 'before' => '/** phpcs:ignoreFile */', @@ -771,11 +771,11 @@ public static function dataDisableSelected() 'expectedErrors' => 1, ], 'disable: single sniff, docblock' => [ - 'before' => '/**'.PHP_EOL.' * phpcs:disable Generic.Commenting.Todo'.PHP_EOL.' */ ', + 'before' => "/**\n * phpcs:disable Generic.Commenting.Todo\n */ ", 'expectedErrors' => 1, ], 'disable: single sniff, docblock, with @' => [ - 'before' => '/**'.PHP_EOL.' * @phpcs:disable Generic.Commenting.Todo'.PHP_EOL.' */ ', + 'before' => "/**\n * @phpcs:disable Generic.Commenting.Todo\n */ ", 'expectedErrors' => 1, ], @@ -784,7 +784,7 @@ public static function dataDisableSelected() 'before' => '// phpcs:disable Generic.Commenting.Todo,Generic.PHP.LowerCaseConstant', ], 'disable: multiple sniff in multiple comments' => [ - 'before' => '// phpcs:disable Generic.Commenting.Todo'.PHP_EOL.'// phpcs:disable Generic.PHP.LowerCaseConstant', + 'before' => "// phpcs:disable Generic.Commenting.Todo\n// phpcs:disable Generic.PHP.LowerCaseConstant", ], // Selectiveness variations. @@ -805,17 +805,17 @@ public static function dataDisableSelected() // Wrong category/sniff/code. 'disable: wrong error code and category' => [ - 'before' => '/**'.PHP_EOL.' * phpcs:disable Generic.PHP.LowerCaseConstant.Upper,Generic.Comments'.PHP_EOL.' */ ', + 'before' => "/**\n * phpcs:disable Generic.PHP.LowerCaseConstant.Upper,Generic.Comments\n */ ", 'expectedErrors' => 1, 'expectedWarnings' => 1, ], 'disable: wrong category, docblock' => [ - 'before' => '/**'.PHP_EOL.' * phpcs:disable Generic.Files'.PHP_EOL.' */ ', + 'before' => "/**\n * phpcs:disable Generic.Files\n */ ", 'expectedErrors' => 1, 'expectedWarnings' => 1, ], 'disable: wrong category, docblock, with @' => [ - 'before' => '/**'.PHP_EOL.' * @phpcs:disable Generic.Files'.PHP_EOL.' */ ', + 'before' => "/**\n * @phpcs:disable Generic.Files\n */ ", 'expectedErrors' => 1, 'expectedWarnings' => 1, ], @@ -1070,7 +1070,7 @@ public static function dataIgnoreSelected() 'expectedWarnings' => 1, ], 'disable: single sniff; ignore: single sniff' => [ - 'before' => '// phpcs:disable Generic.Commenting.Todo'.PHP_EOL.'// phpcs:ignore Generic.PHP.LowerCaseConstant', + 'before' => "// phpcs:disable Generic.Commenting.Todo\n// phpcs:ignore Generic.PHP.LowerCaseConstant", 'expectedErrors' => 1, 'expectedWarnings' => 0, ], From 0451fd96d31001b9bfc8168823a211e8deec0da5 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 17 Mar 2025 06:21:22 +0100 Subject: [PATCH 2/2] File::addMessage(): do not ignore `Internal` errors when scanning selectively When either the `--sniffs=...` CLI parameter is used, or the `--exclude=...` CLI parameter, the `File::addMessage()` method bows out when an error is passed which is not for one of the selected sniffs/is for one of the excluded sniffs. Unfortunately, this "bowing out" did not take `Internal` errors into account, meaning those were now hidden, while those should _always_ be thrown as they generally inform the end-user of something seriously wrong (mixed line endings/no code found etc). Fixed now. Includes updating four test files to allow for seeing internal errors. Also includes some dedicated tests to make sure that this doesn't interfere with the ability to silence `Internal` errors from within a ruleset file. --- src/Files/File.php | 6 +- .../Tests/Files/ByteOrderMarkUnitTest.php | 18 ++- .../DisallowAlternativePHPTagsUnitTest.php | 8 ++ .../PHP/DisallowShortOpenTagUnitTest.php | 9 +- .../Squiz/Tests/PHP/EmbeddedPhpUnitTest.php | 11 +- ...ddMessageSelectiveInternalHandlingTest.php | 126 ++++++++++++++++++ ...ddMessageSelectiveInternalHandlingTest.xml | 9 ++ 7 files changed, 180 insertions(+), 7 deletions(-) create mode 100644 tests/Core/File/AddMessageSelectiveInternalHandlingTest.php create mode 100644 tests/Core/File/AddMessageSelectiveInternalHandlingTest.xml diff --git a/src/Files/File.php b/src/Files/File.php index dbaaf3b204..a7a5eae846 100644 --- a/src/Files/File.php +++ b/src/Files/File.php @@ -865,9 +865,11 @@ protected function addMessage($error, $message, $line, $column, $code, $data, $s } // Filter out any messages for sniffs that shouldn't have run - // due to the use of the --sniffs command line argument. + // due to the use of the --sniffs or --exclude command line argument, + // but don't filter out "Internal" messages. if ($includeAll === false - && ((empty($this->configCache['sniffs']) === false + && (($parts[0] !== 'Internal' + && empty($this->configCache['sniffs']) === false && in_array(strtolower($listenerCode), $this->configCache['sniffs'], true) === false) || (empty($this->configCache['exclude']) === false && in_array(strtolower($listenerCode), $this->configCache['exclude'], true) === true)) diff --git a/src/Standards/Generic/Tests/Files/ByteOrderMarkUnitTest.php b/src/Standards/Generic/Tests/Files/ByteOrderMarkUnitTest.php index e616e74a4c..634158def0 100644 --- a/src/Standards/Generic/Tests/Files/ByteOrderMarkUnitTest.php +++ b/src/Standards/Generic/Tests/Files/ByteOrderMarkUnitTest.php @@ -51,11 +51,25 @@ public function getErrorList($testFile='') * The key of the array should represent the line number and the value * should represent the number of warnings that should occur on that line. * + * @param string $testFile The name of the file being tested. + * * @return array */ - public function getWarningList() + public function getWarningList($testFile='') { - return []; + switch ($testFile) { + case 'ByteOrderMarkUnitTest.3.inc': + case 'ByteOrderMarkUnitTest.4.inc': + case 'ByteOrderMarkUnitTest.5.inc': + if ((bool) ini_get('short_open_tag') === false) { + // Warning about "no code found in file". + return [1 => 1]; + } + return []; + + default: + return []; + } }//end getWarningList() diff --git a/src/Standards/Generic/Tests/PHP/DisallowAlternativePHPTagsUnitTest.php b/src/Standards/Generic/Tests/PHP/DisallowAlternativePHPTagsUnitTest.php index de88b5cb07..fabeeed17e 100644 --- a/src/Standards/Generic/Tests/PHP/DisallowAlternativePHPTagsUnitTest.php +++ b/src/Standards/Generic/Tests/PHP/DisallowAlternativePHPTagsUnitTest.php @@ -61,7 +61,15 @@ public function getErrorList($testFile='') public function getWarningList($testFile='') { if ($testFile === 'DisallowAlternativePHPTagsUnitTest.2.inc') { + // Check if the Internal.NoCodeFound error can be expected on line 1. + $option = (bool) ini_get('short_open_tag'); + $line1 = 1; + if ($option === true) { + $line1 = 0; + } + return [ + 1 => $line1, 2 => 1, 3 => 1, 4 => 1, diff --git a/src/Standards/Generic/Tests/PHP/DisallowShortOpenTagUnitTest.php b/src/Standards/Generic/Tests/PHP/DisallowShortOpenTagUnitTest.php index a4eafd66af..46a6d7ee98 100644 --- a/src/Standards/Generic/Tests/PHP/DisallowShortOpenTagUnitTest.php +++ b/src/Standards/Generic/Tests/PHP/DisallowShortOpenTagUnitTest.php @@ -91,10 +91,15 @@ public function getErrorList($testFile='') public function getWarningList($testFile='') { switch ($testFile) { - case 'DisallowShortOpenTagUnitTest.1.inc': - return []; case 'DisallowShortOpenTagUnitTest.3.inc': + // Check if the Internal.NoCodeFound error can be expected on line 1. + $option = (bool) ini_get('short_open_tag'); + $line1 = 1; + if ($option === true) { + $line1 = 0; + } return [ + 1 => $line1, 3 => 1, 6 => 1, 11 => 1, diff --git a/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.php b/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.php index 88c658484a..04fd767068 100644 --- a/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.php +++ b/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.php @@ -229,10 +229,19 @@ public function getErrorList($testFile='') * The key of the array should represent the line number and the value * should represent the number of warnings that should occur on that line. * + * @param string $testFile The name of the file being tested. + * * @return array */ - public function getWarningList() + public function getWarningList($testFile='') { + if ($testFile === 'EmbeddedPhpUnitTest.24.inc' + && (bool) ini_get('short_open_tag') === false + ) { + // Warning about "no code found in file". + return [1 => 1]; + } + return []; }//end getWarningList() diff --git a/tests/Core/File/AddMessageSelectiveInternalHandlingTest.php b/tests/Core/File/AddMessageSelectiveInternalHandlingTest.php new file mode 100644 index 0000000000..d32e480290 --- /dev/null +++ b/tests/Core/File/AddMessageSelectiveInternalHandlingTest.php @@ -0,0 +1,126 @@ +getPhpcsFile($sniffs, $exclude); + + $added = $phpcsFile->addError('No code found', 0, 'Internal.NoCodeFound'); + $this->assertFalse($added); + + }//end testRulesetIgnoredInternalErrorIsIgnored() + + + /** + * Verify that if an Internal code is NOT silenced from the ruleset, sniff selection doesn't silence it. + * + * @param string $sniffs Sniff selection. + * @param string $exclude Sniff exclusions. + * + * @dataProvider dataSniffSelection + * + * @return void + */ + public function testOtherInternalErrorIsNotIgnored($sniffs, $exclude) + { + $phpcsFile = $this->getPhpcsFile($sniffs, $exclude); + + $added = $phpcsFile->addError('Some other error', 0, 'Internal.SomeError'); + $this->assertTrue($added); + + }//end testOtherInternalErrorIsNotIgnored() + + + /** + * Data provider. + * + * @see testA() + * + * @return array> + */ + public static function dataSniffSelection() + { + return [ + 'Ruleset only, no CLI selection' => [ + 'sniffs' => '', + 'exclude' => '', + ], + 'Ruleset + CLI sniffs selection' => [ + 'sniffs' => 'Generic.Files.ByteOrderMark,Generic.PHP.DisallowShortOpenTag', + 'exclude' => '', + ], + 'Ruleset + CLI exclude selection' => [ + 'sniffs' => '', + 'exclude' => 'Generic.Files.ByteOrderMark,Generic.PHP.DisallowShortOpenTag', + ], + ]; + + }//end dataSniffSelection() + + + /** + * Test Helper to get a File object for use in these tests. + * + * @param string $sniffs Sniff selection. + * @param string $exclude Sniff exclusions. + * + * @return \PHP_CodeSniffer\Files\DummyFile + */ + private function getPhpcsFile($sniffs, $exclude) + { + // Set up the ruleset. + $standard = __DIR__.'/AddMessageSelectiveInternalHandlingTest.xml'; + $args = ["--standard=$standard"]; + + if (empty($sniffs) === false) { + $args[] = "--sniffs=$sniffs"; + } + + if (empty($exclude) === false) { + $args[] = "--exclude=$exclude"; + } + + $config = new ConfigDouble($args); + $ruleset = new Ruleset($config); + + $content = 'parse(); + + return $phpcsFile; + + }//end getPhpcsFile() + + +}//end class diff --git a/tests/Core/File/AddMessageSelectiveInternalHandlingTest.xml b/tests/Core/File/AddMessageSelectiveInternalHandlingTest.xml new file mode 100644 index 0000000000..49ce798b1a --- /dev/null +++ b/tests/Core/File/AddMessageSelectiveInternalHandlingTest.xml @@ -0,0 +1,9 @@ + + + + + 0 + + + +