Community
Participate
Working Groups
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*
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"
