Automattic / VIP-Coding-Standards

PHP_CodeSniffer ruleset to enforce WordPress VIP coding standards.
https://wpvip.com/documentation/how-to-install-php-code-sniffer-for-wordpress-com-vip/
Other
236 stars 40 forks source link

WPCS 3.0: Fix incompatibilities by using PHPCSUtils #734

Closed GaryJones closed 1 year ago

GaryJones commented 1 year ago

~Once #742 has been merged, this can be rebased to allow the workflow tests to run. #742 makes PHPCS 3.7.1 as the minimum version for VIPCS, which is the constraint on PHPCSUtils 1.0.~ Now rebased.


Several methods were removed or moved from the Sniff class in WPCS, so these changes were spotted when running unit tests and getting errors. There are undoubtedly many more opportunities to use PHPCSUtils versions of functions to replace existing VIPCS code to improve the quality of the sniffs, but these are not tackled here.

These changes don't require WPCS 3.0 - we're just making the same types of changes that WPCS 3.0, by using PHPCSUtils as a direct dependency.

Composer: Add PHPCSUtils as required dependency

VIPCS uses PHPCSUtils directly (such as for MessageHelper), so it should be marked as such in the Composer config.

Sniff::strip_quotes(): switch to use the PHPCSUtils version

The original Sniff::strip_quotes() method was removed for WPCS 3.0.

Sniff::find_array_open_close(): switch to use the PHPCSUtils version

The original Sniff::find_array_open_close() method was removed for WPCS 3.0.

Sniff::get_function_call_parameter(): switch to use the PHPCSUtils version

The original Sniff::get_function_call_parameter() method was removed for WPCS 3.0.

Use PHPCSUtils MessageHelper::addMessage

This is more tested than the WPCS Sniff::addMessage(), and the original was removed.

GaryJones commented 1 year ago

Tests pass fine locally, so not sure why GitHub Actions is having an issue finding and using PHPCSUtils.

Edit: Log shows only phpcsstandards/phpcsutils (1.0.0-alpha3) was installed, not alpha4. Edit: But they were for test runs using PHPCS 3.5 and 3.6, which we no longer support anyway...

GaryJones commented 1 year ago

I'd recommend rebasing the PR (without those last two commits) on develop and changing the commit order.

Done.