Community
Participate
Working Groups
Build Identifier: I20100129-1300 The autocast after instanceof feature was introduced in bug 193909. However, the following cases were left unsupported: 1)The feature doesnt work when completion is asked after an if statement containing instanceof expression but where another if / while / some statement lies in between viz. class X { void methodFromX(){} void foo(Object a){ int i = 1; if(a instanceof X){ if(i == 1) { a.[CTRL-SPACE] // EXPECTED PROPOSAL : methodFromX, and if accepted also a cast to X // However, we dont get this currently } } } } 2) It doesnt work when used in conjunction with binary expressions ( ==, != , &&, ||, &, | ) , viz. class X { void methodFromX(){} void foo(Object a){ int i = 1; if(a instanceof X){ if(i == 1 && a.[CTRL-SPACE]) { // EXPECTED PROPOSAL : methodFromX, and if accepted also a cast to X // However, we dont get this currently } } } } We can change the && with any other binary expression in the above code, and we still dont get the required proposal. Reproducible: Always Steps to Reproduce: 1. Use the above snippets 2. Press CTRL SPACE where indicated 3. See that the expected proposals dont come
Created attachment 160299 [details] proposed fix v0.5 + regression tests CompletionParser#buildMoreCompletionEnclosingContext(Statement) has been modified to identify an instanceof expression in the expression stack, in the case that it is not in the immediate context of the location where content assist is requested. This is done to make sure that the if statement containing the instanceof expression is made the enclosing node when required. Also CompletionParser#attachOrphanNode() has been modified to identify the case for binary expressions.
class X { void methodFromX(){} void foo(Object a, Object b){ int i = 1; if (a instanceof X) { if (b instanceof Y) { if (i == 1) { a.[ctrl-space] } } } } } class Y { void methodFromY() {} } Is this supposed to work ? The introduction of the inner instanceof check inhibits a couple of proposals from showing up.
> Is this supposed to work ? The introduction of the inner instanceof > check inhibits a couple of proposals from showing up. No, this doesnt work. Sorry I forgot to point this case out, but the above patch doesnt attempt to fix this case.
Created attachment 162384 [details] improved fix v1.0 + regression tests This fix is an improvement over the earlier one. With it cases similar to the one in comment# 2 also work fine. The enclosing node is built in such a way that it consists of all the if statements that are actually enclosing the completion node. This helps us to find out the one with the instanceof expression which is relevant to the completion node in CompletionEngine#findFieldsAndMethodsFromCastedReceiver(). The handling for binary expressions is the same as in earlier patch. Added regression tests - testBug304006a to j.
Adding these cases as well: 1) void m(CharSequence cs) { String lower= null; if (cs instanceof String) { cs.to // good } if (cs instanceof String) { return cs.to // good } if (cs instanceof String) { lower= cs.to // bad } } Should propose methods from String in the case marked 'bad' 2) CharSequence cs; if(cs instance of String && cs.to|) should propose toLower, toUpper, etc from String
*** Bug 244820 has been marked as a duplicate of this bug. ***
(In reply to comment #6) > *** Bug 244820 has been marked as a duplicate of this bug. *** There is instanceof constructs in the picture in both cases, yes. But I think it is wrong to classify these as duplicates just on that count and package all fixes into one umbrella bug even though it would simply the developer's job. From the pov of code review and future lookups it is always better to keep separate issues separate. After a few years if one were to pull up the diffs that were made for a certain fix, it would be painful to "other" changes there.
Please ignore comment 5. The cases reported there have been handled in bug 244820 and bug 261534 respectively. Also, on Srikanth's advice, opened bug 311146 to handle case 2 reported in description to avoid bloating this bug fix with too many cases.
Created attachment 172709 [details] proposed fix v2.0 + regression tests The new patch is pretty much the same as the previous one except that it doesnt handle the case (2) reported in comment 0. Srikanth, can you please re-review? Thanks!
(In reply to comment #9) > Created an attachment (id=172709) [details] > proposed fix v2.0 + regression tests > > The new patch is pretty much the same as the previous one except that it doesnt > handle the case (2) reported in comment 0. > Srikanth, can you please re-review? Thanks! Patch looks good.
Released in HEAD for 3.7M1. Added tests: CompletionTests#testBug304006a(), CompletionTests#testBug304006b().... CompletionTests#testBug304006e()
Verified for 3.7 M1 using build I20100802-1800
