ObjectDeclarations::getDeclaredConstants(): mult-declarations are not handled correctly

[jrfnl]

Bug Description

PHP allows for multi-constant declarations, but the ObjectDeclarations::getDeclaredConstants() method - or rather, the underlying ObjectDeclarations::analyzeOOStructure() method - does not take those into account.

Given the following reproduction Scenario

The issue happens when running this code:

var_dump(ObjectDeclarations::getDeclaredConstants($phpcsFile, $pointerToClassToken));

... over a file containing this code:

class testMultiConst {
    public const string CONST_A = 'name',
        CONST_B = 'foo',
        CONST_C = 'bar';
}

I'd expect the following behaviour

The return value of the ObjectDeclarations::getDeclaredConstants() method to contain three entries:

[
    'CONST_A' => #,
    'CONST_B' => #,
    'CONST_C' => #,
]

Instead this happened

The return value of the ObjectDeclarations::getDeclaredConstants() method only contains one entry:

[
    'CONST_A' => #,
]

Additional Context (optional)

This needs a good think as the stackPtr returned for CONST_A is currently the stackPtr to the const keyword, which allows for passing the resulting tokens to methods like Constants::getProperties().

The const token is basically the entry point for most analysis on class constant declarations.

However, for multi-constant declarations, that would mean that the stackPtr for each of the constants found in one multi-declaration would be the same const token.

Now, while this keeps the method compatible with methods like Constants::getProperties(), if the method is used to analyse the default value of each constant, things get more complicated as the sniff using the method, would then still need to analyse the constant declaration to find each individual constant name.
Along the same lines, if the method is used to check if a class constant is declared, an isset($ooConst['SOME_NAME']) may return false, while the constant SOME_NAME is in actual fact declared, but as the second or later constant name in a multi-declaration.

One idea would be to change the return value format to contain an array for each constant name found, like so:

[
    'CONST_A' => ['constPtr' => #, 'namePtr' => #],
    'CONST_B' => ['constPtr' => #, 'namePtr' => #],
    'CONST_C' => ['constPtr' => #, 'namePtr' => #],
]

... but changing the return value format would be a breaking change.

Maybe the ObjectDeclarations::getDeclaredConstants() method should get an optional third parameter $returnFormat, which could then be used to toggle between the "old" format and the new format, with the default being the "old" format.
In version 2.0, the default value for the parameter could then be changed to default to the "new" format.

Or maybe the new parameter should be $includeMultiConstants, with the default being false and returning the return value as it currently is (disregard multi-declarations, stackPtr to const).
And if the $includeMultiConstants parameter would be set to true, it would return all names for multi-declarations and the stackPtr would be set to the token for each constant name.

That way, users could get the return value which would be most useful to their usecase by toggling the new parameter.

Either way, these are just some thoughts on this. Input welcome.

Note: multi-property declarations are handled correctly, but for property declaration analysis, the T_VARIABLE token is the entry point for further analysis. Also see #691

Tested Against develop branch?

• I have verified the issue still exists in the develop branch of this package.