Rule `<exclude-pattern>` with `type="relative"` don't work
Describe the bug
The rule <exclude-pattern type="relative">don't work when is used with type="relative".
I have a local env that have the /public/ folder in base directory and also in my project I have another directory called /public/. Example: User/Sites/public/source/development/public/example.php.
So I set a custom ruleset file as example below but PHPCS is not considering any file to check (independe of the path, not just to /public/ folder as expected behavior).
Custom ruleset
(...)
<description>Example Description</description>
<arg name="basepath" value="./"/>
<exclude-pattern>/vendor/*</exclude-pattern>
<exclude-pattern>/cypress/*</exclude-pattern>
<exclude-pattern>/.github/*</exclude-pattern>
<exclude-pattern>/tests/*</exclude-pattern>
<exclude-pattern type="relative">^public/.*</exclude-pattern>
(...)
Checking the PHPCS source code inside a src/Filters/Filter.php I found the root cause. This class expect receive a $basedir as a second parameter.
But in all cases that is called from src/Files/FileList.php in fact it is receiving a file path as value to parameter.
So this will impact directly on method shouldIgnorePath inside src/Filters/Filter.php that is trying to build the relative path to be used to check the rule to ignore the file or not. At this part of the method it try to use substr to remove the $basedir from $path but as the both has the same value the $relativePath will be an empty string and for sure the file won't be ignored as expected.
To reproduce
Steps to reproduce the behavior:
- Create any
<exclude-pattern>with thetype="relative". - Go to
vendor/squizlabs/php_codesniffer/src/Filters/Filter.phpfile at the line237. And add a simple code check the output.
var_dump('DEBUG: $relativePath after substr :', $relativePath);
exit;
- Run
./vendor/bin/phpcs -p .orphpcs -p .depends on how are configured in you env. - So you will see the output with an empty string
Expected behavior
Expects that rule <exclude-pattern type="relative"> ignore the files that match with it. The $relativePath at the method shouldIgnorePath inside src/Filters/Filter.php don't be empty to work as expected.
Versions
| Operating System | MacOS 15.3 |
| PHP version | 8.0 |
| PHP_CodeSniffer version | 3.7.2 |
| Standard | MySource, PEAR, PSR1, PSR2, PSR12, Squiz, Zend, PHPCompatibility, PHPCompatibilityParagonieRandomCompat, PHPCompatibilityParagonieSodiumCompat, PHPCompatibilityWP, PHPCSUtils, WordPress, WordPress-Core, WordPress-Docs and WordPress-Extra |
| Install type | Composer - local |
Additional context
None.
Please confirm
- [X] I have searched the issue list and am not opening a duplicate issue.
- [X] I have read the Contribution Guidelines and this is not a support question.
- [X] I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
- [X] I have verified the issue still exists in the
masterbranch of PHP_CodeSniffer.
I have the intention to open a PR to fix this in the next few days but I registered the issue to help anyones that maybe have the same problema.
Proposed solution
Use $config->basepath in $this->basedir instead of receive $basedir as a parameter to class.
@eduardovillao I haven't looked into this yet, but using $config->basepath is definitely incorrect.
Yes, I checked the definition of basepath and probably you are right @jrfnl Do you have any suggestions?
Maybe create a new config item basedir to be reused here and other places? Or do you think don't make sense this to be a config item?
@eduardovillao No suggestions until I find the time to investigate. I do remember I fixed some bugs in this part of the codebase a few years back, so you may want to have a look through the file history to understand the code.
Understanding what the code doing is not a problem because is simple. But thanks for your time replying.
To context, related to this: https://github.com/squizlabs/PHP_CodeSniffer/issues/981
@eduardovillao Unfortunately, I was unable to reproduce this issue.
Can you check the following and see if / where my reproduction setup differs from yours?
Structure
base/
public/
source/
development/
composer.json
phpcs.xml
public/
example.php
vendor/
Ruleset
<?xml version="1.0"?>
<ruleset name="Coding Standards">
<description>Apply Coding Standards</description>
<arg name="basepath" value="./"/>
<exclude-pattern>/vendor/</exclude-pattern>
<exclude-pattern type="relative">^public/.*</exclude-pattern>
<rule ref="Generic.Arrays.DisallowLongArraySyntax"/>
</ruleset>
Steps
-
Locally install PHPCS 3.7.2 and the installer plugin with Composer (When prompted, press
yto enable the plugin):composer require --dev squizlabs/php_codesniffer:3.7.2 dealerdirect/phpcodesniffer-composer-installer:* -
Add the following after
vendor/squizlabs/php_codesniffer/src/Filters/Filter.php:237:var_dump('DEBUG: $relativePath after substr :', $relativePath); exit; -
Run PHPCS with the progress flag:
./vendor/bin/phpcs -p .
Output
string(35) "DEBUG: $relativePath after substr :"
string(6) "public"
Example workaround: https://gerrit.wikimedia.org/r/c/operations/mediawiki-config/+/1170616/
Avoid type="relative" which doesn't work for files passed directly to PHPCS, as CI does for changed files on commits that update these.
As a workaround, we can make it a regular pattern and accept the hypothetical that, if a file named "dblists-index.php" or the subdir+filename "wmf-config/interwiki.php" one day appears in a nested subtree of this repository, that they too will be excluded.
Ref https://github.com/squizlabs/PHP_CodeSniffer/issues/981
<!-- Auto generated files. -->
- <exclude-pattern type="relative">^dblists-index\.php</exclude-pattern>
- <exclude-pattern type="relative">^wmf-config/interwiki\.php</exclude-pattern>
- <exclude-pattern type="relative">^wmf-config/interwiki-labs\.php</exclude-pattern>
+ <exclude-pattern>dblists-index.php</exclude-pattern>
+ <exclude-pattern>wmf-config/interwiki.php</exclude-pattern>
+ <exclude-pattern>wmf-config/interwiki-labs.php</exclude-pattern>