Add logic to useless return/continue cleanups to prevent logic change

- add logic to UselessContinueCleanUp and UselessReturnCleanUp so they do not remove a return or continue statement if it is in an if then and there is an else with a single statement and the user has also specified to reduce indentation - add new test to CleanUpTest - fixes #1638
eclipse-jdt · Sep 11, 2024 · 4896983 · 4896983
1 parent e82cd2c
commit 4896983
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 2 deletions.
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
@@ -17109,6 +17109,72 @@ public void doNotRemoveContinueWithFollowingCode(List<String> texts, boolean isV
 		assertRefactoringHasNoChange(new ICompilationUnit[] { cu });
 	}
 
+	@Test
+	public void testIssue1638() throws Exception {
+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+		String input= """
+			package test1;
+
+			public class E1 {
+			    public void doNotRemoveUselessReturn(boolean arg) {
+				    if (arg) {
+					    return;
+					} else {
+						System.out.println("here");
+					}
+			    }
+
+			    public void doNotRemoveUselessContinue(boolean arg) {
+					while (arg) {
+						arg = bar();
+						if (arg) {
+							continue;
+						} else {
+							System.out.println("Hello World");
+						}
+					}
+			    }
+
+				public boolean bar() {
+					return true;
+				}
+			}
+			""";
+		ICompilationUnit cu= pack1.createCompilationUnit("E1.java", input, false, null);
+
+		enable(CleanUpConstants.REMOVE_USELESS_CONTINUE);
+		enable(CleanUpConstants.REMOVE_USELESS_RETURN);
+		enable(CleanUpConstants.REDUCE_INDENTATION);
+
+		String output= """
+			package test1;
+
+			public class E1 {
+			    public void doNotRemoveUselessReturn(boolean arg) {
+				    if (arg) {
+					    return;
+					}
+			        System.out.println("here");
+			    }
+
+			    public void doNotRemoveUselessContinue(boolean arg) {
+					while (arg) {
+						arg = bar();
+						if (arg) {
+							continue;
+						}
+			            System.out.println("Hello World");
+					}
+			    }
+
+				public boolean bar() {
+					return true;
+				}
+			}
+			""";
+		assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { output });
+	}
+
 	@Test
 	public void testUnloopedWhile() throws Exception {
 		// Given

diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/UselessContinueCleanUp.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/UselessContinueCleanUp.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2020 Fabrice TIERCELIN and others.
+ * Copyright (c) 2020, 2024 Fabrice TIERCELIN and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -150,6 +150,16 @@ private boolean isLastStatementInLoop(final Statement node) {
 							return true;
 						}
 
+						if (parent instanceof IfStatement ifStatement && ifStatement.getElseStatement() != null
+								&& node.getLocationInParent() != IfStatement.ELSE_STATEMENT_PROPERTY) {
+							if (isEnabled(CleanUpConstants.REDUCE_INDENTATION)) {
+								Statement elseStatement= ifStatement.getElseStatement();
+								if (!(elseStatement instanceof Block elseBlock) || elseBlock.statements().size() == 1) {
+									return false;
+								}
+							}
+						}
+
 						if (parent instanceof MethodDeclaration) {
 							return false;
 						}

diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/UselessReturnCleanUp.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/UselessReturnCleanUp.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2020 Fabrice TIERCELIN and others.
+ * Copyright (c) 2020, 2024 Fabrice TIERCELIN and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -149,6 +149,16 @@ private boolean isLastStatement(final Statement node) {
 							return false;
 						}
 
+						if (parent instanceof IfStatement ifStatement && ifStatement.getElseStatement() != null
+								&& node.getLocationInParent() != IfStatement.ELSE_STATEMENT_PROPERTY) {
+							if (isEnabled(CleanUpConstants.REDUCE_INDENTATION)) {
+								Statement elseStatement= ifStatement.getElseStatement();
+								if (!(elseStatement instanceof Block elseBlock) || elseBlock.statements().size() == 1) {
+									return false;
+								}
+							}
+						}
+
 						if (parent instanceof Statement) {
 							return isLastStatement((Statement) parent);
 						}