sebastianbergmann / php-code-coverage

Library that provides collection, processing, and rendering functionality for PHP code coverage information.
BSD 3-Clause "New" or "Revised" License
8.76k stars 370 forks source link

Use of PHP-Parser 5.0 causes fatal when project under test also polyfills tokens #1025

Closed jrfnl closed 6 months ago

jrfnl commented 6 months ago
Q A
php-code-coverage version 9.2.30
PHP version 8.0.30
Driver Xdebug
Xdebug version (if used) 3.3.0
Installation Method Composer
Usage Method PHPUnit
PHPUnit version (if used) 9.6.15

Description

I seem to have caught a weird one...

PHP_CodeSniffer also polyfills PHP tokens and uses text strings for the polyfills. Changing this would be a breaking change and is not currently on the table.

PHP_CodeSniffer uses PHPUnit for its tests and I have recently enabled recording of code coverage. This worked well until a few days ago when PHP Parser 5.0 got released.

Now, I see the following error, but only when the tests are run with code coverage on PHP 8.0: PHP Fatal error: Uncaught TypeError: Cannot assign string to property PhpToken::$id of type int in /home/runner/work/PHP_CodeSniffer/PHP_CodeSniffer/vendor/nikic/php-parser/lib/PhpParser/Lexer.php:96

Things seem to work fine with PHP 8.1+.

The error looks to be related to the following change in PHP Parser: https://github.com/nikic/PHP-Parser/blob/master/UPGRADE-5.0.md#changes-to-token-representation

I've already tried my go-to solution of warming the cache prior to running the tests, as previously discussed in #798, unfortunately this does not seem to do the trick, it just moves the error to the cache warming step, even though that step shouldn't be running any of the PHPCS code IIRC.

I've done some test runs on different PHP versions to isolate the problem. The results can be seen here: https://github.com/PHPCSStandards/PHP_CodeSniffer/actions/runs/7469399693

For now, I'm going to change the GH Actions workflow which is running into this issue to run code coverage on PHP 8.1 instead of PHP 8.0, but I figured it would still be useful to report this.

Additional info

Composer requirements used in the problem run:

    "require": {
        "php": ">=7.2.0",
        "ext-simplexml": "*",
        "ext-tokenizer": "*",
        "ext-xmlwriter": "*"
    },
    "require-dev": {
        "phpunit/phpunit": "^8.0 || ^9.3.4"
    },

PHPUnit config file used: https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/4.x/feature/ci-prevent-issues-with-code-coverage/phpunit.xml.dist

GH Actions script used for the above linked test run: https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/4.x/feature/ci-prevent-issues-with-code-coverage/.github/workflows/test.yml

Please let me know if you need additional information or want me to run some tests.

sebastianbergmann commented 6 months ago

PHPUnit uses php-code-coverage, php-code-coverage uses PHP-Parser. You are testing PHP_CodeSniffer which conflicts with PHP-Parser. I am sorry, but I do not know what I should do about that.

jrfnl commented 6 months ago

@sebastianbergmann The weird thing about the above is that the conflict apparently only happens on PHP 8.0, not PHP 8.1+, even though the exact same versions of PHPUnit, PHP Code Coverage and PHP Parser are being used on PHP 8.0 vs PHP 8.1+.

That's really the issue I'm reporting.

I'm okay with closing the ticket as, as I wrote above, I can work around it by using PHP 8.1 for code coverage, but figured that some awareness that not everything is as it should be on PHP 8.0 would be a good thing.

sebastianbergmann commented 6 months ago

The weird thing about the above is that the conflict apparently only happens on PHP 8.0, not PHP 8.1+, even though the exact same versions of PHPUnit, PHP Code Coverage and PHP Parser are being used on PHP 8.0 vs PHP 8.1+.

There is PHP version-specific code in PHP-Parser's polyfill.

I'm okay with closing the ticket as, as I wrote above

Then I will do just that, sorry :-(

jrfnl commented 6 months ago

The weird thing about the above is that the conflict apparently only happens on PHP 8.0, not PHP 8.1+, even though the exact same versions of PHPUnit, PHP Code Coverage and PHP Parser are being used on PHP 8.0 vs PHP 8.1+.

There is PHP version-specific code in PHP-Parser's polyfill.

That code applies to all PHP 8.x versions though - PHP_VERSION_ID >= 80000 - and should not cause this issue.