Community
Participate
Working Groups
Build Identifier: M20090917-0800 Searching for IMPLEMENTORS of, e.g. "java.lang.Object" also returns classes whose supertypes/superinterfaces use "java.lang.Object" as a type parameters. Considering the following lines in the Javadoc of SearchPattern.createPattern(String, ...) "search for implementers of java.lang.Runnable: createSearchPattern("java.lang.Runnable", IJavaSearchConstants.TYPE, IJavaSearchConstants.IMPLEMENTORS, true);" I am wondering whether (a) the observed behaviour is intended, or (b) the API documentation is outdated (there is no 'createSearchPattern' method anymore). Either way, one of them needs fixing. Reproducible: Always Steps to Reproduce: 1. The test class in the search scope public class Test implements Comparable<Object> { @Override public int compareTo(Object o) { return 0; } } 2. create the following search pattern final SearchPattern p = SearchPattern.createPattern( "java.lang.Object", IJavaSearchConstants.TYPE, IJavaSearchConstants.IMPLEMENTORS, SearchPattern.R_PATTERN_MATCH);
(In reply to comment #0) This is a bug and am looking into it.
Created attachment 177717 [details] Experimental patch Frederic, For the given test case, the search doesn't take into consideration the fact that the parameter arguments are not required for super type references. The context of the reference can be known only while the file is being parsed or while the reference is getting reported. I see that the context is being checked only while the reference is getting reported rather than the file is being parsed. Is there any reason why this is not being done? Please look at the attached patch to get more sense of what I am talking about.
Frederic, please look at comment 2.
(In reply to comment #2) > Created an attachment (id=177717) [details] > Experimental patch > > Frederic, For the given test case, the search doesn't take into consideration > the fact that the parameter arguments are not required for super type > references. The context of the reference can be known only while the file is > being parsed or while the reference is getting reported. I see that the context > is being checked only while the reference is getting reported rather than the > file is being parsed. Is there any reason why this is not being done? Please > look at the attached patch to get more sense of what I am talking about. Remove inaccurate matches as soon as possible is the better for Search Engine. So, if it's possible to filter matches during the parse, then it's better to do it at that time. So, in this sense, your intention is correct. However, I do not like the idea to use a context of the MatchLocator to do this filtering. Doing that, you added crossed-references between the MatchLocatorParser and the MatchLocator and also between the PatterLocator and the MatchLocator. That really worries me as that often shows a design issue and also leads to potential performances/leaks issues. I think a simpler solution is possible. I'll be back soon with a proposal after I've tested it a little bit...
Created attachment 178046 [details] Other experimental patch Here's my proposal. The change is specific to SuperTypeReferenceLocator which removes matches when they correspond to a type argument of the given type reference. Note that there's no need to recurse among the type arguments as the match(TypeReference node, MatchingNodeSet nodeSet) method is called for each single type reference...
Created attachment 178120 [details] a test (In reply to comment #5) Removing the matches is a nice idea, but createOrPattern(...) could have problems. Though this probably is not used through UI or not typically used, we should keep in mind of it so that it is handled properly whenever it is widely used. Attached is a testcase which shows the problem.
Created attachment 178133 [details] Proposed patch (In reply to comment #6) > Created an attachment (id=178120) [details] > a test > > (In reply to comment #5) > Removing the matches is a nice idea, but createOrPattern(...) could have > problems. Though this probably is not used through UI or not typically used, > we should keep in mind of it so that it is handled properly whenever it is > widely used. > Attached is a testcase which shows the problem. Well spotted! The AndPattern is not a problem as the expected results is the intersection of the matches, hence removing the ones for the implementors one is still ok... but I agree that the OrPattern is definitely annoying :-( So, here's a patch which is the combination of our two proposals which fixes your additional test case, even after having made it a little bit more complex...
(In reply to comment #7) Frederic, Yes, this patch is clean and looks good. However, I am wondering if it really makes sense to classify this as a flavor and if it does really belong to the other group.
(In reply to comment #8) > (In reply to comment #7) > Frederic, Yes, this patch is clean and looks good. However, I am wondering if > it really makes sense to classify this as a flavor and if it does really belong > to the other group. IMO, this is really a flavor... That means this is something specific for the matches found through this method: they are obviously super types. That's why we also found a similar filter for fine grain specialization at the same places...
Released the patch onto HEAD. Frederic, Thank you.
verified for 3.7M2 using build I20100909-1700.
