-
Notifications
You must be signed in to change notification settings - Fork 10
Name resolution is icky #182
Comments
So in the case of
These have names that are always just a single T_STRING token. And have their getName() methods return a TokenNode instead of NameNode. So instead of $classNode->getName()->getAbsolutePath(); would have $classNode->getFullyQualifiedName(); References to class/function/constants can be fully qualified, qualified (relative), or unqualified. In these cases I think it makes sense to keep NameNode as is. As it already has things like: $nameNode->isRelative();
$nameNode->getBasePath();
// rename to getFullyQualifiedName() to be consistent with proposal to ClassNode above.
$nameNode->getAbsolutePath(); The only other piece of cruff in NameNode is fact it treats handling fact NameNode is used for the name part of namespace statement and use declarations, which are always fully qualified but do not start with NS_SEPARATOR '' . In other words there shouldn't be instanceof checks on the parent in NameNode (perhaps fixed by having an optional parameter in the constructor that can specify the path is absolute?) For class methods should we make it be \Full\ClassPath::methodName() to separate it from a constant on the class.. \Full\ClassPath::MY_CONST , since the uppercase convention is only a coding convention? Then what about properties on classes.. should they also have a getFullyQualifiedName.. \Full\ClassPath::$myProperty ? |
Overall, I think the most important thing is for the API to be consistent. So, any kind of namespace-aware node should implement NameResolutionInterface. For things that reference a name, I completely agree that it makes sense to keep NameNode as it is. (It might make sense to rename it to NameResolutionNode and have it implement NameResolutionInterface, for consistency.) I do think we should support class methods, constants, and properties. Methods and constants should be in the form \Path\to\Class::name (no parentheses for methods), and properties should be in the form \Path\to\Class::$property. For now, I don't think we need to bother with fully-qualified names of class properties. AFAIK, the only place that'd be legal in PHP code is if the property is public and static. To summarize, then... Nodes whose name is a T_STRING will provide their own name resolution logic by implementing NameResolutionInterface. This includes ClassNode, FunctionDeclarationNode, ConstantNode, and ClassMethodNode (am I forgetting any?) On the other hand, nodes which include a NameNode -- which would be just about anything with a getName() method, including NamespaceNode, FunctionCallNode, ClassMethodCallNode, UseDeclarationNode, and so forth -- will also implement NameResolutionInterface for consistency, but the NameResolutionInterface methods will simply call the equivalent method of the contained NameNode. (We can make a trait for this.) Does that sound like a plan? |
As discussed on IRC, we agree that NameNode has outgrown itself. It's trying to do too much.
I think that we should redefine NameNode. Names, as written in code, should be distinct from the name resolution logic (right now, NameNode tries to do both).
NameNode, as I envision it, is a countable array-like object:
In this paradigm, NameNode is very dumb. It has no awareness of the wider context in which it exists. Its getText() method is basically
implode('\\', $this->parts)
.The name resolution logic, I think, should be encapsulated by an interface -- NameResolutionInterface, perhaps. I dunno, we can name it later.
PHP defines three "branches" of name resolution logic: unqualified name (foo), qualified name (harrr\foo), and fully qualified name (\harrr\foo). NameResolutionInterface mirrors this directly.
NameResolutionInterface will be implemented by node types which can be affected by namespacing:
For ParameterNode type hints, we could create a NameResolutionNode (a subclass of NameNode) which is a completely stand-alone implementation of NameResolutionInterface, similar in nature to what NameNode currently is.
Each node type implementing NameResolutionInterface can resolve its own name however it needs to do it. ClassMethodNode, for instance, can implement getFullyQualifiedName() such that it returns \Full\Class\Path::methodName.
The point is, if name resolution is going to vary by node type, then node types should be able to implement their own name resolution logic. If we do it this way, we can retire NameNode and all the cruft therein.
The text was updated successfully, but these errors were encountered: