Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support nikic/PHP-Parser 5.x #357

Closed
1 task
staabm opened this issue Feb 21, 2024 · 7 comments
Closed
1 task

Support nikic/PHP-Parser 5.x #357

staabm opened this issue Feb 21, 2024 · 7 comments

Comments

@staabm
Copy link

staabm commented Feb 21, 2024

Is your feature request related to a problem?

incompatibility of PHP-Parser 4.x and 5.x, see nikic/PHP-Parser#981

Describe the solution you'd like

PHP-CS ships with a bunch of polyfill values for Tokens in https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/45c61f55b6ead1a86212c0a75c7a635456a2844e/src/Util/Tokens.php

PHP-Parser 5.x requires these token values to be integers. currently these polyfill tokens contain string values.
PHP-Parser 4.x did not define the value type of these constants.

phpDocumentor contained a similar issue, which was fixed in phpDocumentor/TypeResolver#197

Additional context (optional)

  • I intend to create a pull request to implement this feature.
@jrfnl
Copy link
Member

jrfnl commented Feb 21, 2024

@staabm I'm aware of this, but I am not aware of a real-life use-case which would justify making breaking changes in PHPCS for something introduced at a later date in an unrelated tool, which should not be loaded while PHPCS is running anyway.

What I mean is:

  • PHPCS does not load anything via the Composer autoloader which would cause this conflict to be a problem for other projects.
  • Similarly, PHP-Parser should not be (auto-)loaded when running PHPCS, so again, this shouldn't cause a problem.

The only real-life case I've seen/found so far is for the code coverage runs on PHPCS itself and only when running on PHP 8.0 with PHPUnit 9.x (9.3 or higher), which is a situation in which both projects would be loaded at the same time. And that has already been solved in #245.

@jrfnl
Copy link
Member

jrfnl commented Feb 21, 2024

P.S.: aside from the above, I suggest reporting this to PHP-Parser as they are the ones causing the problem.

@staabm
Copy link
Author

staabm commented Feb 21, 2024

I suggest reporting this to PHP-Parser

its already reported, see nikic/PHP-Parser#981

lets see whether llaville can clarify his use-case

@jrfnl
Copy link
Member

jrfnl commented Feb 21, 2024

@staabm In that case, let's close this issue as reported to the wrong project.

@staabm
Copy link
Author

staabm commented Feb 21, 2024

IMO the cause of the problem is here - but its up to you

@jrfnl
Copy link
Member

jrfnl commented Feb 21, 2024

@staabm No, the cause of the problem is in PHP-Parser. PHPCS does not use PHP-Parser, so there is no reason for PHPCS to comply with whatever arbitrary demands (okay, not completely arbitrary, but still) PHP-Parser puts on these global PHP constants, which PHPCS polyfills.

@jrfnl jrfnl closed this as not planned Won't fix, can't repro, duplicate, stale Feb 21, 2024
@jrfnl
Copy link
Member

jrfnl commented Feb 22, 2024

For the record: the root cause of the use case which was the trigger for this issue was an old version of phpDocumenter/TypeResolver being used and had nothing to do with PHPCS (at all).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants