Update parser token constant values to ints instead of strings

[mukuru-shaun]

I just came across a incompatibility with this package, and nikic/php-parser.

This package defines a few "parser token" constants here:

TypeResolver/src/Types/ContextFactory.php

Lines 51 to 57 in e03361c

	if (!defined('T_NAME_QUALIFIED')) {
	define('T_NAME_QUALIFIED', 'T_NAME_QUALIFIED');
	}
	if (!defined('T_NAME_FULLY_QUALIFIED')) {
	define('T_NAME_FULLY_QUALIFIED', 'T_NAME_FULLY_QUALIFIED');
	}

nikic/php-parser does something similar (but perhaps more comprehensive?) here: https://github.com/nikic/PHP-Parser/blob/master/lib/PhpParser/compatibility_tokens.php
The nikic/php-parser defines a superset of the tokens defined in this package, with the key difference being they are given int values instead of string values.

This leads to an issue, where if the code in this package is loaded first, the few tokens that this package has defined will have string values, and break when being used with nikic/php-parser as they expect integer values.

The PHP documentation seems to indicate that user land defined parser tokens should indeed be int values: https://www.php.net/manual/en/tokens.php

[phil-davis]

This fixes my problem reported in nikic/PHP-Parser#974

I can change the values to:

if (!defined('T_NAME_QUALIFIED')) {
    define('T_NAME_QUALIFIED', 393);
}

if (!defined('T_NAME_FULLY_QUALIFIED')) {
    define('T_NAME_FULLY_QUALIFIED', 392);
}

to exactly match the ints used in https://github.com/nikic/PHP-Parser/blob/master/lib/PhpParser/Parser/Php7.php

Or I can use the values suggested in the PR:

if (!defined('T_NAME_QUALIFIED')) {
    define('T_NAME_QUALIFIED', 10001);
}

if (!defined('T_NAME_FULLY_QUALIFIED')) {
    define('T_NAME_FULLY_QUALIFIED', 10002);
}

Both work.

I suppose that the value just needs to a be int that is unique - 2 token names must not have the same value.

[jaapio]

Thank you very much for this fix.

I understand the issue, and I agree we should solve this. However I'm in doubt, this is a breaking change for people using this library in the wrong way. It should not really matter as the values of the constants are changing between php versions. It feels like it's safer to use high values that are not used by php itself, as we will avoid conflicts in the future.

Nevertheless it feels like this will be come an issue for a huge number of users having php parser and this library as dependency in their list of dependencies.

Maybe we should just remove the fallback values as this library is not supporting php 7 anymore?

[mukuru-shaun]

Hi @jaapio , thanks so much for getting back to me.

It feels like it's safer to use high values that are not used by php itself, as we will avoid conflicts in the future

Using those high numbers is literally what the PHP documentation recommends, which is why I did that in this PR. I do think it could create an issue where another library sets one of those values to a different token it defines, then there could be edge case issues.
I think the way that php-parser sets the value by checking that they are all set and unique is probably the most comprehensive, but I thought it might be best to PR the simplest fix and we can go from there 😄

[jaapio]

The pr is merged and a new version has been tagged. Thanks for this nice patch, I think we should concider dropping this forward compatibility right away when we stop support for php 7.4 which can be done in v2.

I will create an issue for that.

[phil-davis]

I just came across a incompatibility with this package, and nikic/php-parser

confirmed fixed for me - php-parser and type-resolver are working together. Thanks.