PHPCS: fix bug in ruleset #223
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hmm... this is an awkward one and may need fixing in PHPCS 4.0, but in the mean time, let's fix it here. The base `Yoast` ruleset includes select sniffs from the `WordPressVIPMinimum` ruleset. These sniff do not apply to the PHPUnit Polyfills package. Now, when an `<exclude name="..."/>` rule references a ruleset name to _exclude_ those sniffs, that whole standard is read and included, after which the individual sniffs from the ruleset are excluded again. This also means that if such a ruleset _also_ includes sniffs from other standards and/or customizations (changes in severity/excludes/messaging/properties etc) to sniffs from other standards, those sniffs from other standards will still be included and the customizations still be applied. This will often silently lead to unexpected side-effects, like in the case of the exclusion of the `WordPressVIPMinimum` ruleset, which meant that certain rules we _do_ want applied, like "no superfluous whitespace at the end of a line", got unintentionally excluded. Fixed now by excluding the individual sniffs from the `WordPressVIPMinimum` standard which were included by the `Yoast` ruleset, instead of excluding the whole `WordPressVIPMinimum` standard. Includes various fixes to clean up the codebase for things which are now (correctly) being flagged again.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hmm... this is an awkward one and may need fixing in PHPCS 4.0, but in the mean time, let's fix it here.
The base
Yoastruleset includes select sniffs from theWordPressVIPMinimumruleset. These sniff do not apply to the PHPUnit Polyfills package.Now, when an
<exclude name="..."/>rule references a ruleset name to exclude those sniffs, that whole standard is read and included, after which the individual sniffs from the ruleset are excluded again.This also means that if such a ruleset also includes sniffs from other standards and/or customizations (changes in severity/excludes/messaging/properties etc) to sniffs from other standards, those sniffs from other standards will still be included and the customizations still be applied.
This will often silently lead to unexpected side-effects, like in the case of the exclusion of the
WordPressVIPMinimumruleset, which meant that certain rules we do want applied, like "no superfluous whitespace at the end of a line", got unintentionally excluded.Fixed now by excluding the individual sniffs from the
WordPressVIPMinimumstandard which were included by theYoastruleset, instead of excluding the wholeWordPressVIPMinimumstandard.Includes various fixes to clean up the codebase for things which are now (correctly) being flagged again.