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

Wrong result in "Search for references" and "Call Hierarchy" with constructor overloading #1090

Closed
mauromol opened this issue Apr 20, 2020 · 11 comments
Assignees
Labels
Milestone

Comments

@mauromol
Copy link

Consider the attached Eclipse/Gradle project:
Test61.zip

Go to:
test61.MyBean.MyBean(SessionFactory, DetachedCriteria, List<Order>, BiConsumer<Session, ? super T>)

Try to hit Ctrl+Alt+H or Ctrl+Shift+G: a reference is found in Test61.foo(), but the constructor actually called from there is rather test61.MyBean.MyBean(SessionFactory, DetachedCriteria, Order...).

I was not able to reproduce this problem with a smaller test case using simple types like String, Integer, etc.. Maybe the difference is that the involved types are binary references to an external JAR (Hibernate, in this case).

@eric-milles
Copy link
Member

ready to test

@mauromol
Copy link
Author

mauromol commented Apr 23, 2020

There's an improvement here (I had to rebuild the index), but I still see some inconsistencies. In the above project, change classes in this way:

package test61;

import java.util.List;
import java.util.Map;
import java.util.function.BiConsumer;
import java.util.function.Function;

import org.hibernate.Session;
import org.hibernate.SessionFactory;
import org.hibernate.criterion.DetachedCriteria;
import org.hibernate.criterion.Order;

public class MyBean<T> {

	public MyBean(final SessionFactory sessionFactory,
			final DetachedCriteria baseCriteria,
			final List<Order> defaultOrders,
			final BiConsumer<Session, ? super T> postProcessor) {
		this(sessionFactory, baseCriteria, defaultOrders, null, postProcessor);
	}

	public MyBean(final SessionFactory sessionFactory,
			final DetachedCriteria baseCriteria,
			final List<Order> defaultOrders,
			final Map<String, ? extends Iterable<String>> customSortFields) {
	}

	public MyBean(final SessionFactory sessionFactory,
			final DetachedCriteria baseCriteria,
			final List<Order> defaultOrders,
			final Map<String, ? extends Iterable<String>> customSortFields,
			final BiConsumer<Session, ? super T> postProcessor) {
	}

	public MyBean(final SessionFactory sessionFactory,
			final DetachedCriteria baseCriteria, final Order... defaultOrders) {
	}
}

(please note the addition of the this(...) call).

And:

package test61

import org.hibernate.SessionFactory
import org.hibernate.criterion.DetachedCriteria
import org.hibernate.criterion.Order

class Test61 {

	SessionFactory shopSessionFactory
	
	void foo() {
		def b = new MyBean(shopSessionFactory, bar('a', 'b', 'c'))
	}
	
	void bar() {
		def b = new MyBean(shopSessionFactory, bar('a', 'b', 'c'), Order.asc('foo'), Order.asc('bar'))
	}
	
	DetachedCriteria bar(String a, String, b, String c) {
		
	}
}

Now, invoke Call Hierarchy for test61.MyBean.MyBean(SessionFactory, DetachedCriteria, List<Order>, BiConsumer<Session, ? super T>): it finds an only result, which its call to this(...), which is wrong, because that call actually invokes test61.MyBean.MyBean(SessionFactory, DetachedCriteria, List<Order>, Map<String, ? extends Iterable<String>>, BiConsumer<Session, ? super T>). This is just Java code, so I'm not sure Greclipse has any responsibility for this, but I'm pretty sure that this wrong hit was not showing up before applying the latest Greclipse update (I can say this because, in my real project where I found this, I always had that this(...) call).

Now, invoke "Search for references" for test61.MyBean.MyBean(SessionFactory, DetachedCriteria, List<Order>, BiConsumer<Session, ? super T>):

  • the this(...) call found by Call Hierarchy is not shown (correct)
  • references from Test61.foo() and Test61.bar() are shown (both wrong)

Two interesting observations:

  • if you comment out Test61.bar() method altogether and refresh the "Search" view, not only the Test61.bar() hit disappears, but also the Test61.foo() one does
  • if you change restore Test61.bar() method but change it so that the second Order.asc('bar') parameter to the MyBean constructor call is removed (leaving then just Order.asc('foo')) and refresh the "Search" view, only Test61.foo() reference is found, while Test61.bar() is not any more

@mauromol
Copy link
Author

I suspect this change may have broken Call Hierarchy for pure-Java code as well, because it's now failing to find references to a simple setXXX method with a double argument in pure Java code...

@eric-milles
Copy link
Member

The original case and the case demonstrated in your bar() method both have to do with the varargs parameter. One note that might help you in the future: when an exact match is not found, the first constructor is used. So if you want to have a "catch all" like the variadic orders, you can put that one first. I know it is just a workaround, but it may help you out while waiting for a fix.

@eric-milles
Copy link
Member

ready to test

@mauromol
Copy link
Author

Unfortunately, I don't see improvements here in 3.8.0.v202004240308-e1912, even after I rebuilt the Java index.

With this:

package test61

import org.hibernate.SessionFactory
import org.hibernate.criterion.DetachedCriteria
import org.hibernate.criterion.Order

class Test61 {

	SessionFactory shopSessionFactory
	
	void foo() {
		def b = new MyBean(shopSessionFactory, bar('a', 'b', 'c'))
	}

	void bar() {
		def b = new MyBean(shopSessionFactory, bar('a', 'b', 'c'), Order.asc('foo'), Order.asc('bar'))
	}

	DetachedCriteria bar(String a, String, b, String c) {
		
	}
}

foo() and bar() are found.

With this:

package test61

import org.hibernate.SessionFactory
import org.hibernate.criterion.DetachedCriteria
import org.hibernate.criterion.Order

class Test61 {

	SessionFactory shopSessionFactory
	
	void foo() {
		def b = new MyBean(shopSessionFactory, bar('a', 'b', 'c'))
	}

	void bar() {
		def b = new MyBean(shopSessionFactory, bar('a', 'b', 'c'), Order.asc('foo'))
	}

	DetachedCriteria bar(String a, String, b, String c) {
		
	}
}

foo() is found.

With this:

package test61

import org.hibernate.SessionFactory
import org.hibernate.criterion.DetachedCriteria
import org.hibernate.criterion.Order

class Test61 {

	SessionFactory shopSessionFactory
	
	void foo() {
		def b = new MyBean(shopSessionFactory, bar('a', 'b', 'c'))
	}

	void bar2() {
		def b = new MyBean(shopSessionFactory, bar('a', 'b', 'c'), Order.asc('foo'), Order.asc('bar'))
	}

	DetachedCriteria bar(String a, String, b, String c) {
		
	}
}

no match is found (!?!? Just changed bar() name to bar2()!!!)

Also, call hierarchy seems to be broken (see my comments above), and I'm pretty sure it was not before these changes.

@eric-milles
Copy link
Member

There must be some extra thing going on here. I imported your project and tested your foo and bar scenarios. I have test cases for 0, 1 and 2 arguments to the variadic parameter. When you change some unrelated element and the search results change, that tends to suggest something is missing in the indexing visitor. It must tag a source unit as having the potential for matches before the match locator will build AST and look deeply for search hits.

The "broken" call hierarchy seems unrelated. I did not change anything that would touch the Java results, unless there is an exception being thrown along the way. But this would show up in your Error Log view.

@eric-milles
Copy link
Member

Note: You have "DetachedCriteria bar(String a, String, b, String c)" with an extra comma between "String" and "b". This may be having strange effects. Groovy is fine with this, seeing your declaration as "bar(String a, Object String, Object b, String c)".

@mauromol
Copy link
Author

Yes, it seems like the broken bar declaration is indeed causing the weird behaviour I see, I did some quick tests after fixing it to bar(String a, String b, String c) and results seem to be consistent now. Anyway, if this is valid Groovy code I think some fix is still needed.

With regards to the broken Call Hierarchy: I just checked with the exact same projects, same IDE version, but Greclipse 3.8.0.v202004230438-e1912 and call hierarchy for the aforementioned setXXX method works well, while it does not work on the system where I'm testing 3.8.0.v202004242058-e1912.
Also, the following issue does not occur on the previous version:

Now, invoke Call Hierarchy for test61.MyBean.MyBean(SessionFactory, DetachedCriteria, List, BiConsumer<Session, ? super T>): it finds an only result, which its call to this(...), which is wrong, because that call actually invokes test61.MyBean.MyBean(SessionFactory, DetachedCriteria, List, Map<String, ? extends Iterable>, BiConsumer<Session, ? super T>)

What I see in the system where it is broken is that wrong Call Hierarchy results are computed very quickly (while in the other system it takes a while to show the correct results), but no exception is produced in the error log.

I can try to upgrade the workspace where things are working well to see if they get broken there too.
Is there a way to downgrade to a previous snapshot version? I really need Call Hierarchy...

@mauromol
Copy link
Author

Actually, I just upgraded Greclipse on the other workspace and Call Hierarchy still works there... Maybe I have some corruption in the workspace where it's broken. Rebuilding the index does not help, neither cleaning all projects... I may try to recreate the workspace from scratch...
Suggestions welcome...

(actually, I was wondering whether that button "Rebuild Index" in Eclipse preferences does anything, I get no feedback at all when I press it...).

@mauromol
Copy link
Author

For the records, I fixed my broken Call Hierarchy by creating a new workspace from scratch and then re-importing my projects...

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

No branches or pull requests

2 participants