NameResolver should be able to resolve unqualified ConstFetch in namespace

[lisachenko]

Looks like constants in namespaces aren't resolved:

namespace Foo;

const A = 42;
function ($a = A) {}

Here, NameResolver will leave the name unqualified for the constant fetch. However, it should be FQN: Foo\A

[nikic]

Generally we do not know whether an unqualified constant fetch in a namespace refers to the constant in the namespace or to a global constant. As such they are not resolved by the NameResolver. If you want to do additional resolution based on your specific knowledge of which constants exist or not, you should resolve them manually.

[lisachenko]

Hmm, it's interesting https://3v4l.org/BBeFX
HHVM always tries to resolve constant name to the global namespace, but PHP resolves any constant to the current namespace, so ReflectionParameter->getDefaultValueConstantName() returns FQN, see an output:

Checking parameter for Bar\foo
string(11) "Bar\E_ERROR"
int(1)

However, values are still from the global namespace, so you are right, an additional logic should check this fact to resolve this correctly (if it's possible on parse level).

[felixfbecker]

I would really like to have an option to enable this. Let me explain my use case:
I need to implement "Goto definition". For constants and functions, I need to follow the rules of PHP: first look for the constant in the namespace, then fall back to global. That would be easy if I had the FQN, I would first search the definition index with the namespaced FQN, if not found strip the namespace and search the global scope. But the current behavior is counter-intuitive, I have only the "global" name, and no way to find out the namespaced name.

[nikic]

The resolved name could be added as an attribute -- would avoid the need for an option. For now I'd suggest manually keeping track of the current namespace and using Name::concat().

[felixfbecker]

Currently using this piece of code as a workaround (I have decorated all nodes with parent references):

if ($parent instanceof Node\Expr\FuncCall || $parent instanceof Node\Expr\ConstFetch) {
            // Find and try with namespace
            $n = $parent;
            while (isset($n)) {
                $n = $n->getAttribute('parentNode');
                if ($n instanceof Node\Stmt\Namespace_) {
                    return (string)$n->name . '\\' . $name;
                }
            }
        }
        return $name;

It is not pretty though because I do this every time a request is made, while I would rather like to save this on the node once with a NodeVisitor.

[nikic]

90834bf Does this suit you? Note that the attribute is on the name itself, not the FuncCall/ConstFetch node.

[nikic]

Btw, don't forget that some symbols in PHP are case-insensitive. In particular class and function names always are, so you should lowercase these when inserting them into the definition table. Constants are more complicated, because they exist in both case-sensitive and case-insensitive variants.

[felixfbecker]

Sure, that would work fine! For consistency though, would be nice to have namespacedName on the node directly like for classes:

PHP-Parser/lib/PhpParser/NodeVisitor/NameResolver.php

Line 238 in 90834bf

$node->namespacedName = Name::concat($this->namespace, $node->name);

And thanks for the tip!

[nikic]

Unless the mechanism is changed entirely (#312) I won't use dynamic object properties (the namespacedName property predates the introduction of attributes).

[felixfbecker]

@nikic are you planning another beta release?

[nikic]

@felixfbecker Yeah, I expect another beta sometime soon.

[nikic]

"Sometime soon" turned out to be two weeks, but here we go: https://github.com/nikic/PHP-Parser/releases/tag/v3.0.0beta2

[felixfbecker]

Thanks. Had to release PHPLS 3.0 though, which simply depends on c0f0edf directly 😉