-
-
Notifications
You must be signed in to change notification settings - Fork 53
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
phpcs:enable
can sometimes override phpcs:ignore
#111
Comments
Hmm, I can't create the PR corresponding to squizlabs/PHP_CodeSniffer#3891 yet, either I'll have to wait for GitHub to respond to my support request for "rerouting" my fork or I'll have to create a new fork. |
@anomiex True, though you should be able to then just add the new fork + the new repo as extra remotes to your local git copy, which will allow you to push the existing branch up to the new fork. |
The current method, listing codes to disable and a list of exceptions to that list, still has trouble with some cases. For example, disabling a standard, re-enabling a category within that standard, then ignoring or disabling a sniff within that category cannot be handled. We'd need a list of exceptions to the exceptions, and possibly a list of exceptions to that list too, and figuring out how to keep those lists up to date as new directives are encountered could prove to be confusing. Since the standard→category→sniff→code hierarchy is supposed to be thought of as a tree, let's store the ignore list that way instead. Manipulating the branches of the tree is straightforward no matter what directives are encountered. In this implementation I've favored speed over space: there are cases where we could prune a subtree that would evaluate to "ignore" or "don't ignore" for any possible input, but detecting that doesn't seem worth the time when it's not likely there will be so many enable or disable directives that the wasted space will be a problem. Fixes PHPCSStandards#111
The current method, listing codes to disable and a list of exceptions to that list, still has trouble with some cases. For example, disabling a standard, re-enabling a category within that standard, then ignoring or disabling a sniff within that category cannot be handled. We'd need a list of exceptions to the exceptions, and possibly a list of exceptions to that list too, and figuring out how to keep those lists up to date as new directives are encountered could prove to be confusing. Since the standard→category→sniff→code hierarchy is supposed to be thought of as a tree, let's store the ignore list that way instead. Manipulating the branches of the tree is straightforward no matter what directives are encountered. In this implementation I've favored speed over space: there are cases where we could prune a subtree that would evaluate to "ignore" or "don't ignore" for any possible input, but detecting that doesn't seem worth the time when it's not likely there will be so many enable or disable directives that the wasted space will be a problem. Fixes PHPCSStandards#111
Duplicated from squizlabs/PHP_CodeSniffer#3889 (and updated from later comments)
Describe the bug
phpcs:enable
can sometimes wind up overriding a laterphpcs:ignore
for the rule. This can particularly happen when multiplephpcs:enable
comments are present in the file.Code sample
Custom ruleset
N/A
To reproduce
Steps to reproduce the behavior:
test1.php
andtest2.php
with the code samples above.phpcs -s --standard=PSR12 test1.php test2.php
Expected behavior
The error on test1.php line 9 should have been ignored due to the
phpcs:ignore
comment on that line.The error on test2.php line 6 should have been ignored due to the specific rule having been disabled.
Versions (please complete the following information)
Additional context
It seems that in this situation it's setting
$ignoring['.except'][...]
, which overridesphpcs:ignore
.Please confirm:
master
branch of PHP_CodeSniffer.The text was updated successfully, but these errors were encountered: