Warn about a pull up method overriding calls to super class method (#…

…1682)

- add logic ot PullUpRefactoringProcessor to check if pulling up a
  method may override existing calls to a super class method of the
  same name and change program semantics
- add new test to PullUpTests
- fixes #1677

org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/refactoring.properties

@@ -663,7 +663,7 @@ PullUpRefactoring_calculating_required=Computing required members...
 PullUpRefactoring_category_description=Changes to pull up members
 PullUpRefactoring_Field_declared_in_class=Field ''{0}'' is declared in class ''{1}''. Moving it may result in changed program semantics.
 PullUpRefactoring_add_override_annotation=Add '@Override' annotation
-PullUpRefactoring_Method_declared_in_class=Method ''{0}'' is declared in class ''{1}''. Moving it may result in changed program semantics.
+PullUpRefactoring_Method_declared_in_class=Pulling up ''{1}'' up may override existing calls to ''{2}'' and change program semantics.
 PullUpRefactoring_field_cannot_be_accessed=Field ''{0}'' cannot be accessed from ''{1}''
 PullUpRefactoring_method_cannot_be_accessed=Method ''{0}'' cannot be accessed from ''{1}''
 PullUpRefactoring_Type_declared_in_class=Type ''{0}'' is declared in class ''{1}''. Moving it may result in changed program semantics.

org.eclipse.jdt.core.manipulation/refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/PullUpRefactoringProcessor.java

@@ -804,6 +804,9 @@ private static void upgradeMap(final Map<IMember, Set<IMember>> result, final Ma
 	/** The cached set of skipped supertypes */
 	private Set<IType> fCachedSkippedSuperTypes;
 
+	/** The cached set of all supertypes */
+	private Set<IType> fCachedSuperTypes;
+
 	/** Whether an argument has been added to handle this references in pulled up methods */
 	private boolean fArgumentAddedToHandleThis;
 
@@ -1160,7 +1163,7 @@ private void checkFieldTypes(final IProgressMonitor monitor, final RefactoringSt
 	@Override
 	public RefactoringStatus checkFinalConditions(final IProgressMonitor monitor, final CheckConditionsContext context) throws CoreException, OperationCanceledException {
 		try {
-			SubMonitor subMonitor= SubMonitor.convert(monitor, RefactoringCoreMessages.PullUpRefactoring_checking, 13);
+			SubMonitor subMonitor= SubMonitor.convert(monitor, RefactoringCoreMessages.PullUpRefactoring_checking, 14);
 			clearCaches();
 
 			final RefactoringStatus result= new RefactoringStatus();
@@ -1177,6 +1180,7 @@ public RefactoringStatus checkFinalConditions(final IProgressMonitor monitor, fi
 			result.merge(checkAccesses(subMonitor.newChild(1)));
 			result.merge(checkMembersInTypeAndAllSubtypes(subMonitor.newChild(2)));
 			result.merge(checkIfSkippingOverElements(subMonitor.newChild(1)));
+			result.merge(checkIfOverridingSuperClass(subMonitor.newChild(1)));
 			if (monitor.isCanceled())
 				throw new OperationCanceledException();
 			if (!JdtFlags.isAbstract(getDestinationType()) && getAbstractMethods().length > 0)
@@ -1385,7 +1389,7 @@ private RefactoringStatus checkIfMethodDeclaredIn(final IMethod iMethod, final I
 		final IMethod methodInType= JavaModelUtil.findMethod(iMethod.getElementName(), iMethod.getParameterTypes(), iMethod.isConstructor(), type);
 		if (methodInType == null || !methodInType.exists())
 			return null;
-		final String[] keys= { JavaElementLabelsCore.getTextLabel(methodInType, JavaElementLabelsCore.ALL_FULLY_QUALIFIED), JavaElementLabelsCore.getTextLabel(type, JavaElementLabelsCore.ALL_FULLY_QUALIFIED)};
+		final String[] keys= { JavaElementLabelsCore.getTextLabel(methodInType, JavaElementLabelsCore.ALL_FULLY_QUALIFIED), iMethod.getElementName(), JavaElementLabelsCore.getTextLabel(methodInType, JavaElementLabelsCore.ALL_FULLY_QUALIFIED) };
 		final String msg= Messages.format(RefactoringCoreMessages.PullUpRefactoring_Method_declared_in_class, keys);
 		final RefactoringStatusContext context= JavaStatusContext.create(methodInType);
 		return RefactoringStatus.createWarningStatus(msg, context);
@@ -1408,6 +1412,23 @@ private RefactoringStatus checkIfSkippingOverElements(final IProgressMonitor mon
 		}
 	}
 
+	private RefactoringStatus checkIfOverridingSuperClass(final IProgressMonitor monitor) throws JavaModelException {
+		SubMonitor subMonitor= SubMonitor.convert(monitor, RefactoringCoreMessages.PullUpRefactoring_checking, 1);
+		try {
+			final Set<IType> superTypeSet= getAllSuperTypes(subMonitor.newChild(1));
+			final IType[] superTypes= superTypeSet.toArray(new IType[superTypeSet.size()]);
+			final RefactoringStatus result= new RefactoringStatus();
+			for (final IMember element : fMembersToMove) {
+				for (IType element2 : superTypes) {
+					result.merge(checkIfDeclaredIn(element, element2));
+				}
+			}
+			return result;
+		} finally {
+			monitor.done();
+		}
+
+	}
 	private RefactoringStatus checkIfTypeDeclaredIn(final IType iType, final IType type) {
 		final IType typeInType= type.getType(iType.getElementName());
 		if (!typeInType.exists())
@@ -2284,6 +2305,26 @@ private Set<IType> getSkippedSuperTypes(final IProgressMonitor monitor) throws J
 		}
 	}
 
+	// All super classes of the destination
+	private Set<IType> getAllSuperTypes(final IProgressMonitor monitor) throws JavaModelException {
+		SubMonitor subMonitor= SubMonitor.convert(monitor, RefactoringCoreMessages.PullUpRefactoring_checking, 1);
+		try {
+			if (fCachedSuperTypes != null)
+				return fCachedSkippedSuperTypes;
+			final ITypeHierarchy hierarchy= getDestinationTypeHierarchy(subMonitor.newChild(1));
+			fCachedSuperTypes= new HashSet<>(2);
+			IType current= hierarchy.getSuperclass(getDeclaringType());
+			while (current != null && !current.getFullyQualifiedName().equals("java.lang.Object")) { //$NON-NLS-1$
+				fCachedSuperTypes.add(current);
+				current= hierarchy.getSuperclass(current);
+			}
+			return fCachedSuperTypes;
+		} finally {
+			monitor.done();
+		}
+
+	}
+
 	private RefactoringStatus initialize(final JavaRefactoringArguments extended) {
 		String handle= extended.getAttribute(JavaRefactoringDescriptorUtil.ATTRIBUTE_INPUT);
 		if (handle != null) {

org.eclipse.jdt.ui.tests.refactoring/resources/PullUp/testFail35/in/A.java

@@ -0,0 +1,19 @@
+package p;
+
+class A {
+	void method() {
+		System.out.println("ParentClass method");
+	}
+}
+
+class B extends SubClass {
+	void method() {
+		System.out.println("Class B method");
+	}
+}
+
+class SubClass extends A {
+	void test() {
+		method();
+	}
+}

org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/PullUpTests.java

@@ -1857,6 +1857,11 @@ public void testFail34() throws Exception {
 		helper2(new String[] { "foo" }, new String[][] { new String[0] }, true, false, 0);
 	}
 
+	@Test
+	public void testFail35() throws Exception {
+		helper2(new String[] { "method" }, new String[][] { new String[0] }, true, false, 0);
+	}
+
 	//----------------------------------------------------------
 	@Test
 	public void testField0() throws Exception {