Community
Participate
Working Groups
BETA_JAVA7, but most probably not Java 7 related The new Quick Fix to convert a multi-catch to separate catch blocks tried to use newCatchClause.setBody( (Block) rewrite.createCopyTarget(catchClause.getBody())); in QuickAssistProcessor#getUnrollMultiCatchProposals(..), but this mangled e.g. public class E { void foo() { try { System.out.println("foo"); } catch (IllegalArgumentException | NullPointerException ex) { ex.printStackTrace(); } } } into void foo() { try { System.out.println("foo"); } catch (IllegalArgumentException ex) { ex.printStackTrace(); } catch (NullPointerException ex) { ex.printStackTrace(); } } As a workaround, we now copy statement-by-statement, but this should not be necessary, and it can lose comments if they are separated by newlines. UI tests are in AssistQuickFixTest17.
Satyam, please take a look.
I've extracted our workarounds into QuickAssistProcessor#setCatchClauseBody(CatchClause, ASTRewrite, CatchClause). This would be nice to have for BETA_JAVA7 (though we can also ship 3.7.1 with the workaround).
Bug 110059 is probably a similar problem.
(In reply to comment #3) > Bug 110059 is probably a similar problem. They are slightly different. Reason for this bug: Blocks like catch clauses needs a prefix like try to be given to the formatter to format. Formatter doesn't format the prefix. If the string that follows the prefix lies on the same line, that line is not formatted and thus the indentation will not be correct. Reason for bug 110059: In the original string, the line where the blocks starts has an indentation of 4. In the modified string, the line where the block starts has an indentation of 2. Thus, the indentation logic thinks that the code needs to be moved left by 2 and so the problem.
Created attachment 204486 [details] Proposed patch
Olivier, Can you please review this patch?
I think this goes into the right direction, but I think we should find a way to prevent the iteration over the source more than once (inside isFirstLine(..) and getCurrentLine(..)). If isFirstLine(..) returns the position inside the current line, this could prevent iterating the same string again to find the same position. So we should rename it with a name that shows it returns a position and then check that position. If position == 0, then this should be seen as the first line. Doing that, it should not do more work than it used too.
Created attachment 204560 [details] Proposed patch Here is an improved version. This can be optimized further by calling the formatter only if the line start differs from the previous start. Do you think it is worth?
(In reply to comment #8) > Created attachment 204560 [details] > Proposed patch > > Here is an improved version. This can be optimized further by calling the > formatter only if the line start differs from the previous start. Do you think > it is worth? Forgot to add - In this patch, I have modified the other place where getCurrentLine() is being called so that getCurrentLine() can be removed.
Looks good to me.
Thanks Olivier. Release the patch in HEAD. commit 6e51f9fbccbd96945f6930d913a998a8346308ac
Not yet perfect :) Things work nicely in the following case - all exceptions are on one line ------------------------------------------------------------------------------- void foo(String s) throws Exception { try { if (s == null) throw new NullPointerException(); else if (s.length() == 0) throw new IllegalArgumentException(); else throw new Exception(); } catch (NullPointerException | IllegalArgumentException e) { e.printStackTrace(); } } ------------------------------------------------------------------------------- However, in the following case, where all exceptions are not on one line to begin with, the formatting is still bad ------------------------------------------------------------------------------- void foo2(String s) throws Exception { try { if (s == null) throw new NullPointerException(); else if (s.length() == 0) throw new IllegalArgumentException(); else throw new Exception(); } catch (NullPointerException | IllegalArgumentException e) { e.printStackTrace(); } } ------------------------------------------------------------------------------- Steps to reproduce - Uncomment line 1627 in org.eclipse.jdt.internal.ui.text.correction.QuickAssistProcessor.setCatchClauseBody(CatchClause, ASTRewrite, CatchClause) - Comment lines 1630-1633 in the same method. - Try 'Use separate catch blocks' quick assist on above mentioned foo2(String)
(In reply to comment #12) This is similar to bug 110059. Hence, I will keep this resolved and fix the other part there.
(In reply to comment #12) > Steps to reproduce > - Uncomment line 1627 in > org.eclipse.jdt.internal.ui.text.correction.QuickAssistProcessor.setCatchClauseBody(CatchClause, > ASTRewrite, CatchClause) > - Comment lines 1630-1633 in the same method. > - Try 'Use separate catch blocks' quick assist on above mentioned foo2(String) Thanks for the instructions. Verified for 3.8 M3 using build id: N20111022-2000.
