Skip to content

Commit

Permalink
String support for if/else to switch cleanup (#1514)
Browse files Browse the repository at this point in the history 
- Fix SwitchFixCore to support "String" switches as well
- add new test to CleanUpTest12
  • Loading branch information
@carstenartur
carstenartur authored Jul 17, 2024
1 parent d23a721 commit b72ad9e
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 1 deletion.
34 changes: 33 additions & 1 deletion ...t.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SwitchFixCore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@
import org.eclipse.jdt.core.dom.Expression;
import org.eclipse.jdt.core.dom.FieldAccess;
import org.eclipse.jdt.core.dom.ForStatement;
import org.eclipse.jdt.core.dom.IMethodBinding;
import org.eclipse.jdt.core.dom.IfStatement;
import org.eclipse.jdt.core.dom.InfixExpression;
import org.eclipse.jdt.core.dom.MethodInvocation;
import org.eclipse.jdt.core.dom.Name;
import org.eclipse.jdt.core.dom.Statement;
import org.eclipse.jdt.core.dom.SuperFieldAccess;
Expand Down Expand Up @@ -263,11 +265,28 @@ private Variable extractVariableAndValues(final Expression expression) {

if (infixExpression != null) {
return extractVariableAndValuesFromInfixExpression(infixExpression);
} else if (expression instanceof MethodInvocation) {
MethodInvocation method= (MethodInvocation)expression;
if (method.resolveMethodBinding() != null) {
if (!"equals".equals(method.getName().getIdentifier())) return null; //$NON-NLS-1$
List<?> arguments= method.arguments();
if (arguments.size() != 1) return null;
IMethodBinding methodBinding= method.resolveMethodBinding();
String qualifiedName= methodBinding.getDeclaringClass().getQualifiedName();
if ("java.lang.String".equals(qualifiedName)) { //$NON-NLS-1$
return extractVariableAndValuesFromEqualsExpression(method.getExpression(),(Expression)(arguments.get(0)));
}
}
}

return null;
}

private Variable extractVariableAndValuesFromEqualsExpression(final Expression leftOperand,final Expression rightOperand) {
Variable variable= extractVariableWithConstantStringValue(leftOperand, rightOperand);
return variable != null ? variable : extractVariableWithConstantStringValue(rightOperand, leftOperand);
}

private Variable extractVariableAndValuesFromInfixExpression(final InfixExpression infixExpression) {
if (ASTNodes.hasOperator(infixExpression, InfixExpression.Operator.CONDITIONAL_OR, InfixExpression.Operator.OR, InfixExpression.Operator.XOR)) {
List<Expression> operands= ASTNodes.allOperands(infixExpression);
Expand Down Expand Up @@ -305,7 +324,9 @@ private Variable extractVariableAndValuesFromInfixExpression(final InfixExpressi

private Variable extractVariableWithConstantValue(final Expression variable, final Expression constant) {
if ((variable instanceof Name || variable instanceof FieldAccess || variable instanceof SuperFieldAccess)
&& ASTNodes.hasType(variable, char.class.getCanonicalName(), byte.class.getCanonicalName(), short.class.getCanonicalName(), int.class.getCanonicalName())
&& ASTNodes.hasType(variable, char.class.getCanonicalName(),
byte.class.getCanonicalName(), short.class.getCanonicalName(),
int.class.getCanonicalName())
&& constant.resolveTypeBinding() != null
&& constant.resolveTypeBinding().isPrimitive()
&& constant.resolveConstantExpressionValue() != null) {
Expand All @@ -314,6 +335,17 @@ private Variable extractVariableWithConstantValue(final Expression variable, fin

return null;
}

private Variable extractVariableWithConstantStringValue(final Expression variable, final Expression constant) {
if ((variable instanceof Name || variable instanceof FieldAccess || variable instanceof SuperFieldAccess)
&& ASTNodes.hasType(variable, String.class.getCanonicalName())
&& constant.resolveTypeBinding() != null
&& constant.resolveConstantExpressionValue() != null) {
return new Variable(variable, Arrays.asList(constant));
}

return null;
}
}
}

Expand Down
87 changes: 87 additions & 0 deletions org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest12.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1077,6 +1077,93 @@ public int replaceMeltCases(int i1) {
new HashSet<>(Arrays.asList(MultiFixMessages.CodeStyleCleanUp_Switch_description)));
}

@Test
public void testSwitchString() throws Exception {
IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
String given= """
package test1;
public class E {
public static final String VALUE0 = "0";
public static final String VALUE1 = "1";
public static final String VALUE2 = "2";
public static final String VALUE3 = "3";
public void bug1(String i1) {
int i = 0;
if (i1.equals(E.VALUE0)) {
int integer1 = 0;
i = integer1;
} else if (i1.equals(E.VALUE1)) {
char integer1 = 'a';
i = integer1;
} else if (i1.equals(E.VALUE2)) {
char integer1 = 'b';
i = integer1;
} else if (computeit(i1) || i1.equals(E.VALUE3)) {
//
//
}
}
private boolean computeit(String i) {
return i.equals("4") || i.equals("5");
}
}
""";
ICompilationUnit cu= pack.createCompilationUnit("E.java", given, false, null);

enable(CleanUpConstants.USE_SWITCH);

String expected= """
package test1;
public class E {
public static final String VALUE0 = "0";
public static final String VALUE1 = "1";
public static final String VALUE2 = "2";
public static final String VALUE3 = "3";
public void bug1(String i1) {
int i = 0;
switch (i1) {
case E.VALUE0 : {
int integer1 = 0;
i = integer1;
break;
}
case E.VALUE1 : {
char integer1 = 'a';
i = integer1;
break;
}
case E.VALUE2 : {
char integer1 = 'b';
i = integer1;
break;
}
default :
if (computeit(i1) || i1.equals(E.VALUE3)) {
//
//
}
break;
}
}
private boolean computeit(String i) {
return i.equals("4") || i.equals("5");
}
}
""";

assertNotEquals("The class must be changed", given, expected);
assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { expected },
new HashSet<>(Arrays.asList(MultiFixMessages.CodeStyleCleanUp_Switch_description)));
}

@Test
public void testDoNotUseSwitch() throws Exception {
IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
Expand Down

0 comments on commit b72ad9e

Please sign in to comment.