redhat-developer / vscode-java

Java Language Support for Visual Studio Code
Eclipse Public License 2.0
2.08k stars 441 forks source link

Peek/Go To Implementations points to method with same name #3823

Open mamilic opened 1 month ago

mamilic commented 1 month ago

When I try to navigate to implementations of a method it will point me to a method with same name in wrapper class, but that wrapper class does not implement the interface of that method.

Environment
Steps To Reproduce
  1. Clone https://github.com/eclipse-jdtls/eclipse.jdt.ls
  2. Go to JavaClientConnection.java
  3. Navigate to sendActionableNotification of the interface JavaLanguageClient line 59
  4. Try to Go To Implementations, it will point to method in wrapper class JavaClientConnection line 159, however the JavaClientConnection does not implement JavaLanguageClient
rgrunber commented 2 weeks ago

I noticed Eclipse does the same thing. This happens because of https://github.com/eclipse-jdtls/eclipse.jdt.ls/blob/f155e1fc90618b558899c3722606feccfc89e210/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/ImplementationCollector.java#L217 . In Eclipse you can find the code at https://github.com/eclipse-jdt/eclipse.jdt.ui/blob/fcf7f0e944fe8cf23b0a395e4bfcf8d7270a6a6a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/ui/actions/FindDeclarationsAction.java#L87 .

The search is performed on the following element : void sendActionableNotification(ActionableNotification) [in JavaLanguageClient [in JavaClientConnection [in [Working copy] JavaClientConnection.java [in org.eclipse.jdt.ls.core.internal [in src [in org.eclipse.jdt.ls.core]]]]]] and is limited to only declarations within the type hierarchy of JavaLanguageClient. However, there are 2 extra conditions :

So any method named sendActionableNotification(ActionableNotification) (same parameter) will match. The only reason I can think of to do this is to catch "wrapper"-like methods.

We could fix it by just removing those conditions and the method wouldn't show up, but I'm curious if there's something else i'm missing about this behaviour. @snjeza any ideas why it behaves like this, or is "find wrapper-like methods" the best explanation ?

snjeza commented 2 weeks ago

any ideas why it behaves like this, or is "find wrapper-like methods" the best explanation ?

This is an upstream JDT issue. You can try Code Minings that works fine - https://github.com/eclipse-jdt/eclipse.jdt.ui/blob/0871ea2dc2dc80dc35cad068ab34cda7efc6f3b1/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/codemining/JavaImplementationCodeMining.java#L208-L214

Test class:

package test;

public interface IFoo {
    void test();
    class Internal {
        void test() {

        };
    }
    class Foo implements IFoo {
        public void test() {

        }
    }

}

Image

cc @angelozerr

mamilic commented 1 week ago

@rgrunber , thanks for detailed explanation, would you like me to open an issue upstream?

angelozerr commented 1 week ago

any ideas why it behaves like this, or is "find wrapper-like methods" the best explanation ?

This is an upstream JDT issue. You can try Code Minings that works fine - https://github.com/eclipse-jdt/eclipse.jdt.ui/blob/0871ea2dc2dc80dc35cad068ab34cda7efc6f3b1/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/codemining/JavaImplementationCodeMining.java#L208-L214

Test class:

package test;

public interface IFoo {
  void test();
  class Internal {
      void test() {

      };
  }
  class Foo implements IFoo {
      public void test() {

      }
  }

}

Image Image

cc @angelozerr

I don't remember the code I have written @snjeza :)

rgrunber commented 1 week ago

That's fine. I have no doubt the code lens part is correct. The methods aren't shown because they technically aren't implementations. I was more curious, what the motivation for the "declarations" search is in Eclipse IDE. It almost seems like they're trying to intentionally capture methods that don't implement the method interface, but that might behave as if they did.