TypeResolver

TypeResolver copied to clipboard

After some discussion with @nikic on https://github.com/phpDocumentor/TypeResolver/pull/9/files#r34962566 it looks like ContextFactory::getNamespaceAliases() doesn't cope with multiple imports with the same aliased name. For example:

namespace Foo;
use Foo\X;
class A extends X {
    use Zap;
}
use Bar\X;
class B extends X {
    use Baz;
}

If you call ::getNamespaceAliases() on this perfectly valid code, you'll see:

[
    'X' => 'Bar\X'
]

This is not correct, but because it is keyed by the alias name, it is too much of a simple representation to allow multiple named aliases. It's maybe quite an edge case, but it does indeed appear to be a bug.

@asgrim can this one be closed? It looks like you already provided a patch for this issue?

I don't think it's resolved yet - #9 resolved #8 but we discovered this issue #11 after discussion on that PR.

In that case I will dive into this to see if it can be resolved

Anything I can do to help here?

Feel free to make a pr :blush:

@asgrim @jaapio I can't think of any clean way to handle this edge case. I think we should probably just detect if a namespace is being redeclared and throw an exception or something.

Seems reasonable aye :+1: it's a real edge case

The issue here is that php is somehow interpreting files from top to bottom, we are simply parsing a file in the same way but are trying to map that in to an array. I would expect that we could be able to create an context based on the class that is requested.

In the situation where you are requesting for a context for a specific namespace this won't work. But since all types that are resolved are somehow related to an Reflection type we should be able to have some way to figure out what is the correct context. If the user is still requesting a context for a namespace we could throw an exception.

I do fully agree that this isn't a very easy and straightforward solution but it will work for the situations we have to handle in phpdocumentor. And I think this approach might work for BetterReflection as well?

Assuming the API remains the same, nothing should need to change for us, we pretty much just pass any type resolution to here anyway :+1: