Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default parameters: wrong Call Hiearchy results found for method call #773

Closed
mauromol opened this issue Nov 30, 2018 · 4 comments
Closed
Assignees
Milestone

Comments

@mauromol
Copy link

mauromol commented Nov 30, 2018

Consider this:

package test42

class GBean {

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

And this:

package test42;

class Test43 {

	void test() {
		def b2 = new GBean('foo')
		
		b2.doSomething()
		b2.doSomething('foo')
		b2.doSomething('foo', 'bar')
	}
}

Put the cursor over test42.GBean.doSomething() and hit Ctrl+Alt+H: it finds both b2.doSomething() and b2.doSomething('foo'), while it should find only the former.

Please note that b2.doSomething('foo') is (correctly) found also when requesting Call Hiearchy for test42.GBean.doSomething(String, String).

@mauromol mauromol changed the title Default parameters: wrong Call Hiearchy results found Default parameters: wrong Call Hiearchy results found for method call Nov 30, 2018
eric-milles referenced this issue Jan 15, 2019
GroovyCompUnitDecl maps MethodNode -> AbstractMethodDeclaration
MethodScope.createMethod maps AbstractMethodDeclaration -> MethodBinding
JDTClassNode.methodBindingToMethodNode maps MethodBinding -> MethodNode
@eric-milles
Copy link
Member

This is implemented in org.eclipse.jdt.groovy.search.MethodReferenceSearchRequestor. When testing a potential match in acceptASTNode, there is an algorithm for matching the call arguments to the parameter types of the method overloads. At this time, methods generated by default params are redirected to the method that generated them. This was a fix for GRECLIPSE-1233.

If this is switched to exact matching, there will still need to be something that accounts for inexact matches. Like if you have "doIt()" and "doIt(int)" and the call expr is "doIt(1, 2, 3)".

@eric-milles
Copy link
Member

eric-milles commented Jan 15, 2019

I am testing a possible solution now. And I need to work out a conflict with #373, since it was fixed in the call argument matching branch and it now goes down the parameter matching branch.

Some known limitations: The searching for multiple method signatures happens only for the Groovy-specific path. So if your Test43 was a Java class, only one reference would be found when searching any of the 3 method calls. Also, if GBean were a binary reference, there would be no link between doSomething(String) and doSomething(String,String) so the search would find only one reference.

@eric-milles
Copy link
Member

Ready to test

@mauromol
Copy link
Author

This case works fine in 3.3.0.xx-201901152207-e49 and I also verified that there's no regression for #373.

I will open another ticket to investigate solutions for the case in which Test43 is Java code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants