Skip to content

Commit 61d18c1

Browse files
jrfnljrfnl
committed
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.
1 parent 119d4cf commit 61d18c1

File tree

7 files changed

+180
-7
lines changed

7 files changed

+180
-7
lines changed

‎src/Files/File.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -865,9 +865,11 @@ protected function addMessage($error, $message, $line, $column, $code, $data, $s
865865
}
866866

867867
// Filter out any messages for sniffs that shouldn't have run
868-
// due to the use of the --sniffs command line argument.
868+
// due to the use of the --sniffs or --exclude command line argument,
869+
// but don't filter out "Internal" messages.
869870
if ($includeAll === false
870-
&& ((empty($this->configCache['sniffs']) === false
871+
&& (($parts[0] !== 'Internal'
872+
&& empty($this->configCache['sniffs']) === false
871873
&& in_array(strtolower($listenerCode), $this->configCache['sniffs'], true) === false)
872874
|| (empty($this->configCache['exclude']) === false
873875
&& in_array(strtolower($listenerCode), $this->configCache['exclude'], true) === true))

‎src/Standards/Generic/Tests/Files/ByteOrderMarkUnitTest.php‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,25 @@ public function getErrorList($testFile='')
5151
* The key of the array should represent the line number and the value
5252
* should represent the number of warnings that should occur on that line.
5353
*
54+
* @param string $testFile The name of the file being tested.
55+
*
5456
* @return array<int, int>
5557
*/
56-
public function getWarningList()
58+
public function getWarningList($testFile='')
5759
{
58-
return [];
60+
switch ($testFile) {
61+
case 'ByteOrderMarkUnitTest.3.inc':
62+
case 'ByteOrderMarkUnitTest.4.inc':
63+
case 'ByteOrderMarkUnitTest.5.inc':
64+
if ((bool) ini_get('short_open_tag') === false) {
65+
// Warning about "no code found in file".
66+
return [1 => 1];
67+
}
68+
return [];
69+
70+
default:
71+
return [];
72+
}
5973

6074
}//end getWarningList()
6175

‎src/Standards/Generic/Tests/PHP/DisallowAlternativePHPTagsUnitTest.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,15 @@ public function getErrorList($testFile='')
6161
public function getWarningList($testFile='')
6262
{
6363
if ($testFile === 'DisallowAlternativePHPTagsUnitTest.2.inc') {
64+
// Check if the Internal.NoCodeFound error can be expected on line 1.
65+
$option = (bool) ini_get('short_open_tag');
66+
$line1 = 1;
67+
if ($option === true) {
68+
$line1 = 0;
69+
}
70+
6471
return [
72+
1 => $line1,
6573
2 => 1,
6674
3 => 1,
6775
4 => 1,

‎src/Standards/Generic/Tests/PHP/DisallowShortOpenTagUnitTest.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,15 @@ public function getErrorList($testFile='')
9191
public function getWarningList($testFile='')
9292
{
9393
switch ($testFile) {
94-
case 'DisallowShortOpenTagUnitTest.1.inc':
95-
return [];
9694
case 'DisallowShortOpenTagUnitTest.3.inc':
95+
// Check if the Internal.NoCodeFound error can be expected on line 1.
96+
$option = (bool) ini_get('short_open_tag');
97+
$line1 = 1;
98+
if ($option === true) {
99+
$line1 = 0;
100+
}
97101
return [
102+
1 => $line1,
98103
3 => 1,
99104
6 => 1,
100105
11 => 1,

‎src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.php‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,19 @@ public function getErrorList($testFile='')
229229
* The key of the array should represent the line number and the value
230230
* should represent the number of warnings that should occur on that line.
231231
*
232+
* @param string $testFile The name of the file being tested.
233+
*
232234
* @return array<int, int>
233235
*/
234-
public function getWarningList()
236+
public function getWarningList($testFile='')
235237
{
238+
if ($testFile === 'EmbeddedPhpUnitTest.24.inc'
239+
&& (bool) ini_get('short_open_tag') === false
240+
) {
241+
// Warning about "no code found in file".
242+
return [1 => 1];
243+
}
244+
236245
return [];
237246

238247
}//end getWarningList()

‎tests/Core/File/AddMessageSelectiveInternalHandlingTest.php‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
<?php
2+
/**
3+
* Tests that handling of Internal errors in combination with sniff selection.
4+
*
5+
* @copyright 2025 PHPCSStandards and contributors
6+
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
7+
*/
8+
9+
namespace PHP_CodeSniffer\Tests\Core\File;
10+
11+
use PHP_CodeSniffer\Files\DummyFile;
12+
use PHP_CodeSniffer\Ruleset;
13+
use PHP_CodeSniffer\Tests\ConfigDouble;
14+
use PHPUnit\Framework\TestCase;
15+
16+
/**
17+
* Tests that handling of Internal errors in combination with sniff selection.
18+
*
19+
* @covers \PHP_CodeSniffer\Files\File::addMessage
20+
*/
21+
final class AddMessageSelectiveInternalHandlingTest extends TestCase
22+
{
23+
24+
25+
/**
26+
* Verify that if an Internal code is silenced from the ruleset, it will stay silenced, independently of sniff selection.
27+
*
28+
* @param string $sniffs Sniff selection.
29+
* @param string $exclude Sniff exclusions.
30+
*
31+
* @dataProvider dataSniffSelection
32+
*
33+
* @return void
34+
*/
35+
public function testRulesetIgnoredInternalErrorIsIgnored($sniffs, $exclude)
36+
{
37+
$phpcsFile = $this->getPhpcsFile($sniffs, $exclude);
38+
39+
$added = $phpcsFile->addError('No code found', 0, 'Internal.NoCodeFound');
40+
$this->assertFalse($added);
41+
42+
}//end testRulesetIgnoredInternalErrorIsIgnored()
43+
44+
45+
/**
46+
* Verify that if an Internal code is NOT silenced from the ruleset, sniff selection doesn't silence it.
47+
*
48+
* @param string $sniffs Sniff selection.
49+
* @param string $exclude Sniff exclusions.
50+
*
51+
* @dataProvider dataSniffSelection
52+
*
53+
* @return void
54+
*/
55+
public function testOtherInternalErrorIsNotIgnored($sniffs, $exclude)
56+
{
57+
$phpcsFile = $this->getPhpcsFile($sniffs, $exclude);
58+
59+
$added = $phpcsFile->addError('Some other error', 0, 'Internal.SomeError');
60+
$this->assertTrue($added);
61+
62+
}//end testOtherInternalErrorIsNotIgnored()
63+
64+
65+
/**
66+
* Data provider.
67+
*
68+
* @see testA()
69+
*
70+
* @return array<string, array<string, string>>
71+
*/
72+
public static function dataSniffSelection()
73+
{
74+
return [
75+
'Ruleset only, no CLI selection' => [
76+
'sniffs' => '',
77+
'exclude' => '',
78+
],
79+
'Ruleset + CLI sniffs selection' => [
80+
'sniffs' => 'Generic.Files.ByteOrderMark,Generic.PHP.DisallowShortOpenTag',
81+
'exclude' => '',
82+
],
83+
'Ruleset + CLI exclude selection' => [
84+
'sniffs' => '',
85+
'exclude' => 'Generic.Files.ByteOrderMark,Generic.PHP.DisallowShortOpenTag',
86+
],
87+
];
88+
89+
}//end dataSniffSelection()
90+
91+
92+
/**
93+
* Test Helper to get a File object for use in these tests.
94+
*
95+
* @param string $sniffs Sniff selection.
96+
* @param string $exclude Sniff exclusions.
97+
*
98+
* @return \PHP_CodeSniffer\Files\DummyFile
99+
*/
100+
private function getPhpcsFile($sniffs, $exclude)
101+
{
102+
// Set up the ruleset.
103+
$standard = __DIR__.'/AddMessageSelectiveInternalHandlingTest.xml';
104+
$args = ["--standard=$standard"];
105+
106+
if (empty($sniffs) === false) {
107+
$args[] = "--sniffs=$sniffs";
108+
}
109+
110+
if (empty($exclude) === false) {
111+
$args[] = "--exclude=$exclude";
112+
}
113+
114+
$config = new ConfigDouble($args);
115+
$ruleset = new Ruleset($config);
116+
117+
$content = '<?php '."\necho 'hello!';\n";
118+
$phpcsFile = new DummyFile($content, $ruleset, $config);
119+
$phpcsFile->parse();
120+
121+
return $phpcsFile;
122+
123+
}//end getPhpcsFile()
124+
125+
126+
}//end class

‎tests/Core/File/AddMessageSelectiveInternalHandlingTest.xml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?xml version="1.0"?>
2+
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="AddMessageSelectiveInternalHandlingTest" xsi:noNamespaceSchemaLocation="https://raw.githubusercontent.com/PHPCSStandards/PHP_CodeSniffer/master/phpcs.xsd">
3+
4+
<rule ref="Internal.NoCodeFound">
5+
<severity>0</severity>
6+
</rule>
7+
8+
<rule ref="PSR1"/>
9+
</ruleset>

0 commit comments

Comments
 (0)