PHP_CodeSniffer icon indicating copy to clipboard operation
PHP_CodeSniffer copied to clipboard

Rule `<exclude-pattern>` with `type="relative"` don't work

Open eduardovillao opened this issue 11 months ago • 7 comments

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.

Image

But in all cases that is called from src/Files/FileList.php in fact it is receiving a file path as value to parameter.

Image

Image

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.

Image

Image

To reproduce

Steps to reproduce the behavior:

  1. Create any <exclude-pattern> with the type="relative".
  2. Go to vendor/squizlabs/php_codesniffer/src/Filters/Filter.php file at the line 237. And add a simple code check the output.
var_dump('DEBUG: $relativePath after substr :', $relativePath);
exit;

Image

  1. Run ./vendor/bin/phpcs -p . or phpcs -p . depends on how are configured in you env.
  2. So you will see the output with an empty string

Image

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 master branch of PHP_CodeSniffer.

eduardovillao avatar Mar 07 '25 18:03 eduardovillao

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.

Image

eduardovillao avatar Mar 07 '25 18:03 eduardovillao

@eduardovillao I haven't looked into this yet, but using $config->basepath is definitely incorrect.

jrfnl avatar Mar 07 '25 21:03 jrfnl

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 avatar Mar 07 '25 21:03 eduardovillao

@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.

jrfnl avatar Mar 07 '25 21:03 jrfnl

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 avatar Mar 07 '25 22:03 eduardovillao

@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

  1. Locally install PHPCS 3.7.2 and the installer plugin with Composer (When prompted, press y to enable the plugin):

    composer require --dev squizlabs/php_codesniffer:3.7.2 dealerdirect/phpcodesniffer-composer-installer:*
    
  2. Add the following after vendor/squizlabs/php_codesniffer/src/Filters/Filter.php:237:

    var_dump('DEBUG: $relativePath after substr :', $relativePath);
    exit;
    
  3. Run PHPCS with the progress flag:

    ./vendor/bin/phpcs -p .
    

Output

string(35) "DEBUG: $relativePath after substr :"
string(6) "public"

costdev avatar Apr 06 '25 06:04 costdev

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>

Krinkle avatar Jul 24 '25 15:07 Krinkle