Fix class namespaces from uses

[lupinitylabs]

This pull request adresses a long-standing issue the ide-helper has if @param and @return types in methods do not include the fully qualified namespace.

Why this is an issue

Both our code style and PHPStorm encourages importing namespaces via use statements and then removing the unnecessary FQNs everywhere, including the PHPdoc blocks. When doing this, the ide-helper does not know how to resolve the types anymore and just puts them all in the namespace of the current class.

Observed behavior

If we have a class like this, the ide-helper is totally clueless where the Collection and Builder classes come from:

<?php

namespace Sample\Namespace;

use Illuminate\Support\Collection;
use Illuminate\Database\Eloquent\Builder;

class SampleClass {
    /**
     * @param Collection    $collection
     * @param Builder       $builder
     *
     * @return Collection
     */
    function someMethod(Collection $collection, Builder $builder) {
        return collect();
    }
}     

This will result in the ide-helper producing a PHPdoc like this:

/**
 *
 *
 * @param \Sample\Namespace\Collection $collection
 * @param \Sample\Namespace\Builder $builder
 * @return \Sample\Namespace\Collection
 * @static
*/

Expected behavior

Instead, you would expect ide-helper to respect the imports and produce the following PHPdoc instead:

/**
 *
 *
 * @param \Illuminate\Support\Collection $collection
 * @param \Illuminate\Database\Eloquent\Builder $builder
 * @return \Illuminate\Support\Collection
 * @static
*/

About the pull request

The solution is designed to be minimally invasive and just fix class names that do not exist after processing by the ReturnTag class. If the class could not be found in the determined scope, the class imports are searched, and if a corresponding use statement is found, the namespace is applied accordingly. This also takes aliasing into account.

Although this requires a scan of the class sources, and the uses are, in the current form, unnecessarily re-read for each method in a class, the process is still sufficiently fast. However, I would advise to use a runtime cache to steamline the process. I leave that up for discussion, and if desired, I'll see if I can add this to this pull request within the next couple of days.

[lupinitylabs]

There seems to be something wrong with the tests, too... I didn't get them to run properly in my dev environment (issues with line endings), and it looks like travis-ci is reporting my new test and 2 former tests as failing...

The unmodified master failed for me as well, so I ignored it. You might want to look into that...

[lupinitylabs]

OK, found and fixed the issues with the tests. You seem to expect trailing whitespaces on some lines, which have been automatically removed by PHPStorm... I replaced the multi-line string assignment with a representation that leaves no guessing.

[lupinitylabs]

You might also want to refactor the helper functions as they might come in handy in model generation etc. too, reading #905 ... the import of the use statements could be utilized to verify if the namespaces were already imported and to use the alias instead of the FQN in those cases.

[mfn]

That… may take some time to review 😄

[mfn]

A few days ago #954 landed which uses phpdocumentor/type-resolver

Can this PR make use of it instead of manually having to deal with the tokenization process?

[lupinitylabs]

A few days ago #954 landed which uses phpdocumentor/type-resolver

Can this PR make use of it instead of manually having to deal with the tokenization process?

Yes, this may work. phpdocumentor practically seems to use the same approach, and I just fiddled around with it briefly and came to the conclusion that using the ContextFactory might work just fine:

$contextFactory = new ContextFactory();
$context = $contextFactory->createForNamespace('Some\Namespace', file_get_contents('FileToAnalyze.php'));
var_dump((string)$typeResolver->resolve('SomeClass', $context));

This resolves SomeClass just fine, even if it was imported. So if the type-resolver is used anyways, we can use this approach to save some redundant code.

However, it might be a better solution to always use the type resolver in the first place instead of just using it when the naïve approach failed? This is a design decision a core developer should take though.