From ed57071feceb4deb0aa791edb399da6d213a7ab5 Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Tue, 31 Jan 2023 19:10:47 +0000 Subject: [PATCH] Fix checks for PHP 8.1 and above (#746) * Ruleset tests: Add label before test runs Make it clear which ruleset being tested, even on a test failure. * Ruleset Tests: 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 For the VIPCS ruleset tests with PHP 8.1 or above it would skip over a bunch of lines, and mark 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 relate 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 this as a command-line argument when PHPCS is called to run the ruleset test. When WPCS 3.0 is the minimum supported, these changes can be reverted. * 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 For the VIPCS coding standards check with PHP 8.1 or above it 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 setting of several command-line arguments. With the `prefixes` enabled, extra sniff behaviour is enabled, and VIPCS has no 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. * 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. --------- Co-authored-by: Rebecca Hum <16962021+rebeccahum@users.noreply.github.com> --- .phpcs.xml.dist | 1 + bin/unit-tests | 4 ++-- tests/RulesetTest.php | 8 ++++++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist index 424f4095..0b06835d 100644 --- a/.phpcs.xml.dist +++ b/.phpcs.xml.dist @@ -19,6 +19,7 @@ + diff --git a/bin/unit-tests b/bin/unit-tests index 0c84e924..8ffc5acc 100755 --- a/bin/unit-tests +++ b/bin/unit-tests @@ -17,7 +17,7 @@ # if [[ $(php -r 'echo PHP_VERSION_ID;') -ge 80100 ]]; then - "$(pwd)/vendor/bin/phpunit" --filter WordPressVIPMinimum "$(pwd)/vendor/squizlabs/php_codesniffer/tests/AllTests.php" --no-coverage --no-configuration --bootstrap=./tests/bootstrap.php --dont-report-useless-tests $@ + "$(pwd)/vendor/bin/phpunit" --filter=WordPressVIPMinimum "$(pwd)/vendor/squizlabs/php_codesniffer/tests/AllTests.php" --no-coverage --no-configuration --bootstrap=./tests/bootstrap.php --dont-report-useless-tests $@ else - "$(pwd)/vendor/bin/phpunit" --filter WordPressVIPMinimum "$(pwd)/vendor/squizlabs/php_codesniffer/tests/AllTests.php" --no-coverage $@ + "$(pwd)/vendor/bin/phpunit" --filter=WordPressVIPMinimum "$(pwd)/vendor/squizlabs/php_codesniffer/tests/AllTests.php" --no-coverage $@ fi diff --git a/tests/RulesetTest.php b/tests/RulesetTest.php index d67a3454..ca9b15d0 100644 --- a/tests/RulesetTest.php +++ b/tests/RulesetTest.php @@ -98,6 +98,9 @@ public function __construct( $ruleset, $expected = [] ) { $this->phpcs_bin = realpath( $phpcs_bin ); } + // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + printf( 'Testing the ' . $this->ruleset . ' ruleset.' . PHP_EOL ); + $output = $this->collect_phpcs_result(); if ( ! is_object( $output ) || empty( $output ) ) { @@ -143,11 +146,12 @@ private function collect_phpcs_result() { } $shell = sprintf( - '%1$s%2$s --severity=1 --standard=%3$s --report=json ./%3$s/ruleset-test.inc', - $php, // Current PHP executable if avaiable. + '%1$s%2$s --severity=1 --standard=%3$s --report=json --runtime-set minimum_supported_wp_version 0 ./%3$s/ruleset-test.inc', + $php, // Current PHP executable if available. $this->phpcs_bin, $this->ruleset ); + // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.system_calls_shell_exec -- This is test code, not production. $output = shell_exec( $shell );