Treat namespaced names as single token (reduced version)#5827
Closed
nikic wants to merge 2 commits intophp:masterfrom
Closed
Treat namespaced names as single token (reduced version)#5827nikic wants to merge 2 commits intophp:masterfrom
nikic wants to merge 2 commits intophp:masterfrom
Conversation
nikic
force-pushed
the
namespaced-names-minimal
branch
from
79eb561 to
6cf2597
Compare
TysonAndre
reviewed
Jul 20, 2020
Contributor
There was a problem hiding this comment.
One minor oversight in the implementation that should be easy to address (It's easy to forget about. I assume the author will address this, and supporting tokens would generally be assumed to be part of the final implementation of any RFC adding a new token type, for anyone wondering. I've made this mistake in drafts of other unrelated RFC implementations I've worked on):
- Constants should be provided to userland for T_NAME_QUALIFIED, T_NAME_FULLY_QUALIFIED, and T_NAME_RELATIVE
- The implementation of
char *get_token_type_nameshould be updated with the new token types, so that calls from PHP to token_name() (and possibly error messages)
At least 2 parts of ext/tokenizer/tokenizer_data.c need to be updated
void tokenizer_register_constants(INIT_FUNC_ARGS) {
REGISTER_LONG_CONSTANT("T_THROW", T_THROW, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("T_INCLUDE", T_INCLUDE, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("T_INCLUDE_ONCE", T_INCLUDE_ONCE, CONST_CS | CONST_PERSISTENT);char *get_token_type_name(int token_type)
{
switch (token_type) {
case T_THROW: return "T_THROW";
case T_INCLUDE: return "T_INCLUDE";
case T_INCLUDE_ONCE: return "T_INCLUDE_ONCE";Currently (the exact value may be different, I also merged another RFC implementation for this php build),
php > echo token_name(T_NAME_QUALIFIED);
Warning: Uncaught Error: Undefined constant "T_NAME_QUALIFIED" in php shell code:1
Stack trace:
#0 {main}
thrown in php shell code on line 1
php > echo json_encode(token_get_all('<?php use Foo\Bar as X;'));
[[389,"<?php ",1],[352,"use",1],[392," ",1],[314,"Foo\\Bar",1],[392," ",1],[336,"as",1],[392," ",1],[311,"X",1],";"]
php > echo token_name(314);
UNKNOWN
Member
Author
There was a problem hiding this comment.
Thanks, I had indeed forgotten about this. Should be fixed now!
nikic
force-pushed
the
namespaced-names-minimal
branch
from
6cf2597 to
ad37f01
Compare
fabpot
added a commit
to symfony/symfony
that referenced
this pull request
Aug 10, 2020
…errabus) This PR was merged into the 3.4 branch. Discussion ---------- [ClassLoader][Routing] Fix namespace parsing on php 8 | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | N/A | License | MIT | Doc PR | N/A The way namespace declarations are parsed has changed in php 8 (see php/php-src#5827). This PR should fix the corresponding issues in the ClassLoader and Routing components. Note that Doctrine Annotations suffers from the same issue (doctrine/annotations#339). I had to run the Routing tests locally with doctrine/annotations#344 applied. Commits ------- 3d293b2 [ClassLoader][Routing] Fix namespace parsing on php 8.
jrfnl
added a commit
to PHPCompatibility/PHPCompatibility
that referenced
this pull request
Nov 1, 2022
…clarations
As of PHP 8.0, reserved keywords can be used in namespace declarations, with the exception of the `namespace` keyword at the start of a namespace name.
> In the interest of consistency, the namespace declaration will accept any name, including isolated reserved keywords:
>
> ```php
> namespace iter\fn; // Legal
> namespace fn; // Legal
> ```
> This is to avoid a discrepancy where defining symbols like `iter\fn\operator` is allowed, but `fn\operator` is not. The only restriction is that the namespace name cannot start with a `namespace` segment:
>
> ```php
> namespace namespace; // Illegal
> namespace namespace\x; // Illegal
> ```
This commit:
* Adjusts the `ForbiddenNames` sniff to prevent triggering an error for reserved keywords being used in a namespace name when the `testVersion` has a minimum version of PHP `8.0` or higher.
* Adjusts the `ForbiddenNames` sniff to be cross-version compatible with different PHP/PHPCS versions for the new "namespaced names as single token" tokens.
This addresses issue 1226 for this sniff.
* Adjusts the testcase generation file to exclude the testcase with a namespace declaration using `namespace` at the start of the name, including a regenerated test case file as the behaviour is different compared to the other keywords.
* Special cases the "namespace declaration" tests.
These will now trigger an error when PHP < 8.0 needs to be supported, but will no longer trigger an error when PHP 8.0 is the minimum supported version according to the `testVersion` setting.
* Adds separate test cases to the `ForbiddenNamesUnitTest.3.inc` test file to verify the correct handling of a namespace name starting with `namespace`.
* Annotates that the testcase on line 8 in the `ForbiddenNamesUnitTest.3.inc` test case file will only trigger errors when PHP < 8 is part of the `testVersion` setting and adjusts the unit test to make sure a `testVersion` is passed for that test.
Ref:
* https://wiki.php.net/rfc/namespaced_names_as_token
* php/php-src#5827
* php/php-src@7a3dcc3
jrfnl
added a commit
to PHPCompatibility/PHPCompatibility
that referenced
this pull request
Nov 1, 2022
…clarations
As of PHP 8.0, reserved keywords can be used in namespace declarations, with the exception of the `namespace` keyword at the start of a namespace name.
> In the interest of consistency, the namespace declaration will accept any name, including isolated reserved keywords:
>
> ```php
> namespace iter\fn; // Legal
> namespace fn; // Legal
> ```
> This is to avoid a discrepancy where defining symbols like `iter\fn\operator` is allowed, but `fn\operator` is not. The only restriction is that the namespace name cannot start with a `namespace` segment:
>
> ```php
> namespace namespace; // Illegal
> namespace namespace\x; // Illegal
> ```
This commit:
* Adjusts the `ForbiddenNames` sniff to prevent triggering an error for reserved keywords being used in a namespace name when the `testVersion` has a minimum version of PHP `8.0` or higher.
* Adjusts the `ForbiddenNames` sniff to be cross-version compatible with different PHP/PHPCS versions for the new "namespaced names as single token" tokens.
This addresses issue 1226 for this sniff.
* Adjusts the testcase generation file to exclude the testcase with a namespace declaration using `namespace` at the start of the name, including a regenerated test case file as the behaviour is different compared to the other keywords.
* Special cases the "namespace declaration" tests.
These will now trigger an error when PHP < 8.0 needs to be supported, but will no longer trigger an error when PHP 8.0 is the minimum supported version according to the `testVersion` setting.
* Adds separate test cases to the `ForbiddenNamesUnitTest.3.inc` test file to verify the correct handling of a namespace name starting with `namespace`.
* Annotates that the testcase on line 8 in the `ForbiddenNamesUnitTest.3.inc` test case file will only trigger errors when PHP < 8 is part of the `testVersion` setting and adjusts the unit test to make sure a `testVersion` is passed for that test.
Ref:
* https://wiki.php.net/rfc/namespaced_names_as_token
* php/php-src#5827
* php/php-src@7a3dcc3
jrfnl
added a commit
to PHPCompatibility/PHPCompatibility
that referenced
this pull request
Nov 1, 2022
…clarations
As of PHP 8.0, reserved keywords can be used in namespace declarations, with the exception of the `namespace` keyword at the start of a namespace name.
> In the interest of consistency, the namespace declaration will accept any name, including isolated reserved keywords:
>
> ```php
> namespace iter\fn; // Legal
> namespace fn; // Legal
> ```
> This is to avoid a discrepancy where defining symbols like `iter\fn\operator` is allowed, but `fn\operator` is not. The only restriction is that the namespace name cannot start with a `namespace` segment:
>
> ```php
> namespace namespace; // Illegal
> namespace namespace\x; // Illegal
> ```
This commit:
* Adjusts the `ForbiddenNames` sniff to prevent triggering an error for reserved keywords being used in a namespace name when the `testVersion` has a minimum version of PHP `8.0` or higher.
* Adjusts the `ForbiddenNames` sniff to be cross-version compatible with different PHP/PHPCS versions for the new "namespaced names as single token" tokens.
This addresses issue 1226 for this sniff.
* Adjusts the testcase generation file to exclude the testcase with a namespace declaration using `namespace` at the start of the name, including a regenerated test case file as the behaviour is different compared to the other keywords.
* Special cases the "namespace declaration" tests.
These will now trigger an error when PHP < 8.0 needs to be supported, but will no longer trigger an error when PHP 8.0 is the minimum supported version according to the `testVersion` setting.
* Adds separate test cases to the `ForbiddenNamesUnitTest.3.inc` test file to verify the correct handling of a namespace name starting with `namespace`.
* Annotates that the testcase on line 8 in the `ForbiddenNamesUnitTest.3.inc` test case file will only trigger errors when PHP < 8 is part of the `testVersion` setting and adjusts the unit test to make sure a `testVersion` is passed for that test.
Ref:
* https://wiki.php.net/rfc/namespaced_names_as_token
* php/php-src#5827
* php/php-src@7a3dcc3
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
2 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.
This is a smaller version of #5720, which treats namespaced names as a single token and allows any reserved keywords in the namespace declaration, but does not relax reserved keyword restrictions for class names, function names, etc.
RFC: https://wiki.php.net/rfc/namespaced_names_as_token