Community
Participate
Working Groups
build I20090427-1800 While verifying bug 84720, i noticed that some proposals have a too higher relevance value. 1) create Test.java public class Test { String foo0 () {return null;} float foo1 () {return 0;} int foo2() {return 0;} Long foo3() {return null;} long foo4() {return 0;} void test() { Long i = foo| } } 2) do ctrl+space at | foo1() has the same relevance as foo4() but float is not compatible with Long. foo1() should have the same relevance as foo0().
Created attachment 135101 [details] Proposed patch Swapped the parameters to the isBoxingCompatibleWith method invocation, which takes care of the incompatibility issue. Also modified the previous test cases to test the given cases.
The fix looks good but the modified regression test is not equivalent to the previous version. The previous has a base type (int) as expected type and the new a Class (Long) as expected type. Both tests would be useful, so i would keep both.
Created attachment 135507 [details] Patch with updated tests There was an existing test case that had the expected type as a wrapper. I modified that to primitive. Now we have tests both with primitive and wrapper class as expected types. Hope this serves.
Created attachment 135535 [details] Patch with updated tests Retaining the older tests and their order as they are.
Comment on attachment 135535 [details] Patch with updated tests patch looks good
Frederic - Could you review the patch ?
(In reply to comment #6) > Frederic - Could you review the patch ? > The change in CompletionEngine looks good, but the test seems wrong to me: 1) the int<->Integer auto-boxing should not have been removed 2) the introduced Long and Double classes are not declared in the test case, hence may it slightly different than the initial one...
Created attachment 135757 [details] Updated patch Fred's suggestions included in the tests. Some of the redundant tests have been removed to keep the test simpler and at the same time retaining the Long as the expected type to keep the test case as close to the bug's description as possible.
Olivier, could review the last patch ?
Kent, could you review the last patch ?
+1. Patch looks good.
Comment on attachment 135757 [details] Updated patch Patch looks good
Released for 3.5RC2 Tests updated CompletionTests_1_5#testCompletionWithUnboxing_1()
Verified for 3.5RC2 using I20090521-2000.
