Bug 329727

Summary:

Invalid check in the isConstructor() method of the IMethod implementation.

Product:

[Eclipse Project] JDT

Reporter:

Dietrich M. <dietrich.mostowoj>

Component:

Core

Assignee:

Jay Arthanareeswaran <jarthana>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

amj87.iitr, Olivier_Thomann, satyam.kandula

Version:

3.7

Flags:

satyam.kandula: review+

Target Milestone:

3.7 M4

Hardware:

OS:

Windows XP

Whiteboard:

Attachments:

Description	Flags
Proposed patch	none
updated patch	jarthana: review?

Description Dietrich M.

2010-11-09 02:53:02 EST

Build Identifier: M20100211-1343

The implementation of the org.eclipse.jdt.core.IMethod interface (org.eclipse.jdt.internal.core.BinaryMethod/SourceMethod) provides a check before requesting the method element info in order to increase the performance of the isConstructor() method. But unfortunately this check doesn't work for inner classes constructors. The element name of the inner class constructor is separated by the '$' character but the parent element name not (e.g constructor element name -> OuterClass$InnerClass parent element name -> InnerClass). In this case the check is failed and the constructor is not determined.

Reproducible: Always

Steps to Reproduce:
1. Used org.eclipse.jdt.core.search.SearchEngine to search in the workspace for methods and constructors

Comment 1 Ayushman Jain

2010-11-09 06:21:59 EST

Jay, please follow up. Thanks!

Comment 2 Jay Arthanareeswaran

2010-11-09 09:14:03 EST

I am unable to reproduce this with the build I20101028-1441.

Could you try with a newer build and if you still find the problem, could you please provide a test case that I can use to reproduce?

Comment 3 Dietrich M.

2010-11-10 02:46:40 EST

(In reply to comment #2)
> I am unable to reproduce this with the build I20101028-1441.
> Could you try with a newer build and if you still find the problem, could you
> please provide a test case that I can use to reproduce?

I created a simple PDE application with an action that is performing this peace of code:
        
        final String txtPattern = "Snapsho*";
		
        SearchPattern pattern = SearchPattern.createPattern(txtPattern,
        		IJavaSearchConstants.CONSTRUCTOR, 
        		IJavaSearchConstants.DECLARATIONS,
                SearchPattern.R_CASE_SENSITIVE | 
                SearchPattern.R_PATTERN_MATCH);
        
        SearchParticipant[] participants = new SearchParticipant[1];
        participants[0] = SearchEngine.getDefaultSearchParticipant();
                        
        SearchRequestor requestor = new SearchRequestor() {
          @Override
	  public void acceptSearchMatch(SearchMatch match) 
          throws CoreException {
		// assert method
		assert match.getElement() instanceof IMethod;
		// assert constructor
                assert ((IMethod) match.getElement()).isConstructor();	
	  }
         };

        try {
         SearchEngine engine = new SearchEngine();
         IJavaSearchScope scope = engine.createWorkspaceScope();
         engine.search(pattern, participants, scope, requestor, 
                       new NullProgressMonitor());
        } catch (CoreException e) {
          e.printStackTrace();            
        }

The search engine finds the constructor of the following class :
   com.sun.xml.internal.bind.v2.runtime.unmarshaller.LocatorEx$Snapshot
But the getElementName() method of the returnd IMethod provides "LocatorEx$Snapshot" and the parent.getElementName()method provides "Snapshot", so the isConstructor() method returns false and the second assert in the requestor fails.

Comment 4 Dietrich M.

2010-11-10 02:50:30 EST

Last version I used.

Version: 3.6.1
Build id: M20100909-0800

Comment 5 Jay Arthanareeswaran

2010-11-10 05:21:40 EST

Thanks for the code, Dietrich! I am able to reproduce the problem now. The problem occurs only with binary types.

I will investigate why we remove the enclosing type name from the inner type name (ClassFile#getType) where as the method name is untouched.

Comment 6 Jay Arthanareeswaran

2010-11-11 06:11:44 EST

Created attachment 182893 [details]
Proposed patch

While constructing a BinaryMethod in ClassFileMatchLocator#locateMatches(), the constructor is allowed to keep the enclosing name as part of it's element name. It causes problem since the type name itself doesn't have the enclosing name.

This has been fixed in the patch and tests added.

Satyam, can you please review the patch?

Comment 7 Satyam Kandula

2010-11-16 11:00:53 EST

I was wondering if info.getSourceName() could be called instead of calling info.getName() and then processing. The search bug tests generally go in JavaSearchBugsTests.java - So, please move the test there.

Comment 8 Jay Arthanareeswaran

2010-11-18 01:46:45 EST

Created attachment 183365 [details]
updated patch

Indeed, info.getSourceName() will work very well. Thanks, Satyam. The patch has been updated accordingly. And I have moved the tests to JavaSearchBugsTests.

Comment 9 Satyam Kandula

2010-11-22 03:21:56 EST

(In reply to comment #8)
> Created an attachment (id=183365) [details]
> updated patch
> 
> Indeed, info.getSourceName() will work very well. Thanks, Satyam. The patch has
> been updated accordingly. And I have moved the tests to JavaSearchBugsTests.
The Patch looks good. +1

Comment 10 Jay Arthanareeswaran

2010-11-22 05:12:22 EST

Released in HEAD for 3.7 M4.

Comment 11 Satyam Kandula

2010-12-07 05:35:21 EST

Verified for 3.7M4 using build I20101205-2000