Bug 574824 - [cleanup] Switch expression cleanup generates invalid code
- fix SwitchExpressionsFixCore.SwitchStatementsFinder to handle
a switch case that falls off the end of the statement (i.e. no break)
- fix SwitchExpressionsFixCore.SwitchExpressionsFixOperation.rewriteAST()
method to properly copy over type and final modifier of existing
statement
- add new test to CleanUpTest14
Change-Id: I4238530e7782eb6bb3278d0cce82ecbdbcc1d9b5
Reviewed-on: https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/183069
Tested-by: JDT Bot <jdt-bot@eclipse.org>
Tested-by: Jeff Johnston <jjohnstn@redhat.com>
Reviewed-by: Jeff Johnston <jjohnstn@redhat.com>
diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SwitchExpressionsFixCore.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SwitchExpressionsFixCore.java
index b384823..b89c7c7 100644
--- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SwitchExpressionsFixCore.java
+++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SwitchExpressionsFixCore.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2020 Red Hat Inc. and others.
+ * Copyright (c) 2020, 2021 Red Hat Inc. and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -13,6 +13,7 @@
*******************************************************************************/
package org.eclipse.jdt.internal.corext.fix;
+import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.LinkedHashMap;
@@ -44,6 +45,7 @@
import org.eclipse.jdt.core.dom.ITypeBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;
import org.eclipse.jdt.core.dom.IfStatement;
+import org.eclipse.jdt.core.dom.Modifier.ModifierKeyword;
import org.eclipse.jdt.core.dom.Name;
import org.eclipse.jdt.core.dom.ReturnStatement;
import org.eclipse.jdt.core.dom.Statement;
@@ -57,6 +59,7 @@
import org.eclipse.jdt.core.dom.WhileStatement;
import org.eclipse.jdt.core.dom.YieldStatement;
import org.eclipse.jdt.core.dom.rewrite.ASTRewrite;
+import org.eclipse.jdt.core.dom.rewrite.ImportRewrite;
import org.eclipse.jdt.core.dom.rewrite.ListRewrite;
import org.eclipse.jdt.core.manipulation.ICleanUpFixCore;
@@ -157,6 +160,14 @@
}
}
+ // look for final case that has implicit break statement
+ if (currentCase != null) {
+ if (currentBlock != null && currentBlock.isEmpty()) {
+ return null;
+ }
+ caseMap.put(currentCase, currentBlock);
+ }
+
String commonAssignmentName= null;
IBinding assignmentBinding= null;
for (Map.Entry<SwitchCase, List<Statement>> entry : caseMap.entrySet()) {
@@ -371,6 +382,11 @@
newVarFragment.setName(ast.newSimpleName(varName));
newVarFragment.setInitializer(newSwitchExpression);
VariableDeclarationStatement newVar= ast.newVariableDeclarationStatement(newVarFragment);
+ ImportRewrite importRewrite= cuRewrite.getImportRewrite();
+ newVar.setType(importRewrite.addImport(((IVariableBinding) assignmentBinding).getType(), ast));
+ if (varDeclarationStatement != null && Modifier.isFinal(varDeclarationStatement.getModifiers())) {
+ newVar.modifiers().add(ast.newModifier(ModifierKeyword.FINAL_KEYWORD));
+ }
replaceWithLeadingComments(cuRewrite, listRewrite, varDeclarationStatement, group, newVar);
listRewrite.remove(switchStatement, group);
return;
diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest14.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest14.java
index 06213f7..910ade9 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest14.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest14.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2020 Red Hat Inc. and others.
+ * Copyright (c) 2020, 2021 Red Hat Inc. and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -67,7 +67,6 @@
+ " case 3: throw new RuntimeException(); // throw comment\n" //
+ " default:\n" //
+ " i = 8; // value 8\n" //
- + " break;\n" //
+ " }\n" //
+ " return i;\n" //
+ " }\n" //
@@ -260,6 +259,55 @@
assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1 }, new String[] { expected1 }, null);
}
+ @Test
+ public void testConvertToSwitchExpressionBug574824() throws Exception {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+ String sample= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "import java.io.File;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public void foo(String[] args) {\n" //
+ + " // comment 1\n" //
+ + " final File file;\n" //
+ + " switch (args[1]) {\n" //
+ + " case \"foo\":\n" //
+ + " file = new File(\"foo.txt\");\n" //
+ + " break;\n" //
+ + " case \"bar\":\n" //
+ + " file = new File(\"bar.txt\");\n" //
+ + " break;\n" //
+ + " default:\n" //
+ + " file = new File(\"foobar.txt\");\n" //
+ + " }\n" //
+ + " System.err.println(file);\n" //
+ + " }\n" //
+ + "}\n";
+ ICompilationUnit cu1= pack1.createCompilationUnit("E.java", sample, false, null);
+
+ enable(CleanUpConstants.CONTROL_STATEMENTS_CONVERT_TO_SWITCH_EXPRESSIONS);
+
+ sample= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "import java.io.File;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public void foo(String[] args) {\n" //
+ + " // comment 1\n" //
+ + " final File file = switch (args[1]) {\n" //
+ + " case \"foo\" -> new File(\"foo.txt\");\n" //
+ + " case \"bar\" -> new File(\"bar.txt\");\n" //
+ + " default -> new File(\"foobar.txt\");\n" //
+ + " };\n" //
+ + " System.err.println(file);\n" //
+ + " }\n" //
+ + "}\n";
+ String expected1= sample;
+
+ assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1 }, new String[] { expected1 }, null);
+ }
@Test
public void testDoNotConvertToSwitchExpressionNoBreak() throws Exception {
