Skip to content

Commit

Permalink
Fix checks for PHP 8.1 and above (#746)
Browse files Browse the repository at this point in the history
* 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: WordPress/WordPress-Coding-Standards#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: WordPress/WordPress-Coding-Standards#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 <[email protected]>
  • Loading branch information
GaryJones and rebeccahum authored Jan 31, 2023
1 parent 24af5df commit ed57071
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 4 deletions.
1 change: 1 addition & 0 deletions .phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<rule ref="WordPress-Extra">
<exclude name="WordPress.Files.FileName"/>
<exclude name="WordPress.NamingConventions.ValidVariableName"/>
<exclude name="WordPress.NamingConventions.PrefixAllGlobals"/>
<exclude name="Generic.Arrays.DisallowShortArraySyntax"/>
<exclude name="WordPress.PHP.YodaConditions"/>
</rule>
Expand Down
4 changes: 2 additions & 2 deletions bin/unit-tests
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 6 additions & 2 deletions tests/RulesetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) ) {
Expand Down Expand Up @@ -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 );

Expand Down

0 comments on commit ed57071

Please sign in to comment.