-
-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
exclude-pattern
not respected
#683
Comments
@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:
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:
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
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.
This will need further investigation and a fix, which probably needs to build on the prior fix from squizlabs/PHP_CodeSniffer#2501. Detailed bisect analysis results and findingsPHPCS 3.1.0
Findings
PHPCS 3.5.0
Findings
PHPCS 3.5.4
Findings
PHPCS 3.11.1 /
|
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 exercism/php#857 -, the naming conventions from PHPCS are still not followed for your sniff. I've submitted a fix for this (tomasnorre/exercism-php#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: #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.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.The text was updated successfully, but these errors were encountered: