Community
Participate
Working Groups
The quick fix for the dead code detection removes the code as well as the condition, which works in case of only 'if' block but if an 'if-else ' block is used it leaves behind the curly braces with the valid code. Try this simple test case: if (false){ x= 1; }else { x= 0; } then click on remove in the quick fix. It leaves this : { x= 0; }
Build: I20081209-0100.
*** Bug 302600 has been marked as a duplicate of this bug. ***
Test Case for pasting into Package Explorer: void test() { Object o = new Object(); if (o != null) { if (o == null) { System.out.println("hello"); } else { System.out.println("bye"); } } }
Created attachment 174084 [details] Patch Removes the curly braces when there are single or multiple statements in the if or else unreachable block. Ex: void test2() { Object o= new Object(); if (o != null) { if (o == null) { System.out.println("hello"); } else { System.out.println("bye"); System.out.println("bye-bye"); } } }
It's not that easy, that's why I didn't do it in the first place ;-) package snippet; public class Snippet { public static void main(String[] args) { test(true); test(false); } static void test(boolean b) { Object o = new Object(); if (o != null) { if (b) if (o == null) { System.out.println("hello"); } else { System.out.println("bye " + b); System.out.println("bye " + b); } } } } You have to think about all constructs in the AST where a block could be unused and rule out all possible side effects when removing the block. Code pointers that could help: - ControlStatementsFix#createRemoveBlockFix(CompilationUnit, ASTNode) - ASTNodes#isControlStatementBody(StructuralPropertyDescriptor) We already have a helper method ASTNodes#substituteMustBeParenthesized(..) for parentheses. Maybe we need something similar for Blocks? We should also have a tests for this in LocalCorrectionsQuickFixTest.
Created attachment 183641 [details] Patch_2 (In reply to comment #5) > It's not that easy, that's why I didn't do it in the first place ;-) > > package snippet; > public class Snippet { > public static void main(String[] args) { > test(true); > test(false); > } > > static void test(boolean b) { > Object o = new Object(); > if (o != null) { > if (b) > if (o == null) { > System.out.println("hello"); > } else { > System.out.println("bye " + b); > System.out.println("bye " + b); > } > } > } > } > > You have to think about all constructs in the AST where a block could be unused > and rule out all possible side effects when removing the block. > > Code pointers that could help: > - ControlStatementsFix#createRemoveBlockFix(CompilationUnit, ASTNode) > - ASTNodes#isControlStatementBody(StructuralPropertyDescriptor) > We already have a helper method ASTNodes#substituteMustBeParenthesized(..) for > parentheses. Maybe we need something similar for Blocks? > > We should also have a tests for this in LocalCorrectionsQuickFixTest. I looked at all the code mentioned but I think we could solve this by checking if the parent node is a Block or not and only if it is then copy the statements in the block and replace the removed node without adding the extra parenthesis. The existing behavior is to add the parenthesis around the node that would replace the removed node by default, so we dont need to add extra code like ASTNodes#substituteMustBeParenthesized(..), ASTNodes#isControlStatementBody(StructuralPropertyDescriptor) to check, instead we could just check first whether the node itself is a Block, if it is then if the parent node is also a Block then remove the parenthesis , else keep the parenthesis. Here's another example : static void test3() { Object o = new Object(); if (o != null) if (true) if (true) if (o != null) { System.out.println("hello"); System.out.println("hi"); } else { System.out.println("bye"); System.out.println("bye-bye"); } else System.out.println("out"); else System.out.println("outermost"); }
Created attachment 184298 [details] patch 3 (In reply to comment #6) > Created an attachment (id=183641) [details] [diff] > Patch_2 Looks good, but the parent of the removed block could also be a SwitchStatement. I've added this to the patch. I've tested a bit more and found another bug with this snippet: static void test(boolean b) { Object o = new Object(); if (o != null) if (b) if (o == null) System.out.println("x " + b); System.out.println("y " + b); } This was already wrong before, but this is the place where the tips from comment 5 apply. I've also fixed that problem in this patch. Raksha, please release if you agree with my changes and add regression tests for the problematic cases to LocalCorrectionsQuickFixTest (fix existing tests).
(In reply to comment #7) > Created an attachment (id=184298) [details] [diff] > patch 3 > > I've tested a bit more and found another bug with this snippet: > > static void test(boolean b) { > Object o = new Object(); > if (o != null) > if (b) > if (o == null) > System.out.println("x " + b); > System.out.println("y " + b); > } > > This was already wrong before, but this is the place where the tips from > comment 5 apply. I've also fixed that problem in this patch. Ah yes. Good catch! > > Raksha, please release if you agree with my changes and add regression tests > for the problematic cases to LocalCorrectionsQuickFixTest (fix existing tests). Yes. Added the tests and committed the above patch to HEAD.
Created attachment 184353 [details] Patch + tests ..and here's the patch.
Verified in I20101206-1800.
