Bug 72380 - [generalize type] introduces duplicate methodsI20200803-1800I20200803-0210I20200801-1800I20200801-0010I20200731-2040I20200731-1800

- Add check for overloaded methods
- Add test case

Change-Id: I4d0c4f0149eb7296e7d9c6b98d828df67feb3d91
Signed-off-by: Kenneth Styrberg <kenneth@kean.nu>

4 files changed, 94 insertions, 5 deletions

diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParameterTypeWithOverrideMethod_in.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParameterTypeWithOverrideMethod_in.java
new file mode 100644
index 0000000000..b7c9a1a41f
--- /dev/null
+++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParameterTypeWithOverrideMethod_in.java
@@ -0,0 +1,16 @@
+class A_testParameterTypeWithOverrideMethod_in {
+	static class E {
+		public void bar(String s1, Float g, String s) {
+			g.foo();
+		}
+	}
+
+	static class F extends E {
+		@Override
+		public void bar(String s1, Float g, String s) {
+		}
+
+		public void bar(String s1, Object i, String s) {
+		}
+	}
+}
diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParameterTypeWithOverrideMethod_out.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParameterTypeWithOverrideMethod_out.java
new file mode 100644
index 0000000000..2ee60cb3f1
--- /dev/null
+++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParameterTypeWithOverrideMethod_out.java
@@ -0,0 +1,16 @@
+class A_testParameterTypeWithOverrideMethod_in {
+	static class E {
+		public void bar(String s1, Number g, String s) {
+			g.foo();
+		}
+	}
+
+	static class F extends E {
+		@Override
+		public void bar(String s1, Number g, String s) {
+		}
+
+		public void bar(String s1, Object i, String s) {
+		}
+	}
+}
diff --git a/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ChangeTypeRefactoringTests.java b/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ChangeTypeRefactoringTests.java
index f243a720a6..dd50e17e70 100644
--- a/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ChangeTypeRefactoringTests.java
+++ b/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ChangeTypeRefactoringTests.java
@@ -266,6 +266,16 @@ public class ChangeTypeRefactoringTests extends GenericRefactoringTest {
 		Assert.assertTrue(types.contains("java.util.List"));
 		Assert.assertTrue(types.contains("java.util.Collection"));
 	}
+
+	@Test
+	public void testParameterTypeWithOverrideMethod() throws Exception {
+		Collection<String> types= helper1(3, 36, 3, 41, "java.lang.Number").getValidTypeNames();
+		Assert.assertEquals(3, types.size());
+		Assert.assertTrue(types.contains("java.io.Serializable"));
+		Assert.assertTrue(types.contains("java.lang.Comparable<java.lang.Float>"));
+		Assert.assertTrue(types.contains("java.lang.Number"));
+		Assert.assertFalse(types.contains("java.lang.Object"));
+	}
 	@Test
 	public void testParameterDeclWithOverride() throws Exception {
 		Collection<String> types= helper1(10, 25, 10, 39, "java.util.AbstractCollection").getValidTypeNames();
diff --git a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/ChangeTypeRefactoring.java b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/ChangeTypeRefactoring.java
index 163a0f7672..240ffad5b7 100644
--- a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/ChangeTypeRefactoring.java
+++ b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/ChangeTypeRefactoring.java
@@ -1167,20 +1167,22 @@ public class ChangeTypeRefactoring extends Refactoring {
 	 * @throws JavaModelException
 	 */
 	private Collection<ITypeBinding> computeValidTypes(ITypeBinding originalType,
-													Collection<ConstraintVariable> relevantVars,
-													Collection<ITypeConstraint> relevantConstraints,
-													IProgressMonitor pm) throws JavaModelException {
+			Collection<ConstraintVariable> relevantVars,
+			Collection<ITypeConstraint> relevantConstraints,
+			IProgressMonitor pm) throws JavaModelException {
 
 		Collection<ITypeBinding> result= new HashSet<>();
 
-		Collection<ITypeBinding> allTypes = new HashSet<>();
+		Collection<ITypeBinding> allTypes= new HashSet<>();
 		allTypes.addAll(getAllSuperTypes(originalType));
 
 		pm.beginTask(RefactoringCoreMessages.ChangeTypeRefactoring_analyzingMessage, allTypes.size());
 
 		for (ITypeBinding type : allTypes) {
 			if (isValid(type, relevantVars, relevantConstraints, new SubProgressMonitor(pm, 1))) {
-				result.add(type);
+				if (checkTypeParameterConflict(relevantVars, type)) {
+					result.add(type);
+				}
 			}
 		}
 		// "changing" to the original type is a no-op
@@ -1194,6 +1196,51 @@ public class ChangeTypeRefactoring extends Refactoring {
 		return result;
 	}
 
+	/*
+	 * check if replacement type is conflicting with existing type usage for classes with overridden
+	 * method.
+	 *
+	 * @return TRUE if no conflict found
+	 */
+	private boolean checkTypeParameterConflict(Collection<ConstraintVariable> relevantVars, ITypeBinding replaceTypeTo) {
+		for (ConstraintVariable constraintVariable : relevantVars) {
+			if (constraintVariable instanceof ParameterTypeVariable) {
+				ParameterTypeVariable parameterTypeVariable= (ParameterTypeVariable) constraintVariable;
+				ITypeBinding declaringClass= parameterTypeVariable.getMethodBinding().getDeclaringClass();
+				ITypeBinding[] parameterTypeVariableTypes= parameterTypeVariable.getMethodBinding().getParameterTypes();
+				List<IMethodBinding> possibleConflictMethods= new ArrayList<>();
+				for (IMethodBinding declaredMethod : declaringClass.getDeclaredMethods()) {
+					if (declaredMethod.getName().equals(parameterTypeVariable.getMethodBinding().getName()) &&
+							declaredMethod.getParameterTypes().length == parameterTypeVariableTypes.length) {
+						// if method name and number of parameters match it might be a conflict
+						possibleConflictMethods.add(declaredMethod);
+					}
+				}
+				if (possibleConflictMethods.size() < 2) {
+					continue; // no conflicting methods
+				}
+				mLoop: for (IMethodBinding declaredMethod : possibleConflictMethods) {
+					ITypeBinding[] parameterTypes= declaredMethod.getParameterTypes();
+					/* check if other parameter Types are equal, if not this method is ok */
+					for (int i= 0; i < parameterTypes.length; i++) {
+						if (i == fParamIndex) {
+							continue;
+						}
+						if (!Bindings.equals(parameterTypes[i], parameterTypeVariableTypes[i])) {
+							continue mLoop;
+						}
+					}
+					// if all method parameter Types are equal then we have a conflict with the Type we want to replace to
+					// so we return false to remove it from allowed types
+					if (Bindings.equals(parameterTypes[fParamIndex], replaceTypeTo)) {
+						return false;
+					}
+				}
+			}
+		}
+		return true;
+	}
+
 	/**
 	 * Determines if a given type satisfies a set of type constraints.
 	 * @param type