Bug 567567 - [AutoRefactor immigration #29/137] [cleanup & saveaction]
Redundant comparison statement
Change-Id: I66b97fcb5015dd162f3458e69c626d5a8840727e
Signed-off-by: Fabrice Tiercelin <fabrice.tiercelin@yahoo.fr>
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 8c891ed..792995a 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
@@ -129,6 +129,7 @@
public static String AutoboxingCleanup_description;
public static String UnboxingCleanup_description;
public static String PushDownNegationCleanup_description;
+ public static String RedundantComparisonStatementCleanup_description;
public static String RedundantSuperCallCleanup_description;
public static String MergeConditionalBlocksCleanup_description;
public static String RedundantFallingThroughBlockEndCleanup_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 e7c1fb9..7f64742 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
@@ -110,6 +110,7 @@
AutoboxingCleanup_description = Use Autoboxing
UnboxingCleanup_description = Use Unboxing
PushDownNegationCleanup_description = Push down negation
+RedundantComparisonStatementCleanup_description=Remove redundant comparison statement
RedundantSuperCallCleanup_description=Remove redundant super() call in constructor
MergeConditionalBlocksCleanup_description=Merge conditions of if/else if/else that have the same blocks
RedundantFallingThroughBlockEndCleanup_description=Remove redundant end of block with jump statement
diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/dom/ASTNodes.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/dom/ASTNodes.java
index b07da53..27f9485 100644
--- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/dom/ASTNodes.java
+++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/dom/ASTNodes.java
@@ -1065,6 +1065,18 @@
}
/**
+ * Returns whether the provided statement has the provided type.
+ *
+ * @param statement the statement to test
+ * @param stmtClass the type to test the statement against
+ * @return {@code true} if the provided statement has the provided type,
+ * {@code false} otherwise
+ */
+ public static boolean is(final Statement statement, final Class<? extends Statement> stmtClass) {
+ return as(statement, stmtClass) != null;
+ }
+
+ /**
* Returns whether the provided expression has the provided type.
*
* @param expression the expression to test
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 ba5b519..e7ee3d0 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
@@ -1176,6 +1176,18 @@
public static final String PUSH_DOWN_NEGATION= "cleanup.push_down_negation"; //$NON-NLS-1$
/**
+ * Removes useless bad value checks before assignments or return statements.
+ * <p>
+ * Possible values: {TRUE, FALSE}
+ * <p>
+ *
+ * @see CleanUpOptionsCore#TRUE
+ * @see CleanUpOptionsCore#FALSE
+ * @since 4.18
+ */
+ public static final String REMOVE_REDUNDANT_COMPARISON_STATEMENT= "cleanup.comparison_statement"; //$NON-NLS-1$
+
+ /**
* Remove super() call in constructor.
* <p>
* Possible values: {TRUE, FALSE}
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 defc01f..98a8544 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
@@ -5480,6 +5480,639 @@
}
@Test
+ public void testRedundantComparisonStatement() throws Exception {
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String input= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "import java.util.Date;\n" //
+ + "import java.util.List;\n" //
+ + "import java.util.Map;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " private static final String DEFAULT = \"\";\n" //
+ + " private String input;\n" //
+ + "\n" //
+ + " public String refactorLocalVariable1(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " if (input == null) {\n" //
+ + " output = null;\n" //
+ + " } else {\n" //
+ + " output = /* Keep this comment too */ input;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorLocalVariable2(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " if (null == input) {\n" //
+ + " output = null;\n" //
+ + " } else {\n" //
+ + " output = input;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorLocalVariable3(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " if (input != null) {\n" //
+ + " output = input;\n" //
+ + " } else {\n" //
+ + " output = null;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorLocalVariable4(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " if (null != input) {\n" //
+ + " output = input;\n" //
+ + " } else {\n" //
+ + " output = null;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public int removeHardCodedNumber(int input) {\n" //
+ + " int output;\n" //
+ + " // Keep this comment\n" //
+ + " if (123 != input) {\n" //
+ + " output = input;\n" //
+ + " } else {\n" //
+ + " output = 123;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public char removeHardCodedCharacter(char input) {\n" //
+ + " char output;\n" //
+ + " // Keep this comment\n" //
+ + " if (input == 'a') {\n" //
+ + " output = 'a';\n" //
+ + " } else {\n" //
+ + " output = input;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public int removeHardCodedExpression(int input) {\n" //
+ + " int output;\n" //
+ + " // Keep this comment\n" //
+ + " if (input != 1 + 2 + 3) {\n" //
+ + " output = input;\n" //
+ + " } else {\n" //
+ + " output = 3 + 2 + 1;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorLocalVariable5(String input, boolean isValid) {\n" //
+ + " String output = null;\n" //
+ + " if (isValid)\n" //
+ + " if (input != null) {\n" //
+ + " output = input;\n" //
+ + " } else {\n" //
+ + " output = null;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void refactorFieldAssign1(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (input == null) {\n" //
+ + " this.input = null;\n" //
+ + " } else {\n" //
+ + " this.input = input;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void refactorFieldAssign2(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (null == input) {\n" //
+ + " this.input = null;\n" //
+ + " } else {\n" //
+ + " this.input = input;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void refactorFieldAssign3(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (input != null) {\n" //
+ + " this.input = input;\n" //
+ + " } else {\n" //
+ + " this.input = null;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void refactorFieldAssign4(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (null != input) {\n" //
+ + " this.input = input;\n" //
+ + " } else {\n" //
+ + " this.input = null;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturn1(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (input == null) {\n" //
+ + " return null;\n" //
+ + " } else {\n" //
+ + " return /* Keep this comment too */ input;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturn2(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (null == input) {\n" //
+ + " return null;\n" //
+ + " } else {\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturn3(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (input != null) {\n" //
+ + " return input;\n" //
+ + " } else {\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturn4(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (null != input) {\n" //
+ + " return input;\n" //
+ + " } else {\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturnNoElse1(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (input == null) {\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturnNoElse2(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (null == input) {\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturnNoElse3(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (input != null) {\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturnNoElse4(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (null != input) {\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstant1(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " if (input != null) {\n" //
+ + " output = /* Keep this comment too */ null;\n" //
+ + " } else {\n" //
+ + " output = input;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstant2(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " if (null != input) {\n" //
+ + " output = null;\n" //
+ + " } else {\n" //
+ + " output = input;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstant3(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " if (input == null) {\n" //
+ + " output = input;\n" //
+ + " } else {\n" //
+ + " output = null;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstant4(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " if (null == input) {\n" //
+ + " output = input;\n" //
+ + " } else {\n" //
+ + " output = null;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturn1(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (input != null) {\n" //
+ + " return /* Keep this comment too */ null;\n" //
+ + " } else {\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturn2(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (null != input) {\n" //
+ + " return null;\n" //
+ + " } else {\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturn3(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (input == null) {\n" //
+ + " return input;\n" //
+ + " } else {\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturn4(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (null == input) {\n" //
+ + " return input;\n" //
+ + " } else {\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturnNoElse1(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (input != null) {\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturnNoElse2(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (null != input) {\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturnNoElse3(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (input == null) {\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturnNoElse4(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " if (null == input) {\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + "}\n";
+ ICompilationUnit cu= pack.createCompilationUnit("E.java", input, false, null);
+
+ enable(CleanUpConstants.REMOVE_REDUNDANT_COMPARISON_STATEMENT);
+
+ String output= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "import java.util.Date;\n" //
+ + "import java.util.List;\n" //
+ + "import java.util.Map;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " private static final String DEFAULT = \"\";\n" //
+ + " private String input;\n" //
+ + "\n" //
+ + " public String refactorLocalVariable1(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " output = /* Keep this comment too */ input;\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorLocalVariable2(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " output = input;\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorLocalVariable3(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " output = input;\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorLocalVariable4(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " output = input;\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public int removeHardCodedNumber(int input) {\n" //
+ + " int output;\n" //
+ + " // Keep this comment\n" //
+ + " output = input;\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public char removeHardCodedCharacter(char input) {\n" //
+ + " char output;\n" //
+ + " // Keep this comment\n" //
+ + " output = input;\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public int removeHardCodedExpression(int input) {\n" //
+ + " int output;\n" //
+ + " // Keep this comment\n" //
+ + " output = input;\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorLocalVariable5(String input, boolean isValid) {\n" //
+ + " String output = null;\n" //
+ + " if (isValid)\n" //
+ + " output = input;\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void refactorFieldAssign1(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " this.input = input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void refactorFieldAssign2(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " this.input = input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void refactorFieldAssign3(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " this.input = input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void refactorFieldAssign4(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " this.input = input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturn1(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return /* Keep this comment too */ input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturn2(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturn3(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturn4(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturnNoElse1(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturnNoElse2(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturnNoElse3(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorReturnNoElse4(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return input;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstant1(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " output = /* Keep this comment too */ null;\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstant2(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " output = null;\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstant3(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " output = null;\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstant4(String input) {\n" //
+ + " String output;\n" //
+ + " // Keep this comment\n" //
+ + " output = null;\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturn1(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return /* Keep this comment too */ null;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturn2(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturn3(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturn4(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturnNoElse1(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturnNoElse2(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturnNoElse3(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String refactorConstantReturnNoElse4(String input) {\n" //
+ + " // Keep this comment\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + "}\n";
+ assertGroupCategoryUsed(new ICompilationUnit[] { cu }, new String[] { MultiFixMessages.RedundantComparisonStatementCleanup_description });
+ assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { output });
+ }
+
+ @Test
+ public void testKeepComparisonStatement() throws Exception {
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String sample= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "import java.util.Collection;\n" //
+ + "import java.util.Collections;\n" //
+ + "import java.util.Date;\n" //
+ + "import java.util.List;\n" //
+ + "import java.util.Map;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " private static final String DEFAULT = \"\";\n" //
+ + " private String input;\n" //
+ + "\n" //
+ + " public String doNotRefactorLocalVariable(String input) {\n" //
+ + " String output;\n" //
+ + " if (input == null) {\n" //
+ + " output = DEFAULT;\n" //
+ + " } else {\n" //
+ + " output = input;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String doNotRefactorConstant(String input) {\n" //
+ + " String output;\n" //
+ + " if (input != null) {\n" //
+ + " output = DEFAULT;\n" //
+ + " } else {\n" //
+ + " output = input;\n" //
+ + " }\n" //
+ + " return output;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String doNotRefactorActiveExpression(List<String> input) {\n" //
+ + " String result;\n" //
+ + " if (input.remove(0) == null) {\n" //
+ + " result = null;\n" //
+ + " } else {\n" //
+ + " result = input.remove(0);\n" //
+ + " }\n" //
+ + " return result;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String doNotUseConstantWithoutActiveExpression(List<String> input) {\n" //
+ + " String result;\n" //
+ + " if (input.remove(0) == null) {\n" //
+ + " result = null;\n" //
+ + " } else {\n" //
+ + " result = input.remove(0);\n" //
+ + " }\n" //
+ + " return result;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotRefactorFieldAssignXXX(String input, E other) {\n" //
+ + " if (input == null) {\n" //
+ + " this.input = null;\n" //
+ + " } else {\n" //
+ + " other.input = input;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotRefactorFieldAssign(String input) {\n" //
+ + " if (input == null) {\n" //
+ + " this.input = DEFAULT;\n" //
+ + " } else {\n" //
+ + " this.input = input;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String doNotRefactorConstantReturn(String input) {\n" //
+ + " if (null != input) {\n" //
+ + " return input;\n" //
+ + " } else {\n" //
+ + " return DEFAULT;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public Collection<?> doNotRefactorDifferentReturn(Collection<?> c) {\n" //
+ + " if (c == null) {\n" //
+ + " return Collections.emptySet();\n" //
+ + " } else {\n" //
+ + " return c;\n" //
+ + " }\n" //
+ + " }\n" //
+ + "\n" //
+ + " public Date doNotRefactorActiveAssignment(List<Date> input) {\n" //
+ + " Date date;\n" //
+ + " if (input.remove(0) != null) {\n" //
+ + " date = input.remove(0);\n" //
+ + " } else {\n" //
+ + " date = null;\n" //
+ + " }\n" //
+ + " return date;\n" //
+ + " }\n" //
+ + "\n" //
+ + " public Date doNotRefactorActiveReturn(List<Date> input) {\n" //
+ + " if (input.remove(0) != null) {\n" //
+ + " return input.remove(0);\n" //
+ + " }\n" //
+ + " return null;\n" //
+ + " }\n" //
+ + "}\n";
+ ICompilationUnit cu1= pack.createCompilationUnit("E.java", sample, false, null);
+
+ enable(CleanUpConstants.REMOVE_REDUNDANT_COMPARISON_STATEMENT);
+
+ assertRefactoringHasNoChange(new ICompilationUnit[] { cu1 });
+ }
+
+ @Test
public void testUselessSuperCall() throws Exception {
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
String sample= "" //
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 8ae5144..2d6a47e 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
@@ -98,6 +98,7 @@
options.setOption(USE_AUTOBOXING, CleanUpOptions.FALSE);
options.setOption(USE_UNBOXING, CleanUpOptions.FALSE);
options.setOption(PUSH_DOWN_NEGATION, CleanUpOptions.FALSE);
+ options.setOption(REMOVE_REDUNDANT_COMPARISON_STATEMENT, CleanUpOptions.FALSE);
options.setOption(REDUNDANT_SUPER_CALL, CleanUpOptions.FALSE);
options.setOption(MERGE_CONDITIONAL_BLOCKS, CleanUpOptions.FALSE);
options.setOption(REDUNDANT_FALLING_THROUGH_BLOCK_END, CleanUpOptions.FALSE);
@@ -217,6 +218,7 @@
options.setOption(USE_AUTOBOXING, CleanUpOptions.FALSE);
options.setOption(USE_UNBOXING, CleanUpOptions.FALSE);
options.setOption(PUSH_DOWN_NEGATION, CleanUpOptions.FALSE);
+ options.setOption(REMOVE_REDUNDANT_COMPARISON_STATEMENT, CleanUpOptions.FALSE);
options.setOption(REDUNDANT_SUPER_CALL, CleanUpOptions.FALSE);
options.setOption(MERGE_CONDITIONAL_BLOCKS, CleanUpOptions.FALSE);
options.setOption(REDUNDANT_FALLING_THROUGH_BLOCK_END, CleanUpOptions.FALSE);
diff --git a/org.eclipse.jdt.ui/plugin.xml b/org.eclipse.jdt.ui/plugin.xml
index 2c8623d..0c7915a 100644
--- a/org.eclipse.jdt.ui/plugin.xml
+++ b/org.eclipse.jdt.ui/plugin.xml
@@ -7231,9 +7231,14 @@
runAfter="org.eclipse.jdt.ui.cleanup.unboxing">
</cleanUp>
<cleanUp
+ class="org.eclipse.jdt.internal.ui.fix.RedundantComparisonStatementCleanUp"
+ id="org.eclipse.jdt.ui.cleanup.comparison_statement"
+ runAfter="org.eclipse.jdt.ui.cleanup.push_down_negation">
+ </cleanUp>
+ <cleanUp
class="org.eclipse.jdt.internal.ui.fix.RedundantSuperCallCleanUp"
id="org.eclipse.jdt.ui.cleanup.no_super"
- runAfter="org.eclipse.jdt.ui.cleanup.push_down_negation">
+ runAfter="org.eclipse.jdt.ui.cleanup.comparison_statement">
</cleanUp>
<cleanUp
class="org.eclipse.jdt.internal.ui.fix.MergeConditionalBlocksCleanUp"
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/RedundantComparisonStatementCleanUp.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/RedundantComparisonStatementCleanUp.java
new file mode 100644
index 0000000..70839bd
--- /dev/null
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/RedundantComparisonStatementCleanUp.java
@@ -0,0 +1,259 @@
+/*******************************************************************************
+ * Copyright (c) 2020 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.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.IProgressMonitor;
+
+import org.eclipse.text.edits.TextEditGroup;
+
+import org.eclipse.jdt.core.ICompilationUnit;
+import org.eclipse.jdt.core.dom.ASTVisitor;
+import org.eclipse.jdt.core.dom.Assignment;
+import org.eclipse.jdt.core.dom.Block;
+import org.eclipse.jdt.core.dom.CompilationUnit;
+import org.eclipse.jdt.core.dom.Expression;
+import org.eclipse.jdt.core.dom.IfStatement;
+import org.eclipse.jdt.core.dom.InfixExpression;
+import org.eclipse.jdt.core.dom.ReturnStatement;
+import org.eclipse.jdt.core.dom.Statement;
+import org.eclipse.jdt.core.dom.rewrite.ASTRewrite;
+import org.eclipse.jdt.core.refactoring.CompilationUnitChange;
+
+import org.eclipse.jdt.internal.corext.dom.ASTNodes;
+import org.eclipse.jdt.internal.corext.fix.CleanUpConstants;
+import org.eclipse.jdt.internal.corext.fix.CompilationUnitRewriteOperationsFix;
+import org.eclipse.jdt.internal.corext.fix.CompilationUnitRewriteOperationsFix.CompilationUnitRewriteOperation;
+import org.eclipse.jdt.internal.corext.fix.LinkedProposalModel;
+import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite;
+
+import org.eclipse.jdt.ui.cleanup.CleanUpRequirements;
+import org.eclipse.jdt.ui.cleanup.ICleanUpFix;
+import org.eclipse.jdt.ui.text.java.IProblemLocation;
+
+/**
+ * A fix that removes useless bad value checks before assignments or return statements.
+ * Such useless bad value checks are comparing an expression against bad value,
+ * then either assigning bad value or the expression depending on the result of the bad value check.
+ * It is simpler to directly assign the expression:
+ * <ul>
+ * <li>The expression should be passive.</li>
+ * <li>The excluded value should be hard coded.</li>
+ * </ul>
+ */
+public class RedundantComparisonStatementCleanUp extends AbstractMultiFix implements ICleanUpFix {
+ public RedundantComparisonStatementCleanUp() {
+ this(Collections.emptyMap());
+ }
+
+ public RedundantComparisonStatementCleanUp(Map<String, String> options) {
+ super(options);
+ }
+
+ @Override
+ public CleanUpRequirements getRequirements() {
+ boolean requireAST= isEnabled(CleanUpConstants.REMOVE_REDUNDANT_COMPARISON_STATEMENT);
+ return new CleanUpRequirements(requireAST, false, false, null);
+ }
+
+ @Override
+ public String[] getStepDescriptions() {
+ if (isEnabled(CleanUpConstants.REMOVE_REDUNDANT_COMPARISON_STATEMENT)) {
+ return new String[] { MultiFixMessages.RedundantComparisonStatementCleanup_description };
+ }
+
+ return new String[0];
+ }
+
+ @Override
+ public String getPreview() {
+ if (isEnabled(CleanUpConstants.REMOVE_REDUNDANT_COMPARISON_STATEMENT)) {
+ return "return i;\n\n\n\n\n"; //$NON-NLS-1$
+ }
+
+ return "" //$NON-NLS-1$
+ + "if (i != 123) {\n" //$NON-NLS-1$
+ + " return i;\n" //$NON-NLS-1$
+ + "} else {\n" //$NON-NLS-1$
+ + " return 123;\n" //$NON-NLS-1$
+ + "}\n"; //$NON-NLS-1$
+ }
+
+ @Override
+ protected ICleanUpFix createFix(CompilationUnit unit) throws CoreException {
+ if (!isEnabled(CleanUpConstants.REMOVE_REDUNDANT_COMPARISON_STATEMENT)) {
+ return null;
+ }
+
+ final List<CompilationUnitRewriteOperation> rewriteOperations= new ArrayList<>();
+
+ unit.accept(new ASTVisitor() {
+ @Override
+ public boolean visit(final Block node) {
+ IfAndReturnVisitor ifAndReturnVisitor= new IfAndReturnVisitor(node);
+ node.accept(ifAndReturnVisitor);
+ return ifAndReturnVisitor.result;
+ }
+
+ final class IfAndReturnVisitor extends ASTVisitor {
+ private final Block startNode;
+ private boolean result= true;
+
+ public IfAndReturnVisitor(final Block startNode) {
+ this.startNode= startNode;
+ }
+
+ @Override
+ public boolean visit(final Block node) {
+ return startNode == node;
+ }
+
+ @Override
+ public boolean visit(final IfStatement node) {
+ InfixExpression condition= ASTNodes.as(node.getExpression(), InfixExpression.class);
+ Statement thenStatement= getThenStatement(node);
+ Statement elseStatement= getElseStatement(node, thenStatement);
+
+ if (result
+ && thenStatement != null
+ && elseStatement != null
+ && condition != null
+ && !condition.hasExtendedOperands()
+ && ASTNodes.hasOperator(condition, InfixExpression.Operator.EQUALS, InfixExpression.Operator.NOT_EQUALS)) {
+ boolean isEqual= ASTNodes.hasOperator(condition, InfixExpression.Operator.EQUALS);
+ Assignment thenAssignment= ASTNodes.asExpression(thenStatement, Assignment.class);
+ Assignment elseAssignment= ASTNodes.asExpression(elseStatement, Assignment.class);
+
+ if (ASTNodes.hasOperator(thenAssignment, Assignment.Operator.ASSIGN)
+ && ASTNodes.hasOperator(elseAssignment, Assignment.Operator.ASSIGN)
+ && ASTNodes.match(thenAssignment.getLeftHandSide(), elseAssignment.getLeftHandSide())) {
+ if (isEqual) {
+ return maybeReplace(node, condition, thenAssignment.getRightHandSide(), elseAssignment.getRightHandSide(), elseStatement, null)
+ && maybeReplace(node, condition, elseAssignment.getRightHandSide(), thenAssignment.getRightHandSide(), elseStatement, null);
+ }
+
+ return maybeReplace(node, condition, elseAssignment.getRightHandSide(), thenAssignment.getRightHandSide(), thenStatement, null)
+ && maybeReplace(node, condition, thenAssignment.getRightHandSide(), elseAssignment.getRightHandSide(), thenStatement, null);
+ }
+
+ ReturnStatement thenReturnStatement= ASTNodes.as(thenStatement, ReturnStatement.class);
+ ReturnStatement elseReturnStatement= ASTNodes.as(elseStatement, ReturnStatement.class);
+
+ if (thenReturnStatement != null && elseReturnStatement != null) {
+ if (isEqual) {
+ return maybeReplace(node, condition, thenReturnStatement.getExpression(), elseReturnStatement.getExpression(), elseReturnStatement, thenReturnStatement)
+ && maybeReplace(node, condition, elseReturnStatement.getExpression(), thenReturnStatement.getExpression(), elseReturnStatement, thenReturnStatement);
+ }
+
+ return maybeReplace(node, condition, elseReturnStatement.getExpression(), thenReturnStatement.getExpression(), thenReturnStatement, elseReturnStatement)
+ && maybeReplace(node, condition, thenReturnStatement.getExpression(), elseReturnStatement.getExpression(), thenReturnStatement, elseReturnStatement);
+ }
+ }
+
+ return true;
+ }
+
+ private Statement getThenStatement(final IfStatement node) {
+ List<Statement> thenStatements= ASTNodes.asList(node.getThenStatement());
+
+ if (thenStatements.size() == 1) {
+ return thenStatements.get(0);
+ }
+
+ return null;
+ }
+
+ private Statement getElseStatement(final IfStatement node, final Statement thenStatement) {
+ List<Statement> elseStatements= ASTNodes.asList(node.getElseStatement());
+
+ if (elseStatements.size() == 1) {
+ return elseStatements.get(0);
+ }
+
+ if (ASTNodes.is(thenStatement, ReturnStatement.class)) {
+ return ASTNodes.getNextSibling(node);
+ }
+
+ return null;
+ }
+
+ private boolean maybeReplace(final IfStatement node, final InfixExpression condition, final Expression hardCodedExpression, final Expression valuedExpression,
+ final Statement statementToMove, final ReturnStatement returnToRemove) {
+ if (ASTNodes.isHardCoded(hardCodedExpression)
+ && ASTNodes.isPassiveWithoutFallingThrough(hardCodedExpression)
+ && ASTNodes.isPassive(valuedExpression)
+ && ((ASTNodes.match(condition.getRightOperand(), hardCodedExpression) && ASTNodes.match(condition.getLeftOperand(), valuedExpression))
+ || (ASTNodes.match(condition.getRightOperand(), valuedExpression) && ASTNodes.match(condition.getLeftOperand(), hardCodedExpression)))) {
+ rewriteOperations.add(new RedundantComparisonStatementOperation(node, statementToMove, returnToRemove));
+ result= false;
+ return false;
+ }
+
+ return true;
+ }
+ }
+ });
+
+ if (rewriteOperations.isEmpty()) {
+ return null;
+ }
+
+ return new CompilationUnitRewriteOperationsFix(MultiFixMessages.RedundantComparisonStatementCleanup_description, unit,
+ rewriteOperations.toArray(new CompilationUnitRewriteOperation[0]));
+ }
+
+ @Override
+ public CompilationUnitChange createChange(IProgressMonitor progressMonitor) throws CoreException {
+ return null;
+ }
+
+ @Override
+ public boolean canFix(final ICompilationUnit compilationUnit, final IProblemLocation problem) {
+ return false;
+ }
+
+ @Override
+ protected ICleanUpFix createFix(final CompilationUnit unit, final IProblemLocation[] problems) throws CoreException {
+ return null;
+ }
+
+ private static class RedundantComparisonStatementOperation extends CompilationUnitRewriteOperation {
+ private final IfStatement node;
+ private final Statement toMove;
+ private final ReturnStatement toRemove;
+
+ public RedundantComparisonStatementOperation(final IfStatement node, final Statement toMove, final ReturnStatement toRemove) {
+ this.node= node;
+ this.toMove= toMove;
+ this.toRemove= toRemove;
+ }
+
+ @Override
+ public void rewriteAST(final CompilationUnitRewrite cuRewrite, final LinkedProposalModel linkedModel) throws CoreException {
+ ASTRewrite rewrite= cuRewrite.getASTRewrite();
+ TextEditGroup group= createTextEditGroup(MultiFixMessages.RedundantComparisonStatementCleanup_description, cuRewrite);
+
+ ASTNodes.replaceButKeepComment(rewrite, node, ASTNodes.createMoveTarget(rewrite, toMove), group);
+
+ if (toRemove != null) {
+ rewrite.remove(toRemove, group);
+ }
+ }
+ }
+}
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 47c3c91..ad6ee3d 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
@@ -130,6 +130,7 @@
public static String UnnecessaryCodeTabPage_CheckboxName_Unboxing;
public static String UnnecessaryCodeTabPage_CheckboxName_PatternMatchingForInstanceof;
public static String UnnecessaryCodeTabPage_CheckboxName_PushDownNegation;
+ public static String UnnecessaryCodeTabPage_CheckboxName_ComparisonStatement;
public static String UnnecessaryCodeTabPage_CheckboxName_RedundantSuperCall;
public static String UnnecessaryCodeTabPage_CheckboxName_MergeConditionalBlocks;
public static String UnnecessaryCodeTabPage_CheckboxName_RedundantFallingThroughBlockEnd;
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 26667e0..fb1a9b8 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
@@ -115,6 +115,7 @@
UnnecessaryCodeTabPage_CheckboxName_Unboxing=Use Un&boxing (1.5 or higher)
UnnecessaryCodeTabPage_CheckboxName_PatternMatchingForInstanceof=&Pattern matching for instanceof (Java 14 or higher with feature preview)
UnnecessaryCodeTabPage_CheckboxName_PushDownNegation=Push d&own negation
+UnnecessaryCodeTabPage_CheckboxName_ComparisonStatement=Remove redundant comparison statement
UnnecessaryCodeTabPage_CheckboxName_RedundantSuperCall=Remove redundant s&uper() call in constructor
UnnecessaryCodeTabPage_CheckboxName_MergeConditionalBlocks=Merge &conditions of if/else if/else that have the same blocks
UnnecessaryCodeTabPage_CheckboxName_RedundantFallingThroughBlockEnd=Remove redundant end of block with &jump statement
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 f5a46e3..3fbcdaa 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
@@ -33,6 +33,7 @@
import org.eclipse.jdt.internal.ui.fix.PatternMatchingForInstanceofCleanUp;
import org.eclipse.jdt.internal.ui.fix.PushDownNegationCleanUp;
import org.eclipse.jdt.internal.ui.fix.RedundantFallingThroughBlockEndCleanUp;
+import org.eclipse.jdt.internal.ui.fix.RedundantComparisonStatementCleanUp;
import org.eclipse.jdt.internal.ui.fix.RedundantIfConditionCleanUp;
import org.eclipse.jdt.internal.ui.fix.RedundantModifiersCleanUp;
import org.eclipse.jdt.internal.ui.fix.RedundantSemicolonsCleanUp;
@@ -60,6 +61,7 @@
new AutoboxingCleanUp(values),
new UnboxingCleanUp(values),
new PushDownNegationCleanUp(values),
+ new RedundantComparisonStatementCleanUp(values),
new RedundantSuperCallCleanUp(values),
new MergeConditionalBlocksCleanUp(values),
new RedundantFallingThroughBlockEndCleanUp(values),
@@ -120,6 +122,9 @@
CleanUpModifyDialog.FALSE_TRUE);
registerPreference(pushDownNegation);
+ CheckboxPreference comparisonStatement= createCheckboxPref(unnecessaryGroup, numColumns, CleanUpMessages.UnnecessaryCodeTabPage_CheckboxName_ComparisonStatement, CleanUpConstants.REMOVE_REDUNDANT_COMPARISON_STATEMENT, CleanUpModifyDialog.FALSE_TRUE);
+ registerPreference(comparisonStatement);
+
CheckboxPreference redundantSuperCall= createCheckboxPref(unnecessaryGroup, numColumns, CleanUpMessages.UnnecessaryCodeTabPage_CheckboxName_RedundantSuperCall, CleanUpConstants.REDUNDANT_SUPER_CALL, CleanUpModifyDialog.FALSE_TRUE);
registerPreference(redundantSuperCall);
