[Hotfix] Tokenizer / numeric separators backfill: fix all known issues #2771
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrfnl
force-pushed
the
feature/fix-php-7.4-numeric-separators-backfill
branch
7 times, most recently
from
bc6a0f2 to
fa1f283
Compare
Test case file:
1. Remove a stray invisible whitespace character/null byte from the test case file.
2. Add a large number of additional test cases.
Test file `testBackfill()`:
1. Fixed reversed order of the parameters in the `assertSame()`. The first parameter is `$expected`, the second `$result`.
Having the parameters in the correct order makes debugging the tests easier as the messages send by PHPUnit will be correct.
2. Test that the final token has the correct code **and** the correct type.
3. Correct the expected type for two test cases based on how PHP 7.4 tokenizes these.
4. Add datasets for the new test cases which are to be handled by the backfill.
Test file new `testNoBackfil()`
Add test that numbers using numeric separators which are considered parse errors and/or which aren't relevant to the backfill, do not incorrectly trigger the backfill anyway.
This test verifies that the final token sequence is in line with the token sequence PHP 7.4 would give.
ADD TO: unit tests
When the final content of a numeric token is more than PHP_INT_MAX or when a numeric token contains a `.` or an `E/e`, it should be tokenized as `T_DNUMBER`. This fixes the first set of the unit test failures.
jrfnl
force-pushed
the
feature/fix-php-7.4-numeric-separators-backfill
branch
from
fa1f283 to
adadcd5
Compare
…ed by the backfill The prevents the backfill from kicking in when confronted with invalid use of the numeric literal separator which would be a parse error in PHP 7.4 anyway. This fixes the remaining unit test failures.
jrfnl
force-pushed
the
feature/fix-php-7.4-numeric-separators-backfill
branch
from
adadcd5 to
bb9216e
Compare
|
@gsherwood Could this PR please be milestoned for |
|
Thanks for doing all this work to make it more accurate. |
|
@gsherwood Thanks for the merge! This one was a bit of a thorn in my side ;-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tokenizer / numeric separators backfill: improve the unit tests
Test case file:
Test file
testBackfill():assertSame(). The first parameter is$expected, the second$result.Having the parameters in the correct order makes debugging the tests easier as the messages send by PHPUnit will be correct.
Test file new
testNoBackfill()Add tests to verify that numbers using numeric separators which are considered parse errors and/or which aren't relevant to the backfill, do not incorrectly trigger the backfill anyway.
This test verifies that the final token sequence is in line with the token sequence PHP 7.4 would give.
Tokenizer / numeric separators backfill: fix incorrect token types
Tokenizer / numeric separators backfill: fix incorrect token types
When the final content of a numeric token is more than PHP_INT_MAX or when a numeric token contains a
.or anE/e, it should be tokenized asT_DNUMBER.This fixes the first set of the unit test failures.
Tokenizer / numeric separators backfill: fix parse errors being handled by the backfill
The prevents the backfill from kicking in when confronted with invalid use of the numeric literal separator which would be a parse error in PHP 7.4 anyway.
This fixes the remaining unit test failures.
Fixes all currently known issues which remained after #2546
Note: on Windows the unit tests will currently not pass when run against PHP 7.4.0 due to a bug in PHP itself. See: https://bugs.php.net/bug.php?id=78978