Find references does not return the usings of class or interface

[BladeMF]

If a class is searched for references, say Class3, this line won't be returned:
use Temp2\Folder\Class3;

Since that is a reference to the class/interface/function, maybe it should be returned?

[BladeMF]

Doesn't return use MyTrait; either.

[dantleech]

Because it's not really interesting. It may also increase significantly increase the size of the index.

So we have these options:

1/ Increase the size of the index and expand the search criteria to specify if these symbols should be found or not.
2/ Parse each file which has a reference and get the use statements -- we only care about use statements which are in a file which references the class.

I'd consider option #2 - for each reference update the name imports for that reference.

For use MyTrait that should be in the index - indeed there is a class for it: https://github.com/phpactor/workspace-query/blob/ad39574df0e53cec73562015bb864b4043e2bc49/lib/Adapter/Tolerant/Indexer/TraitUseClauseIndexer.php#L16

[BladeMF]

Not sure why TraitUseClauseIndex didn't work. I didn't see it and added code in the ClassLike indexer. Will remove my code and find the problem with TraitUseClauseIndex.

Option 2/ is not bad. The only thing that bothers me is that a Rename action should rename all the usings, not just the ones which contain class references, because the user would be annoyed if we didn't rename the others. Most of the time they wouldn't see the logic in that I'm afraid. That requires an index to be useful. As for the size of the index, I can measure it with or without it.

[BladeMF]

TraitUseClauseIndex is not finished. I am unsure if two indexers indexing ClassRecords is a good idea (TraitUseClauseIndex and ClassLikeReferenceIndexer), what do you think?

[dantleech]

The only thing that bothers me is that a Rename action should rename all the usings

Any use without a reference to the class it imports is dead code - and should/would be removed by a cs fixer.

indexing the use statements also means, by default, that the use statements are found when searching for references, which most of the time isn't what you want (it's redundant).

I am unsure if two indexers indexing

Hmm . I'm not sure. From a quick look it seems that the class-like-reference indexer should already pick up trait use but due to the bug in TP it doesn't.

You could try using TolerantQualifiedNameResolver::getResolvedName($qualifiedName) instead of ->getResolvedName() in the class-like reference finder (as in the trait-use-clause-indexer). That helper class is there specifically because of the trait-use issue.

[dantleech]

The TraitUseClauseIndexer is only working for implementations i.e. it's associating the use-containing class as an implementation of the trait - so that's doing a different job and can be left alone.

I think to support trait use in the references we just need to use TolerantQualifiedNameResolver::getResolvedName($qualifiedName) instead of ->getResolvedName() in the ClassLikeReferenceIdexer (as in the TraitUseClauseIndexer)

[BladeMF]

The TraitUseClauseIndexer is only working for implementations i.e. it's associating the use-containing class as an implementation of the trait - so that's doing a different job and can be left alone.

I am not sure. I wrote these tests and they don't work:

    public function provideClasses(): Generator
    {
        yield 'use trait (basic)' => [
            "// File: src/file1.php\n<?php class C { use T; }",
            'T',
            [0, 0, 1]
        ];

        yield 'use trait (namespaced)' => [
            "// File: src/file1.php\n<?php use N\T; class C { use T; }",
            'N\T',
            [0, 0, 2]
        ];
    }

Am I missing something?

[BladeMF]

I think to support trait use in the references we just need to use TolerantQualifiedNameResolver::getResolvedName($qualifiedName) instead of ->getResolvedName() in the ClassLikeReferenceIdexer (as in the TraitUseClauseIndexer)

I agree.

The TraitUseClauseIndexer is only working for implementations i.e. it's associating the use-containing class as an implementation of the trait - so that's doing a different job and can be left alone.

Why not add both - implementation and reference? Cool.

[BladeMF]

indexing the use statements also means, by default, that the use statements are found when searching for references, which most of the time isn't what you want (it's redundant).

I am inclined to agree. I can have the Renamer search the namespace use clauses wherever it finds references.

[dantleech]

Cool - the trait use indexer is missing a dedicated test class, but it is tested in the TolerantIndexBuilderTest (the test should be broken out at some point)

[BladeMF]

I added tests for everything. We can comment on the PR.

[dantleech]

You haven't made the PR yet or?

[BladeMF]

I haven't. I need to fix my renamer first.