From b72ad9e266e9670604949b32aee1415448e0cd89 Mon Sep 17 00:00:00 2001 From: Carsten Hammer Date: Wed, 17 Jul 2024 22:30:54 +0200 Subject: [PATCH] String support for if/else to switch cleanup (#1514) - Fix SwitchFixCore to support "String" switches as well - add new test to CleanUpTest12 --- .../internal/corext/fix/SwitchFixCore.java | 34 +++++++- .../jdt/ui/tests/quickfix/CleanUpTest12.java | 87 +++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SwitchFixCore.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SwitchFixCore.java index 33320b402dc..d8ebb034aa7 100644 --- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SwitchFixCore.java +++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SwitchFixCore.java @@ -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; @@ -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 operands= ASTNodes.allOperands(infixExpression); @@ -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) { @@ -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; + } } } diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest12.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest12.java index 5dfd48549f1..d21a9fc852f 100644 --- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest12.java +++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest12.java @@ -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);