Community
Participate
Working Groups
Created attachment 177946 [details] proposed fix I20100831-1001 Steps - Do a type search for 'worker' => no * is appended in this case - good - Do a method search for 'Worker.run()' => it essentially searches for '*worker.run()' and hence returns results like 'InternalWorker.run()' etc - bad The correct search should be for '*.worker.run()', attached patch should fix the problem.
Do you have a more precise reproducible test case? I cannot get the described symptoms. All the matches I've found are potential ones and the attached patch does not change anything...
>Do you have a more precise reproducible test case? Do a method declaration search for "Worker.run" via search dialog ==> java.util.concurrent.ThreadPoolExecutor.Worker.run() javax.swing.SwingWorker.run() sun.jdbc.odbc.ee.PoolWorker.run()
(In reply to comment #2) > >Do you have a more precise reproducible test case? > Do a method declaration search for "Worker.run" via search dialog > ==> > java.util.concurrent.ThreadPoolExecutor.Worker.run() > javax.swing.SwingWorker.run() > sun.jdbc.odbc.ee.PoolWorker.run() Yep, I got it now... :-) The missing precise key word was 'declaration' as I was doing as references search...
Deepak, Thanks for the patch but there is one small problem with the patch. It misses declarations from default packages. Let me see if I can get to fix that.
(In reply to comment #4) > Deepak, Thanks for the patch but there is one small problem with the patch. It > misses declarations from default packages. Let me see if I can get to fix that. Satyam, Please note that this patch also misses binary inner class as their name is something like EnclosingClass$Inner which fails to match the "*.worker" pattern...
Created attachment 178028 [details] Jar file exhibiting the inner class issue Search for type declaration of "Worker" on a project which has the attached jar on its classpath misses the Test.Worker inner class with the patch...
(In reply to comment #5) Frederic, Yes this test also fails. Thanks for this additional test and I will take care of it also whenever I get the fix.
Note that this bug is not limited to JAR / binary types and has been there since day one.
Also, it looks like all methods named 'run' but with completely different type name are reported as potential matches. This looks wrong to me.
(In reply to comment #9) > Also, it looks like all methods named 'run' but with completely different type > name are reported as potential matches. This looks wrong to me Those bindings which couldn't be resolved end up in the potential matches. We cannot do much with them.
>Those bindings which couldn't be resolved end up in the potential matches. We >cannot do much with them. Why can't they be resolved? It happens in the simplest case: 1. new workspace 2. new Java project 3. search for "Worker.run"
(In reply to comment #11) > >Those bindings which couldn't be resolved end up in the potential matches. We > >cannot do much with them. > Why can't they be resolved? It happens in the simplest case: > 1. new workspace > 2. new Java project > 3. search for "Worker.run" As you would have noticed, this types should be from some dependent plugins. Most likely,that particular type's hierarchy is not completely resolved. However, I do agree that there could be some mistake there, mistake in not able to resolve that.
Remember that this is a method *declaration* search. I don't think any types whose name doesn't match should be considered by this kind of search at all.
(In reply to comment #13) > Remember that this is a method *declaration* search. I don't think any types > whose name doesn't match should be considered by this kind of search at all. Don't you think it should check for method declarations of sub classes? I believe it will be required for refactorings.
(In reply to comment #9) > Also, it looks like all methods named 'run' but with completely different type > name are reported as potential matches. This looks wrong to me. Yup, these are coming from nested/anonymous classes and yes we should fix this.
(In reply to comment #15) > Yup, these are coming from nested/anonymous classes and yes we should fix this. Filed bug 325418 to track this.
(In reply to comment #13) > Remember that this is a method *declaration* search. I don't think any types > whose name doesn't match should be considered by this kind of search at all. Are you telling that the subtypes should not be returned for method *declaration* search? Javadoc for DECLARATIONS tells ########### DECLARATIONS: will search declarations matching with the corresponding element. In case the element is a method, declarations of matching methods in sub-types will also be found, allowing to find declarations of abstract methods, etc. ##########
> In case the element is a method, declarations of matching methods in sub-types > will also be found, allowing to find declarations of abstract methods, etc. You're right, and we can't change that API. So the way to go is to add a 'limitTo' flag, e.g. IJavaSearchConstants.IGNORE_SUB_TYPES that implements a much faster search that only considers matching type names (if the type name is empty or "*", it should of course consider all types, but it still doesn't need to build any hierarchies).
(In reply to comment #18) > So the way to go is to add a 'limitTo' flag, e.g. > IJavaSearchConstants.IGNORE_SUB_TYPES that implements a much faster search that > only considers matching type names (if the type name is empty or "*", it should > of course consider all types, but it still doesn't need to build any > hierarchies). Yes, I agree with this and will open a separate bug for this.
...as I suspected, this is not limited to Declaration Search - Fresh Workspace - Create the following 2 classes package p; class A { void foo() { } public static void main(String[] args) { new A().foo(); } } package p; class AnotherA { void foo() { } public static void main(String[] args) { new AnotherA().foo(); } } - Do a Java Search for 'A.foo()' with 'Method' and 'References' options selected (Search in = Java Sources) => 2 matches are returned! (I also suspect that the fix will be same for declaration and reference search.)
Moving to M4 to keep more time for APT work.
Moving again.
Created attachment 187262 [details] Proposed patch on HEAD
Jay, Can you please review? Thanks.
Created attachment 187271 [details] Proposed patch on HEAD There is a problem in the test of the previous patch. Fixed it. No source changes.
Patch looks good to me.
Thanks Jay, Released the patch on HEAD.
Thanks Olivier for letting me know. This patch doesn't indeed fix the case mentioned in comment 20. Hence, reopening this bug.
Looks like comment 20 is a special case. If the length of the pattern is 1 (here the length of A is 1). bug 323213 runs into the same problem.
(In reply to comment #29) > Looks like comment 20 is a special case. If the length of the pattern is 1 > (here the length of A is 1). bug 323213 runs into the same problem. Actually it is not the length 1 that is the special case, but AnotherA starting and ending with A which being the pattern is making the test case fail. CharOperation.match() doesn't handle this case well!
Created attachment 187525 [details] Patch in consideration Patch that fixes the special test case in comment 20. This also fixes bug 323213. Test to follow.
Created attachment 187533 [details] Proposed patch Added tests also. I am running the full test suite.
Patch looks good to me. The regression test should be moved to the CharOperationTest test suite.
Moved the test to CharOperationTest and Released on HEAD.
Verified with I20110125-2012.
*** Bug 323213 has been marked as a duplicate of this bug. ***
