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

Fix checks for PHP 8.1 and above #746

Closed GaryJones closed 1 year ago

GaryJones commented 1 year ago

Running composer test (or composer check after #741 is merged) would lead to several issues under PHP 8.1 and 8.2. This PR fixes those issues.

Ruleset tests: Add label before test runs

Make it clear which ruleset is being tested, even on a test failure. This just makes it easier to debug.

Ruleset Tests: Workaround WPCS trim() bug

PHP 8.1 increased the strictness of types used with native functions, and this meant WPCS 2.3.0 had a bug: https://github.com/WordPress/WordPress-Coding-Standards/issues/2035

For the VIPCS ruleset tests with PHP 8.1 or above, it would skip over a bunch of lines in the test file and report other lines as expecting errors or warnings incorrectly.

Running:

vendor/squizlabs/php_codesniffer/bin/phpcs --standard=WordPressVIPMinimum --severity=1 WordPressVIPMinimum/ruleset-test.inc -s

...shows all the expected violations, but also this for line 1:

An error occurred during processing; checking has been aborted. The error message was: trim(): Passing null to parameter #1 ($string) of type string is deprecated in .../vipcs/vendor/wp-coding-standards/wpcs/WordPress/Sniff.php on line 1144 (Internal.Exception)

Line 1144 relates to the retrieval of the minimum_supported_wp_version config value. Since it appeared to be coming through as null, the RulesetTest.php now includes setting a trivial value via a command-line argument when PHPCS is called to run the ruleset test.

When WPCS 3.0 is the minimum supported, this change can be reverted since the original bug would have been fixed.

CS: Work around WPCS trim() bug

PHP 8.1 increased the strictness of types used with native functions, and this meant WPCS 2.3.0 had a bug: https://github.com/WordPress/WordPress-Coding-Standards/issues/2035

The VIPCS coding standards check with PHP 8.1 or above would report incorrect violations from sniffs that included the retrieval of optional command-line arguments. These violations did not appear when running PHPCS with PHP 8.0 or below.

Since the retrieval of the optional command-line arguments appeared to be coming through as null, causing the problems, the command to run PHPCS on the VIPCS codebase now includes the setting of several command-line arguments.

With the prefixes enabled, extra sniff behaviour is enabled, and VIPCS does not need to adhere to consistent prefixes (unlike actual WordPress plugin and theme codebases). As such, the PrefixAllGlobals rule is excluded for VIPCS.

When WPCS 3.0 is the minimum supported, these changes can be reverted since the original bug would have been fixed.

Unit tests: Fix PHPUnit deprecation

When running the unit tests with PHP 8.1 or above, PHPUnit would give a deprecation notice:

PHP Deprecated: strlen(): Passing null to parameter #1 ($string) of type string is deprecated in .../vipcs/vendor/phpunit/phpunit/src/Util/Getopt.php on line 159

The tests still ran successfully, though.

This notice originated from the use of --filter WordPressVIPMinimum and was fixed by adding an equals sign: --filter=WordPressVIPMinimum.

Under both versions, the same number of tests and test files and unique error codes were created.