Namespaces in docblocks are not scoped

[ragulka]

I stumbled upon an issue where my IDE was complaining that a certain method from a scoped dependency did not return the required return type.

I found that it did, in fact, return the correct (scoped) type, but the method had a docblock with a @return annotation, which was still using the non-scoped namespace for the return type.

It looks like PHP Scoper does not support scoping namespaces inside docblocks (or any comments), so we may end up with something like this:

namespace Scoped\Illuminate\Database\Eloquent\Relations;

class HasMany extends HasOneOrMany
{
    /**
     * Convert the relationship to a "has one" relationship.
     *
     * @return \Illuminate\Database\Eloquent\Relations\HasOne
     */
    public function one()
    {
        return HasOne::noConstraints(fn() => new HasOne($this->getQuery(), $this->parent, $this->foreignKey, $this->localKey));
    }
}

Notice that while we the namespace is prefixed Scoped\Illuminate\Database\Eloquent\Relations, the @return annotation uses the non-scoped FQDN \Illuminate\Database\Eloquent\Relations\HasOne. This means that whenever HasMany::one() is called, IDEs will think that the wrong return type is used.

This would not happen if the annotations would use imported class names, rather than FQDNs, but we cannot affect that.

While this isn't a critical issue (the code runs just fine), it's a hurdle for static code analysis and a pain point in terms of DX.

I'd like to investigate if we can use some post-processing to fix this, after PHP scoper has done it's thing. Perhaps we can get a map of prefixed namespaces, and use that to run a search & replace inside the comments? Or perhaps we can use a custom patcher?

[maxrice]

@ragulka good find. I don't know if I'd spend much time fixing this, as the "proper" solution is probably just to use an import for those, rather than the fully-qualified namespace. I sort of hate adding an import for namespaces that are only used in docblocks, but aside from my own preferences, I don't have a good reason for avoiding the import.

FWIW phpstorm also constantly suggests "qualifier could be replaced with an import" when you use a fully-qualified namespace in a docblock 🤷

[ragulka]

I don't know if I'd spend much time fixing this, as the "proper" solution is probably just to use an import for those, rather than the fully-qualified namespace

@maxrice I agree that's the best solution if the scoped package is our own. Unfortunately, we can't import namespaces in 3rd party packages like illuminate/database, which is where this issue originates from.

[maxrice]

oh oh, just re-read the code snippet, I see what you mean now. Yeah worth investing some time in that case I'd say.