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

fix: handle deprecated class LNumber in nikic/php-parser v5 #148

Merged
merged 1 commit into from
May 17, 2024

Conversation

nikophil
Copy link
Collaborator

I think since the Automapper is the only entrypoint of this whole lib, we can add class_alias() in this class, and problem would be solved.

WDYT?

@joelwurtz
Copy link
Member

joelwurtz commented May 17, 2024

Not a fan of this, i would keep declaring the alias where it is really needed, not on the AutoMapper class, main problem is that if we change this later (like some lib that don't depend on the AutoMapper class) we would not have those alias

@GromNaN
Copy link
Contributor

GromNaN commented May 17, 2024

Using class_alias for classes that don't belong to this library can lead to other issues (like an other library checking if the class exist ...). That's bad that php-parser doesn't provide a clean migration path. The best would be that php-parser 4.x would have provided the new classes directly.

A clean alternative would be to create factory functions here, that uses the correct class name depending on the php-parser version.

@nikophil
Copy link
Collaborator Author

nikophil commented May 17, 2024

like an other library checking if the class exist ...

you're right, I did not think about it

The best would be that php-parser 4.x would have provided the new classes directly.

indeed 🤷 this deprecation layer is not really clean IMO

A clean alternative would be to create factory functions here, that uses the correct class name depending on the php-parser version.

this was actually what I done in an earlier version 😅

@nikophil nikophil force-pushed the fix/php-parser-deprecated-class branch from af0b990 to 9f81989 Compare May 17, 2024 12:01
@nikophil nikophil requested review from joelwurtz and Korbeil May 17, 2024 12:06
@joelwurtz joelwurtz merged commit 39fd301 into jolicode:main May 17, 2024
5 checks passed
* Constructs an integer number scalar node.
*
* @param int $value Value of the number
* @param array<string, mixed> $attributes Additional attributes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This functions should be @internal

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

Successfully merging this pull request may close these issues.

3 participants