Skip to content

Commit

Permalink
PHPCS: fix bug in ruleset
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jrfnl committed Jan 5, 2025
1 parent 69025f6 commit f3d1f36
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 1 deletion.
5 changes: 4 additions & 1 deletion .phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@
<exclude name="WordPress.WP"/>
<exclude name="Yoast.Yoast.JsonEncodeAlternative"/>
<exclude name="Yoast.NamingConventions.ObjectNameDepth.MaxExceeded"/>
<exclude name="WordPressVIPMinimum"/>
<exclude name="WordPressVIPMinimum.Classes.DeclarationCompatibility"/>
<exclude name="WordPressVIPMinimum.Hooks.AlwaysReturnInFilter"/>
<exclude name="WordPressVIPMinimum.Security.EscapingVoidReturnFunctions"/>
<exclude name="WordPressVIPMinimum.Security.ProperEscapingFunction"/>

<!-- Exclude select "modern PHP" sniffs, which conflict with the minimum supported PHP version of this package. -->
<exclude name="SlevomatCodingStandard.Classes.ModernClassNameReference"/><!-- PHP 5.5+ -->
Expand Down
2 changes: 2 additions & 0 deletions src/TestListeners/TestListenerSnakeCaseMethods.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

/**
* Renamed snake_case TestListener method collection used by the TestListenerDefaultImplementation traits.
*
* @phpcs:disable VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- This is intentional as these are template methods.
*/
trait TestListenerSnakeCaseMethods {

Expand Down
2 changes: 2 additions & 0 deletions tests/TestListeners/Fixtures/TestListenerImplementation.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
/**
* TestListener implementation for testing the TestListener cross-version
* TestListenerDefaultImplementation trait.
*
* @phpcs:disable VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- This is intentional. These params can be used, but don't have to be.
*/
class TestListenerImplementation implements TestListener {

Expand Down

0 comments on commit f3d1f36

Please sign in to comment.