Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReturnType of "PhpParser\Node\Name\FullyQualified" - but no leading backslash leads to FQN added to current namespace #134

Open
ancpru opened this issue Apr 27, 2019 · 2 comments

Comments

@ancpru
Copy link

ancpru commented Apr 27, 2019

Something strange seems to happen here:

$object->getReturnType() returns a object of PhpParser\Node\Name\FullyQualified despite it's not fully qualified in the source. So far no big deal. But: the string conversion does not contain a leading backslash.

   protected function doCreate($object, StrategyContainer $strategies, ?Context $context = null)
    {
        $docBlock = $this->createDocBlock($strategies, $object->getDocComment(), $context);

        $returnType = null;
        if ($object->getReturnType() !== null) {
            $typeResolver = new TypeResolver();
            if ($object->getReturnType() instanceof NullableType) {
                $typeString = '?' . $object->getReturnType()->type;
            } else {
                $typeString = (string) $object->getReturnType();
            }

            $returnType = $typeResolver->resolve($typeString, $context);

The subsequent call of $typeResolver->resolve($typeString, $context); then leads to a concatination of the namespace of the file with the (not wellformed) FQN (e.g. \My\Current\Namespace\MyCurrent\Namespace\Class)

I did not tried it yet, but it might be enough to make sure that the string contains a leading backslash:

if (($object instanceof \PhpParser\Node\Name\FullyQualified) && ($typeString[0] !== '\\'  ) {
  $typeString = '\\' .$typeString;
}

Update:

Checking on instance of \PhpParser\Node\Name and $returnType->isFullyQualified() might be the even cleaner approach.

@jaapio
Copy link
Member

jaapio commented Apr 28, 2019

Thanks for reporting this issue. I will investigate what is going on. I think it could be an issue that should be fixed here or it should be solved in the type resolver. Since only there the type is known.

It would be very helpfull when you are able to provide an example file which triggers this issue.

@ancpru
Copy link
Author

ancpru commented Apr 28, 2019

Thanks for reporting this issue. I will investigate what is going on. I think it could be an issue that should be fixed here or it should be solved in the type resolver. Since only there the type is known.

It would be very helpfull when you are able to provide an example file which triggers this issue.

Hi,

I also thought about the type resolver first. Easy way: If the given string begins with the current namespace given in context, then it is already a fqsen. There is just one case where it does not work: when the user really repeats the namespace: \Foo\Bar\Foo\Bar. Then then a relative namespace to the second \Foo\Bar would not be noticed.

But in general I think the problem is, that phpparser does not convert to a fqsen-format despite it has a fqsen in direct string conversion.

using $node->toCodeString() instead of (string)$node may fix the problem:

That's the implemenation of FullyQualified:

    public function toCodeString() : string {
        return '\\' . $this->toString();
    }

Drawback:
Relative has this implementation:

    public function toCodeString() : string {
        return 'namespace\\' . $this->toString();
    }

thus relative references would begin start with "namespace" in the string (which might not be the terrible thing - TypeResolver would just need to handle it (if given name starts with "namespace" just attach it to the namespace)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants