Community
Participate
Working Groups
Version: 3.5.0 Build id: I20090313-0100 1. Create a Java project with the following: public class C { public void Methodaaaa1() {} public void Methodaaaa2() {} public void Methodaaaa3() {} public void Methodaaaa4() {} public void Methodaaaa5() {} public void Methodaaaa6() {} public void Methodaaaa7() {} public void Methodaaaa8() {} public void Methodaaaa9() {} public void Methodaaaa10() {} public void Methodaaaa11() {} public void Methodaaaa12() {} public boolean Methodboolean() { return false;} public Boolean Methodboolean2() { return false;} public void foo() { if (Meth|) } } 2. At the '|' do a <Ctrl-Space> to complete. 3. You will see that all the void typed proposals appear in the list. Should these even be proposed ? Perhaps not. 4. See that this is one instance of a context where void types are not suitable, not the only one. In general whenever the completion engine has a definite notion of expected type, it would appear void types should not be proposed.
See bug# 25876 & bug# 43897. We don't want to eliminate void typed proposals altogether, we may want to diminish their relevance so they are offered below others.
Jay, can you take a look at this ? Normally we use the notion of expected type to boost the relavance of a proposal. In this case, we may want to diminish the relavance. Thanks.
Consider this test: public class C { void foo00() {} void foo01(){} String foo0 () {return null;} float foo1 () {return 0;} int foo2() {return 0;} Long foo3() {return null;} long foo4() {return 0;} Integer foo5() {return 0;} void test() { foo| } } On Ctrl+Space, we get all foo methods get the same relevance value and ultimately listed alphabetically. My question is, if the completion node doesn't have an expected type, should we give higher relevance to void methods?
In this case I would use the same relevance for all methods because it is not so rare to want to use a method without using the returned value (eg. List.add(Object)).
Created attachment 135919 [details] Proposed patch The fix introduces a new relevance for methods returning void values with a value of -5. This way the relevance of such methods get reduced by 5. Test cases have been modified to relflect/test this behavior.
I looked the patch and i have several suggestions: - I do not understand why the void check is done in the for loop inside computeRelevanceForExpectingType(). I think it should be done at the beginning of the method with something like: private int computeRelevanceForExpectingType(TypeBinding proposalType){ if (proposalType == TypeBinding.VOID) return VOID; ... - Before this patch all the relevance constants were positive so there is no risk to compute a negative relevance. This patch change this rule and add a negative constant. To limit the risk of a negative relevance when adding a new constant in RelevanceConstants the new rules could be: - a relevance is the sum of constants defined in RelevanceConstants - each constant can be added only one time in the relevance of a proposal - the sum of all negative constants and R_DEFAULT must be positive You should add a comment in RelevanceConstants which explicitly describe these new rules. We should fix this bug for 3.6 as it is too late for 3.5.
Agree with you on the changes, David. In fact, this change unearthed many more test cases that may have to be changed. And since this is going only for 3.6, I will work on fixing the test cases and submit a new patch after that.
Created attachment 140340 [details] Updated Patch Attached the patch with the modifications as per suggestions and modified test cases.
Released Jay's fix in HEAD for 3.6M1
Verified for 3.6M1
(In reply to comment #10) > Verified for 3.6M1 > ... using I20090802-2000 build
