Community
Participate
Working Groups
3.2 A problem that is still unsolved: The TypeNameRequestor returns matches of a type search in a given scope with the information of: packageName, simpleTypeName, enclosingTypeNames and path. At some point we need to resolve this information to a IType. Given the fields above and the IJavaSearchScope is very tricky and shouldn't be done by user code. Best place for this API would be IJavaSearchScope, but as the API is implementable so we might consider something like TypeNameRequestor.resolveType(scope, packageName, enclosingTypeNames, simpleTypeName) Or look into taking over TypeInfo from jdt.ui. A lightweight element that efficiently stores type search results. Note that our TypeInfo.resolveType is already quite complex, but can't solve the case described in bug 119707.
I'd rather imagine defining a new API on SearchEngine taking a TypeElementRequestor for feeding results. A type element requestor would #accept(IType) or even better for future evolution, directly a SearchRequestor, where SearchMatches would be more lightweight (no position information for instance)
#accept(IType) is bad as we will access the flags of it right after. So for the all type dialog this will become very expensive. The second suggestion is more or less what we did with TypeInfo which stores the lightweight information (qualified name, context, flags). That would be a good solution.
Why not simply using a partial SearchMatch, as opposed to yet another match object...
Note that even in JDT/Core, we will fail to find existing type in bug 119707 comment 2 test case... To fix it, we'd need to add a new API on JavaCore: create(IFile, IJavaProject). Then giving the java project where the generic class is defined as parameter to this method would return an existing handler...
Created attachment 50710 [details] Proposed patch Here's a first draft which implements this new API... Patch summary: + Added new SearchMatch: TypeDeclarationNameMatch which keeps information returned by the searchAllTypeNames operation and may provide corresponding IType when requested + Added SearchEngine.searchAllTypeNames method with a SearchRequestor instead of TypeNameRequestor. Results from this request are TypeDeclarationNameMatch accepted in given requestor... + Deprecates SearchEngine searchAllTypeNames(char[], char[], int, int, IJavaSearchScope, TypeNameRequestor, int, IProgressMonitor) API method to avoid too many different searchAllTypeNames methods which may confuse users... + Also added JavaCore.create(IFile,IJavaProject) to solve issue shown by specific test case of bug 119707.
Created attachment 50713 [details] JDT/UI patch to test this implementation Here's the patch of the changes I made in JDT/UI to use this new API and verified that with proposed JDT/Core patch, bug 119707 is fixed... This patch is based on version v20060920-1200.
Created attachment 50714 [details] JDT/UI patch to test this implementation Sorry, previous patch was an old version...
At first sight, the API looks smooth and nicely fitting. But at the second look it has some major weeknesses: The result is returned as a subclass of SearchMatch, but almost all API of SearchMatch is is not applicable to the type name search: length/offset/accuracy/participant/resource. Unfortunatly this information are currently all stored in fields in the SearchMatch, adding 5 fields to a type name match not needed. In the case of the open types dialog, memory is a concern as the user can enter a pattern so that all types in the system are the result and as many 'TypeNameSearchMatch' in the memory while the dialog is up. In the old days of the all types cache, we even added code that tried to reuse (split and internalize) the path / package and type name char arrays to reduce the memory footprint further. So yes, the field could be moved from the SearchMath to specific subclasses to solve this. But mostly for API reasons I would prefer separate API TypeNameRequestor and TypeNameMatch. As we already have a TypeNameRequestor, I suggest to stick with it. TypeNameMatch would be a good addition and users can create it themselves inside a TypeNameRequestor. Or, we can offer a TypeNameMatchRequestor that just implements acceptType and has a new abstract method acceptTypeNameSearchResult. public abstract class TypeNameMatchRequestor extends TypeNameRequestor { public void acceptType(int modifiers, char[] packageName, char[] simpleTypeName, char[][] enclosingTypeNames, String path) { acceptTypeNameMatch(new TypeNameMatch(modifiers, packageName, simpleTypeName, enclosingTypeNames, path)); } public abstract void acceptTypeNameMatch(TypeNameMatch match); } public class TypeNameMatch { public TypeNameMatch(int modifiers, char[] packageName, char[] simpleTypeName, char[][] enclosingTypeNames, String path) { } getModifiers/PackageName/simpleTypeName/EnclosingTypeNames/Path IType resolveType(); }
For bug 119707, wouldn't 'JavaCore.create(IResource, IJavaProject)' be more general and necessary to cover class folders as well?
(In reply to comment #8) Your arguments sound reasonable and I agree that returning a sub-class of SearchMatch does not look like an optimal solution... However, if we decide to create a separated class to store TypeNameRequestor returned information, I do not think we should use a class name with Match in it. This could be misleading for users which may wrongly think that it could inherit from SearchMatch. As it's pure index information, perhaps we can use a name with 'Index' and create a new API hierarchy like: SearchIndexInfo + TypeDeclarationIndexInfo This hierarchy, then might be extended in future (only if really needed) with other declarations stored in indexes: + MethodDeclarationIndexInfo + FieldDeclarationIndexInfo
Agree, 'Match' might be confusing. I don't like IndexInfo however. 'TypeSearchResult' would be my suggestion. I wouldn't intruduce a object hierarchy until we really need one.
(In reply to comment #9) > For bug 119707, wouldn't 'JavaCore.create(IResource, IJavaProject)' be more > general and necessary to cover class folders as well? > I don't think this is necessary. Returned handle for class folders is already correct (e.g. exists) as JavaModelManager.create(IFolder) scans all existing wksp projects when default one returns an non-existing handle. There's also no problem with other IResource as IProject and IWorkspaceRoot... That's why I only added a method on IFile...
Created attachment 51323 [details] Proposed patch Zip file including patch + .class for test to put in org.eclipse.jdt.core.tests.model/workspace/JavaSearchBugs/lib folder to make all tests pass...
Released for 3.3 M3 in HEAD stream.
*** Bug 157832 has been marked as a duplicate of this bug. ***
Reopen, as this API needs to be polished...
Created attachment 51563 [details] New implementation for this functionality
Released for 3.3 M3 in HEAD stream
Verified for 3.3M3 with 20061030-0800
