Skip to content

Commit

Permalink
Fixed bug #2391 : Sniff-specific ignore rules inside rulesets are fil…
Browse files Browse the repository at this point in the history
…tering out too many files

Moved is_array check earlier in shouldIgnorePath() as:
    $this->config->ignored was an array of pattern => string,
where as
    $this->ruleset->getIgnorePatterns() was an array of ref => [pattern => string]

Which meant the original array_merge was not merging the arrays correctly, which caused all files to be excluded as when the rule ref name appeared in the projects path.
  • Loading branch information
peternolland committed May 23, 2019
1 parent ad34a36 commit 8b713e1
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 14 deletions.
2 changes: 2 additions & 0 deletions package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
-- The new error code is Squiz.PHP.DisallowMultipleAssignments.FoundInControlStructure
-- All other multiple assignment cases use the existing error code Squiz.PHP.DisallowMultipleAssignments.Found
-- Thanks to Juliette Reinders Folmer for the patch
- Fixed bug #2391 : Sniff-specific ignore rules inside rulesets are filtering out too many files
-- Thanks to Willington Vega for the patch
- Fixed bug #2478 : FunctionCommentThrowTag.WrongNumber when exception is thrown once but built conditionally
- Fixed bug #2479 : Generic.WhiteSpace.ScopeIndent error when using array destructing with exact indent checking
- Fixed bug #2498 : Squiz.Arrays.ArrayDeclaration.MultiLineNotAllowed autofix breaks heredoc
Expand Down
19 changes: 12 additions & 7 deletions src/Filters/Filter.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,17 @@ protected function shouldIgnorePath($path)
$this->ignoreDirPatterns = [];
$this->ignoreFilePatterns = [];

$ignorePatterns = array_merge($this->config->ignored, $this->ruleset->getIgnorePatterns());
$ignorePatterns = $this->config->ignored;
$rulesetIgnorePatterns = $this->ruleset->getIgnorePatterns();
foreach ($rulesetIgnorePatterns as $pattern => $type) {
// Ignore standard/sniff specific exclude rules.
if (is_array($type) === true) {
continue;
}

$ignorePatterns[$pattern] = $type;
}

foreach ($ignorePatterns as $pattern => $type) {
// If the ignore pattern ends with /* then it is ignoring an entire directory.
if (substr($pattern, -2) === '/*') {
Expand All @@ -214,7 +224,7 @@ protected function shouldIgnorePath($path)
$this->ignoreFilePatterns[$pattern] = $type;
}
}
}
}//end if

$relativePath = $path;
if (strpos($path, $this->basedir) === 0) {
Expand All @@ -229,11 +239,6 @@ protected function shouldIgnorePath($path)
}

foreach ($ignorePatterns as $pattern => $type) {
// Ignore standard/sniff specific exclude rules.
if (is_array($type) === true) {
continue;
}

// Maintains backwards compatibility in case the ignore pattern does
// not have a relative/absolute value.
if (is_int($pattern) === true) {
Expand Down
3 changes: 2 additions & 1 deletion tests/Core/Filters/Filter/AcceptTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class AcceptTest extends TestCase
public static function setUpBeforeClass()
{
$standard = __DIR__.'/'.basename(__FILE__, '.php').'.xml';
self::$config = new Config(["--standard=$standard"]);
self::$config = new Config(["--standard=$standard", "--ignore=*/somethingelse/*"]);
self::$ruleset = new Ruleset(self::$config);

}//end setUpBeforeClass()
Expand Down Expand Up @@ -88,6 +88,7 @@ public function dataExcludePatterns()
[
'/path/to/src/Main.php',
'/path/to/src/Something/Main.php',
'/path/to/src/Somethingelse/Main.php',
'/path/to/src/Other/Main.php',
],
['/path/to/src/Main.php'],
Expand Down
12 changes: 6 additions & 6 deletions tests/Core/Filters/Filter/AcceptTest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="AcceptTest" xsi:noNamespaceSchemaLocation="phpcs.xsd">
<description>Ruleset to test the filtering based on exclude patterns.</description>

<!-- Directory pattern. -->
<exclude-pattern>*/something/*</exclude-pattern>
<!-- File pattern. -->
<exclude-pattern>*/Other/Main\.php$</exclude-pattern>
<!-- Directory pattern. -->
<exclude-pattern>*/something/*</exclude-pattern>
<!-- File pattern. -->
<exclude-pattern>*/Other/Main\.php$</exclude-pattern>

<rule ref="Generic">
<!-- Standard specific directory pattern. -->
<!-- Standard specific directory pattern. -->
<exclude-pattern>/anything/*</exclude-pattern>
<!-- Standard specific file pattern. -->
<!-- Standard specific file pattern. -->
<exclude-pattern>/YetAnother/Main\.php</exclude-pattern>
</rule>
</ruleset>

0 comments on commit 8b713e1

Please sign in to comment.