phptools-docs icon indicating copy to clipboard operation
phptools-docs copied to clipboard

Find all references works wrong with public methods

Open AndreiOrmanji opened this issue 2 years ago • 13 comments

Descrition: I've created an interface A with 1 method process and 1 implementation (class B) and one class that has a method with the same name but it does not implement interface A Example from description: image

Expected result: No references found for method C::process()

Actual result: All methods with the same name are returned, even when signature does not match.

If this issue will result in PR with fix, please check it at least with my code:

<?php
interface A
{
    public function process(): void;
}

class B implements A
{
    public function process(): void
    {
        echo 1;
    }
}

class C
{
    public function process(): void
    {
        echo 2;
    }
}

Expected result for all methods process:

  1. "Find all references" on A::process() should return reference on itself and B::process()
  2. "Find all references" on B::process() should return reference on itself and A::process()
  3. "Find all references" on C::process() should return reference on itself only

Environment details: Extension version: v1.33.12934 VSCode: 1.77.3 (running in WSL via Remote Development package on distro Ubuntu 20.04.6 LTS) OS: Windows 10 build 19043.2364

AndreiOrmanji avatar Apr 17 '23 09:04 AndreiOrmanji

Thank you @AndreiOrmanji for the detailed issue!

I'll try to replicate it and fix it!

jakubmisek avatar Apr 17 '23 14:04 jakubmisek

@jakubmisek It seems the problem exists for a long time. https://community.devsense.com/d/697-find-all-references-works-wrong/11

AndreiOrmanji avatar Apr 21 '23 12:04 AndreiOrmanji

@AndreiOrmanji Thank you for your issue. We'll be looking into it.

jakubmisek avatar Apr 22 '23 12:04 jakubmisek

Preparing a pre-release, that fixes most of the find all references inaccuracies.

jakubmisek avatar Apr 29 '23 21:04 jakubmisek

@jakubmisek I've installed today pre-release version to check and the extension still finds wrong references. For some reason Find all references for C::process() returns references from vendor of non-related methods, but with the same name. image

Description of fixes says "Find All References performs type analysis if necessary to provide better results". Why does "find all references" not use ONLY type analysis? I can not say for anyone else but for me is important to get a list of references that are 100% related to symbol, i'm searching. Other cases that can not be found by type analysis should be searched manually, or a new feature should be introduced to show "less relevant" variants (with the name "find weak references" or something like that).

AndreiOrmanji avatar May 01 '23 07:05 AndreiOrmanji

@AndreiOrmanji thank you for testing it out so quickly.

You're right;

There still seems to be a bug related to untitled documents and/or phar files. I'll keep working on it.

jakubmisek avatar May 01 '23 07:05 jakubmisek

@jakubmisek It's not limited to untitled and phar files. Example:

     $result->{$prop} = is_a($attr, A::class, true) ? (new $attr())->run($step) : $attr;

I have 2 interfaces(A and B) with method run and nor A extends B, nor B extends A and for BOTH of them this row will be shown as a reference.

I did not tried if the extension takes into account arguments' count (but I'm sure that argument type is not taken into account)

AndreiOrmanji avatar May 01 '23 10:05 AndreiOrmanji

@AndreiOrmanji it seems we don't handle is_a() yet - I'll do that, then (new $attr)->run might get resolved correctly

jakubmisek avatar May 01 '23 13:05 jakubmisek

@AndreiOrmanji it seems we don't handle is_a() yet - I'll do that, then (new $attr)->run might get resolved correctly

The main question here is why extension thinks about all objects without specified type that they are of ALL TYPES AT ONCE instead of MIXED?

I would like not to see places where run method is called on an object of unknown type as if it was called by a specific type with run method defined in it.

AndreiOrmanji avatar May 01 '23 15:05 AndreiOrmanji

@AndreiOrmanji I agree - I'll try to fine-tune it a little.

jakubmisek avatar May 16 '23 15:05 jakubmisek

@jakubmisek, It seems that huge amount of work is done to improve the "find all references" feature. Thank you and all your team for your work. Currently (extension version 1.37.13534) for the same code provided in issue description returns the following output for case 3 ("Find all references" on C::process()): image

Can it be adjusted to fix this case?

AndreiOrmanji avatar Aug 06 '23 08:08 AndreiOrmanji

find all references still returns incorrect results, it's the only thing that stops me from using this extension

cDonut avatar Oct 19 '23 07:10 cDonut

@cDonut thank you for reminding me!

jakubmisek avatar Oct 19 '23 13:10 jakubmisek