Bug 569726 - [cleanup] "Use '==' or '^' on booleans" rule should improve
redundant ternary expression
Change-Id: Id8285e00d367304ec67bc8ab6ee3f24e80c97a87
Signed-off-by: Fabrice Tiercelin <fabrice.tiercelin@yahoo.fr>
diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java
index 8389584..419e2ea 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java
@@ -5811,6 +5811,26 @@
+ " boolean newBoolean1 = (staticField > 0) && (staticField < 100) || (staticField <= 0) && (staticField >= 100);\n" //
+ " boolean newBoolean2 = (staticField > 0) && (staticField < 100) || (staticField >= 100) && !(staticField > 0);\n" //
+ " }\n" //
+ + "\n" //
+ + " public void replaceTernaryWithPrimitiveTypes(boolean b1, boolean b2) {\n" //
+ + " // Keep this comment\n" //
+ + " boolean newBoolean1 = b1 ? !b2 : b2;\n" //
+ + " boolean newBoolean2 = b1 ? b2 : !b2;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void replaceTernaryWithExpressions(int i1, int i2, int i3, int i4) {\n" //
+ + " // Keep this comment\n" //
+ + " boolean newBoolean1 = (i1 == i2) ? !(i3 == i4) : (i3 == i4);\n" //
+ + " boolean newBoolean2 = (i1 == i2) ? (i3 <= i4) : !(i4 >= i3);\n" //
+ + " boolean newBoolean3 = (i1 == i2) ? (i3 != i4) : (i3 == i4);\n" //
+ + " boolean newBoolean4 = (i1 == i2) ? (i3 < i4) : (i4 <= i3);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void replaceTernaryWithFields() {\n" //
+ + " // Keep this comment\n" //
+ + " boolean newBoolean1 = (staticField > 0) ? (staticField < 100) : (staticField >= 100);\n" //
+ + " boolean newBoolean2 = (staticField > 0) ? (staticField < 100) : !(staticField < 100);\n" //
+ + " }\n" //
+ "}\n";
ICompilationUnit cu= pack.createCompilationUnit("E.java", given, false, null);
@@ -5859,6 +5879,26 @@
+ " boolean newBoolean1 = (staticField > 0) == (staticField < 100);\n" //
+ " boolean newBoolean2 = (staticField > 0) == (staticField < 100);\n" //
+ " }\n" //
+ + "\n" //
+ + " public void replaceTernaryWithPrimitiveTypes(boolean b1, boolean b2) {\n" //
+ + " // Keep this comment\n" //
+ + " boolean newBoolean1 = b1 ^ b2;\n" //
+ + " boolean newBoolean2 = b1 == b2;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void replaceTernaryWithExpressions(int i1, int i2, int i3, int i4) {\n" //
+ + " // Keep this comment\n" //
+ + " boolean newBoolean1 = (i1 == i2) ^ (i3 == i4);\n" //
+ + " boolean newBoolean2 = (i1 == i2) == (i3 <= i4);\n" //
+ + " boolean newBoolean3 = (i1 == i2) == (i3 != i4);\n" //
+ + " boolean newBoolean4 = (i1 == i2) == (i3 < i4);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void replaceTernaryWithFields() {\n" //
+ + " // Keep this comment\n" //
+ + " boolean newBoolean1 = (staticField > 0) == (staticField < 100);\n" //
+ + " boolean newBoolean2 = (staticField > 0) == (staticField < 100);\n" //
+ + " }\n" //
+ "}\n";
assertNotEquals("The class must be changed", given, expected);
@@ -5918,6 +5958,22 @@
+ " boolean newBoolean2 = b1 && new SideEffect() instanceof SideEffect\n" //
+ " || !b1 && !(new SideEffect() instanceof SideEffect);\n" //
+ " }\n" //
+ + "\n" //
+ + " public boolean doNotReplaceNullableObjects(Boolean booleanObject1, Boolean booleanObject2) {\n" //
+ + " return booleanObject1 ? booleanObject2 : !booleanObject2;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotReplaceTernaryWithIncrements(int i1, int i2, int i3, int i4) {\n" //
+ + " boolean newBoolean1 = (i1 == i2) ? !(i3 == i4++) : (i3 == i4++);\n" //
+ + " boolean newBoolean2 = (i1 == i2) ? !(i3 == ++i4) : (i3 == ++i4);\n" //
+ + " boolean newBoolean3 = (i1 == i2) ? !(i3 == i4--) : (i3 == i4--);\n" //
+ + " boolean newBoolean4 = (i1 == i2) ? !(i3 == --i4) : (i3 == --i4);\n" //
+ + "\n" //
+ + " boolean newBoolean5 = (i1 == i2) ? (i3 == i4++) : !(i3 == i4++);\n" //
+ + " boolean newBoolean6 = (i1 == i2) ? (i3 == ++i4) : !(i3 == ++i4);\n" //
+ + " boolean newBoolean7 = (i1 == i2) ? (i3 == i4--) : !(i3 == i4--);\n" //
+ + " boolean newBoolean8 = (i1 == i2) ? (i3 == --i4) : !(i3 == --i4);\n" //
+ + " }\n" //
+ "}\n";
ICompilationUnit cu= pack.createCompilationUnit("E.java", sample, false, null);
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/StrictlyEqualOrDifferentCleanUp.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/StrictlyEqualOrDifferentCleanUp.java
index eb45e1b..4ce2401 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/StrictlyEqualOrDifferentCleanUp.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/StrictlyEqualOrDifferentCleanUp.java
@@ -28,6 +28,7 @@
import org.eclipse.jdt.core.dom.AST;
import org.eclipse.jdt.core.dom.ASTVisitor;
import org.eclipse.jdt.core.dom.CompilationUnit;
+import org.eclipse.jdt.core.dom.ConditionalExpression;
import org.eclipse.jdt.core.dom.Expression;
import org.eclipse.jdt.core.dom.InfixExpression;
import org.eclipse.jdt.core.dom.PrefixExpression;
@@ -84,12 +85,16 @@
if (isEnabled(CleanUpConstants.STRICTLY_EQUAL_OR_DIFFERENT)) {
return "" //$NON-NLS-1$
+ "boolean newBoolean1 = isValid == (i > 0);\n" //$NON-NLS-1$
- + "boolean newBoolean2 = isValid ^ isEnabled;\n"; //$NON-NLS-1$
+ + "boolean newBoolean2 = isValid ^ isEnabled;\n" //$NON-NLS-1$
+ + "boolean newBoolean3 = isActive == (0 <= i);\n" //$NON-NLS-1$
+ + "boolean newBoolean4 = isActive ^ isEnabled;\n"; //$NON-NLS-1$
}
return "" //$NON-NLS-1$
+ "boolean newBoolean1 = isValid && (i > 0) || !isValid && (i <= 0);\n" //$NON-NLS-1$
- + "boolean newBoolean2 = !isValid && isEnabled || isValid && !isEnabled;\n"; //$NON-NLS-1$
+ + "boolean newBoolean2 = !isValid && isEnabled || isValid && !isEnabled;\n" //$NON-NLS-1$
+ + "boolean newBoolean3 = isActive ? (0 <= i) : (i < 0);\n" //$NON-NLS-1$
+ + "boolean newBoolean4 = !isActive ? isEnabled : !isEnabled;\n"; //$NON-NLS-1$
}
@Override
@@ -146,15 +151,35 @@
return true;
}
+ @Override
+ public boolean visit(final ConditionalExpression visited) {
+ if (ASTNodes.hasType(visited.getThenExpression(), boolean.class.getCanonicalName())
+ && ASTNodes.hasType(visited.getElseExpression(), boolean.class.getCanonicalName())
+ && ASTNodes.isPassive(visited.getThenExpression())
+ && ASTNodes.isPassive(visited.getElseExpression())
+ && ASTSemanticMatcher.INSTANCE.matchNegative(visited.getThenExpression(), visited.getElseExpression())) {
+ AtomicBoolean isFirstExpressionPositive= new AtomicBoolean();
+ AtomicBoolean isSecondExpressionPositive= new AtomicBoolean();
+
+ Expression firstBasicExpression= getBasisExpression(visited.getExpression(), isFirstExpressionPositive);
+ Expression secondBasicExpression= getBasisExpression(visited.getThenExpression(), isSecondExpressionPositive);
+
+ rewriteOperations.add(new StrictlyEqualOrDifferentOperation(visited, firstBasicExpression, secondBasicExpression, isFirstExpressionPositive.get() == isSecondExpressionPositive.get()));
+ return false;
+ }
+
+ return true;
+ }
+
private Expression getBasisExpression(final Expression originalExpression, final AtomicBoolean isExpressionPositive) {
PrefixExpression negateExpression= ASTNodes.as(originalExpression, PrefixExpression.class);
if (ASTNodes.hasOperator(negateExpression, PrefixExpression.Operator.NOT)) {
- isExpressionPositive.set(false);
+ isExpressionPositive.lazySet(false);
return negateExpression.getOperand();
}
- isExpressionPositive.set(true);
+ isExpressionPositive.lazySet(true);
return originalExpression;
}
});
@@ -183,12 +208,12 @@
}
private static class StrictlyEqualOrDifferentOperation extends CompilationUnitRewriteOperation {
- private final InfixExpression visited;
+ private final Expression visited;
private final Expression firstExpression;
private final Expression secondExpression;
private final boolean isEquality;
- public StrictlyEqualOrDifferentOperation(final InfixExpression visited, final Expression firstExpression, final Expression secondExpression, final boolean isEquality) {
+ public StrictlyEqualOrDifferentOperation(final Expression visited, final Expression firstExpression, final Expression secondExpression, final boolean isEquality) {
this.visited= visited;
this.firstExpression= firstExpression;
this.secondExpression= secondExpression;
