Bug 325418 - [search] Search for method declarations returns spurious potential matches for anonymous classes
Summary: [search] Search for method declarations returns spurious potential matches fo...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.6   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.7 M5   Edit
Assignee: Satyam Kandula CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 250958 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-09-16 05:22 EDT by Satyam Kandula CLA
Modified: 2011-01-25 09:15 EST (History)
6 users (show)

See Also:
srikanth_sankaran: review+


Attachments
Proposed patch (11.77 KB, text/plain)
2010-10-12 04:47 EDT, Satyam Kandula CLA
no flags Details
Proposed patch syncing with HEAD (11.77 KB, patch)
2010-10-27 05:14 EDT, Satyam Kandula CLA
satyam.kandula: review?
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Satyam Kandula CLA 2010-09-16 05:22:34 EDT
Search for a method declaration of Worker.run() returns some incorrect anonymous classes as potential matches. 
To reproduce:
1. Open a fresh workspace
2. Create a java project
3. Use a JDK attached to source code
4. Search for method declarations of Worker.run()
You will see many potential matches for anonymous classes. These could be probably avoided.
Comment 1 Satyam Kandula CLA 2010-10-12 04:41:52 EDT
While reading some class files, an Abortcompilation exception is getting thrown and subsequent source files are not getting parsed to resolve some bindings. As the bindings were not resolved, search engine is returning those matches as potential matches. This happens for only anonymous classes because abort happens at the stage of parsing method statements and their resolution.
Comment 2 Satyam Kandula CLA 2010-10-12 04:47:52 EDT
Created attachment 180648 [details]
Proposed patch

The exception happens while parsing types which use the enclosing method's type parameters. For finding the signatures we do look at the enclosing type's parameters but not methods. 
In this patch, while calculating the type's parameters, I have made the type take the enclosing method's type parameter. 

With this patch, I don't have any potential matches with my workspace.
Comment 3 Satyam Kandula CLA 2010-10-12 04:48:29 EDT
Srikanth, Please review.
Comment 4 Satyam Kandula CLA 2010-10-13 08:23:01 EDT
The source of this bug seems to impact refactoring too as reported in the second part of bug 293861 comment 62.
Comment 5 Srikanth Sankaran CLA 2010-10-21 00:22:29 EDT
(In reply to comment #3)
> Srikanth, Please review.

The patch as it is fails to apply. Could you regenerate after synching
with HEAD and repost for review ? Thanks!
Comment 6 Satyam Kandula CLA 2010-10-27 05:14:54 EDT
Created attachment 181811 [details]
Proposed patch syncing with HEAD

Srikanth, I have regenerated the patch. Please review.
Comment 7 Srikanth Sankaran CLA 2010-11-19 08:05:57 EST
My sincere apologies, this has been in my queue
for a long time. Everytime I plan to get to it,
I get overwhelmed by half a dozen of those 1.4/1.5
defects to work on. 

I'll prioritize it and get this review early next
week -- Thanks.
Comment 8 Srikanth Sankaran CLA 2010-11-22 01:56:24 EST
I am not quite sure manipulating the arity of the 
local/anonymous type to make it include the type
parameters of the enclosing method is the right way
to go.

Given these are local/anonymous types and given further
that these are binary types we are talking about, it may
not be as risky to munge things this way. Nevertheless,
akin to being able to query the type parameters of an
enclosing type in the case of inner type, I would recommend
we investigate and see if a cleaner approach may be possible.
Comment 9 Satyam Kandula CLA 2010-12-01 05:17:56 EST
I get your point. However, we don't store the enclosing method information anywhere except from the class file Object itself( ClassFileReader ) :(. We do need this information at LookupEnvironment#getTypeFromTypeSignature() where it could get costly to access the class file object. One proper way could be is to create  an object like LocalTypeBinding which extends BinaryBinding and has enclosing method, but it is probably not necessary except for this problem. What do you think?
Comment 10 Srikanth Sankaran CLA 2011-01-17 03:09:36 EST
(In reply to comment #9)
> I get your point. However, we don't store the enclosing method information
> anywhere except from the class file Object itself( ClassFileReader ) :(. We do
> need this information at LookupEnvironment#getTypeFromTypeSignature() where it
> could get costly to access the class file object. One proper way could be is to
> create  an object like LocalTypeBinding which extends BinaryBinding and has
> enclosing method, but it is probably not necessary except for this problem.
> What do you think?

Thanks for the explanation Satyam, under the circumstances, I suggest
you proceed with the approach you have.
Comment 11 Satyam Kandula CLA 2011-01-17 08:24:34 EST
Thank you Srikanth.
Released this on HEAD
Comment 12 Satyam Kandula CLA 2011-01-17 08:27:04 EST
*** Bug 250958 has been marked as a duplicate of this bug. ***
Comment 13 Olivier Thomann CLA 2011-01-25 09:15:19 EST
Verified for 3.7M5 using I20110124-1800