Skip to content

Commit 1cad124

Browse files
jrfnljrfnl
committed
Filter: bugfix for shouldIgnorePath() ignoring paths containing the name of a standard
The file list filtering in the `Filter` class should only take global exclude patterns into account and should ignore sniff specific exclude patterns, which are used/handled in the `File::addMessage()` method. This commit fixes this. Includes: * Reworking the initial unit test to use `setUpBeforeClass()` for the fixtures and a data provider to easily allow for more test cases without code duplication. * Adding an additional unit test to verify that top-level exclude patterns are correctly respected. * Renaming the test ruleset file to use the `xml` file extension. * Adding the newly introduced unit test files to the `package.xml` file lists. Fixes 2391
1 parent b813cd6 commit 1cad124

File tree

5 files changed

+121
-20
lines changed

5 files changed

+121
-20
lines changed

‎package.xml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,12 @@ http://pear.php.net/dtd/package-2.0.xsd">
138138
<file baseinstalldir="" name="IsReferenceTest.inc" role="test" />
139139
<file baseinstalldir="" name="IsReferenceTest.php" role="test" />
140140
</dir>
141+
<dir name="Filters">
142+
<dir name="Filter">
143+
<file baseinstalldir="" name="AcceptTest.xml" role="test" />
144+
<file baseinstalldir="" name="AcceptTest.php" role="test" />
145+
</dir>
146+
</dir>
141147
<file baseinstalldir="" name="AllTests.php" role="test" />
142148
<file baseinstalldir="" name="ErrorSuppressionTest.php" role="test" />
143149
<file baseinstalldir="" name="IsCamelCapsTest.php" role="test" />
@@ -1856,6 +1862,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
18561862
<install as="CodeSniffer/Core/File/GetMethodPropertiesTest.inc" name="tests/Core/File/GetMethodPropertiesTest.inc" />
18571863
<install as="CodeSniffer/Core/File/IsReferenceTest.php" name="tests/Core/File/IsReferenceTest.php" />
18581864
<install as="CodeSniffer/Core/File/IsReferenceTest.inc" name="tests/Core/File/IsReferenceTest.inc" />
1865+
<install as="CodeSniffer/Core/Filters/Filter/AcceptTest.php" name="tests/Core/Filters/Filter/AcceptTest.php" />
1866+
<install as="CodeSniffer/Core/Filters/Filter/AcceptTest.xml" name="tests/Core/Filters/Filter/AcceptTest.xml" />
18591867
<install as="CodeSniffer/Standards/AllSniffs.php" name="tests/Standards/AllSniffs.php" />
18601868
<install as="CodeSniffer/Standards/AbstractSniffUnitTest.php" name="tests/Standards/AbstractSniffUnitTest.php" />
18611869
</filelist>
@@ -1889,6 +1897,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
18891897
<install as="CodeSniffer/Core/File/GetMethodPropertiesTest.inc" name="tests/Core/File/GetMethodPropertiesTest.inc" />
18901898
<install as="CodeSniffer/Core/File/IsReferenceTest.php" name="tests/Core/File/IsReferenceTest.php" />
18911899
<install as="CodeSniffer/Core/File/IsReferenceTest.inc" name="tests/Core/File/IsReferenceTest.inc" />
1900+
<install as="CodeSniffer/Core/Filters/Filter/AcceptTest.php" name="tests/Core/Filters/Filter/AcceptTest.php" />
1901+
<install as="CodeSniffer/Core/Filters/Filter/AcceptTest.xml" name="tests/Core/Filters/Filter/AcceptTest.xml" />
18921902
<install as="CodeSniffer/Standards/AllSniffs.php" name="tests/Standards/AllSniffs.php" />
18931903
<install as="CodeSniffer/Standards/AbstractSniffUnitTest.php" name="tests/Standards/AbstractSniffUnitTest.php" />
18941904
<ignore name="bin/phpcs.bat" />

‎src/Filters/Filter.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,11 @@ protected function shouldIgnorePath($path)
229229
}
230230

231231
foreach ($ignorePatterns as $pattern => $type) {
232+
// Ignore standard/sniff specific exclude rules.
233+
if (is_array($type) === true) {
234+
continue;
235+
}
236+
232237
// Maintains backwards compatibility in case the ignore pattern does
233238
// not have a relative/absolute value.
234239
if (is_int($pattern) === true) {

‎tests/Core/Filters/Filter/AcceptTest.inc

Lines changed: 0 additions & 8 deletions
This file was deleted.

‎tests/Core/Filters/Filter/AcceptTest.php

Lines changed: 90 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
* Tests for the \PHP_CodeSniffer\Filters\Filter::accept method.
44
*
55
* @author Willington Vega <wvega@wvega.com>
6-
* @copyright 2006-2018 Squiz Pty Ltd (ABN 77 084 670 600)
6+
* @author Juliette Reinders Folmer <phpcs_nospam@adviesenzo.nl>
7+
* @copyright 2019 Squiz Pty Ltd (ABN 77 084 670 600)
78
* @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
89
*/
910

@@ -17,33 +18,110 @@
1718
class AcceptTest extends TestCase
1819
{
1920

21+
/**
22+
* The Config object.
23+
*
24+
* @var \PHP_CodeSniffer\Config
25+
*/
26+
protected static $config;
27+
28+
/**
29+
* The Ruleset object.
30+
*
31+
* @var \PHP_CodeSniffer\Ruleset
32+
*/
33+
protected static $ruleset;
34+
2035

2136
/**
22-
* Test paths that include the name of a standard with associated
23-
* exclude-patterns are still accepted.
37+
* Initialize the config and ruleset objects based on the `AcceptTest.xml` ruleset file.
2438
*
2539
* @return void
2640
*/
27-
public function testExcludePatternsForStandards()
41+
public static function setUpBeforeClass()
2842
{
29-
$standard = __DIR__.'/'.basename(__FILE__, '.php').'.inc';
30-
$config = new Config(["--standard=$standard"]);
31-
$ruleset = new Ruleset($config);
43+
$standard = __DIR__.'/'.basename(__FILE__, '.php').'.xml';
44+
self::$config = new Config(["--standard=$standard"]);
45+
self::$ruleset = new Ruleset(self::$config);
46+
47+
}//end setUpBeforeClass()
3248

33-
$paths = ['/path/to/generic-project/src/Main.php'];
3449

35-
$fakeDI = new \RecursiveArrayIterator($paths);
36-
$filter = new Filter($fakeDI, '/path/to/generic-project/src', $config, $ruleset);
50+
/**
51+
* Test filtering a file list for excluded paths.
52+
*
53+
* @param array $inputPaths List of file paths to be filtered.
54+
* @param array $expectedOutput Expected filtering result.
55+
*
56+
* @dataProvider dataExcludePatterns
57+
*
58+
* @return void
59+
*/
60+
public function testExcludePatterns($inputPaths, $expectedOutput)
61+
{
62+
$fakeDI = new \RecursiveArrayIterator($inputPaths);
63+
$filter = new Filter($fakeDI, '/', self::$config, self::$ruleset);
3764
$iterator = new \RecursiveIteratorIterator($filter);
3865
$files = [];
3966

4067
foreach ($iterator as $file) {
4168
$files[] = $file;
4269
}
4370

44-
$this->assertEquals($paths, $files);
71+
$this->assertEquals($expectedOutput, $files);
72+
73+
}//end testExcludePatterns()
74+
75+
76+
/**
77+
* Data provider.
78+
*
79+
* @see testExcludePatterns
80+
*
81+
* @return array
82+
*/
83+
public function dataExcludePatterns()
84+
{
85+
$testCases = [
86+
// Test top-level exclude patterns.
87+
[
88+
[
89+
'/path/to/src/Main.php',
90+
'/path/to/src/Something/Main.php',
91+
'/path/to/src/Other/Main.php',
92+
],
93+
['/path/to/src/Main.php'],
94+
],
95+
96+
// Test ignoring standard/sniff specific exclude patterns.
97+
[
98+
[
99+
'/path/to/src/generic-project/Main.php',
100+
'/path/to/src/generic/Main.php',
101+
'/path/to/src/anything-generic/Main.php',
102+
],
103+
[
104+
'/path/to/src/generic-project/Main.php',
105+
'/path/to/src/generic/Main.php',
106+
'/path/to/src/anything-generic/Main.php',
107+
],
108+
],
109+
];
110+
111+
// Allow these tests to work on Windows as well.
112+
if (DIRECTORY_SEPARATOR === '\\') {
113+
foreach ($testCases as $key => $case) {
114+
foreach ($case as $nr => $param) {
115+
foreach ($param as $file => $value) {
116+
$testCases[$key][$nr][$file] = strtr($value, '/', '\\');
117+
}
118+
}
119+
}
120+
}
121+
122+
return $testCases;
45123

46-
}//end testExcludePatternsForStandards()
124+
}//end dataExcludePatterns()
47125

48126

49127
}//end class

‎tests/Core/Filters/Filter/AcceptTest.xml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?xml version="1.0"?>
2+
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="AcceptTest" xsi:noNamespaceSchemaLocation="phpcs.xsd">
3+
<description>Ruleset to test the filtering based on exclude patterns.</description>
4+
5+
<!-- Directory pattern. -->
6+
<exclude-pattern>*/something/*</exclude-pattern>
7+
<!-- File pattern. -->
8+
<exclude-pattern>*/Other/Main\.php$</exclude-pattern>
9+
10+
<rule ref="Generic">
11+
<!-- Standard specific directory pattern. -->
12+
<exclude-pattern>/anything/*</exclude-pattern>
13+
<!-- Standard specific file pattern. -->
14+
<exclude-pattern>/YetAnother/Main\.php</exclude-pattern>
15+
</rule>
16+
</ruleset>

0 commit comments

Comments
 (0)