PHP 8.0 tests: fail with composer packages

[htdat]

Expected/Desired Behavior

No error

Summary

Two packages have errors:

• 1, exussum12/coverage-checker - Can be fixed as mentioned below PHP 8.0 tests: fail with composer packages #638 (comment)
• 2, squizlabs/php_codesniffer - as a part of automattic/vipwpcs. Wait for the new release of WPCS as mentioned below PHP 8.0 tests: fail with composer packages #638 (comment)

Two errors in detail

From https://github.com/Automattic/Edit-Flow/runs/2407816247?check_suite_focus=true

1:

PHP Fatal error:  Array and string offset access syntax with curly braces is no longer supported in /home/runner/work/Edit-Flow/Edit-Flow/vendor/exussum12/coverage-checker/src/ArgParser.php on line 25

2:

bash bin/phpcs-diff.sh
From https://github.com/Automattic/Edit-Flow
 * [new branch]      dependabot/npm_and_yarn/elliptic-6.5.4 -> origin/dependabot/npm_and_yarn/elliptic-6.5.4
....
....
....
PHP Fatal error:  Uncaught TypeError: vsprintf(): Argument #2 ($values) must be of type array, string given in /home/runner/work/Edit-Flow/Edit-Flow/vendor/squizlabs/php_codesniffer/src/Files/File.php:1056
Stack trace:
#0 /home/runner/work/Edit-Flow/Edit-Flow/vendor/squizlabs/php_codesniffer/src/Files/File.php(1056): vsprintf()
#1 /home/runner/work/Edit-Flow/Edit-Flow/vendor/squizlabs/php_codesniffer/src/Files/File.php(672): PHP_CodeSniffer\Files\File->addMessage()
#2 /home/runner/work/Edit-Flow/Edit-Flow/vendor/squizlabs/php_codesniffer/src/Files/File.php(780): PHP_CodeSniffer\Files\File->addError()
#3 /home/runner/work/Edit-Flow/Edit-Flow/vendor/wp-coding-standards/wpcs/WordPress/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php(205): PHP_CodeSniffer\Files\File->addFixableError()
#4 /home/runner/work/Edit-Flow/Edit-Flow/vendor/wp-coding-standards/wpcs/WordPress/Sniff.php(910): WordPressCS\WordPress\Sniffs\WhiteSpace\ControlStructureSpacingSniff->process_token()
#5 /home/runner/work/Edit-Flow/Edit-Flow/vendor/squizlabs/php_codesniffer/src/Files/File.php(498): WordPressCS\WordPress\Sniff->process()
#6 /home/runner/work/Edit-Flow/Edit-Flow/vendor/squizlabs/php_codesniffer/src/Files/LocalFile.php(92): PHP_CodeSniffer\Files\File->process()
#7 /home/runner/work/Edit-Flow/Edit-Flow/vendor/squizlabs/php_codesniffer/src/Runner.php(630): PHP_CodeSniffer\Files\LocalFile->process()
#8 /home/runner/work/Edit-Flow/Edit-Flow/vendor/squizlabs/php_codesniffer/src/Runner.php(434): PHP_CodeSniffer\Runner->processFile()
#9 /home/runner/work/Edit-Flow/Edit-Flow/vendor/squizlabs/php_codesniffer/src/Runner.php(114): PHP_CodeSniffer\Runner->run()
#10 /home/runner/work/Edit-Flow/Edit-Flow/vendor/squizlabs/php_codesniffer/bin/phpcs(18): PHP_CodeSniffer\Runner->runPHPCS()
#11 {main}
  thrown in /home/runner/work/Edit-Flow/Edit-Flow/vendor/squizlabs/php_codesniffer/src/Files/File.php on line 1056

(Optional) Additional notes

Only issues with PHP 8.0.
There is no issue from PHP 5.6 to 7.4.

[htdat]

The second issue is being asked here exussum12/coverageChecker#66

[htdat]

PHP Fatal error: Uncaught TypeError: vsprintf(): Argument #2 ($values) must be of type array, string given in /home/runner/work/Edit-Flow/Edit-Flow/vendor/squizlabs/php_codesniffer/src/Files/File.php:1056

Relevant issue squizlabs/PHP_CodeSniffer#3196
It has been fixed here WordPress/WordPress-Coding-Standards#1935
But WPCS ver 3.0.0 including this fix has not been released.

[htdat]

The second issue is being asked here exussum12/coverageChecker#66

The author released v1.0.0.

I tried this in my forked repo https://github.com/htdat/Edit-Flow/commit/3b61a16f575360111564974729554c5c17d2b961 and it worked very well as shown here https://github.com/htdat/Edit-Flow/actions/runs/784319557

Note: ^0.11.2 || ^1.0.0 is required. Otherwise, it will create a compatibility issues with PHP 5.6 and this package like the following:

- Root composer.json requires exussum12/coverage-checker ^1.0.0 -> satisfiable by exussum12/coverage-checker[1.0.0].
- exussum12/coverage-checker 1.0.0 requires php >=7.0 -> your php version (5.6.40) does not satisfy that requirement.

[htdat]

All of these packages are related to ./bin/phpcs-diff.sh and I am seeing if we can replace this with one of these two options:

• Composer Package https://github.com/sirbrillig/phpcs-changed, which is being used in Jetpack. It's tricky that this supports PHP >= 7.1 only.
• Use this command vendor/bin/phpcs --report=bin/phpcs-eslint-report.php | eslines --quiet like the way vip-go-mu-plugins is doing. The downside of this is that it requires phpcs-eslint-report.php file and require eslines while current PHP tests worflow does not use npm at all.

[htdat]

Update: I have played around for two options above but the main issue is still related WPCS is not ready for PHP 8.0 (#638 (comment) above).

Therefore I am proposing a PR to just partly fix this issue and improve ./bin/phpcs-diff.sh a bit.

[mikeyarce]

@htdat if I'm understanding correctly:

• Currently PHP 8.0 tests are failing
• This is due to two packages, exussum12/coverage-checker and automattic/vipwpcs
• We can fix the problem for exussum12/coverage-checker but not vipwpcs yet
• We are waiting for WPCS 3.0 to be released for the other fix

If that's right, I think maybe we should split this issue into two and address the first problem now, and then just wait until WPCS 3.0 is released to address the other one. How does that sound?

[htdat]

@mikeyarce

If that's right, I think maybe we should split this issue into two and address the first problem now, and then just wait until WPCS 3.0 is released to address the other one. How does that sound?

Yes, this looks very good. I've decomposed this issue into two issues:

• PHP 8.0 - PHPCS tests - incompatibility with vipwpcs and wpcs  #646
• PHP 8.0 - PHPCS tests - incompatibility with exussum12/coverage-checker #647

I am going to close this issue.