Fix refactor method false error regarding ambiguous return value (#673)

- fixes #671 - add BreakStatement method in InputFlowAnalyzer.LoopReentranceVisitor to ignore break statements that precede the selected code - fix binding equality check in ExtractMethodAnalyzer - add new test to ExtractMethodTests
eclipse-jdt · Jul 20, 2023 · ab84e5d · ab84e5d
1 parent 6c6af29
commit ab84e5d
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 4 deletions.
diff --git a/...re extension/org/eclipse/jdt/internal/corext/refactoring/code/flow/InputFlowAnalyzer.java b/...re extension/org/eclipse/jdt/internal/corext/refactoring/code/flow/InputFlowAnalyzer.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2019 IBM Corporation and others.
+ * Copyright (c) 2000, 2023 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -27,6 +27,7 @@
 import org.eclipse.jdt.core.dom.ASTNode;
 import org.eclipse.jdt.core.dom.AbstractTypeDeclaration;
 import org.eclipse.jdt.core.dom.BodyDeclaration;
+import org.eclipse.jdt.core.dom.BreakStatement;
 import org.eclipse.jdt.core.dom.ConditionalExpression;
 import org.eclipse.jdt.core.dom.DoStatement;
 import org.eclipse.jdt.core.dom.EnhancedForStatement;
@@ -69,6 +70,12 @@ public void process(ASTNode node) {
 			}
 		}
 		@Override
+		public void endVisit(BreakStatement node) {
+			if (node.getStartPosition() + node.getLength() <= fSelection.getExclusiveEnd())
+				return;
+			super.endVisit(node);
+		}
+		@Override
 		public void endVisit(DoStatement node) {
 			if (skipNode(node))
 				return;

diff --git a/...resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_in/A_testIssue671.java b/...resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_in/A_testIssue671.java
@@ -0,0 +1,28 @@
+package validSelection_in;
+
+public class A_testIssue671 {
+	protected void foo() {
+		while (true) {
+			List<String> line = List.of();
+			if (line == null) {
+				break;
+			}
+			/*[*/// comment
+			MyType plcTableType;
+			String plcTableTypeText = "";
+			if (plcTableTypeText.isEmpty()) {
+				plcTableType = null;
+			} else {
+				plcTableType = new MyType();
+			}
+			// comment 2/*]*/
+
+			if (plcTableType == null) {
+				return;
+			}
+		}
+	}
+
+	public static class MyType {
+	}
+}
diff --git a/...esources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_out/A_testIssue671.java b/...esources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_out/A_testIssue671.java
@@ -0,0 +1,33 @@
+package validSelection_out;
+
+public class A_testIssue671 {
+	protected void foo() {
+		while (true) {
+			List<String> line = List.of();
+			if (line == null) {
+				break;
+			}
+			MyType plcTableType = extracted();
+
+			if (plcTableType == null) {
+				return;
+			}
+		}
+	}
+
+	protected MyType extracted() {
+		/*[*/// comment
+		MyType plcTableType;
+		String plcTableTypeText = "";
+		if (plcTableTypeText.isEmpty()) {
+			plcTableType = null;
+		} else {
+			plcTableType = new MyType();
+		}
+		// comment 2/*]*/
+		return plcTableType;
+	}
+
+	public static class MyType {
+	}
+}
diff --git a/...tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractMethodTests.java b/...tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractMethodTests.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2021 IBM Corporation and others.
+ * Copyright (c) 2000, 2023 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -2675,4 +2675,10 @@ public void test2004() throws Exception {
 	public void test2005() throws Exception {
 		wikiTest();
 	}
+
+	//--- Test Issues
+	@Test
+	public void testIssue671() throws Exception {
+		validSelectionTestChecked();
+	}
 }
diff --git a/...e refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodAnalyzer.java b/...e refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodAnalyzer.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2021 IBM Corporation and others.
+ * Copyright (c) 2000, 2023 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -646,7 +646,7 @@ private void computeOutput(RefactoringStatus status) {
 		outer: for (int i= 0; i < returnValues.length && localReads.size() < returnValues.length; i++) {
 			IVariableBinding binding= returnValues[i];
 			for (IVariableBinding read : reads) {
-				if (read == binding) {
+				if (read.isEqualTo(binding)) {
 					localReads.add(binding);
 					fReturnValue= binding;
 					continue outer;