PHP 8.0 | Tokenizer: Handle reserved keywords being used in namespaced names

[jrfnl]

PR #3063 and it's PHPCS 4.x sister-PR addressed the namespaced name token changes as outlined in #3041.

The underlying PHP RFC, however, contains another change which is so far unaccounted for: reserved keywords can now be used in namespaced names.

The impact of this change is limited to PHP 8.0 code which actually uses reserved keywords in namespaced names, so the fact that PHPCS does not handle this yet, will probably not affect many people for now.

I've been doing some research to see what the impact is of this and have come to the conclusion that we will need to adjust the tokenizer for this in PHPCS 3.x.
With the retokenization to the PHP 8 tokens in PHPCS 4.x, I expect that there will be far less issues in PHPCS 4.x, though a slightly different fix may still be needed there too, but I will check that later just to be sure.

The problem is two-fold:

PHP 8.0

When PHPCS is run on PHP 8.0, single-level namespace names and single-level within a group use statement, are the only ones which will really cause a problem. Note: the below code sample is valid PHP 8.0 code.

namespace Class;

use Foreach;

use Package\{
    Include
};

The examples from the above code sample are likely to also cause issues in PHPCS 4.x, but I expect that those will be the only problem cases in PHPCS 4.x.

Multi-level namespace names are split into T_STRING tokens separated by T_NS_SEPARATOR tokens, independently of the token content, so reserved keywords in multi-level namespaced names are not affected. See PR #3063.

PHP 5.4-7.4

When PHPCS is run on PHP 5.4-7.4 against code written for PHP 8.0, the problem is much more extensive as reserved keywords used in namespaced names will, in that case, always tokenize as their keyword token, which will cause lots of sniffs looking for a particular keyword to throw false positives and will cause at least one sniff to go into an infinite loop (which sniff is still to be identified).

namespace For\Include\Class;

use Goto\Interface;

\Echo\Private\function_name();

Sniffs which are impacted by this.

The below lists are based on an initial scan with some test code against all PHPCS native standards. This type of scan will only show false positives, not false negatives, so in practice, there are likely to be more sniffs affected, but these lists give some indication of the problem.

As some of these sniffs contain auto-fixers, it can be presumed that the result of running phpcbf on PHP 8.0 code using reserved keywords in namespace names will cause the fixers to introduce parse errors into the code.

Also, these lists are limited to the PHPCS native sniffs, while sniffs in external standards will, of course, also be affected.

When running PHPCS on PHP 8.0

• Generic.Arrays.DisallowLongArraySyntax
• Generic.Classes.OpeningBraceSameLine
• Generic.ControlStructures.InlineControlStructure
• Generic.Files.OneObjectStructurePerFile
• Generic.NamingConventions.InterfaceNameSuffix
• Generic.PHP.DiscourageGoto
• PEAR.Classes.ClassDeclaration
• PEAR.Commenting.ClassComment
• PEAR.Commenting.FunctionComment
• PEAR.Files.IncludingFile
• PEAR.NamingConventions.ValidClassName
• PSR2.Classes.ClassDeclaration
• PSR2.Namespaces.UseDeclaration
• PSR12.Classes.ClassInstantiation
• PSR12.Files.DeclareStatement
• PSR12.Files.ImportStatement
• PSR12.Operators.OperatorSpacing
• Squiz.Classes.ValidClassName
• Squiz.ControlStructures.ForEachLoopDeclaration
• Squiz.ControlStructures.ForLoopDeclaration
• Squiz.Files.FileExtension
• Squiz.WhiteSpace.ScopeKeywordSpacing
• Squiz.ControlStructures.ForEachLoopDeclaration
• Squiz.ControlStructures.ForLoopDeclaration
• Squiz.Functions.GlobalFunction
• Squiz.Operators.ValidLogicalOperators
• Squiz.PHP.DisallowBooleanStatement
• Squiz.WhiteSpace.LogicalOperatorSpacing

There is also at least one sniff (in the Squiz standard) which causes an Internal.Exception

When running PHPCS on PHP 5.4-7.4

(partial list as the PSR1, PSR2, PSR12 and Squiz standards go into an infinite loop)

• Generic.Arrays.DisallowLongArraySyntax
• Generic.Classes.OpeningBraceSameLine
• Generic.CodeAnalysis.UnconditionalIfStatement
• Generic.ControlStructures.InlineControlStructure
• Generic.Files.OneObjectStructurePerFile
• Generic.Functions.OpeningFunctionBraceBsdAllman
• Generic.NamingConventions.CamelCapsFunctionName
• Generic.NamingConventions.InterfaceNameSuffix
• Generic.NamingConventions.TraitNameSuffix
• Generic.PHP.DiscourageGoto
• Generic.WhiteSpace.LanguageConstructSpacing
• Generic.WhiteSpace.ScopeIndent
• PEAR.Classes.ClassDeclaration
• PEAR.Commenting.ClassComment
• PEAR.Commenting.FunctionComment
• PEAR.ControlStructures.ControlSignature
• PEAR.Files.IncludingFile
• PEAR.Functions.FunctionDeclaration
• PEAR.NamingConventions.ValidClassName
• PEAR.NamingConventions.ValidFunctionName
• PEAR.WhiteSpace.ScopeIndent
• PEAR.WhiteSpace.ScopeClosingBrace
• Squiz.Functions.GlobalFunction

Proposed fix

I propose to add code to the Tokenizer\PHP class to retokenize reserved keywords used in namespace(d) names to T_STRING.

A similar, but far more limited fix will need to be applied to PHPCS 4.x for single-level namespace name use.

@gsherwood Any objections to what I suggest here ?

[jrfnl]

To ensure that reserved keywords in namespace(d) names are handled correctly, I've created a ridiculous amount of tests via a script.

Having all these tests is possibly a little over the top, at the same time, a) it does safeguard things properly and b) even though it's > 3000 tests, it only adds ~10 seconds to the test run.

@gsherwood What's your opinion on this ? I could include the script I used to generate the tests in the PR, so they can easily be regenerated if something would need to change with the code samples.

[gsherwood]

Any objections to what I suggest here ?

No objections. Makes perfect sense.

What's your opinion on this ?

It sounds like it's a bit much to me. I would include a few tests for different cases, possibly using different reserved keywords in each case, just to get some general coverage.

[timotheemoulin]

It looks like the error is still there in the latest version (or am I missing something?)

[fredden]

@timotheemoulin this issue is still open. I don't see a pull request which closes it. Please do submit such a pull request if you are keen to get this sorted out.

[jrfnl]

@timotheemoulin @fredden Actually, PR #3484 and its follow-ups should have fixed the majority of issues.

Those fixes are included in the latest releases.

@timotheemoulin If you are seeing a specific issue related to this, please provide example code with which the issue can be reproduced. If it is not exactly the issue being described here, please open a new ticket for this.

[jrfnl]

Closing as replaced by PHPCSStandards/PHP_CodeSniffer#24