Open tomasnorre opened 1 day ago
@tomasnorre Thanks for opening the issue.
I've already been looking into this and can confirm the issue.
To reproduce and investigate this, I've used a minimal setup with control:
Directory layout:
- project root
- src
- Sniffs
- SniffsDirSniff.php
- SniffsDirWithExcludeSniff.php
- StrictTypes (directory)
- CategorySniff.php
- CategoryWithExcludeSniff.php
The content of the sniff files is the same with only minimal variation:
namespace Exercism\Sniffs;
use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;
class SniffsDirSniff implements Sniff
{
public function register()
{
return [T_OPEN_TAG];
}
public function process(File $phpcsFile, $stackPtr)
{
$phpcsFile->addError('Running', $stackPtr, 'Control');
}
}
The only differences between the four sniffs are:
Exercism\Sniffs\StrictTypes
.*WithExclude
sniffs is updated from Control
to ThisErrorShouldHaveBeenExcluded
.Ruleset:
<?xml version="1.0"?>
<ruleset
name="Exercism PHP Coding Standard"
>
<file>src</file>
<rule ref="src/Sniffs/SniffsDirSniff.php"/>
<!-- This works. -->
<rule ref="src/Sniffs/SniffsDirWithExcludeSniff.php">
<exclude-pattern>src/*</exclude-pattern>
</rule>
<rule ref="src/Sniffs/StrictTypes/CategorySniff.php"/>
<!-- This doesn't work, but should. -->
<rule ref="src/Sniffs/StrictTypes/CategoryWithExcludeSniff.php">
<exclude-pattern>src/*</exclude-pattern>
</rule>
</ruleset>
Command run from the project root:
phpcs -ps --basepath=.
Result
FILE: src\Sniffs\SniffsDirSniff.php
------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 1 LINE
------------------------------------------------------------------------------------------------
1 | ERROR | Running (.Sniffs.SniffsDir.Control)
1 | ERROR | Running (Exercism.StrictTypes.Category.Control)
1 | ERROR | Running (Exercism.StrictTypes.CategoryWithExclude.ThisErrorShouldHaveBeenExcluded)
------------------------------------------------------------------------------------------------
FILE: src\Sniffs\SniffsDirWithExcludeSniff.php
------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 1 LINE
------------------------------------------------------------------------------------------------
1 | ERROR | Running (.Sniffs.SniffsDir.Control)
1 | ERROR | Running (Exercism.StrictTypes.Category.Control)
1 | ERROR | Running (Exercism.StrictTypes.CategoryWithExclude.ThisErrorShouldHaveBeenExcluded)
------------------------------------------------------------------------------------------------
FILE: src\Sniffs\StrictTypes\CategorySniff.php
------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 1 LINE
------------------------------------------------------------------------------------------------
1 | ERROR | Running (.Sniffs.SniffsDir.Control)
1 | ERROR | Running (Exercism.StrictTypes.Category.Control)
1 | ERROR | Running (Exercism.StrictTypes.CategoryWithExclude.ThisErrorShouldHaveBeenExcluded)
------------------------------------------------------------------------------------------------
FILE: src\Sniffs\StrictTypes\CategoryWithExcludeSniff.php
------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 1 LINE
------------------------------------------------------------------------------------------------
1 | ERROR | Running (.Sniffs.SniffsDir.Control)
1 | ERROR | Running (Exercism.StrictTypes.Category.Control)
1 | ERROR | Running (Exercism.StrictTypes.CategoryWithExclude.ThisErrorShouldHaveBeenExcluded)
------------------------------------------------------------------------------------------------
As the original discussion mentioned that it "used to work", I've done some git bisecting to validate this and see where things changed, starting at PHPCS 3.1.0.
My findings based on that are, however, different: This never worked (between PHPCS 3.1.0 and now) and it used to be worse.
SniffsDir
.
This was reported in issue squizlabs/PHP_CodeSniffer#2497 and fixed via PR squizlabs/PHP_CodeSniffer#2501.This will need further investigation and a fix, which probably needs to build on the prior fix from squizlabs/PHP_CodeSniffer#2501.
Thanks @jrfnl for your thorough investigation. I'm not super familiar with PHPCS it self, but if there is anything I can do to help out, let me know.
Thanks @jrfnl for your thorough investigation. I'm not super familiar with PHPCS it self, but if there is anything I can do to help out, let me know.
Well, actually...
Sorry, I don't know where my head was, but I've taken another look at this and suddenly realized that the problem is that - even after the proposed update in https://github.com/exercism/php/pull/857 -, the naming conventions from PHPCS are still not followed for your sniff.
I've submitted a fix for this (https://github.com/tomasnorre/exercism-php/pull/2) and with that fix included, things are working as expected.
With that in mind, I'm going to mark this ticket as a close candidate.
While, yes, this is a bug (as something is not working), I don't think PHPCS should have to support sniff file includes when the sniffs do not comply with the PHPCS naming conventions.
The fact that this is "partially" supported now is more by accident than by design and I think it is a good idea to formally and completely drop support for sniffs not complying with the PHPCS naming conventions in PHPCS 4.0.
Thanks for your PR and your further investigation. I totally overlooked that extra directory missing too.
Created based on discussion: https://github.com/PHPCSStandards/PHP_CodeSniffer/discussions/680
Describe the bug
After adjusting (1) the Sniff according to https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki/Coding-Standard-Tutorial#creating-the-sniff my
exclude-pattern
isn't respected anymore.Code sample
ExplainStrictTypesSniff.php
Custom ruleset
When I run
vendor/bin/phpcs -s
I don't get an error code, and the two errors are from theexclude-pattern
files from above.1) https://github.com/exercism/php/pull/857
To reproduce
Steps to reproduce the behavior:
KindergartenGarden.php
in a folder called.meta
with the code sample aboveExplainStrictTypesSniff.php
in foldersrc/Sniffs/StrictTypes/
from the code abovephpcs.xml
in base-folder with content from abovevendor/bin/phpcs -s .meta/KindergartenGarden.php
Expected behavior
I would expect the error not to be reported as the
.meta
directory is in theexclude-pattern
.Versions (please complete the following information)
Please confirm
master
branch of PHP_CodeSniffer.