eclipse-jdtls / eclipse.jdt.ls

Java language server
1.77k stars 395 forks source link

Add support for callees based on implementors #2780

Closed gayanper closed 2 months ago

gayanper commented 1 year ago

Add support for the following feature which exist in Eclipse IDE into JDT.LS as well.

https://github.com/eclipse-jdt/eclipse.jdt.ui/commit/5f6911229f550c478b21488327f0a09ed13245eb

Summary: When the call hierarchy finds a interface, this feature will try to find implementations and then continue on with the implementation methods to show the callees.

gayanper commented 1 year ago

@rgrunber @jjohnstn How do you suggest to implement this ?

rgrunber commented 1 year ago

Is there a concrete example of this ? Is there a case where an interface method ends up in the call hierarchy ? If I just perform call hierarchy on an interface method, I see the implementors are already used.

gayanper commented 1 year ago

I will provide a sample scenario

gayanper commented 1 year ago
package com.example.demo;

import java.util.ArrayList;
import java.util.List;

public class CallH {
  static StringList names = new StringArrayList();

  public interface StringList {
    String get(int index);
  }

  public static class StringArrayList implements StringList {
    private List<String> strings = new ArrayList<>();

    @Override
    public String get(int index) {
      return strings.get(index);
    }
  }

  public static String fetchName(int pos) {
    return names.get(pos);
  }

  public static void main(String[] args) {
    fetchName(0);
  }
}

Now try to create outgoing call tree on String com.example.demo.CallH.fetchName(int pos).

This is vscode image

This is Eclipse which I expect vscode could do the same image

rgrunber commented 1 year ago

Is there any reason this wasn't enabled by default in Eclipse ?

I think it's reasonable to set this. So if an outgoing call is done through an interface, where there's only 1 implementation, the call hierarchy should follow the single implementor and make it the child of the given element.

The one downside is the hierarchy is "techically" not correct. The 2 get methods should be replaced by just the get(..) from StringArrayList right ?

gayanper commented 11 months ago

Is there any reason this wasn't enabled by default in Eclipse ?

I think we added that in Eclipse so that if users find it heavy when generating the hierarchy for certain workspaces, then they can opt out of that feature. We can do the same in vscode as well by introducing the setting as we did for chain completions isn't it ?

I think it's reasonable to set this. So if an outgoing call is done through an interface, where there's only 1 implementation, the call hierarchy should follow the single implementor and make it the child of the given element.

Isn't it gaining to make it unclear for a user who is discovering that hierarchy for the first time in the code base ?

The one downside is the hierarchy is "techically" not correct. The 2 get methods should be replaced by just the get(..) from StringArrayList right ?

Even if we find only one implementation, should we make this decision that the fetchName uses StringArrayList#get ? In runtime it could be replaced by something else since the reference is a interface type right ?