Directory exclusions do not work as expected when a single file name is passed to phpcs

[toolstack]

GlotPress has been using phpcs in conjunction with TravisCI, we've created a phpcs.xml that includes several directory exclusions like:

<exclude-pattern>/tests/*</exclude-pattern>

However these do not function as expected when TravisCI runs against a PR.

Looking through the phpcs code, the shouldIgnorePath() method in /Filters/Filter.php, includes this code:

        if (is_dir($path) === true) {
            $ignorePatterns = $this->ignoreDirPatterns;
        } else {
            $ignorePatterns = $this->ignoreFilePatterns;
        }

Which checks to see if the path passed in to the function is a directory or a file and picks which exclusions to check (directory or file).

However when a file name is passed in to phpcs, like: (which is what happens on a PR with TravisCI)

phpcs --standard=phpcs.xml tests/phpunit/testcases/test_links.php

the directory exclusions are never checked as only the full path and filename are passed in to shouldIgnorePath().

As this individual file is not a directory, only the file ignore patterns are checked against and the filename.

This is easily observable by instead of using a directory pattern of /tests/*, just using /tests in the exclusion-pattern, which will get added as a file pattern.

A fix appears to be to change the above code to:

        if (is_dir($path) === true) {
            $ignorePatterns = $this->ignoreDirPatterns;
        } else {
            $ignorePatterns = array_merge($this->ignoreFilePatterns, $this->ignoreDirPatterns);
        }

Which ensures that when a file is checked, both file and directory patterns are used.

I can create a PR for the above if that makes sense.

[jrfnl]

Interesting, though I have not been able to reproduce the issue.

[toolstack]

Try the following:

PHP-CodeSniffer/bin$ ./phpcs ../tests

Which will output:

................ 16 / 16 (100%)


Time: 162ms; Memory: 6Mb

Now try:

PHP-CodeSniffer/bin$ ./phpcs --ignore=*/tests/* ../tests

Which will output:

... 3 / 3 (100%)


Time: 49ms; Memory: 6Mb

(which is actually incorrect as it should have excluded the three files in the root of the tests directory)

Finally, try:

PHP-CodeSniffer/bin$ ./phpcs --ignore=*/tests/* ../tests/Core/File/FindExtendedClassNameTest.php

Which will output:

. 1 / 1 (100%)


Time: 47ms; Memory: 6Mb

However the expected output should be no files checked as we've explicitly told phpcs not to check files in the tests directory.

This is a result of how the directory tree is "walked" in each case:

• In the second case, the walk is started inside of the tests directory, so shouldIgnorePath() is never called on the tests directory name itself, just on the three files inside of it. Then shouldIgnorePath() is called on the Code and Standards subdirectories and they are excluded from the walk as they match the directory pattern.

• In the third case, shouldIgnorePath() is never called on any directory name as the root of the walk is a single file, which means the directory exclusions are never checked.

If you run the following command:

PHP-CodeSniffer/bin$ ./phpcs --ignore=*/tests ../tests/Core/File/FindExtendedClassNameTest.php

You'll get the expected output of:

Time: 28ms; Memory: 4Mb

because the pattern is stored as a file pattern instead of a directory pattern and the file is excluded "properly" when it is passed to shouldIgnorePath().

[pattonwebz]

Ran accross this issue as well today. I have */tests/* directory as excluded pattern in the ruleset, Travis still fails on items in the tests directory :'(

[toolstack]

@jrfnl have you had a chance to try my above instructions?

[ikappas]

I've run into this issue as well. Can @gsherwood have a look?

[gsherwood]

This is a bug. PHPCS should ignore the file if the standard's (or CLI specified) ignore patterns match it.

[gsherwood]

The Filter class has an optimisation in it (compared to 2.x) so that it only run a regex check for filters that look like they are excluding entire directories (end with /*) when it gets to a directory path. This saves a lot of time if your standard has a lot of file-specific ignore patterns, as someone previously reported to me.

So that works fine, but the code wasn't using those directory-specific ignore patterns when it encountered an individual file path, which happens when you ask PHPCS to check a specific file.

This behaviour was different from 2.x where all patterns were checked for all paths. So the optimisation looks to have caused this issue, which has now been resolved by checking all ignore patterns for file paths, but only directory ignore patterns for directories.

Thanks for reporting this.

[toolstack]

Thanks @gsherwood for resolving this, any timeline for 3.2 yet?

[gsherwood]

any timeline for 3.2 yet?

No idea yet. My day job is pretty crazy at the moment, but I'm getting through the todo list when I can.

[mtangoo]

Encountered an issue similar to this (I need to confirm before I open bug report

$ phpcs --version
PHP_CodeSniffer version 3.5.1 (stable) by Squiz (http://www.squiz.net)

if I run phpcs --standard=PSR12,PSR2 --extensions=php --ignore=*/vendor/,*/tests/,*/runtime/,*/web/ my_file.php it does not work. If I remove */tests/* or rename it to */test/* then it works fine and I get warnings.

Is this the same issue or I should open different ticket?