-
-
Notifications
You must be signed in to change notification settings - Fork 74
Excluding a class of rules via <exclude name="xxx"/>
, inside a ruleset ... globally excludes them ...
#1081
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
Comments
This is actually the way the ruleset reading was designed and changing this would be a huge breaking change. I do agree it can be counter-intuitive and I'd like to have a think at some point about how this experience can be improved, but that will have to wait for a future major release. In the mean time, changing the
|
Oh.. one caveats and my suggestion above actually solves that problem too - using |
@jrfnl Thanks for the reply! I agree completely that excluding an entire ruleset is ... problematic. Might I suggest that at least the documentation page (really 'example page' vs real docs) be updated? It implies that what I did, based upon what it says, would work. IE using the logic: I want the 'standard' WordPress-Extras ruleset as default configured. But don't include any PSR2 rules that it tries to apply inside of it. Because I then want to apply PSR12 rules, which include PSR2 rules. And I want 'those defaults' to apply. (What I posted was a much simpler example obviously. My real file was basically doing a bunch of WP rule exclusions, and then also realizing that the WP ruleset, added a bunch of universal/etc rules that I didn't want. And also-also that it added PSR2 rules, for example, and CONFIGURED THEM to be differently config than PSR12 does ... For example it does: <rule ref="PSR2.Classes.PropertyDeclaration.Underscore">
<severity>5</severity>
</rule> And I wanted the 'PSR12 standard' version of that to be in place at default severity. In my own case, I solved this by instead of trying to be cute and saying "include this ruleset, but ignore all 'these parts of it'" ... I just figured out what the parts I wanted were used So it works. But, IMO, the docs are claiming something that isn't true. Actually, as written/functional. Shouldn't the not be allowed 'inside' of a ruleset anyway? IE: if an exclude is going to exclude that rule from all included later rulesets. Why not move the exclude 'outside' of the rules and even not allow it inside, since it's truly global? #2cents ... Regardless, thank you again for the response. |
I took a look to do just that and I'm a bit in doubt how to update the page as for the particular example used on the page, the problem doesn't apply. This is the snippet from the "Annotated Ruleset" page I'm looking at: <!--
You can even exclude a whole standard. This example includes
all sniffs from the Squiz standard, but excludes any that come
from the Generic standard.
-->
<rule ref="Squiz">
<exclude name="Generic"/>
</rule> The things, That also highlights, in part, why changing the Ruleset behaviour for when complete standards are excluded is going to be a challenge: it touches on a fundamental design principle in PHP_CodeSniffer: every sniff needs to be part of a standard, but the rulesets for standards are also used to include other rules/make sniff customizations etc. This is not an issue when sniff libraries are set up as sniff collections with a basically empty The problem also only really comes into play when using an I seem to remember support for this was added in PHPCS 3.0.0 and, to be honest, I'm kind of tempted to prohibit complete standard It's something I've been thinking about for a while already, but I haven't been able to come up with an elegant solution which would not lead to tons of support requests (yet). There is also another issue with the rulesets which I find counter-intuitive: a previous All in all, don't expect a solution any time soon - 5.0 at the earliest as any change in ruleset interpretation must go into a new major. For now, if you have any suggestions on how the docs can be improved for the current situation, let me know and I'll update the wiki. |
Describe the bug
I have been attempting to use the
exclude
functionality as shown in the documentation here:https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki/Annotated-Ruleset
Wherein I exclude a class of rules inside of the section that is pulling in a ruleset. Specifically the part of documentation/example here:
As it says, it will 'include all of Squiz but remove any from Squiz that come from Generic"
This is not working for me, as (for above case), it would be completely removing ALL Generic.* rules, even if included by another ruleset. Example below:
Custom ruleset
To reproduce
phpcs -s -e
Expected behavior
Only declarations for PSR2 rules that exist in 'WordPress-Extra' and it's sub-rule-sets it includes ... would be excluded. But since later PSR12 is added in as another ruleset to include enmass ... PSR2 and PSR1 rules, as configured by the PSR12 file(s) ... would apply ... and therefore a
-s -e
would show them.Right now, since you exclude them in the 'WordPress-Extra' context ... they are completely excluded, even if added in by PSR12 ruleset later.
Versions (please complete the following information)
Additional context
n/a
Please confirm
master
branch of PHP_CodeSniffer.The text was updated successfully, but these errors were encountered: