-
-
Notifications
You must be signed in to change notification settings - Fork 82
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
Start PHP 8.0 support #47
Conversation
In order for this to work, we still need to make sure laminas-code does not crash with:
This is a lot of work that requires much more care for this specific package :S |
If you agree, I'll do my best at least to start the support with the current test suite. I've renamed the PR accordingly: is it ok for you? |
@Slamdunk if you have time/willpower to throw at the problem, I can gladly support you! This has been on my to-do list as well, since |
@Slamdunk an alternative (which had already be planned, but can't find where) is to drop all the scanner components from That effectively means that everything that has to do with token changes can be removed, and therefore half the work is gone. |
@@ -538,6 +538,8 @@ protected function scan() | |||
|
|||
case T_VARIABLE: | |||
case T_STRING: | |||
case T_NAME_QUALIFIED: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where should I put the backword compatibility definitions for the new constants?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anywhere you want, as long as it's @internal
, I suppose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Analyzing the source, \Laminas\Code\Scanner\TokenArrayScanner::scan
seems the right place: https://github.com/laminas/laminas-code/pull/47/files#diff-c08ce1832ae9f8788d45e1cf8fa62d743ede600c44a4e622ec20b4b684383320R383
Had a run with --process-isolation
, and everything works fine.
@Ocramius well, I just made tests passing on PHP 8.0, I only need to make the BC, see #47 (comment) |
72b83af
to
8493624
Compare
From what you wrote, you clearly have much wider perspective on the overall future of this library. May I ask you if this PR can be tagged after merge so I can continue my work on |
eb38dd7
to
03e037e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢
Signed-off-by: Filippo Tessarotto <zoeslam@gmail.com>
Signed-off-by: Filippo Tessarotto <zoeslam@gmail.com>
Signed-off-by: Filippo Tessarotto <zoeslam@gmail.com>
Signed-off-by: Filippo Tessarotto <zoeslam@gmail.com>
Signed-off-by: Filippo Tessarotto <zoeslam@gmail.com>
…PHP 8.0 Signed-off-by: Filippo Tessarotto <zoeslam@gmail.com>
Signed-off-by: Filippo Tessarotto <zoeslam@gmail.com>
Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
d55b897
to
e3a1c6a
Compare
Close #46