T_AMPERSAND token

in sebastianbergmann/php-code-coverage#1125 we can see the token-names for tokenizing the file https://github.com/sebastianbergmann/php-code-coverage/blob/555cd5e6f887648198b96ed89c6880d6f61a0733/tests/_files/source_with_class_and_anonymous_function.php

changed

-        <token name="T_AMPERSAND_FOLLOWED_BY_VAR_OR_VARARG">&amp;</token>
+        <token name="T_AMPERSAND">&amp;</token>

with the 2.0 release.

T_AMPERSAND was a token type in PHP4.x and it only existed in theseer/tokenizer, because it was hard-coded in

tokenizer/src/Tokenizer.php

Line 35 in d1dd771

'&' => 'T_AMPERSAND',

since PHP5+ the builtin token names are
T_AMPERSAND_FOLLOWED_BY_VAR_OR_VARARG
and/or T_AMPERSAND_NOT_FOLLOWED_BY_VAR_OR_VARARG

https://3v4l.org/tU46U#veol

it looks like with 2.0.x we are now using T_AMPERSAND more often as we did with the 1.x counterpart.

I am not sure yet what todo about it, but wanted to leave it here so we can discuss the possible impact and/or a fix

dropping this line will make the phpunit/code-coverage regression go away:

tokenizer/src/Tokenizer.php

Line 35 in d1dd771

'&' => 'T_AMPERSAND',

I guess the question is, how we treat the token names in regard to backwards compatbility and whether we need/want to align the token names here with the token names in php-src

My assumption is - and I did not verify that in any way yet - this is caused by the switch to PHPToken::tokenize() and it behaving differently than the old token_get_all. I believe that actually was one reason to introduce the ::tokenize()-method into PHP in the first place, so we have a reliable object only result set?

If that is correct, we might not need the internal Map at all anymore.

So, if we stick with ::tokenize() I guess that's just a change we have to accept, maybe allowing us to clean up the mapping logic as the (use) case should no longer exist.

This class provides an alternative to token_get_all(). While the function returns tokens either as a single-character string, or an array with a token ID, token text and line number, PhpToken::tokenize() normalizes all tokens into PhpToken objects, which makes code operating on tokens more memory efficient and readable.

Reading the documentation helps :-)

So at least my memory was right.

Okay, a quick test shows the Object is equally useless when it comes to names, e.g. the ::name() method returns ) for example instead of an actual token name like T_CLOSE_BRACKET.

So I'm not sure as to what to do here :)

it looks like with 2.0.x we are now using T_AMPERSAND more often as we did with the 1.x counterpart.

To me, that would only make sense when the ::tokenize() result uses & as a name more frequently. Otherwise we did not change anything I can see that should be relevant to this.

I think easiest way forward would be to only keep the Tokenizer map for those single-characters which are still returned by getTokenName()

The leaves the question of which ones are that? ;-)

And, I guess we should change the map access to use ::getTokenName() then, rather than using ->text, right?

see e.g. https://3v4l.org/bfRMm#veol

::getTokenName() then, rather than using ->text, right?

yes

see e.g. https://3v4l.org/bfRMm#veol

Funny.

So from PHP 8.1 onwards, it appears that only the & has been removed. At least that's how I interprete the diff view on 3v4l.

Given we, for 2.0.0 require 8.1+, I guess that means we can indeed remove that line from our map.

oohh I think we can just do: if (isset(self::MAP[$tok->getTokenName()])) {

because of

Object is equally useless when it comes to names, e.g. the ::name() method returns ) for example instead of an actual token name like T_CLOSE_BRACKET.

I'll update the code.

I took the output for the 8.1+ from 3v4l and ran it through sort and uniq:

$ cat single_char.txt | sort | uniq

string(1) "!"
string(1) "%"
string(1) "("
string(1) ")"
string(1) "*"
string(1) "+"
string(1) ","
string(1) "-"
string(1) "/"
string(1) ";"
string(1) "<"
string(1) "="
string(1) ">"
string(1) "@"
string(1) "["
string(1) "]"
string(1) "^"
string(1) "`"
string(1) "{"
string(1) "|"
string(1) "}"
string(1) "~"

So that's 22 chars.
Our Map array still contains more entries after I removed the &:

        '(' => 'T_OPEN_BRACKET',
        ')' => 'T_CLOSE_BRACKET',
        '[' => 'T_OPEN_SQUARE',
        ']' => 'T_CLOSE_SQUARE',
        '{' => 'T_OPEN_CURLY',
        '}' => 'T_CLOSE_CURLY',
        ';' => 'T_SEMICOLON',
        '.' => 'T_DOT',
        ',' => 'T_COMMA',
        '=' => 'T_EQUAL',
        '<' => 'T_LT',
        '>' => 'T_GT',
        '+' => 'T_PLUS',
        '-' => 'T_MINUS',
        '*' => 'T_MULT',
        '/' => 'T_DIV',
        '?' => 'T_QUESTION_MARK',
        '!' => 'T_EXCLAMATION_MARK',
        ':' => 'T_COLON',
        '"' => 'T_DOUBLE_QUOTES',
        '@' => 'T_AT',
        '%' => 'T_PERCENT',
        '|' => 'T_PIPE',
        '$' => 'T_DOLLAR',
        '^' => 'T_CARET',
        '~' => 'T_TILDE',
        '`' => 'T_BACKTICK'

That implies we'd loose more of "our" Tokens.

Are we sure we want that?

The map lookup of course only would work as before if we stick to ->text I guess..

so that means we should stick with if (isset(self::MAP[$tok->text])) { and remove only & because

So from PHP 8.1 onwards, it appears that only the & has been removed. At least that's how I interprete the diff view on 3v4l.

?

so that means we sould stick with if (isset(self::MAP[$tok->text])) { and remove only & because

So from PHP 8.1 onwards, it appears that only the & has been removed. At least that's how I interprete the diff view on 3v4l.

?

There's a button in the 3v4l output to get a visual diff:

So I guess, ... yes?

I'm still confused as to why that should be happening more frequently and would like to understand it better before I commit to that change.

Could it be that token_get_all actually returned a TOKEN for those cases already? And now that we look at ->text value only, we screw it up?

So if we indeed remove the & from the map, we would be fine: There is no reason to assume we still get a single char & as ->text - right? Even something like $x="&"; would tokenize differently..

Okay, token_get_all returns a token array since PHP 8.1 as to be expected, a single char string for 8.0 and 7.4 (didn't test older versions). That means indeed using a ->text lookup is a BC break for &.

That means with PHP 8.1+, we shouldn't have seen T_AMPERSAND at all in the XML output with 1.x, but with PHP 8.0 and earlier. This at least qualifies as an unintended side effect and an inconsistency between PHP versions.

So now we have to decide:

Do we go for the consistency? Than we should keep the code as is, as it also normalizes the two potential cases the 8.1+ tokenizing has into a single token, same as it was before PHP 8.1.
We stick closer to the PHP tokenizing, and remove the "custom" T_AMPERSAND from 2.x.

I'm leaning to removing it as with 2.x we require PHP 8.1+ and there might be some sort of value in having the two explicit tokens represented. Even if I fail to see a use case ;-)

Optionions?

Do we go for the consistency? Than we should keep the code as is, as it also normalizes the two potential cases the 8.1+ tokenizing has into a single token, same as it was before PHP 8.1.

I don't think the job of this lib is to interpret tokens differently than php-src and change meaning/abstract details away

We stick closer to the PHP tokenizing, and remove the "custom" T_AMPERSAND from 2.x.

👍

Do we go for the consistency? Than we should keep the code as is, as it also normalizes the two potential cases the 8.1+ tokenizing has into a single token, same as it was before PHP 8.1.

I don't think the job of this lib is to interpret tokens differently than php-src and change meaning/abstract details away

I disagree with that statement.
If you'd want a library behaving like this statement suggests, we'd need to remove the map completely.

But back when I started this library, I added a bunch of custom tokens for the very reason: To get actual Tokens for things where PHP does not.

Regardless, given that PHP 8.1+ now does have Tokens for &, i removed my custom one with Commit 024008b

I'll roll a new release.

Navigation Menu

Repository navigation

T_AMPERSAND token #44

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

T_AMPERSAND token #44

Description

Activity

staabm commented on Dec 8, 2025

theseer commented on Dec 8, 2025

theseer commented on Dec 8, 2025

theseer commented on Dec 8, 2025

theseer commented on Dec 8, 2025

staabm commented on Dec 8, 2025

theseer commented on Dec 8, 2025

staabm commented on Dec 8, 2025

staabm commented on Dec 8, 2025

theseer commented on Dec 8, 2025

staabm commented on Dec 8, 2025

theseer commented on Dec 8, 2025

theseer commented on Dec 8, 2025

theseer commented on Dec 8, 2025

staabm commented on Dec 8, 2025

theseer commented on Dec 8, 2025

theseer commented on Dec 8, 2025

theseer commented on Dec 8, 2025

theseer commented on Dec 8, 2025

theseer commented on Dec 8, 2025

staabm commented on Dec 8, 2025

theseer commented on Dec 8, 2025

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions