Skip to content
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: fix bug in ruleset #85

Merged
merged 1 commit into from
Jan 5, 2025
Merged

PHPCS: fix bug in ruleset #85

merged 1 commit into from
Jan 5, 2025

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Jan 5, 2025

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 WP TestUtils 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.

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 WP TestUtils 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.
@jrfnl jrfnl added this to the 1.x Next Release milestone Jan 5, 2025
@coveralls
Copy link

Coverage Status

coverage: 99.167%. remained the same
when pulling 34ba59b on JRF/work-round-phpcs-bug
into 3a07eed on develop.

@jrfnl jrfnl merged commit 430933b into develop Jan 5, 2025
19 checks passed
@jrfnl jrfnl deleted the JRF/work-round-phpcs-bug branch January 5, 2025 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants