Bug 572790 - [AutoRefactor #74/153] 1 if rather than duplicate
Merge consecutive if statements with same code block that end with a
jump statement:
Given:
if (i1 == 0) {
System.out.println("The same code");
return;
}
if (i1 == 1) {
System.out.println("The same code");
return;
}
System.out.println("Next code");
When:
Applying "One if rather than duplicate blocks that fall through" clean
up...
Then:
if ((i1 == 0) || (i1 == 1)) {
System.out.println("The same code");
return;
}
System.out.println("Next code");
Change-Id: I94d94c3f1bb101f74a10d320baff0ae3a938a4d6
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 2461d05..02d573d 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
@@ -186,6 +186,7 @@
public static String ObjectsEqualsCleanup_description;
public static String OperandFactorizationCleanUp_description;
+ public static String OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp_description;
public static String InvertEqualsCleanUp_description;
public static String CheckSignOfBitwiseOperation_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 f6846ae..6090e9e 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
@@ -168,6 +168,7 @@
ObjectsEqualsCleanup_description=Use Objects.equals() in the equals method implementation
OperandFactorizationCleanUp_description=Replace (X && Y) || (X && Z) by (X && (Y || Z))
+OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp_description=Single 'if' statement rather than duplicate blocks that fall through
InvertEqualsCleanUp_description=Avoid Object.equals() or String.equalsIgnoreCase() on null objects
CheckSignOfBitwiseOperation_description=Use != 0 instead of > 0 when comparing the result of a bitwise expression
diff --git a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/OneIfRatherThanDuplicateBlocksThatFallThroughCleanUpCore.java b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/OneIfRatherThanDuplicateBlocksThatFallThroughCleanUpCore.java
new file mode 100644
index 0000000..7008603
--- /dev/null
+++ b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/OneIfRatherThanDuplicateBlocksThatFallThroughCleanUpCore.java
@@ -0,0 +1,94 @@
+/*******************************************************************************
+ * 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.CleanUpConstants;
+import org.eclipse.jdt.internal.corext.fix.OneIfRatherThanDuplicateBlocksThatFallThroughFixCore;
+
+public class OneIfRatherThanDuplicateBlocksThatFallThroughCleanUpCore extends AbstractCleanUpCore {
+ public OneIfRatherThanDuplicateBlocksThatFallThroughCleanUpCore(final Map<String, String> options) {
+ super(options);
+ }
+
+ public OneIfRatherThanDuplicateBlocksThatFallThroughCleanUpCore() {
+ }
+
+ @Override
+ public CleanUpRequirementsCore getRequirementsCore() {
+ return new CleanUpRequirementsCore(requireAST(), false, false, null);
+ }
+
+ public boolean requireAST() {
+ return isEnabled(CleanUpConstants.ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH);
+ }
+
+ @Override
+ public ICleanUpFixCore createFixCore(final CleanUpContextCore context) throws CoreException {
+ CompilationUnit compilationUnit= context.getAST();
+
+ if (compilationUnit == null || !isEnabled(CleanUpConstants.ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH)) {
+ return null;
+ }
+
+ return OneIfRatherThanDuplicateBlocksThatFallThroughFixCore.createCleanUp(compilationUnit);
+ }
+
+ @Override
+ public String[] getStepDescriptions() {
+ if (isEnabled(CleanUpConstants.ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH)) {
+ return new String[] {MultiFixMessages.OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp_description};
+ }
+
+ return new String[0];
+ }
+
+ @Override
+ public String getPreview() {
+ StringBuilder bld= new StringBuilder();
+
+ if (isEnabled(CleanUpConstants.ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH)) {
+ bld.append("if (isActive || isValid) {\n"); //$NON-NLS-1$
+ } else {
+ bld.append("if (isActive) {\n"); //$NON-NLS-1$
+ }
+
+ bld.append(" System.out.println(\"The same code\");\n"); //$NON-NLS-1$
+ bld.append(" throw new NullPointerException();\n"); //$NON-NLS-1$
+ bld.append("}\n"); //$NON-NLS-1$
+
+ if (!isEnabled(CleanUpConstants.ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH)) {
+ bld.append("if (isValid) {\n"); //$NON-NLS-1$
+ bld.append(" System.out.println(\"The same code\");\n"); //$NON-NLS-1$
+ bld.append(" throw new NullPointerException();\n"); //$NON-NLS-1$
+ bld.append("}\n"); //$NON-NLS-1$
+ }
+
+ bld.append("System.out.println(\"Next code\");\n"); //$NON-NLS-1$
+
+ if (isEnabled(CleanUpConstants.ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH)) {
+ bld.append("\n\n\n\n"); //$NON-NLS-1$
+ }
+
+ return bld.toString();
+ }
+}
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 1f02713..652118c 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
@@ -1596,6 +1596,18 @@
public static final String CONTROLFLOW_MERGE= "cleanup.controlflow_merge"; //$NON-NLS-1$
/**
+ * Merge consecutive <code>if</code> statements with same code block that end with a jump statement.
+ * <p>
+ * Possible values: {TRUE, FALSE}
+ * <p>
+ *
+ * @see CleanUpOptionsCore#TRUE
+ * @see CleanUpOptionsCore#FALSE
+ * @since 4.20
+ */
+ public static final String ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH= "cleanup.one_if_rather_than_duplicate_blocks_that_fall_through"; //$NON-NLS-1$
+
+ /**
* Merges blocks that end with a jump statement into the following same code.
* <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 f9943e6..edbc35f 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
@@ -170,6 +170,8 @@
public static String ArrayWithCurlyFix_description;
public static String ReturnExpressionFix_description;
+ public static String OneIfRatherThanDuplicateBlocksThatFallThroughFix_description;
+
public static String TypeAnnotationFix_move;
public static String TypeAnnotationFix_remove;
public static String ConstantsCleanUpFix_refactor;
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 d3ccfd0..7f88ec9 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
@@ -157,6 +157,8 @@
ArrayWithCurlyFix_description=Create array with curly
ReturnExpressionFix_description=Remove variable assignment before return
+OneIfRatherThanDuplicateBlocksThatFallThroughFix_description=Single 'if' statement rather than duplicate blocks that fall through
+
TypeAnnotationFix_move=Move type annotation
TypeAnnotationFix_remove=Remove type annotation
ConstantsCleanUpFix_refactor=Replace system property with Java method
diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/OneIfRatherThanDuplicateBlocksThatFallThroughFixCore.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/OneIfRatherThanDuplicateBlocksThatFallThroughFixCore.java
new file mode 100644
index 0000000..86013cb
--- /dev/null
+++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/OneIfRatherThanDuplicateBlocksThatFallThroughFixCore.java
@@ -0,0 +1,177 @@
+/*******************************************************************************
+ * 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 java.util.concurrent.atomic.AtomicInteger;
+
+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.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.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 OneIfRatherThanDuplicateBlocksThatFallThroughFixCore extends CompilationUnitRewriteOperationsFixCore {
+ public static final class OneIfRatherThanDuplicateBlocksThatFallThroughFinder extends ASTVisitor {
+ private List<CompilationUnitRewriteOperation> fResult;
+
+ public OneIfRatherThanDuplicateBlocksThatFallThroughFinder(List<CompilationUnitRewriteOperation> ops) {
+ fResult= ops;
+ }
+
+ @Override
+ public boolean visit(final Block visited) {
+ SuccessiveIfVisitor successiveIfVisitor= new SuccessiveIfVisitor(visited);
+ visited.accept(successiveIfVisitor);
+ return successiveIfVisitor.result;
+ }
+
+ private final class SuccessiveIfVisitor extends ASTVisitor {
+ private final Block startNode;
+ private boolean result= true;
+
+ public SuccessiveIfVisitor(final Block startNode) {
+ this.startNode= startNode;
+ }
+
+ @Override
+ public boolean visit(final Block node) {
+ return startNode == node;
+ }
+
+ @Override
+ public boolean visit(final IfStatement visited) {
+ if (result && ASTNodes.fallsThrough(visited.getThenStatement())) {
+ List<IfStatement> duplicateIfBlocks= new ArrayList<>(ASTNodes.EXCESSIVE_OPERAND_NUMBER - 1);
+ AtomicInteger operandCount= new AtomicInteger(ASTNodes.getNbOperands(visited.getExpression()));
+ duplicateIfBlocks.add(visited);
+
+ while (addOneMoreIf(duplicateIfBlocks, operandCount)) {
+ // OK continue
+ }
+
+ if (duplicateIfBlocks.size() > 1) {
+ fResult.add(new OneIfRatherThanDuplicateBlocksThatFallThroughOperation(duplicateIfBlocks));
+ result= false;
+ return false;
+ }
+ }
+
+ return true;
+ }
+
+ private boolean addOneMoreIf(final List<IfStatement> duplicateIfBlocks, final AtomicInteger operandCount) {
+ IfStatement lastBlock= duplicateIfBlocks.get(duplicateIfBlocks.size() - 1);
+
+ if (lastBlock.getElseStatement() == null) {
+ IfStatement nextSibling= ASTNodes.as(ASTNodes.getNextSibling(lastBlock), IfStatement.class);
+
+ if (nextSibling != null
+ && nextSibling.getElseStatement() == null
+ && operandCount.get() + ASTNodes.getNbOperands(nextSibling.getExpression()) < ASTNodes.EXCESSIVE_OPERAND_NUMBER
+ && ASTNodes.match(lastBlock.getThenStatement(), nextSibling.getThenStatement())) {
+ operandCount.addAndGet(ASTNodes.getNbOperands(nextSibling.getExpression()));
+ duplicateIfBlocks.add(nextSibling);
+ return true;
+ }
+ }
+
+ return false;
+ }
+ }
+ }
+
+ private static class OneIfRatherThanDuplicateBlocksThatFallThroughOperation extends CompilationUnitRewriteOperation {
+ private final List<IfStatement> duplicateIfBlocks;
+
+ public OneIfRatherThanDuplicateBlocksThatFallThroughOperation(final List<IfStatement> duplicateIfBlocks) {
+ this.duplicateIfBlocks= duplicateIfBlocks;
+ }
+
+ @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.OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp_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);
+ }
+ });
+
+ List<Expression> newConditions= new ArrayList<>(duplicateIfBlocks.size());
+
+ for (IfStatement ifStatement : duplicateIfBlocks) {
+ InfixExpression oneOriginalCondition= ASTNodes.as(ifStatement.getExpression(), InfixExpression.class);
+
+ if (ASTNodes.hasOperator(oneOriginalCondition, InfixExpression.Operator.CONDITIONAL_OR)) {
+ newConditions.add(ASTNodes.createMoveTarget(rewrite, ASTNodes.getUnparenthesedExpression(ifStatement.getExpression())));
+ } else {
+ newConditions.add(ASTNodeFactory.parenthesizeIfNeeded(ast, ASTNodes.createMoveTarget(rewrite, ifStatement.getExpression())));
+ }
+ }
+
+ InfixExpression newCondition= ast.newInfixExpression();
+ newCondition.setOperator(InfixExpression.Operator.CONDITIONAL_OR);
+ newCondition.setLeftOperand(newConditions.remove(0));
+ newCondition.setRightOperand(newConditions.remove(0));
+ newCondition.extendedOperands().addAll(newConditions);
+
+ for (int i= 0; i < duplicateIfBlocks.size() - 1; i++) {
+ ASTNodes.removeButKeepComment(rewrite, duplicateIfBlocks.get(i), group);
+ }
+
+ rewrite.replace(duplicateIfBlocks.get(duplicateIfBlocks.size() - 1).getExpression(), newCondition, group);
+ }
+ }
+
+ public static ICleanUpFixCore createCleanUp(final CompilationUnit compilationUnit) {
+ List<CompilationUnitRewriteOperation> operations= new ArrayList<>();
+ OneIfRatherThanDuplicateBlocksThatFallThroughFinder finder= new OneIfRatherThanDuplicateBlocksThatFallThroughFinder(operations);
+ compilationUnit.accept(finder);
+
+ if (operations.isEmpty()) {
+ return null;
+ }
+
+ CompilationUnitRewriteOperationsFixCore.CompilationUnitRewriteOperation[] ops= operations.toArray(new CompilationUnitRewriteOperationsFixCore.CompilationUnitRewriteOperation[0]);
+ return new OneIfRatherThanDuplicateBlocksThatFallThroughFixCore(FixMessages.OneIfRatherThanDuplicateBlocksThatFallThroughFix_description, compilationUnit, ops);
+ }
+
+ protected OneIfRatherThanDuplicateBlocksThatFallThroughFixCore(final String name, final CompilationUnit compilationUnit, final CompilationUnitRewriteOperationsFixCore.CompilationUnitRewriteOperation[] fixRewriteOperations) {
+ super(name, compilationUnit, fixRewriteOperations);
+ }
+}
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 260bf40..f707b3b 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
@@ -85,6 +85,7 @@
import org.eclipse.jdt.internal.ui.fix.LazyLogicalCleanUp;
import org.eclipse.jdt.internal.ui.fix.MapCloningCleanUp;
import org.eclipse.jdt.internal.ui.fix.MergeConditionalBlocksCleanUp;
+import org.eclipse.jdt.internal.ui.fix.OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp;
import org.eclipse.jdt.internal.ui.fix.PlainReplacementCleanUp;
import org.eclipse.jdt.internal.ui.fix.PrimitiveComparisonCleanUp;
import org.eclipse.jdt.internal.ui.fix.PrimitiveIntRatherThanWrapperCleanUp;
@@ -754,6 +755,22 @@
}
@Test
+ public void testOneIfRatherThanDuplicateBlocksThatFallThroughCleanUp() throws Exception {
+ CleanUpRefactoring cleanUpRefactoring= new CleanUpRefactoring();
+ addAllCUs(cleanUpRefactoring, MyTestSetup.fJProject1.getChildren());
+
+ Map<String, String> node= getNullSettings();
+
+ node.put(CleanUpConstants.ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH, CleanUpOptions.TRUE);
+
+ storeSettings(node);
+
+ cleanUpRefactoring.addCleanUp(new OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp());
+
+ doCleanUp(cleanUpRefactoring);
+ }
+
+ @Test
public void testComparingOnCriteriaCleanUp() 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 0942995..70d0cef 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
@@ -18258,6 +18258,327 @@
}
@Test
+ public void testOneIfRatherThanDuplicateBlocksThatFallThrough() throws Exception {
+ // Given
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String given= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public void mergeConditionsWithReturn(int i1) {\n" //
+ + " // Keep this comment\n" //
+ + " if (i1 == 0) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " // Keep this comment too\n" //
+ + " if (i1 == 1) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void mergeConditionsWithThrow(int i1) throws Exception {\n" //
+ + " // Keep this comment\n" //
+ + " if (i1 == 0) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " i1--;\n" //
+ + " throw new Exception();\n" //
+ + " }\n" //
+ + " // Keep this comment too\n" //
+ + " if (i1 == 1) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " --i1;\n" //
+ + " throw new Exception();\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void mergeConditionsWithContinue() {\n" //
+ + " for (int i1 = 0; i1 < 10; i1++) {\n" //
+ + " // Keep this comment\n" //
+ + " if (i1 == 0) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " i1++;\n" //
+ + " continue;\n" //
+ + " }\n" //
+ + " // Keep this comment too\n" //
+ + " if (i1 == 1) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " ++i1;\n" //
+ + " continue;\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + " System.out.println(\"Another code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void mergeConditionsWithBreak() {\n" //
+ + " for (int i1 = 0; i1 < 10; i1++) {\n" //
+ + " // Keep this comment\n" //
+ + " if (i1 == 0) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " i1++;\n" //
+ + " break;\n" //
+ + " }\n" //
+ + " // Keep this comment too\n" //
+ + " if (i1 == 1) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " i1 = i1 + 1;\n" //
+ + " break;\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + " System.out.println(\"Another code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void mergeConditionsWithReturnAndThrow(int i1, int i2) throws Exception {\n" //
+ + " // Keep this comment\n" //
+ + " if (i1 == 0) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " if (i2 == 0) {\n" //
+ + " return;\n" //
+ + " } else {\n" //
+ + " throw new Exception(\"Error #\" + i1++);\n" //
+ + " }\n" //
+ + " }\n" //
+ + " if (i1 == 1) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " if (i2 == 0) {\n" //
+ + " return;\n" //
+ + " } else {\n" //
+ + " throw new Exception(\"Error #\" + i1++);\n" //
+ + " }\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void mergeSeveralConditions(int i1) {\n" //
+ + " // Keep this comment\n" //
+ + " if (i1 == 0) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " if (i1 == 1) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " if (i1 == 2) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " if (i1 == 3) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void mergeORConditions(boolean isValid, boolean isActive, boolean isEnabled, boolean isFound) {\n" //
+ + " // Keep this comment\n" //
+ + " if (isValid || isActive) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " if (isEnabled || isFound) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "}\n";
+
+ String expected= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public void mergeConditionsWithReturn(int i1) {\n" //
+ + " // Keep this comment\n" //
+ + " // Keep this comment too\n" //
+ + " if ((i1 == 0) || (i1 == 1)) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void mergeConditionsWithThrow(int i1) throws Exception {\n" //
+ + " // Keep this comment\n" //
+ + " // Keep this comment too\n" //
+ + " if ((i1 == 0) || (i1 == 1)) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " --i1;\n" //
+ + " throw new Exception();\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void mergeConditionsWithContinue() {\n" //
+ + " for (int i1 = 0; i1 < 10; i1++) {\n" //
+ + " // Keep this comment\n" //
+ + " // Keep this comment too\n" //
+ + " if ((i1 == 0) || (i1 == 1)) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " ++i1;\n" //
+ + " continue;\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + " System.out.println(\"Another code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void mergeConditionsWithBreak() {\n" //
+ + " for (int i1 = 0; i1 < 10; i1++) {\n" //
+ + " // Keep this comment\n" //
+ + " // Keep this comment too\n" //
+ + " if ((i1 == 0) || (i1 == 1)) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " i1 = i1 + 1;\n" //
+ + " break;\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + " System.out.println(\"Another code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void mergeConditionsWithReturnAndThrow(int i1, int i2) throws Exception {\n" //
+ + " // Keep this comment\n" //
+ + " if ((i1 == 0) || (i1 == 1)) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " if (i2 == 0) {\n" //
+ + " return;\n" //
+ + " } else {\n" //
+ + " throw new Exception(\"Error #\" + i1++);\n" //
+ + " }\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void mergeSeveralConditions(int i1) {\n" //
+ + " // Keep this comment\n" //
+ + " if ((i1 == 0) || (i1 == 1) || (i1 == 2)\n" //
+ + " || (i1 == 3)) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void mergeORConditions(boolean isValid, boolean isActive, boolean isEnabled, boolean isFound) {\n" //
+ + " // Keep this comment\n" //
+ + " if (isValid || isActive || isEnabled || isFound) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "}\n";
+
+ // When
+ ICompilationUnit cu= pack.createCompilationUnit("E.java", given, false, null);
+ enable(CleanUpConstants.ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH);
+
+ // Then
+ assertNotEquals("The class must be changed", given, expected);
+ assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { expected }, new HashSet<>(Arrays.asList(MultiFixMessages.OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp_description)));
+ }
+
+ @Test
+ public void testDoNotUseOneIfRatherThanDuplicateBlocksThatFallThrough() throws Exception {
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String sample= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public void doNotMergeConditionsWithConditionalReturn(int i1, int i2) {\n" //
+ + " if (i1 == 0) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " if (i2 == 0) {\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " }\n" //
+ + " if (i1 == 1) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " if (i2 == 0) {\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotMergeMoreThanFourOperands(int i1) {\n" //
+ + " if ((i1 == 0) || (i1 == 1) || (i1 == 2) || (i1 == 3)) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " if (i1 == 4) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotMergeConditionsWithoutJump(int i) {\n" //
+ + " if (i == 0) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " }\n" //
+ + " if (i == 1) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotMergeDifferentBlocks(int i) {\n" //
+ + " if (i == 0) {\n" //
+ + " System.out.println(\"A code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " if (i == 1) {\n" //
+ + " System.out.println(\"Another code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotMergeConditionsWithElse(int i1, int counter) {\n" //
+ + " // Keep this comment\n" //
+ + " if (i1 == 0) {\n" //
+ + " System.out.println(\"The count is: \" + counter++);\n" //
+ + " return;\n" //
+ + " } else {\n" //
+ + " System.out.println(\"The count is: \" + ++counter);\n" //
+ + " }\n" //
+ + " if (i1 == 1) {\n" //
+ + " System.out.println(\"The count is: \" + counter++);\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotMergeConditionsWithAnotherElse(int i) {\n" //
+ + " // Keep this comment\n" //
+ + " if (i == 0) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " }\n" //
+ + " if (i == 1) {\n" //
+ + " System.out.println(\"The same code\");\n" //
+ + " return;\n" //
+ + " } else {\n" //
+ + " System.out.println(\"Another code\");\n" //
+ + " }\n" //
+ + " System.out.println(\"Next code\");\n" //
+ + " }\n" //
+ + "}\n";
+ ICompilationUnit cu= pack.createCompilationUnit("E.java", sample, false, null);
+
+ enable(CleanUpConstants.ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH);
+
+ assertRefactoringHasNoChange(new ICompilationUnit[] { cu });
+ }
+
+ @Test
public void testRegExPrecompilationInLambda() 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 33b4403..aa59a3d 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
@@ -168,6 +168,7 @@
options.setOption(STRICTLY_EQUAL_OR_DIFFERENT, CleanUpOptions.FALSE);
options.setOption(MERGE_CONDITIONAL_BLOCKS, CleanUpOptions.FALSE);
options.setOption(CONTROLFLOW_MERGE, CleanUpOptions.FALSE);
+ options.setOption(ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH, CleanUpOptions.TRUE);
// Java Features
options.setOption(USE_PATTERN_MATCHING_FOR_INSTANCEOF, CleanUpOptions.FALSE);
@@ -340,6 +341,7 @@
options.setOption(STRICTLY_EQUAL_OR_DIFFERENT, CleanUpOptions.FALSE);
options.setOption(MERGE_CONDITIONAL_BLOCKS, CleanUpOptions.FALSE);
options.setOption(CONTROLFLOW_MERGE, CleanUpOptions.FALSE);
+ options.setOption(ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH, CleanUpOptions.FALSE);
// Java Features
options.setOption(USE_PATTERN_MATCHING_FOR_INSTANCEOF, CleanUpOptions.FALSE);
diff --git a/org.eclipse.jdt.ui/plugin.xml b/org.eclipse.jdt.ui/plugin.xml
index c5d1b6d..eac0415 100644
--- a/org.eclipse.jdt.ui/plugin.xml
+++ b/org.eclipse.jdt.ui/plugin.xml
@@ -7403,9 +7403,14 @@
runAfter="org.eclipse.jdt.ui.cleanup.merge_conditional_blocks">
</cleanUp>
<cleanUp
+ class="org.eclipse.jdt.internal.ui.fix.OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp"
+ id="org.eclipse.jdt.ui.one_if_rather_than_duplicate_blocks_that_fall_through"
+ runAfter="org.eclipse.jdt.ui.cleanup.controlflow_merge">
+ </cleanUp>
+ <cleanUp
class="org.eclipse.jdt.internal.ui.fix.RedundantFallingThroughBlockEndCleanUp"
id="org.eclipse.jdt.ui.cleanup.redundant_falling_through_block_end"
- runAfter="org.eclipse.jdt.ui.cleanup.controlflow_merge">
+ runAfter="org.eclipse.jdt.ui.one_if_rather_than_duplicate_blocks_that_fall_through">
</cleanUp>
<cleanUp
class="org.eclipse.jdt.internal.ui.fix.RedundantIfConditionCleanUp"
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp.java
new file mode 100644
index 0000000..445f3ff
--- /dev/null
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp.java
@@ -0,0 +1,68 @@
+/*******************************************************************************
+ * 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 merges consecutive <code>if</code> statements with same code block that end with a jump statement:
+ * <ul>
+ * <li>It collapses 5 <code>if</code> statements maximum.</li>
+ * </ul>
+ */
+public class OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp extends AbstractCleanUp {
+ private OneIfRatherThanDuplicateBlocksThatFallThroughCleanUpCore coreCleanUp= new OneIfRatherThanDuplicateBlocksThatFallThroughCleanUpCore();
+
+ public OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp(final Map<String, String> options) {
+ setOptions(options);
+ }
+
+ public OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp() {
+ }
+
+ @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 d46a6d13..d473f7e 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
@@ -180,6 +180,7 @@
public static String DuplicateCodeTabPage_CheckboxName_StrictlyEqualOrDifferent;
public static String DuplicateCodeTabPage_CheckboxName_MergeConditionalBlocks;
public static String DuplicateCodeTabPage_CheckboxName_ControlFlowMerge;
+ public static String DuplicateCodeTabPage_CheckboxName_OneIfRatherThanDuplicateBlocksThatFallThrough;
public static String DuplicateCodeTabPage_CheckboxName_RedundantFallingThroughBlockEnd;
public static String DuplicateCodeTabPage_CheckboxName_RedundantIfCondition;
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 4eedd10..107645f 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
@@ -156,6 +156,7 @@
DuplicateCodeTabPage_CheckboxName_StrictlyEqualOrDifferent=Use '==' or '^' on booleans
DuplicateCodeTabPage_CheckboxName_MergeConditionalBlocks=Merge &conditions of if/else if/else that have the same blocks
DuplicateCodeTabPage_CheckboxName_ControlFlowMerge=Pull down common code from if/else statement
+DuplicateCodeTabPage_CheckboxName_OneIfRatherThanDuplicateBlocksThatFallThrough=Single 'if' statement rather than duplicate blocks that fall through
DuplicateCodeTabPage_CheckboxName_RedundantFallingThroughBlockEnd=Remove redundant end of block with &jump statement
DuplicateCodeTabPage_CheckboxName_RedundantIfCondition=R&emove redundant if condition
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/DuplicateCodeTabPage.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/DuplicateCodeTabPage.java
index a60a314..62c1108 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/DuplicateCodeTabPage.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/DuplicateCodeTabPage.java
@@ -23,6 +23,7 @@
import org.eclipse.jdt.internal.ui.fix.AbstractCleanUp;
import org.eclipse.jdt.internal.ui.fix.ControlFlowMergeCleanUp;
import org.eclipse.jdt.internal.ui.fix.MergeConditionalBlocksCleanUp;
+import org.eclipse.jdt.internal.ui.fix.OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp;
import org.eclipse.jdt.internal.ui.fix.OperandFactorizationCleanUp;
import org.eclipse.jdt.internal.ui.fix.RedundantFallingThroughBlockEndCleanUp;
import org.eclipse.jdt.internal.ui.fix.RedundantIfConditionCleanUp;
@@ -40,6 +41,7 @@
new StrictlyEqualOrDifferentCleanUp(values),
new MergeConditionalBlocksCleanUp(values),
new ControlFlowMergeCleanUp(values),
+ new OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp(values),
new RedundantFallingThroughBlockEndCleanUp(values),
new RedundantIfConditionCleanUp(values)
};
@@ -64,6 +66,9 @@
final CheckboxPreference controlFlowMerge= createCheckboxPref(duplicateGroup, numColumns, CleanUpMessages.DuplicateCodeTabPage_CheckboxName_ControlFlowMerge, CleanUpConstants.CONTROLFLOW_MERGE, CleanUpModifyDialog.FALSE_TRUE);
registerPreference(controlFlowMerge);
+ final CheckboxPreference oneIfRatherThanDuplicateBlocksThatFallThrough= createCheckboxPref(duplicateGroup, numColumns, CleanUpMessages.DuplicateCodeTabPage_CheckboxName_OneIfRatherThanDuplicateBlocksThatFallThrough, CleanUpConstants.ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH, CleanUpModifyDialog.FALSE_TRUE);
+ registerPreference(oneIfRatherThanDuplicateBlocksThatFallThrough);
+
final CheckboxPreference redundantFallingThroughBlockEnd= createCheckboxPref(duplicateGroup, numColumns, CleanUpMessages.DuplicateCodeTabPage_CheckboxName_RedundantFallingThroughBlockEnd, CleanUpConstants.REDUNDANT_FALLING_THROUGH_BLOCK_END, CleanUpModifyDialog.FALSE_TRUE);
registerPreference(redundantFallingThroughBlockEnd);
