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

3.0: support WordPressCS 3.0 #779

Merged
merged 11 commits into from
Aug 25, 2023
Merged

3.0: support WordPressCS 3.0 #779

merged 11 commits into from
Aug 25, 2023

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Aug 24, 2023

Composer: update WordPressCS + PHPCS dependencies

WordPressCS 3.0.0 has been released and requires a minimum PHPCS version of 3.7.2.

This commit updates the WordPressCS and PHPCS dependencies and the documentation referring to those, in all the relevant places.

Includes:

  • Removing the DealerDirect plugin from require as it comes with WordPressCS 3.0.0 automatically (via PHPCSUtils) and not having the explicit dependency may prevent conflicts with allowed versions in the future.
  • Updating the PHP version on which the CS check for the VIPCS native code is being run. With WPCS 3.0, this check is no longer limited to PHP 7.4.
  • Removing a work-around for WPCS < 3.0 vs PHP 8.0
  • Updating the VIPCS native .phpcs.xml.dist ruleset.

Ref: https://github.com/WordPress/WordPress-Coding-Standards/releases/tag/3.0.0

WPCS 3.0 | Revert previously applied work-arounds

These are no longer needed with WPCS 3.0.0.

Ref: #746

Security/EscapingVoidReturnFunctions: switch to using WPCS PrintingFunctionsTrait

Note: this does mean that the sniff will now also support a public customPrintingFunctions property which can be adjusted in a custom ruleset.

AbstractVariableRestrictionsSniff: use WPCS ContextHelper::is_in_isset_or_empty()

AbstractVariableRestrictionsSniff: use WPCS RulesetPropertyHelper::merge_custom_array()

AbstractArrayAssignmentRestrictions children: adjust for changes in received $val

The $val parameter received by the callback() method will no longer automatically have been stripped of quotes.

This adjusts the callback() methods of the sniff which extend the WPCS AbstractArrayAssignmentRestrictions sniff to take this into account.

VIPMinimum ruleset: replace strict comparison sniff

Includes adding PHPCSExtra to the composer.json configuration as that is now a direct dependency of VIPCS as VIPCS now uses one of its sniffs in the ruleset.

Fixes #603

VIPMinimum ruleset: replace assignment-in-condition sniff

... which has been split.

VIPMinimum ruleset: update excludes for the WP/AlternativeFunctions sniff

... to prevent introducing new duplicate error messages.

Update ruleset tests: account for new errors from WPCS

  • The WordPress.Security.ValidatedSanitizedInput sniff will now also examine $_SESSION variables.
    As the test is about another sniff, let's just ignore the notices coming from WordPress.Security.ValidatedSanitizedInput.

WordPressVIPMinimum RulesetTest: fix it

To be honest, I'm completely stumped. I can't, for the life of me, think of a reason why the update of the WPCS dependency would suddenly cause all sorts of notices from the VariableAnalysis to show, where they previously did not.

  • It can't be related to the version update of the VariableAnalysis sniff in VIPCS 2.3.4, nor the move of the properties from the Go ruleset to the Minimum ruleset as, in that case, the build for the earlier PR/commit should also have been failing.
  • It can't be related to the VIPCS native VariableAnalysis sniff being removed as, in that case, the build for the earlier PR/commit should also have been failing.
  • VA and WPCS currently have two overlapping CS related dependencies.
    • PHP_CodeSniffer itself, but as they both use the same version, that can not be a reason for the change in the results.
    • The Composer plugin, but that shouldn't influence the scan results, other than if a ruleset would not be registered, but if that were the case (for VA), the tests should have been failing before on line 264, which specifically tests the VA standard triggering a notice (and yes, I checked, it did do so correctly before and after).
  • No new parse errors were accidentally introduced into the test files.

As things are, I'm out of ideas of why these warnings are now suddenly showing up, so all I can do is fix them (by selectively silencing the notices from VA).

Note: this is only happening in the Minimum ruleset test as VIPGo silences the UnusedVariable notice completely.

Closes #727 (as WPCS 3.0.0 will no longer throw deprecations on PHP 8.x)

jrfnl and others added 11 commits August 24, 2023 18:25
WordPressCS 3.0.0 has been released and requires a minimum PHPCS version of 3.7.2.

This commit updates the WordPressCS and PHPCS dependencies and the documentation referring to those, in all the relevant places.

Includes:
* Removing the DealerDirect plugin from `require` as it comes with WordPressCS 3.0.0 automatically (via PHPCSUtils) and not having the explicit dependency may prevent conflicts with allowed versions in the future.
* Updating the PHP version on which the CS check for the VIPCS native code is being run. With WPCS 3.0, this check is no longer limited to PHP 7.4.
* Removing a work-around for WPCS < 3.0 vs PHP 8.0
* Updating the VIPCS native `.phpcs.xml.dist` ruleset.

Ref: https://github.com/WordPress/WordPress-Coding-Standards/releases/tag/3.0.0
These are no longer needed with WPCS 3.0.0.

Ref: 746
…nctionsTrait

Note: this does mean that the sniff will now also support a `public` `customPrintingFunctions` property which can be adjusted in a custom ruleset.

Co-authored-by: Gary Jones <gary.jones@automattic.com>
…t_or_empty()

Co-authored-by: Gary Jones <gary.jones@automattic.com>
…rge_custom_array()

Co-authored-by: Gary Jones <gary.jones@automattic.com>
…eceived `$val`

The `$val` parameter received by the `callback()` method will no longer automatically have been stripped of quotes.

This adjusts the `callback()` methods of the sniff which extend the WPCS `AbstractArrayAssignmentRestrictions` sniff to take this into account.
Includes adding PHPCSExtra to the `composer.json` configuration as that is now a direct dependency of VIPCS as VIPCS now uses one of its sniffs in the ruleset.

Co-authored-by: Gary Jones <gary.jones@automattic.com>
…niff

... to prevent introducing new duplicate error messages.

Co-authored-by: Gary Jones <gary.jones@automattic.com>
* The `WordPress.Security.ValidatedSanitizedInput` sniff will now also examine `$_SESSION` variables.
    As the test is about another sniff, let's just ignore the notices coming from `WordPress.Security.ValidatedSanitizedInput`.
To be honest, I'm completely stumped. I can't, for the life of me, think of a reason why the update of the WPCS dependency would suddenly cause all sorts of notices from the `VariableAnalysis` to show, where they previously did not.

* It can't be related to the version update of the `VariableAnalysis` sniff in VIPCS 2.3.4, nor the move of the properties from the Go ruleset to the Minimum ruleset as, in that case, the build for the earlier PR/commit should also have been failing.
* It can't be related to the VIPCS native `VariableAnalysis` sniff being removed as, in that case, the build for the earlier PR/commit should also have been failing.
* VA and WPCS currently have two overlapping CS related dependencies.
    - PHP_CodeSniffer itself, but as they both use the same version, that can not be a reason for the change in the results.
    - The Composer plugin, but that shouldn't influence the scan results, other than if a ruleset would not be registered, but if that were the case (for VA), the tests should have been failing before on line 264, which specifically tests the VA standard triggering a notice (and yes, I checked, it did do so correctly before and after).
* No new parse errors were accidentally introduced into the test files.

As things are, I'm out of ideas of why these warnings are now suddenly showing up, so all I can do is fix them (by selectively silencing the notices from VA).

Note: this is only happening in the Minimum ruleset test as VIPGo silences the `UnusedVariable` notice completely.
@jrfnl jrfnl added this to the 3.0.0 milestone Aug 24, 2023
@jrfnl jrfnl requested a review from a team as a code owner August 24, 2023 16:50
Copy link
Contributor

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GaryJones GaryJones merged commit 0fd1c85 into develop Aug 25, 2023
32 checks passed
@GaryJones GaryJones deleted the 3.0/updates-for-wpcs-3.0 branch August 25, 2023 13:54
@jrfnl jrfnl mentioned this pull request Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants