Support for PHP 8.1 Explicit Octal Notation in the Tokenizer#3481
Merged
gsherwood merged 8 commits intosquizlabs:masterfrom Dec 17, 2021
Merged
Conversation
added 3 commits
November 20, 2021 18:50
…o777") from two separate T_NUMBER and T_STRING tokens into a single T_NUMBER token with the correct value
…ence (e.g."0o777") from two separate T_NUMBER and T_STRING tokens into a single T_NUMBER token with the correct value
…ill works correctly with Explicit octal values when they have to be backfilled
gsherwood
requested changes
Dec 14, 2021
| { | ||
| $tokens = self::$phpcsFile->getTokens(); | ||
|
|
||
| $number = $this->getTargetToken($testData['marker'], [T_LNUMBER, T_DNUMBER, T_STRING]); |
Member
There was a problem hiding this comment.
Are we only looking for T_LNUMBER tokens here or are there test cases to add where T_DNUMBER and T_STRING would be valid to find? Might need some tests to ensure some strings that look octal are not actually being touched ?
jrfnl
reviewed
Dec 14, 2021
jrfnl
reviewed
Dec 14, 2021
| <?php | ||
|
|
||
| /* testExplicitOctal declaration */ | ||
| $foo = 0o137041; |
Contributor
There was a problem hiding this comment.
Again: what about a number with uppercase o ?
Contributor
There was a problem hiding this comment.
Also wondering if any test could be conceived where the token stream should not be changed (i.e. guard against false positives).
jrfnl
reviewed
Dec 14, 2021
added 5 commits
December 15, 2021 12:03
Check for lower- or upper-case 'o'. Don't adjust $token and $tokens variables; but set the token in $finalTokens, and adjust pointers.
gsherwood
added a commit
that referenced
this pull request
Dec 17, 2021
gsherwood
added a commit
that referenced
this pull request
Dec 17, 2021
gsherwood
added a commit
that referenced
this pull request
Dec 17, 2021
Member
|
Thanks a lot for the contribution and for making those requested changes. I made a couple of minor changes to the tests but didn't add anything about a case where the token should not be changed (can't think of a case off the top of my head). If anyone has any, please post here and I'll add the test case in. |
jrfnl
added a commit
to PHPCSStandards/PHPCSUtils
that referenced
this pull request
Feb 28, 2022
This add support for the explicit octal notation using a `0o`/`0O` prefix as supported in PHP as of PHP 8.1. The only method which needed adjusting was the `Numbers::getCompleteNumber()` method, which now backfills the tokenization when needed. For the `Numbers::isOctalInt()` method, a small tweak to the regex was all that was needed. As for the `Numbers::getDecimalValue()` method: this already handled the conversion correctly due to the use of the PHP native `octdec()` function. From the RFC: > Surprisingly PHP already has support for this notation when using the `octdec()` and `base_convert()` functions. Includes adding unit tests to safeguard support in all relevant methods in the class. Refs: * https://wiki.php.net/rfc/explicit_octal_notation * php/php-src#6360 * squizlabs/PHP_CodeSniffer#3481 * squizlabs/PHP_CodeSniffer#3552
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Added support for Explicit Octal Notation (introduced in PHP 8.1) in the tokenizer.
When running phpcs against PHP < 8.1, an expression like
separates the
0o777into two tokens; aT_LNUMBERwith a value"0"and aT_STRINGwith a value"o777".These need to be treated as single
T_LNUMBERtoken with a value of"0o777"to be processed correctly by phpcs