NameResolver does not appear to resolve nullable types

[Gert-dev]

Hello

When looking at NameResolver, it appears that PHP 7.1 nullable types are not resolved. resolveSignature only checks for Name, but not for NullableType. This seems to apply for both return types as well as parameter types.

It is also hard to work around this as the method is private. However, I can understand this as it may not be a method you wish to expose in the extensible API of the class; on one hand you want to let users have control over what they can override if they so desire, but on the other hand you don't want to have to support an API that was never intended to be used outside of the library either.

IIRC, Symfony solved this by making everything protected, allowing complete extensibility, but marking methods that were really intended for consumption with the @api docblock tag (i.e. you have control, but there are no backwards compatibility guarantees for the elements without the tag). Although, they were also thinking about making some Request-related elements final, so I'm not sure what the best approach is anymore.

Thanks in advance

[nikic]

NullableTypes are supposed to be resolved here: https://github.com/nikic/PHP-Parser/blob/3.x/lib/PhpParser/NodeVisitor/NameResolver.php#L123 Do you have a test case for this issue?

Edit: NullableType resolution was added by a485ecd in 3.0.2, are you sure you're not using an older version?

[Gert-dev]

You're right, I did some more investigating and it turns out I didn't correctly see what the problem was. What is happening is that I"m traversing a tree to find ClassMethod nodes and I'm extending the NameResolver to do the resolving. My enterNode unconditionally calls parent::enterNode. When the NameResolver encounters such a node, it immediately calls resolveSignature to do the resolving. After that, it exits and my method parses the entire class method definition, including its parameters, return type, and so on.

In other words, you did certainly implement it, but because the new resolving code is not in the resolveSignature method (where I expected it to be in my first post), my code, which depends on the resolved types, is executing before the newly added elseif block is executed. It is still executed, but after my code is done exceuting, which explains why your tests are succeeding :-).

To conclude, it's not so much of a bug, just not the behavior I expected. In order to get the types resolved, I now either have to do the entire resolving beforehand and visit the hierarchy twice, or move my code to the leaveNode method for the ClassMethod node in order for the resolving to have already happened. I can't immediately process any ClassMethod (nor Param nodes, for that matters), as type resolving happens only when the parent descends into the name nodes of the types themselves.

I'll leave you to decide if you want to change the behavior or not. If not, I won't hold it against you and I'll look into working around it :-).

[nikic]

Having a fully resolved signature in enterNode sounds like a reasonable use-case. I've moved the resolution code into resolveSignature per your suggestion in 1b59e91.