TODO PhpToken

[WinterSilence]

1. Add new constants:
   T_NAME_FULLY_QUALIFIED, T_NAME_RELATIVE, T_NAME_QUALIFIED, T_NULLSAFE_OBJECT_OPERATOR, T_ATTRIBUTE and T_MATCH.
2. Fix PhpToken::tokenize() result:
   2.1. Implements T_NAME_RELATIVE and T_NAME_QUALIFIED tokens ((T_STRING T_NS_SEPARATOR)xN or (T_NS_SEPARATOR T_STRING)xN).
   2.2. Implement T_NULLSAFE_OBJECT_OPERATOR token (T_STRING '?->' T_STRING)
   2.3. Implement T_MATCH token
   2.3. Implement T_ATTRIBUTE token

[keradus]

what if one is using token_get_all and one of returned outcome would supposed to be T_ATTRIBUTE ? you can prepare polyfill of PhpToken::tokenize to return that registered constant, but how you want to override existing token_get_all ?

and having one covered but not another is falsy polyfill - one will see T_ATTRIBUTE is defined and prepare to use it, and then token_get_all not returning it will be an issue

[WinterSilence]

@keradus no need override token_get_all() to do it, T_ATTRIBUTE detects in PHP 7 as inline comment(T_COMMENT), we can it convert #[...] to tokens T_ATTRIBUTE, T_STRING, ]

[stof]

@WinterSilence we cannot convert it, as we cannot override token_get_all.
And if you expect the caller to convert it, the fact that of polyfilling T_ATTRIBUTE would precisely be the issue, as the caller code would detect T_ATTRIBUTE as supported already (and so not needing conversion)

[WinterSilence]

@stof

we cannot convert it

sorry don't undersand you, can you explain why we cant convert result of token_get_all()? i don't see any problem

[stof]

@WinterSilence for code calling token_get_all(), we cannot make them receive T_ATTRIBUTE instead of T_COMMENT (as we cannot change the behavior of token_get_all())

[WinterSilence]

@stof

convert result of token_get_all()

i.e. we can convert T_COMMENT token with text #[...] into T_ATTRIBUTE, T_STRING, ] tokens.

[stof]

@WinterSilence but that requires the caller to know that it has to do the conversion (which our polyfill cannot do for them), which is precisely what could break if they rely on detecting T_ATTRIBUTE to know whether token_get_all will support T_ATTRIBUTE.

[WinterSilence]

@stof users don't call token_get_all() directly, they call PhpToken::tokenize() and it's not same to token_get_all(). It seem polyfill should return result same to to his prototype. But it's point to discuss.

[stof]

@WinterSilence that's not true for existing code written before PhpToken was part of PHP.

Polyfilling PhpToken must not break code relying on the token_get_all API, which exists in PHP since years and is still used today.

[WinterSilence]

@stof PhpToken::tokenize() not alias to token_get_all(), polyfill must return result same to original.

[nicolas-grekas]

@WinterSilence do you plan to work on this? How much effort would that be? I feel like we don't have to provide such level of forward compatibility, but if you're up to work on it, I'd be happy to review.

[WinterSilence]

@nicolas-grekas if no one helps, then i can impeelments 1-2.2

It's not so hard, for example,

2.3. Implement T_MATCH token

need replace tokens ?+-> to T_NULLSAFE_OBJECT_OPERATOR token

[stof]

@WinterSilence but those T_* constants are also used by token_get_all, which is why polyfilling the constants might break things.

[nicolas-grekas]

@WinterSilence do you have an actual use case or is this for completeness? Because if not driven by a use case, better wait for someone that would really need that before considering.

[WinterSilence]

@stof

which is why polyfilling the constants might break things.

how? this constants not exist in PHP7

@nicolas-grekas as I already say, i can impeelments 1-2.2

[stof]

@WinterSilence that's the whole point. Existing code written with token_get_all and supporting multiple PHP versions might rely on detecting the constant existence to know how token_get_all will behave. For instance, PHP-CS-Fixer has a bunch of \defined('T_ATTRIBUTE') checks in its code based on token_get_all to change its behavior

[stof]

And this is true for other T_* constants. It is not specific to T_ATTRIBUTE

[WinterSilence]

@stof yes, that makes sense, but what about the incomplete implementation of intl?

[stof]

@WinterSilence our intl polyfills don't break feature detection for an existing feature of PHP when polyfilling another one.

[WinterSilence]

@stof

For instance, PHP-CS-Fixer has a bunch of \defined('T_ATTRIBUTE') checks in its code based on token_get_all to change its behavior

It's reason why this package have so many bugs. Detect features by indirect signs is bad practice.

What's if some packages uses similar checks to determine intl extension?

$formatter = class_exists('MessageFormatter') ? new MessageFormatter() : new MyMsgFormatter();
<...>
$calendar = $formatter instanceof MyMsgFormatter ? new MyCalendar() : new IntlCalendar();

Do you want remove polyfill breaks their code? I don't think.

[WinterSilence]

In PHP 7 id's of T_NAME_QUALIFIED and T_NAME_FULLY_QUALIFIED tokens assigned to T_ENCAPSED_AND_WHITESPACE and T_VARIABLE i.e. can't emulate PHP 8 tokens.