Bug 324109

Summary:

[search] Java search shows incorrect results as accurate matches

Product:

[Eclipse Project] JDT

Reporter:

Deepak Azad <deepakazad>

Component:

Core

Assignee:

Satyam Kandula <satyam.kandula>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

amj87.iitr, daniel_megert, frederic_fusier, Olivier_Thomann

Version:

3.7

Target Milestone:

3.7 M2

Hardware:

OS:

Windows XP

Whiteboard:

Attachments:

Description	Flags
workspace	none
search results	none
proposed partial fix	none
Patch to test	none
Patch on Head	none

Description Deepak Azad

2010-08-31 13:04:36 EDT

I am attaching the workspace to reproduce the problem (I have been unable to reproduce this with a fresh workspace.)

Steps:
1. Use the attached workspace
2. Ctrl+H, search for 'Worker.run()', Declarations, and all 4 Search In options selected, Scope=workspace
3. A number of 'run()' methods are returned which are not in 'Worker' type (I will attach a screenshot)

Comment 1 Deepak Azad

2010-08-31 13:20:49 EDT

Created attachment 177865 [details]
workspace

Comment 2 Deepak Azad

2010-08-31 13:23:09 EDT

Created attachment 177866 [details]
search results

Comment 3 Olivier Thomann

2010-08-31 13:24:17 EDT

Satyam, please follow up.

Comment 4 Deepak Azad

2010-08-31 13:38:54 EDT

Created attachment 177870 [details]
proposed partial fix

The problem seems to be in org.eclipse.jdt.internal.core.search.matching.MethodLocator.resolveLevelAsSubtype(...). In org.eclipse.jdt.internal.core.search.matching.PatternLocator.resolveLevelForType(char[], TypeBinding) there is a check for invalid type binding, and I think the same check should be made in resolveLevelAsSubtype as well. 

If the type binding in not valid (for me it was MissingTypeBinding) then the following check in line 685 is invalid : || !type.isAbstract()) && !type.isInterface())
(Also I do not know why the type binding was invalid)

The patch does not fix the problem completely, there were still some incorrect results with this patch. I tried to fix it but I did not understand the use of *_FLAVOR constants here...

Comment 5 Deepak Azad

2010-08-31 21:33:10 EDT

argh.. forgot the build ID I20100824-1210

Comment 6 Satyam Kandula

2010-09-01 07:46:30 EDT

Created attachment 177939 [details]
Patch to test

Deepak, I couldn't get any bad results with your workspace, but I understand the problem. Though I got one junit testcase, I want you to try out the attached patch. 

This patch doesn't include the changes you have proposed but does include the fix similar to the one you have proposed in our discussion - don't include the match flavors while comparing the match levels. 

Please try out this attached patch and let me know how it goes.

Comment 7 Deepak Azad

2010-09-01 08:04:41 EDT

(In reply to comment #6)
> Created an attachment (id=177939) [details] [diff]
> Patch to test

+1 With this patch all the results where type binding is missing are correctly marked as potential matches.

Comment 8 Satyam Kandula

2010-09-02 04:38:58 EDT

Simple test case
########
public class Test extends MissingType{
	public void foo() {
	}
}
#######
Search for method declarations of Worker.foo() reports Test.foo() as Exact Match. This should have been a potential match.

Comment 9 Satyam Kandula

2010-09-02 04:48:18 EDT

Created attachment 178026 [details]
Patch on Head

Patch with minor correction

Comment 10 Deepak Azad

2010-09-02 04:56:29 EDT

(In reply to comment #8)
> Simple test case
> ########
> public class Test extends MissingType{
>     public void foo() {
>     }
> }
> #######
> Search for method declarations of Worker.foo() reports Test.foo() as Exact
> Match. This should have been a potential match.

Ah yes, this is a much simpler way to reproduce the problem :) But I wonder why the type bindings were missing in my workspace - There were no errors and I had tried clean build as well. Satyam, could the type bindings be missing only for search ?

Comment 11 Frederic Fusier

2010-09-02 05:05:49 EDT

(In reply to comment #4)
> 
> The patch does not fix the problem completely, there were still some incorrect
> results with this patch. I tried to fix it but I did not understand the use of
> *_FLAVOR constants here...

The OVERRIDDEN_METHOD_FLAVOR was set to store the fact that if a match was found during sub-types resolution it was in an overridden method. There's currently no API to exhibit this peculiar piece of information but I let it there in case someone made a request for it in the future...

Comment 12 Satyam Kandula

2010-09-02 05:09:59 EDT

Released in HEAD

Comment 13 Satyam Kandula

2010-09-02 05:11:42 EDT

(In reply to comment #10)
> Ah yes, this is a much simpler way to reproduce the problem :) But I wonder why
> the type bindings were missing in my workspace - There were no errors and I had
> tried clean build as well. Satyam, could the type bindings be missing only for
> search ?
The references you were getting should be for those jar whose dependents may not be present. Hence, there is a possibility of running into this problem.

Comment 14 Deepak Azad

2010-09-02 06:36:31 EDT

(In reply to comment #13)
> The references you were getting should be for those jar whose dependents may
> not be present. Hence, there is a possibility of running into this problem.
Hmm ok.I will keep an eye out for this.

Thanks for the fix!

Comment 15 Ayushman Jain

2010-09-14 05:30:15 EDT

Verified for 3.7M2 using build I20100909-1700.

Comment 16 Dani Megert

2010-09-14 06:43:06 EDT

This special cases looks fixed but the general bug of reporting too many matches is still open, see bug 324189.