Default arguments: wrong Call Hiearchy results found for method call in Java class

[mauromol]

Follow up from #773.

Consider this:

package test42

class GBean {

	GBean() {}
	
	GBean(String foo, String bar = 'hello') {}
	
	void doSomething() {}
	
	void doSomething(String foo, String bar = 'bar') {}
}

And this Java class:

package test42;

public class Test43J {

	public void test() {
		GBean b2 = new GBean("foo");
		
		b2.doSomething();
		b2.doSomething("foo");
		b2.doSomething("foo", "bar");
	}
}

Put the cursor over test42.GBean.doSomething(String, String) and hit Ctrl+Alt+H: it finds only b2.doSomething("foo", "bar") , but not b2.doSomething("foo").
Put the cursor over doSomething in b2.doSomething("foo"): references for a method GBean.doSomething(String) are searched, but this method does not exist in sources (similar to #776, where the call was from Groovy code however).

Similar case is for the GBean constructors: no call is detected for GBean(String, String), and references for a constructor GBean(String) (which does not exist in sources) is performed when invoking from the call site in Test43J.

[eric-milles]

ready to test

[mauromol]

Tested with 3.9.0.v202008162300-e1912 (the latest one found with "check for updates") but I see no change in behaviour. Should I wait for a new build?

[eric-milles]

Sorry, yes Eclipse 2019-12 and prior have been moved to 5 daily builds (Sun-Thurs). These changes will not be available in 2019-12 until later this evening.

[mauromol]

OK, so I tested with 3.9.0.v202008180254-e2006 now and this is what I see.
When I hit Ctrl+Alt+H on test42.GBean.doSomething(String, String), a popup is shown requesting me whether I want to search for calls to doSomething(String, String) or doSomething(String).
When I hit Ctrl+Alt+H on b2.doSomething("foo") call, still references for doSomething(String) are searched for and b2.doSomething("foo") only is returned.
Same result for calls to the GBean constructor.

This is coherent and correct from a "binary" point of view, however since we're talking about source navigation I'm not sure this is what the user expects. I mean, what I would expect is that doSomething(String, String) is the only method existing in source, so:

• a Call Hierarchy invoked from it should not show any popup and just find both calls b2.doSomething("foo") and b2.doSomething("foo", "bar")
• a Call Hierarchy invoked from b2.doSomething("foo") should be translated into a call hierarchy for test42.GBean.doSomething(String, String)

IMHO Call Hierarchy is one of the most precious tool to use when you're studying some source code and/or you're evaluating some refactoring. With current behaviour the fact that b2.doSomething("foo") is substantally a call to test42.GBean.doSomething(String, String) (leaving a default parameter unspecified) is somewhat hidden by the IDE and a user might not realise that test42.GBean.doSomething(String, String) is indeed the only method existing in source, which might then be called from other call sites with an explicit second parameter. This may lead the developer to take a wrong decision. IMHO, the fact that, under the hood, the default parameters are exploded into multiple method overloadings should just be an "implementation detail" and should not compromise source analysis.

[eric-milles]

Providing the choice dialog was the most cross-compatible solution. Searching the methods that are created from a single declaration with default values can be done with a QueryParticipant. But the Call Hierarchy search does not involve query participants, so it cannot be made to work this way. Refactoring also had to be considered, which is why you can see a RenameParticipant added in the commit.

[mauromol]

How this works for Java varargs methods? There we have a single source method which can be called with a variable number of arguments too. Even if the implementation is different (a single method exists in binaries with a last array argument), isn't it a similar problem from a source analysis point of view?

[eric-milles]

For a variadic method, there is only one method with an array as the parameter type, with a special flag set to indicate "...". It is possible for search and rename to seek out multiple targets. I have asked about Call Hierarchy and have no answer as to why it does not support extra participants. So I find that Call Hierarchy is the lowest common denominator and try to design solutions with that in mind.

[eric-milles]

Some of this is discussed as part of #936

[eric-milles]

A method with default arguments is somewhere in between a variadic method and, for example, a Groovy property. As noted in #936 the same gap exists for a Groovy property as was raised by this issue. That is, Call Hierarchy is showing results for only one AST node but the single source element is represented by multiple AST nodes.

I found that code select (the action behind ctrl+clicking on or hovering over a source element) can return multiple items. Unfortunately, I also found that returning more than one triggers the choice dialog instead of an "or" search. So I put this change out there to see if the choice dialog was a workable solution with the intent of applying the same solution to Groovy properties if so.

As it stands, we don't have a great experience from a Call Hierarchy perspective. But at least I was able to get navigation, searching and refactoring working the same if initiated from Java or Groovy source. I will try to circle back to this and #936 if/when I can find a way to enhance the Call Hierarchy search.