Bug 572851 - [AutoRefactor #75/153] Redundant truth
Directly checks boolean values instead of comparing them with
true/false:
- The operator can be equals, not equals or XOR,
- The constants can be a literal or a java.lang.Boolean constant,
- One operand should be primitive so no new null pointer exceptions may
occur.
Given:
if (isValid == true) {
int i = 0;
}
When:
Applying "Boolean value rather than comparison" clean up...
Then:
if (isValid) {
int i = 0;
}
Change-Id: If40bd448d4654c66c10587a5c75d1358f385e59d
Signed-off-by: Fabrice Tiercelin <fabrice.tiercelin@yahoo.fr>
Reviewed-on: https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/179318
Reviewed-by: Holger Voormann <eclipse@voormann.de>
Tested-by: JDT Bot <jdt-bot@eclipse.org>
diff --git a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/BooleanValueRatherThanComparisonCleanUpCore.java b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/BooleanValueRatherThanComparisonCleanUpCore.java
new file mode 100644
index 0000000..1450251
--- /dev/null
+++ b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/BooleanValueRatherThanComparisonCleanUpCore.java
@@ -0,0 +1,81 @@
+/*******************************************************************************
+ * Copyright (c) 2021 Fabrice TIERCELIN and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Fabrice TIERCELIN - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.jdt.internal.ui.fix;
+
+import java.util.Map;
+
+import org.eclipse.core.runtime.CoreException;
+
+import org.eclipse.jdt.core.dom.CompilationUnit;
+import org.eclipse.jdt.core.manipulation.CleanUpContextCore;
+import org.eclipse.jdt.core.manipulation.CleanUpRequirementsCore;
+import org.eclipse.jdt.core.manipulation.ICleanUpFixCore;
+
+import org.eclipse.jdt.internal.corext.fix.BooleanValueRatherThanComparisonFixCore;
+import org.eclipse.jdt.internal.corext.fix.CleanUpConstants;
+
+public class BooleanValueRatherThanComparisonCleanUpCore extends AbstractCleanUpCore {
+ public BooleanValueRatherThanComparisonCleanUpCore(final Map<String, String> options) {
+ super(options);
+ }
+
+ public BooleanValueRatherThanComparisonCleanUpCore() {
+ }
+
+ @Override
+ public CleanUpRequirementsCore getRequirementsCore() {
+ return new CleanUpRequirementsCore(requireAST(), false, false, null);
+ }
+
+ public boolean requireAST() {
+ return isEnabled(CleanUpConstants.BOOLEAN_VALUE_RATHER_THAN_COMPARISON);
+ }
+
+ @Override
+ public ICleanUpFixCore createFixCore(final CleanUpContextCore context) throws CoreException {
+ CompilationUnit compilationUnit= context.getAST();
+
+ if (compilationUnit == null || !isEnabled(CleanUpConstants.BOOLEAN_VALUE_RATHER_THAN_COMPARISON)) {
+ return null;
+ }
+
+ return BooleanValueRatherThanComparisonFixCore.createCleanUp(compilationUnit);
+ }
+
+ @Override
+ public String[] getStepDescriptions() {
+ if (isEnabled(CleanUpConstants.BOOLEAN_VALUE_RATHER_THAN_COMPARISON)) {
+ return new String[] {MultiFixMessages.BooleanValueRatherThanComparisonCleanUp_description};
+ }
+
+ return new String[0];
+ }
+
+ @Override
+ public String getPreview() {
+ if (isEnabled(CleanUpConstants.BOOLEAN_VALUE_RATHER_THAN_COMPARISON)) {
+ return "" //$NON-NLS-1$
+ + "boolean booleanValue = isValid;\n" //$NON-NLS-1$
+ + "boolean booleanValue2 = !isValid;\n" //$NON-NLS-1$
+ + "boolean booleanValue3 = i <= 0;\n" //$NON-NLS-1$
+ + "boolean booleanValue4 = !booleanObject;\n"; //$NON-NLS-1$
+ }
+
+ return "" //$NON-NLS-1$
+ + "boolean booleanValue = isValid == true;\n" //$NON-NLS-1$
+ + "boolean booleanValue2 = isValid == false;\n" //$NON-NLS-1$
+ + "boolean booleanValue3 = Boolean.FALSE.equals(i > 0);\n" //$NON-NLS-1$
+ + "boolean booleanValue4 = booleanObject.equals(Boolean.FALSE);\n"; //$NON-NLS-1$
+ }
+}
diff --git a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.java b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.java
index 03c17c4..97bd75e 100644
--- a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.java
+++ b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.java
@@ -159,6 +159,7 @@
public static String AutoboxingCleanup_description;
public static String UnboxingCleanup_description;
public static String PushDownNegationCleanup_description;
+ public static String BooleanValueRatherThanComparisonCleanUp_description;
public static String DoubleNegationCleanUp_description;
public static String RedundantComparisonStatementCleanup_description;
public static String RedundantSuperCallCleanup_description;
diff --git a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.properties b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.properties
index 02f1f39..5912204 100644
--- a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.properties
+++ b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.properties
@@ -141,6 +141,7 @@
AutoboxingCleanup_description=Use Autoboxing
UnboxingCleanup_description=Use Unboxing
PushDownNegationCleanup_description=Push down negation
+BooleanValueRatherThanComparisonCleanUp_description=Boolean value rather than comparison
DoubleNegationCleanUp_description=Avoid double negation
RedundantComparisonStatementCleanup_description=Remove redundant comparison statement
RedundantSuperCallCleanup_description=Remove redundant super() call in constructor
diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/BooleanValueRatherThanComparisonFixCore.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/BooleanValueRatherThanComparisonFixCore.java
new file mode 100644
index 0000000..21a3280
--- /dev/null
+++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/BooleanValueRatherThanComparisonFixCore.java
@@ -0,0 +1,186 @@
+/*******************************************************************************
+ * Copyright (c) 2021 Fabrice TIERCELIN and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Fabrice TIERCELIN - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.jdt.internal.corext.fix;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.eclipse.core.runtime.CoreException;
+
+import org.eclipse.text.edits.TextEditGroup;
+
+import org.eclipse.jdt.core.dom.AST;
+import org.eclipse.jdt.core.dom.ASTNode;
+import org.eclipse.jdt.core.dom.ASTVisitor;
+import org.eclipse.jdt.core.dom.CompilationUnit;
+import org.eclipse.jdt.core.dom.Expression;
+import org.eclipse.jdt.core.dom.InfixExpression;
+import org.eclipse.jdt.core.dom.MethodInvocation;
+import org.eclipse.jdt.core.dom.ParenthesizedExpression;
+import org.eclipse.jdt.core.dom.rewrite.ASTRewrite;
+import org.eclipse.jdt.core.dom.rewrite.TargetSourceRangeComputer;
+import org.eclipse.jdt.core.manipulation.ICleanUpFixCore;
+
+import org.eclipse.jdt.internal.corext.dom.ASTNodeFactory;
+import org.eclipse.jdt.internal.corext.dom.ASTNodes;
+import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite;
+
+import org.eclipse.jdt.internal.ui.fix.MultiFixMessages;
+
+public class BooleanValueRatherThanComparisonFixCore extends CompilationUnitRewriteOperationsFixCore {
+ public static final class BooleanValueRatherThanComparisonFinder extends ASTVisitor {
+ private List<CompilationUnitRewriteOperation> fResult;
+
+ public BooleanValueRatherThanComparisonFinder(List<CompilationUnitRewriteOperation> ops) {
+ fResult= ops;
+ }
+
+ @Override
+ public boolean visit(final MethodInvocation visited) {
+ if (ASTNodes.usesGivenSignature(visited, Boolean.class.getCanonicalName(), "equals", Object.class.getCanonicalName())) { //$NON-NLS-1$
+ Boolean isExpressionTrue= ASTNodes.getBooleanLiteral(visited.getExpression());
+ Expression argument= (Expression) visited.arguments().get(0);
+
+ if (isExpressionTrue != null
+ // A primitive may not create a NPE
+ && ASTNodes.isPrimitive(argument, boolean.class.getSimpleName())) {
+ fResult.add(new BooleanValueRatherThanComparisonOperation(visited, argument, isExpressionTrue));
+ return false;
+ }
+
+ Boolean isArgumentTrue= ASTNodes.getBooleanLiteral(argument);
+
+ // The result has as many NPE threads as the original code so it's OK
+ if (visited.getExpression() != null
+ && (Boolean.FALSE.equals(isArgumentTrue)
+ || Boolean.TRUE.equals(isArgumentTrue) && visited.getLocationInParent() != MethodInvocation.ARGUMENTS_PROPERTY && ASTNodes.hasType(ASTNodes.getTargetType(visited), boolean.class.getSimpleName()))) {
+ fResult.add(new BooleanValueRatherThanComparisonOperation(visited, visited.getExpression(), isArgumentTrue));
+ return false;
+ }
+ }
+
+ return true;
+ }
+
+ @Override
+ public boolean visit(final ParenthesizedExpression visited) {
+ InfixExpression originalCondition= ASTNodes.as(visited, InfixExpression.class);
+
+ if (originalCondition != null) {
+ return maybeRefactorInfixExpression(visited, originalCondition);
+ }
+
+ return true;
+ }
+
+ @Override
+ public boolean visit(final InfixExpression visited) {
+ return maybeRefactorInfixExpression(visited, visited);
+ }
+
+ private boolean maybeRefactorInfixExpression(final Expression visited, final InfixExpression originalCondition) {
+ if (!originalCondition.hasExtendedOperands()
+ && ASTNodes.hasOperator(originalCondition, InfixExpression.Operator.EQUALS, InfixExpression.Operator.NOT_EQUALS, InfixExpression.Operator.XOR)
+ // Either:
+ // - Two boolean primitives: no possible NPE
+ // - One boolean primitive and one Boolean object, this code already run
+ // the risk of an NPE, so we can replace the infix expression without
+ // fearing we would introduce a previously non existing NPE.
+ && (ASTNodes.isPrimitive(originalCondition.getLeftOperand(), boolean.class.getSimpleName()) || ASTNodes.isPrimitive(originalCondition.getRightOperand(), boolean.class.getSimpleName()))) {
+ Expression leftExpression= originalCondition.getLeftOperand();
+ Expression rightExpression= originalCondition.getRightOperand();
+ boolean isEquals= ASTNodes.hasOperator(originalCondition, InfixExpression.Operator.EQUALS);
+
+ return maybeRemoveConstantOperand(visited, leftExpression, rightExpression, isEquals)
+ && maybeRemoveConstantOperand(visited, rightExpression, leftExpression, isEquals);
+ }
+
+ return true;
+ }
+
+ private boolean maybeRemoveConstantOperand(final Expression visited, final Expression dynamicOperand,
+ final Expression hardCodedOperand, final boolean isEquals) {
+ Boolean booleanLiteral= ASTNodes.getBooleanLiteral(hardCodedOperand);
+
+ if (booleanLiteral != null) {
+ boolean isTrue= booleanLiteral == isEquals;
+
+ if (!isTrue
+ || ASTNodes.isPrimitive(dynamicOperand, boolean.class.getSimpleName())
+ || (visited.getLocationInParent() != MethodInvocation.ARGUMENTS_PROPERTY && ASTNodes.hasType(ASTNodes.getTargetType(visited), boolean.class.getSimpleName()))) {
+ fResult.add(new BooleanValueRatherThanComparisonOperation(visited, dynamicOperand, isTrue));
+ return false;
+ }
+ }
+
+ return true;
+ }
+ }
+
+ private static class BooleanValueRatherThanComparisonOperation extends CompilationUnitRewriteOperation {
+ private final ASTNode visited;
+ private final Expression expressionToCopy;
+ private final boolean isTrue;
+
+ public BooleanValueRatherThanComparisonOperation(final ASTNode visited, final Expression expressionToCopy,
+ final boolean isTrue) {
+ this.visited= visited;
+ this.expressionToCopy= expressionToCopy;
+ this.isTrue= isTrue;
+ }
+
+ @Override
+ public void rewriteAST(final CompilationUnitRewrite cuRewrite, final LinkedProposalModelCore linkedModel) throws CoreException {
+ ASTRewrite rewrite= cuRewrite.getASTRewrite();
+ AST ast= cuRewrite.getRoot().getAST();
+ TextEditGroup group= createTextEditGroup(MultiFixMessages.BooleanValueRatherThanComparisonCleanUp_description, cuRewrite);
+ rewrite.setTargetSourceRangeComputer(new TargetSourceRangeComputer() {
+ @Override
+ public SourceRange computeSourceRange(final ASTNode nodeWithComment) {
+ if (Boolean.TRUE.equals(nodeWithComment.getProperty(ASTNodes.UNTOUCH_COMMENT))) {
+ return new SourceRange(nodeWithComment.getStartPosition(), nodeWithComment.getLength());
+ }
+
+ return super.computeSourceRange(nodeWithComment);
+ }
+ });
+
+ Expression operand;
+ if (isTrue) {
+ operand= ASTNodes.createMoveTarget(rewrite, expressionToCopy);
+ } else {
+ operand= ASTNodeFactory.negate(ast, rewrite, expressionToCopy, true);
+ }
+
+ rewrite.replace(visited, ASTNodeFactory.parenthesizeIfNeeded(ast, operand), group);
+ }
+ }
+
+ public static ICleanUpFixCore createCleanUp(final CompilationUnit compilationUnit) {
+ List<CompilationUnitRewriteOperation> operations= new ArrayList<>();
+ BooleanValueRatherThanComparisonFinder finder= new BooleanValueRatherThanComparisonFinder(operations);
+ compilationUnit.accept(finder);
+
+ if (operations.isEmpty()) {
+ return null;
+ }
+
+ CompilationUnitRewriteOperationsFixCore.CompilationUnitRewriteOperation[] ops= operations.toArray(new CompilationUnitRewriteOperationsFixCore.CompilationUnitRewriteOperation[0]);
+ return new BooleanValueRatherThanComparisonFixCore(FixMessages.BooleanValueRatherThanComparisonFix_description, compilationUnit, ops);
+ }
+
+ protected BooleanValueRatherThanComparisonFixCore(final String name, final CompilationUnit compilationUnit, final CompilationUnitRewriteOperationsFixCore.CompilationUnitRewriteOperation[] fixRewriteOperations) {
+ super(name, compilationUnit, fixRewriteOperations);
+ }
+}
diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstants.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstants.java
index 652118c..23b05ba 100644
--- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstants.java
+++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstants.java
@@ -1486,6 +1486,18 @@
public static final String PUSH_DOWN_NEGATION= "cleanup.push_down_negation"; //$NON-NLS-1$
/**
+ * Directly checks boolean values instead of comparing them with <code>true</code>/<code>false</code>.
+ * <p>
+ * Possible values: {TRUE, FALSE}
+ * <p>
+ *
+ * @see CleanUpOptionsCore#TRUE
+ * @see CleanUpOptionsCore#FALSE
+ * @since 4.20
+ */
+ public static final String BOOLEAN_VALUE_RATHER_THAN_COMPARISON= "cleanup.boolean_value_rather_than_comparison"; //$NON-NLS-1$
+
+ /**
* Reduces double negation in boolean expression.
* <p>
* Possible values: {TRUE, FALSE}
diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.java
index 8458b4b..b350c89 100644
--- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.java
+++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.java
@@ -167,6 +167,7 @@
public static String TypeParametersFix_insert_inferred_type_arguments_name;
public static String TypeParametersFix_remove_redundant_type_arguments_description;
public static String TypeParametersFix_remove_redundant_type_arguments_name;
+ public static String BooleanValueRatherThanComparisonFix_description;
public static String RedundantComparatorFix_remove_comparator;
public static String ArrayWithCurlyFix_description;
public static String ReturnExpressionFix_description;
diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.properties b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.properties
index 43a5fb2..bd6a905 100644
--- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.properties
+++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.properties
@@ -154,6 +154,7 @@
TypeParametersFix_insert_inferred_type_arguments_name=Insert inferred type arguments
TypeParametersFix_remove_redundant_type_arguments_description=Remove redundant type arguments
TypeParametersFix_remove_redundant_type_arguments_name=Remove redundant type arguments
+BooleanValueRatherThanComparisonFix_description=Boolean value rather than comparison
RedundantComparatorFix_remove_comparator=Implicit comparator
ArrayWithCurlyFix_description=Create array with curly
ReturnExpressionFix_description=Remove variable assignment before return
diff --git a/org.eclipse.jdt.ui.tests/performance/org/eclipse/jdt/ui/tests/performance/views/CleanUpPerfTest.java b/org.eclipse.jdt.ui.tests/performance/org/eclipse/jdt/ui/tests/performance/views/CleanUpPerfTest.java
index ec3d9fd..02e592a 100644
--- a/org.eclipse.jdt.ui.tests/performance/org/eclipse/jdt/ui/tests/performance/views/CleanUpPerfTest.java
+++ b/org.eclipse.jdt.ui.tests/performance/org/eclipse/jdt/ui/tests/performance/views/CleanUpPerfTest.java
@@ -64,6 +64,7 @@
import org.eclipse.jdt.internal.ui.fix.AutoboxingCleanUp;
import org.eclipse.jdt.internal.ui.fix.BitwiseConditionalExpressionCleanup;
import org.eclipse.jdt.internal.ui.fix.BooleanLiteralCleanUp;
+import org.eclipse.jdt.internal.ui.fix.BooleanValueRatherThanComparisonCleanUp;
import org.eclipse.jdt.internal.ui.fix.BreakLoopCleanUp;
import org.eclipse.jdt.internal.ui.fix.CodeFormatCleanUp;
import org.eclipse.jdt.internal.ui.fix.CodeStyleCleanUp;
@@ -676,6 +677,22 @@
}
@Test
+ public void testBooleanValueRatherThanComparisonCleanUp() throws Exception {
+ CleanUpRefactoring cleanUpRefactoring= new CleanUpRefactoring();
+ addAllCUs(cleanUpRefactoring, MyTestSetup.fJProject1.getChildren());
+
+ Map<String, String> node= getNullSettings();
+
+ node.put(CleanUpConstants.BOOLEAN_VALUE_RATHER_THAN_COMPARISON, CleanUpOptions.TRUE);
+
+ storeSettings(node);
+
+ cleanUpRefactoring.addCleanUp(new BooleanValueRatherThanComparisonCleanUp());
+
+ doCleanUp(cleanUpRefactoring);
+ }
+
+ @Test
public void testDoubleNegationCleanUp() throws Exception {
CleanUpRefactoring cleanUpRefactoring= new CleanUpRefactoring();
addAllCUs(cleanUpRefactoring, MyTestSetup.fJProject1.getChildren());
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 45150ce..be31434 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
@@ -7924,6 +7924,311 @@
}
@Test
+ public void testBooleanValueRatherThanComparison() throws Exception {
+ // Given
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String given= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public void removeMethodCall(boolean isValid, int number) {\n" //
+ + " if (Boolean.TRUE.equals(isValid)) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (Boolean.FALSE.equals(isValid)) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (Boolean.TRUE.equals(number > 0) && isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (Boolean.FALSE.equals(number > 0) && isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public boolean removeMethodCallOnObject(Boolean isValid) {\n" //
+ + " return isValid.equals(Boolean.TRUE);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public Boolean removeMethodCallOnNegativeExpression(Boolean isValid) {\n" //
+ + " return isValid.equals(Boolean.FALSE);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void simplifyPrimitiveBooleanExpression(boolean isValid) {\n" //
+ + " if (isValid == true) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid != false) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid == false) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid != true) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid == Boolean.TRUE) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid != Boolean.FALSE) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid == Boolean.FALSE) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid != Boolean.TRUE) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void removeParenthesis(boolean isValid, boolean isActive) {\n" //
+ + " if ((isValid == true) == isActive) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isActive == (isValid == true)) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if ((isValid == true) != isActive) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isActive != (isValid == true)) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if ((isValid == false) == isActive) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isActive == (isValid == false)) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if ((isValid == false) != isActive) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isActive != (isValid == false)) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void simplifyBooleanWrapperExpression(Boolean isValid) {\n" //
+ + " if (isValid == true) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid != false) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid == false) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid != true) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "}\n";
+
+ String expected= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public void removeMethodCall(boolean isValid, int number) {\n" //
+ + " if (isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (!isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if ((number > 0) && isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if ((number <= 0) && isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public boolean removeMethodCallOnObject(Boolean isValid) {\n" //
+ + " return isValid;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public Boolean removeMethodCallOnNegativeExpression(Boolean isValid) {\n" //
+ + " return !isValid;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void simplifyPrimitiveBooleanExpression(boolean isValid) {\n" //
+ + " if (isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (!isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (!isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (!isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (!isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void removeParenthesis(boolean isValid, boolean isActive) {\n" //
+ + " if (isValid == isActive) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isActive == isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid != isActive) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isActive != isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (!isValid == isActive) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isActive == !isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (!isValid != isActive) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isActive != !isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void simplifyBooleanWrapperExpression(Boolean isValid) {\n" //
+ + " if (isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (!isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (!isValid) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "}\n";
+
+ // When
+ ICompilationUnit cu= pack.createCompilationUnit("E.java", given, false, null);
+ enable(CleanUpConstants.BOOLEAN_VALUE_RATHER_THAN_COMPARISON);
+
+ // Then
+ assertNotEquals("The class must be changed", given, expected);
+ assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { expected }, new HashSet<>(Arrays.asList(MultiFixMessages.BooleanValueRatherThanComparisonCleanUp_description)));
+ }
+
+ @Test
+ public void testDoNotUseBooleanValueRatherThanComparison() throws Exception {
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String sample= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public void doNotRemoveMethodCallOnObjectParameter(Boolean isValid) {\n" //
+ + " if (Boolean.TRUE.equals(isValid)) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (Boolean.FALSE.equals(isValid)) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotRemoveMethodCallOnObjectParameter(boolean isValid, Boolean isActive) {\n" //
+ + " if (isActive.equals(isValid)) {\n" //
+ + " int i = 0;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public Boolean doNotAvoidNPESkippingExpression(Boolean isValid) {\n" //
+ + " return isValid == true;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public Boolean doNotAvoidNPESkippingMethod(Boolean isValid) {\n" //
+ + " return isValid.equals(Boolean.TRUE);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public int doNotSimplifyBooleanWrapperExpression(Boolean isValid) {\n" //
+ + " if (isValid == Boolean.TRUE) {\n" //
+ + " return 1;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid != Boolean.FALSE) {\n" //
+ + " return 2;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid == Boolean.FALSE) {\n" //
+ + " return 3;\n" //
+ + " }\n" //
+ + "\n" //
+ + " if (isValid != Boolean.TRUE) {\n" //
+ + " return 4;\n" //
+ + " }\n" //
+ + "\n" //
+ + " return 0;\n" //
+ + " }\n" //
+ + "}\n";
+ ICompilationUnit cu= pack.createCompilationUnit("E.java", sample, false, null);
+
+ enable(CleanUpConstants.BOOLEAN_VALUE_RATHER_THAN_COMPARISON);
+
+ assertRefactoringHasNoChange(new ICompilationUnit[] { cu });
+ }
+
+ @Test
public void testDoubleNegation() throws Exception {
IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
String input= "" //
diff --git a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstantsOptions.java b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstantsOptions.java
index aa59a3d..6e9a86d 100644
--- a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstantsOptions.java
+++ b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstantsOptions.java
@@ -104,6 +104,7 @@
options.setOption(ARRAYS_FILL, CleanUpOptions.FALSE);
options.setOption(EVALUATE_NULLABLE, CleanUpOptions.FALSE);
options.setOption(PUSH_DOWN_NEGATION, CleanUpOptions.FALSE);
+ options.setOption(BOOLEAN_VALUE_RATHER_THAN_COMPARISON, CleanUpOptions.TRUE);
options.setOption(DOUBLE_NEGATION, CleanUpOptions.FALSE);
options.setOption(REMOVE_REDUNDANT_COMPARISON_STATEMENT, CleanUpOptions.FALSE);
options.setOption(REDUNDANT_SUPER_CALL, CleanUpOptions.FALSE);
@@ -275,6 +276,7 @@
options.setOption(ARRAYS_FILL, CleanUpOptions.FALSE);
options.setOption(EVALUATE_NULLABLE, CleanUpOptions.FALSE);
options.setOption(PUSH_DOWN_NEGATION, CleanUpOptions.FALSE);
+ options.setOption(BOOLEAN_VALUE_RATHER_THAN_COMPARISON, CleanUpOptions.FALSE);
options.setOption(DOUBLE_NEGATION, CleanUpOptions.FALSE);
options.setOption(REMOVE_REDUNDANT_COMPARISON_STATEMENT, CleanUpOptions.FALSE);
options.setOption(REDUNDANT_SUPER_CALL, CleanUpOptions.FALSE);
diff --git a/org.eclipse.jdt.ui/plugin.xml b/org.eclipse.jdt.ui/plugin.xml
index 97211e2..4e080d8 100644
--- a/org.eclipse.jdt.ui/plugin.xml
+++ b/org.eclipse.jdt.ui/plugin.xml
@@ -7328,9 +7328,14 @@
runAfter="org.eclipse.jdt.ui.cleanup.evaluate_nullable">
</cleanUp>
<cleanUp
+ class="org.eclipse.jdt.internal.ui.fix.BooleanValueRatherThanComparisonCleanUp"
+ id="org.eclipse.jdt.ui.boolean_value_rather_than_comparison"
+ runAfter="org.eclipse.jdt.ui.cleanup.push_down_negation">
+ </cleanUp>
+ <cleanUp
class="org.eclipse.jdt.internal.ui.fix.DoubleNegationCleanUp"
id="org.eclipse.jdt.ui.cleanup.double_negation"
- runAfter="org.eclipse.jdt.ui.cleanup.push_down_negation">
+ runAfter="org.eclipse.jdt.ui.boolean_value_rather_than_comparison">
</cleanUp>
<cleanUp
class="org.eclipse.jdt.internal.ui.fix.OverriddenAssignmentCleanUp"
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/BooleanValueRatherThanComparisonCleanUp.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/BooleanValueRatherThanComparisonCleanUp.java
new file mode 100644
index 0000000..d0e1059
--- /dev/null
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/BooleanValueRatherThanComparisonCleanUp.java
@@ -0,0 +1,73 @@
+/*******************************************************************************
+ * Copyright (c) 2021 Fabrice TIERCELIN and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Fabrice TIERCELIN - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.jdt.internal.ui.fix;
+
+import java.util.Map;
+
+import org.eclipse.core.runtime.CoreException;
+
+import org.eclipse.jdt.core.manipulation.ICleanUpFixCore;
+
+import org.eclipse.jdt.ui.cleanup.CleanUpContext;
+import org.eclipse.jdt.ui.cleanup.CleanUpOptions;
+import org.eclipse.jdt.ui.cleanup.CleanUpRequirements;
+import org.eclipse.jdt.ui.cleanup.ICleanUpFix;
+
+/**
+ * A fix that directly checks boolean values instead of comparing them with <code>true</code>/<code>false</code>:
+ * <ul>
+ * <li>The operator can be equals, not equals or XOR,</li>
+ * <li>The constants can be a literal or a <code>java.lang.Boolean</code> constant,</li>
+ * <li>One operand should be primitive so no new null pointer exceptions may occur,</li>
+ * <li>We should never fix NPE as it may trigger zombie code.
+ * A zombie code is a dead code that is dead because an error occurs before.
+ * The day someone fixes the error, the zombie code comes back to life and alters the behavior.</li>
+ * </ul>
+ */
+public class BooleanValueRatherThanComparisonCleanUp extends AbstractCleanUp {
+ private BooleanValueRatherThanComparisonCleanUpCore coreCleanUp= new BooleanValueRatherThanComparisonCleanUpCore();
+
+ public BooleanValueRatherThanComparisonCleanUp(final Map<String, String> options) {
+ setOptions(options);
+ }
+
+ public BooleanValueRatherThanComparisonCleanUp() {
+ }
+
+ @Override
+ public void setOptions(final CleanUpOptions options) {
+ coreCleanUp.setOptions(options);
+ }
+
+ @Override
+ public CleanUpRequirements getRequirements() {
+ return new CleanUpRequirements(coreCleanUp.getRequirementsCore());
+ }
+
+ @Override
+ public ICleanUpFix createFix(final CleanUpContext context) throws CoreException {
+ ICleanUpFixCore fixCore= coreCleanUp.createFixCore(context);
+ return fixCore != null ? new CleanUpFixWrapper(fixCore) : null;
+ }
+
+ @Override
+ public String[] getStepDescriptions() {
+ return coreCleanUp.getStepDescriptions();
+ }
+
+ @Override
+ public String getPreview() {
+ return coreCleanUp.getPreview();
+ }
+}
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.java
index d473f7e..ec5d591 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.java
@@ -134,6 +134,7 @@
public static String UnnecessaryCodeTabPage_CheckboxName_ArraysFill;
public static String UnnecessaryCodeTabPage_CheckboxName_EvaluateNullable;
public static String UnnecessaryCodeTabPage_CheckboxName_PushDownNegation;
+ public static String UnnecessaryCodeTabPage_CheckboxName_BooleanValueRatherThanComparison;
public static String UnnecessaryCodeTabPage_CheckboxName_DoubleNegation;
public static String UnnecessaryCodeTabPage_CheckboxName_ComparisonStatement;
public static String UnnecessaryCodeTabPage_CheckboxName_RedundantSuperCall;
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.properties b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.properties
index 107645f..8b4f1a9 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.properties
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.properties
@@ -120,6 +120,7 @@
UnnecessaryCodeTabPage_CheckboxName_ArraysFill=Use Arrays.&fill() when possible
UnnecessaryCodeTabPage_CheckboxName_EvaluateNullable=Evaluate without null check
UnnecessaryCodeTabPage_CheckboxName_PushDownNegation=Push d&own negation
+UnnecessaryCodeTabPage_CheckboxName_BooleanValueRatherThanComparison=Boolean value rather than comparison
UnnecessaryCodeTabPage_CheckboxName_DoubleNegation=Avoid double negation
UnnecessaryCodeTabPage_CheckboxName_ComparisonStatement=Remove redundant comparison statement
UnnecessaryCodeTabPage_CheckboxName_RedundantSuperCall=Remove redundant s&uper() call in constructor
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/UnnecessaryCodeTabPage.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/UnnecessaryCodeTabPage.java
index c5c1d90..0b04d2a 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/UnnecessaryCodeTabPage.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/UnnecessaryCodeTabPage.java
@@ -24,6 +24,7 @@
import org.eclipse.jdt.internal.ui.fix.AbstractCleanUp;
import org.eclipse.jdt.internal.ui.fix.ArrayWithCurlyCleanUp;
import org.eclipse.jdt.internal.ui.fix.ArraysFillCleanUp;
+import org.eclipse.jdt.internal.ui.fix.BooleanValueRatherThanComparisonCleanUp;
import org.eclipse.jdt.internal.ui.fix.CollectionCloningCleanUp;
import org.eclipse.jdt.internal.ui.fix.DoubleNegationCleanUp;
import org.eclipse.jdt.internal.ui.fix.EmbeddedIfCleanUp;
@@ -61,6 +62,7 @@
new ArraysFillCleanUp(values),
new EvaluateNullableCleanUp(values),
new PushDownNegationCleanUp(values),
+ new BooleanValueRatherThanComparisonCleanUp(values),
new DoubleNegationCleanUp(values),
new RedundantComparisonStatementCleanUp(values),
new RedundantSuperCallCleanUp(values),
@@ -121,6 +123,9 @@
CleanUpModifyDialog.FALSE_TRUE);
registerPreference(pushDownNegation);
+ CheckboxPreference booleanValueRatherThanComparison= createCheckboxPref(unnecessaryGroup, numColumns, CleanUpMessages.UnnecessaryCodeTabPage_CheckboxName_BooleanValueRatherThanComparison, CleanUpConstants.BOOLEAN_VALUE_RATHER_THAN_COMPARISON, CleanUpModifyDialog.FALSE_TRUE);
+ registerPreference(booleanValueRatherThanComparison);
+
CheckboxPreference doubleNegation= createCheckboxPref(unnecessaryGroup, numColumns, CleanUpMessages.UnnecessaryCodeTabPage_CheckboxName_DoubleNegation, CleanUpConstants.DOUBLE_NEGATION, CleanUpModifyDialog.FALSE_TRUE);
registerPreference(doubleNegation);
