Bugzilla – Full Text Bug Listing
| Summary: | [1.7][assist] No proposal inside a multi catch statement after '|' | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Ayushman Jain <amj87.iitr> | ||||||||||
| Component: | Core | Assignee: | Ayushman Jain <amj87.iitr> | ||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||
| Severity: | normal | ||||||||||||
| Priority: | P3 | CC: | markus.kell.r, satyam.kandula, srikanth_sankaran | ||||||||||
| Version: | 3.7 | Flags: | satyam.kandula:
review+
|
||||||||||
| Target Milestone: | 3.7.1 | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
Currently the above reported case works fine in BETA_JAVA7, but following does not:
public class Test {
void foo() {
try {
boo(1);
boo(2);
} catch (IndexOutOfBoundsException | I[CTRL-SPACE]
}
int boo(int i) throws IndexOutOfBoundsException, IllegalArgumentException{
if (i ==1 )
throw new IndexOutOfBoundsException();
if (i == 2)
throw new IllegalArgumentException();
return 1;
}
}
This is because content assist looks for K_BETWEEN_CATCH_AND_RIGHT_PARENTHESIS to be able to deal with the completion node (which is correctly calculated here), but instead finds K_BINARY_OPERATOR.
Created attachment 192011 [details]
patch under test
Created attachment 192077 [details]
proposed fix + tests
This patch introduces a new constant to be pushed on the stack when an '|' is encountered inside the catch clause. This will help the CompletionParser to figure out that the current assistNode occured inside a catch clause and corresponds to a completion of an exception type.
The patch checks for both single type ref and qualified type ref.
Satyam, please review. Thanks! Created attachment 192156 [details]
Another patch
Ayush, I don't think a new constant is required to be pushed onto the stack when we see the OR. Your patch does work and I don't see any side impacts because of it, but I want you to have a look at my proposal here and choose which ever you think is good.
I have couple of comments with your patch though:
1. Is it necessary to push K_OR_OPERATOR_FOR_DISJUNCTIVE_TYPE even though the stack in the top is K_OR_OPERATOR_FOR_DISJUNCTIVE_TYPE? Just an FYI, if you didn't want to have many if's there, you could just use a switch.
2. With both the patches, I don't see any proposal for the variable names. That probably can be addressed in a separate bug.
(In reply to comment #5) > Created attachment 192156 [details] [diff] > Another patch This patch is better. Let me see what it'll take to fix the missing argument name proposal as well. (In reply to comment #6) > Let me see what it'll take to fix the missing argument > name proposal as well. This happens because the resolvedType of a DisjunctiveTypeReference is null. (In reply to comment #7) > (In reply to comment #6) > > Let me see what it'll take to fix the missing argument > > name proposal as well. > > This happens because the resolvedType of a DisjunctiveTypeReference is null. This is a bug in DisjunctiveTypeRef#resolveType(..). Raised bug 341333. With the patch attached there, we now get proposals for variable names as well. Created attachment 192174 [details] proposed fix v2.0 + tests Above patch along with the tests and also the new test to verify that we get the variable name suggestions - CompletionTest#testBug338789e(). This test will only pass with the patch from bug 341333. Satyam, let me know if you have any concerns. I'll release patches for both bugs together. +1. Even the patch you had submitted in bug 341333 seems to be good. Released for 3.7M7 in BETA_JAVA7 branch. Verified using "Eclipse Java Development Tools Patch for Java 7 Support (BETA) 1.0.0.v20110623-0900 org.eclipse.jdt.patch.feature.group Eclipse.org" |
BETA_JAVA7 Currently, we dont get any proposals from content assist inside the catch expression of a multi catch statement. Eg: public class Test { void foo() { try { boo(1); boo(2); } catch (I[CTRL-SPACE] } int boo(int i) throws IndexOutOfBoundsException, IllegalArgumentException{ if (i ==1 ) throw new IndexOutOfBoundsException(); if (i == 2) throw new IllegalArgumentException(); return 1; } } Expected proposals: Exception types I*