Community
Participate
Working Groups
An error is shown when there is a try block without a catch or finally block. But the error does not have a quick fix. Pressing Ctrl + 1 on 'try' gives a quickfix to remove the try block. It will be helpful if there can be one more quickfix (of higher relevance) which add a finally block. You can see the error am referring to by using this snippet import java.io.File; public class CLAZZ { public void method() { try { //press Ctrl + 1 on try to view quickfix, you can remove try block but can not add finally File f = new File("c:\testfile"); f.createNewFile(); // throws IOException, which means I want/have to add finally block } // Error is on this line } }
-------------------------------------------------------------- import java.io.File; public class CLAZZ { public void method() { try { File f = new File("c:\testfile"); f.createNewFile(); } catch (Exception e) { } } } -------------------------------------------------------------- Invoke Ctrl+1 on try => 'Add finally block' quick assist is available. But is not available in snippet from comment 0 which is missing a catch block. We can probably do 2 things - Provide 'Add finally block' quick assist even if the catch block is missing. - Provide the quick assist on the line containing the error as well.
I've started to look at this issue. Following Deepak comment, here after my analysis for the first part : - Provide 'Add finally block' quick assist even if the catch block is missing. When parsing a 'try' block, a corresponding TryStatement is created (see Parser.consumeStatementTry(boolean withFinally, boolean hasResources)). If the 'try' block is missing BOTH a 'catch' AND a 'finally' block, then the Parser.consumeStatementTry is invoked with parameter withFinally = true, which is incorrect. So I suspect the issue to be in the Parser.consumeRule(int act) method. This method is generated using the java.g grammar file (under org.eclipse.jdt.core/grammar). This java.g file needs to be fixed to handle properly a standalone 'try' block. If my analysis is correct, let me know if I should log an issue for the jdt core part.
Noopur, please have a look at the previous comment.
(In reply to sandra lions from comment #2) That's correct. We don't get a standalone 'try' in the AST node 'TryStatement'. When both 'catch' and 'resources' are missing with a 'try', a 'finally' block is inserted in the node by default (if not already available). Also, we do not have any direct way to find out whether the 'finally' in the AST exists in the code snippet or was inserted explicitly. We can create a bug report in JDT Core to fix/clarify this. However, in the case where we try to provide a quick fix at the error (not quick assist), we will know that the 'finally' block has to be inserted if we get IProblem.ParsingErrorInsertToComplete at 'TryStatement' node with some other checks. So, we can implement a quick fix at least until the JDT Core issue is resolved.
To debug AST issues, the ASTView is very helpful. This view should already be available in your runtime workbench if you checked out eclipse.jdt.ui/org.eclipse.jdt.astview. For comment 0, you can see that the TryStatement node and it's FINALLY > Block are marked as recovered (ASTNode.getFlags()). AST recovery is helpful in many cases to ensure we get as many AST constructs as possible and can still process the parts that are not affected by the problem. However, in this specific case I agree that the recovery is problematic, see bug 432539. To detect the recovery, you can just test whether the finally block's getLength() is null. You could also test if the finally block's flags contain RECOVERED, but I don't think that's even necessary. For the syntax error, we already have case IProblem.ParsingErrorInsertToComplete in QuickFixProcessor#process(..). The fix should add similar glue code that eventually calls QuickAssistProcessor.getAddFinallyProposals(..) and reuses the existing quick assist. But let's wait with that until it's clear how the fix for bug 432539 will look like. As a last resort, we could also hack something that doesn't use ASTRewrite at all, but we try to avoid that.
Created attachment 243234 [details] Unit Test + Patch IMPORTANT : The proposed patch depends on Bug 432539 - [parser][ast rewrite] Fake 'finally' block from recovery causes problems in rewrite So you have to apply the workaround on jdt.core proposed by Markus before applying this patch.
For comment 6 : This contribution complies with http://www.eclipse.org/legal/CoO.php
Here are my review comments: 1. When you add quick fixes to the process(..) method, you must also add the problem IDs to hasCorrections(..). This turns the annotation ruler icon from a red X into a lightbulb with red X. 2. The header comment is updated in all modified files, but it is missing the bug url information. Follow the below template: Your Name <email@example.com> - Bug Title - https://bugs.eclipse.org/BUG_NUMBER Otherwise the patch looks good.
Created attachment 244276 [details] Unit Test + Patch version 2 Take into account Manju Mathew feedback.
The changes looks fine. Until Bug 432539 is resolved we will keep this bug open. I have currently disabled the new test which must be re-enabled when Bug 432539 is resolved. Released to 'marsTemp' branch with: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?h=marsTemp&id=90f5f79f6dc7e42ca66eab9f42f15b9af0e5888f
(In reply to Manju Mathew from comment #10) > Released to 'marsTemp' branch with: > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/ > ?h=marsTemp&id=90f5f79f6dc7e42ca66eab9f42f15b9af0e5888f I've reverted this commit. As already found in bug 434188, the compile error IProblem.ParsingErrorInsertToComplete shows up in a lot of cases (see e.g. bug 434188 comment 7), but most of these cases are not fixed by the new quick fix. And as Sandra said, the patch doesn't resolve the compile error without a solution for bug 432539.
Target can be reassigned when bug 432539 is resolved.
