Php_codesniffer: PHP 8.0 | Proposal for handling "namespaced names as single token"

[jrfnl]

What changes in PHP 8

From the RFC:

PHP distinguishes four kinds of namespaced names:

• Unqualified names like Foo, which coincide with identifiers.
• Qualified names like FooBar.
• Fully qualified names like Foo.
• Namespace-relative names like namespaceFoo.

Each of these kinds will be represented by a distinct token:

Foo;
// Before: T_STRING
// After:  T_STRING
// Rule:   {LABEL}

Foo\Bar;
// Before: T_STRING T_NS_SEPARATOR T_STRING
// After:  T_NAME_QUALIFIED
// Rule:   {LABEL}("\\"{LABEL})+

\Foo;
// Before: T_NS_SEPARATOR T_STRING
// After:  T_NAME_FULLY_QUALIFIED
// Rule:   ("\\"{LABEL})+

namespace\Foo;
// Before: T_NAMESPACE T_NS_SEPARATOR T_STRING
// After:  T_NAME_RELATIVE
// Rule:   "namespace"("\\"{LABEL})+

Individual namespace segments may contain reserved keywords:

// This is interpreted as T_LIST (i.e., as a reserved keyword):
List
// All of the following are interpreted as legal namespaced names:
\List
FooBar\List
namespace\List

Whitespace (or comments) is not permitted between namespace separators.

Ref: https://wiki.php.net/rfc/namespaced_names_as_token

TL;DR

So, in short, PHP 8.0 introduces 3 new tokens for identifiers, on top of the existing T_STRING:

• T_NAME_FULLY_QUALIFIED
• T_NAME_RELATIVE
• T_NAME_QUALIFIED

Perspective on this as a sniff author

In the long run, I think these new tokens will make writing sniffs analyzing function calls, class instantiation and the likes much easier, especially when namespace/use statement context needs to be taken into account when matching identifiers.

However, this is also a big breaking change which would require a significant change in how any such sniff is analyzing code.

A significant part of the current test failures against the nightly build for PHPCS itself can be traced back to this particular change. Same for several external standards which I examined.

Proposal for how to handle this change in PHPCS

I'd like to propose the following and would like to invite other sniff authors to give their opinion on this as well:

PHPCS 3.x

For PHPCS 3.x, basically "undo" the change, disregarding any (no longer reserved) keywords.

In practice, this would mean splitting the above tokens on the namespace separator \ and tokenizing the name parts as T_STRING (aside from a potential namespace keyword which would need to be tokenized as T_NAMESPACE).

PHPCS 4.x

For PHPCS 4.x, backfill the PHP 8.0 tokenization for the supported PHP versions < 8.0.

Reasoning

• It moves the _breaking change_ part of this PHP 8.0 change to a major version in PHPCS, which is expected to have breaking changes anyhow.
• It will allow existing sniffs looking for namespace names, use statements, function calls etc to continue to function as before while on PHPCS 3.x.
• It moves the decision on the timing of _when_ to rewrite/adjust these sniffs back to the maintainers of standards as it would coincide with when they will start supporting PHPCS 4.x.
• It moves the decision on whether to concurrently support both types of tokenization to standard maintainers, as it now becomes a decision on whether to concurrently support PHPCS 3.x as well as 4.x in the standard at the same time or to drop support for PHPCS 3.x, or potentially, to have separate branches/versions which work with the different PHPCS versions.

Opinions ?

/cc @gsherwood @photodude @umherirrender @TomasVotruba @sirbrillig @michalbundyra @djoos @louisl @kukulich @carusogabriel @klausi @dereuromark @michalbundyra @jmarcil @weierophinney (and please feel free to ping anyone I missed)

[photodude]

@jrfnl I've kinda dropped out of development activities for the time being. I'm not sure I have much to contribute at the moment. I think the proposal looks good.

[jrfnl]

PR #3063 addresses this for PHPCS 3.x.

I have also prepared the necessary changes for PHPCS 4.x, but PR #3066 should be reviewed & merged + merged into the PHPCS 4.x branch before I can pull the PHPCS 4.x branch for this change without it causing conflicts/leaving stray tokens behind.

[jrfnl]

@gsherwood Should this ticket be re-opened for the PHPCS 4.x part ? Only the 3.x part has been fixed so far.

Once the union types PR (#3032) is merged in PHPCS 3.6.0 and cherrypicked to 4.x, I will pull the PR for this change to the 4.x branch. (would conflict otherwise)

[gsherwood]

I'll move it to the 4.0 project.

[gsherwood]

After some mucking around, I would really need this to be 2 tickets to organise things properly. So have just moved it to 4.0 instead of keeping it in both.

[jrfnl]

PR #3155 has now been pulled to address the changes as proposed for PHPCS 4.x.

[gsherwood]

PR has been merged but leaving In Progress until changelog is written

[gsherwood]

Forgot to add the changelog entry for this, but have done so now.