Compatibility with PHP-Parser

[0zd0]

Describe the bug

When I develop sniffers and use PHPUnit and do coverage, it uses PHP-Parser, which checks that the token is a number, but you fill it with a string, I didn't delve too deeply into the purpose of this, but I found out from the stack

To reproduce

{
    "name": "onepix/wp-coding-standards",
    "type": "phpcodesniffer-standard",
    "description": "PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions",
    "license": "MIT",
    "keywords": [
        "phpcs",
        "standards",
        "static analysis",
        "WordPress"
    ],
    "autoload": {
        "psr-4": {
            "WpOnepixStandard\\": "WpOnepixStandard/"
        }
    },
    "autoload-dev": {
        "psr-4": {
            "PHP_CodeSniffer\\Tests\\": "vendor/squizlabs/php_codesniffer/tests/"
        }
    },
    "require": {
        "php": "^8.3",
        "wp-coding-standards/wpcs": "^3",
        "squizlabs/php_codesniffer": "3.13.4"
    },
    "require-dev": {
        "phpunit/phpunit": "^12",
        "vimeo/psalm": "^6.8"
    },
    "config": {
        "allow-plugins": {
            "dealerdirect/phpcodesniffer-composer-installer": true
        }
    },
    "scripts": {
        "tests:unit": "phpunit -c phpunit.xml --coverage-filter WpOnepixStandard --coverage-clover=coverage-unit.xml"
    }
}

Versions

Operating System	MacOS 15.6.1 (24G90)
PHP version	8.3.26
PHP_CodeSniffer version	3.13.4
Standard	custom
Install type	Composer

Additional context

An error occurred inside PHPUnit.

Message:  Token T_PUBLIC_SET has ID of type string, should be int. You may be using a library with broken token emulation
Location: /IdeaProjects/wp-coding-standards/vendor/nikic/php-parser/lib/PhpParser/compatibility_tokens.php:39

#0 /IdeaProjects/wp-coding-standards/vendor/nikic/php-parser/lib/PhpParser/compatibility_tokens.php(71): PhpParser\defineCompatibilityTokens()
#1 /IdeaProjects/wp-coding-standards/vendor/nikic/php-parser/lib/PhpParser/Lexer.php(5): require('/IdeaProjects/w...')
#2 /IdeaProjects/wp-coding-standards/vendor/squizlabs/php_codesniffer/autoload.php(173): include('/IdeaProjects/w...')
#3 /IdeaProjects/wp-coding-standards/vendor/squizlabs/php_codesniffer/autoload.php(138): PHP_CodeSniffer\Autoload::loadFile('/IdeaProjects/w...')
#4 /IdeaProjects/wp-coding-standards/vendor/nikic/php-parser/lib/PhpParser/ParserFactory.php(16): PHP_CodeSniffer\Autoload::load('PhpParser\\Lexer')
#5 /IdeaProjects/wp-coding-standards/vendor/nikic/php-parser/lib/PhpParser/ParserFactory.php(40): PhpParser\ParserFactory->createForVersion(Object(PhpParser\PhpVersion))
#6 /IdeaProjects/wp-coding-standards/vendor/phpunit/php-code-coverage/src/StaticAnalysis/ParsingSourceAnalyser.php(50): PhpParser\ParserFactory->createForHostVersion()
#7 /IdeaProjects/wp-coding-standards/vendor/phpunit/php-code-coverage/src/StaticAnalysis/CachingSourceAnalyser.php(73): SebastianBergmann\CodeCoverage\StaticAnalysis\ParsingSourceAnalyser->analyse('/IdeaProjects/w...', '<?php\n\ndeclare(...', true, false)
#8 /IdeaProjects/wp-coding-standards/vendor/phpunit/php-code-coverage/src/StaticAnalysis/CacheWarmer.php(31): SebastianBergmann\CodeCoverage\StaticAnalysis\CachingSourceAnalyser->analyse('/IdeaProjects/w...', '<?php\n\ndeclare(...', true, false)
#9 /IdeaProjects/wp-coding-standards/vendor/phpunit/phpunit/src/Runner/CodeCoverage.php(145): SebastianBergmann\CodeCoverage\StaticAnalysis\CacheWarmer->warmCache('/IdeaProjects/w...', true, false, Object(SebastianBergmann\CodeCoverage\Filter))
#10 /IdeaProjects/wp-coding-standards/vendor/phpunit/phpunit/src/TextUI/Application.php(206): PHPUnit\Runner\CodeCoverage->init(Object(PHPUnit\TextUI\Configuration\Configuration), Object(PHPUnit\TextUI\Configuration\CodeCoverageFilterRegistry), false)
#11 /IdeaProjects/wp-coding-standards/vendor/phpunit/phpunit/phpunit(104): PHPUnit\TextUI\Application->run(Array)
#12 /IdeaProjects/wp-coding-standards/vendor/bin/phpunit(122): include('/IdeaProjects/w...')
#13 {main}

Please confirm

• I have searched the issue list and am not opening a duplicate issue.
• I have read the Contribution Guidelines and this is not a support question.
• I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
• I have verified the issue still exists in the 4.x branch of PHP_CodeSniffer.

[jrfnl]

This is a duplicate of issue #357. You may also be interested in reading up on the discussions in nikic/PHP-Parser#981, sebastianbergmann/php-code-coverage#798 and sebastianbergmann/php-code-coverage#1025.

I'm fully aware of this issue, but this is not something which can be changed easily as it could be considered a breaking change.

I agree it's a pain in the neck for running code coverage for PHPCS standards, but to be honest, that's the only time I am aware of this issue coming into play. And running code coverage against the highest + lowest supported PHP versions (PHP 7.2 + 8.4) should be sufficient and if you do that, you will not run into the error (see how code coverage is run for PHPCS itself).

Having said that, now 4.0 has been released and as this is purely about PHP native tokens being polyfilled (which would have a different constant value anyway when running on a PHP version on which the native token is available), let me have another think about this.

As the constant values would not be stable across PHP versions anyway for polyfilled tokens, we could consider making the change - just for the PHP native polyfilled tokens (not the PHPCS native tokens) in a 4.x minor.

And unless PHP would "appropriate" a custom token name already in use by PHPCS in the future, I don't think any change is needed for the PHPCS native tokens.

---

Note for implementation: any change to the token values (polyfilled or not) will impact (=break) the Tokens::tokenName() method, so we'd need to maintain a translation table for the PHPCS polyfilled token constants to their token name.

And in such a translation table, we'd be forced to also consider the values other tools use to polyfill the tokens, in particular PHP Parser and PHPDocumentor/TypeResolver, as in a test situation, either one of those tools could have polyfilled the tokens before we could, meaning any (test) code using the Tokens::tokenName() method would break unless we keep track of the potential values from the various other tools.

At the same time, having code related to these tools which are not used by PHPCS in the PHPCS source code feels terribly wrong.

[0zd0]

@jrfnl So if, on the maximum version of PHP 8.4, filling is done with numbers, what is the problem with doing this instead of strings?

[0zd0]

Therefore, negative numbers are substituted, as in php-parser is filled in

[jrfnl]

So if, on the maximum version of PHP 8.4, filling is done with numbers, what is the problem with doing this instead of strings?

Please read my previous reply to understand that this will break things in PHPCS itself (which would need fixing in some way or other).

Therefore, negative numbers are substituted, as in php-parser is filled in

Also, negative numbers are counter to the current recommendation in the PHP manual, so are not the way to go.