diff --git a/java/java-impl/src/com/intellij/refactoring/memberPushDown/PushDownConflicts.java b/java/java-impl/src/com/intellij/refactoring/memberPushDown/PushDownConflicts.java index f42c6a1406e0..63f28bc88c72 100644 --- a/java/java-impl/src/com/intellij/refactoring/memberPushDown/PushDownConflicts.java +++ b/java/java-impl/src/com/intellij/refactoring/memberPushDown/PushDownConflicts.java @@ -16,6 +16,7 @@ package com.intellij.refactoring.memberPushDown; import com.intellij.codeInsight.AnnotationUtil; +import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.search.searches.ReferencesSearch; import com.intellij.psi.util.InheritanceUtil; @@ -29,6 +30,7 @@ import com.intellij.refactoring.util.classMembers.MemberInfo; import com.intellij.util.containers.MultiMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Set; public class PushDownConflicts { @@ -76,6 +78,26 @@ public class PushDownConflicts { if (annotation != null && myMovedMembers.contains(LambdaUtil.getFunctionalInterfaceMethod(myClass))) { myConflicts.putValue(annotation, RefactoringBundle.message("functional.interface.broken")); } + boolean isAbstract = myClass.hasModifierProperty(PsiModifier.ABSTRACT); + for (PsiMember member : myMovedMembers) { + if (!member.hasModifierProperty(PsiModifier.STATIC) && member instanceof PsiMethod && !myAbstractMembers.contains(member)) { + Set unrelatedDefaults = new LinkedHashSet<>(); + for (PsiMethod superMethod : ((PsiMethod)member).findSuperMethods()) { + if (!isAbstract && superMethod.hasModifierProperty(PsiModifier.ABSTRACT)) { + myConflicts.putValue(member, "Non abstract " + RefactoringUIUtil.getDescription(myClass, false) + " will miss implementation of " + RefactoringUIUtil.getDescription(superMethod, false)); + break; + } + if (superMethod.hasModifierProperty(PsiModifier.DEFAULT)) { + unrelatedDefaults.add(superMethod.getContainingClass()); + if (unrelatedDefaults.size() > 1) { + myConflicts.putValue(member, CommonRefactoringUtil.capitalize(RefactoringUIUtil.getDescription(myClass, false) + " will inherit unrelated defaults from " + + StringUtil.join(unrelatedDefaults, aClass -> RefactoringUIUtil.getDescription(aClass, false)," and "))); + break; + } + } + } + } + } } public void checkTargetClassConflicts(final PsiElement targetElement, final PsiElement context) { diff --git a/java/java-tests/testData/refactoring/pushDown/ClassInheritsUnrelatedDefaultsConflict.java b/java/java-tests/testData/refactoring/pushDown/ClassInheritsUnrelatedDefaultsConflict.java new file mode 100644 index 000000000000..e03eefd6111f --- /dev/null +++ b/java/java-tests/testData/refactoring/pushDown/ClassInheritsUnrelatedDefaultsConflict.java @@ -0,0 +1,16 @@ + +interface A { + default void foo() {} +} + +interface I { + default void foo() { } +} + +class B implements I, A { + @Override + public void foo() { } +} + +class C extends B { +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/pushDown/ClassInheritsUnrelatedDefaultsConflict_after.java b/java/java-tests/testData/refactoring/pushDown/ClassInheritsUnrelatedDefaultsConflict_after.java new file mode 100644 index 000000000000..2d09fd2d646a --- /dev/null +++ b/java/java-tests/testData/refactoring/pushDown/ClassInheritsUnrelatedDefaultsConflict_after.java @@ -0,0 +1,16 @@ + +interface A { + default void foo() {} +} + +interface I { + default void foo() { } +} + +class B implements I, A { +} + +class C extends B { + @Override + public void foo() { } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/pushDown/ClassShouldBeAbstractConflict.java b/java/java-tests/testData/refactoring/pushDown/ClassShouldBeAbstractConflict.java new file mode 100644 index 000000000000..574fa95f3f42 --- /dev/null +++ b/java/java-tests/testData/refactoring/pushDown/ClassShouldBeAbstractConflict.java @@ -0,0 +1,11 @@ +abstract class A { + abstract void foo(); +} + +class B extends A { + @Override + public void foo() { } +} + +class C extends B { +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/pushDown/ClassShouldBeAbstractConflict_after.java b/java/java-tests/testData/refactoring/pushDown/ClassShouldBeAbstractConflict_after.java new file mode 100644 index 000000000000..14f487ccdf33 --- /dev/null +++ b/java/java-tests/testData/refactoring/pushDown/ClassShouldBeAbstractConflict_after.java @@ -0,0 +1,11 @@ +abstract class A { + abstract void foo(); +} + +class B extends A { +} + +class C extends B { + @Override + public void foo() { } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/PushDownTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/PushDownTest.java index 814cf33a4b1d..2547994c34ff 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/PushDownTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/PushDownTest.java @@ -27,7 +27,9 @@ import com.intellij.util.containers.MultiMap; import org.jetbrains.annotations.NotNull; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.function.Consumer; /** * @author anna @@ -107,11 +109,31 @@ public class PushDownTest extends LightRefactoringTestCase { doTest(); } + public void testClassShouldBeAbstractConflict() { + doTest(conflicts -> { + assertSameElements(conflicts.values(), Collections.singletonList("Non abstract class B will miss implementation of method foo()")); + }); + } + + public void testClassInheritsUnrelatedDefaultsConflict() { + doTest(conflicts -> { + assertSameElements(conflicts.values(), Collections.singletonList("Class B will inherit unrelated defaults from interface I and interface A")); + }); + } + private void doTest() { doTest(false); } private void doTest(final boolean failure) { + doTest(conflicts -> { + if (failure == conflicts.isEmpty()) { + fail(failure ? "Conflict was not detected" : "False conflict was detected"); + } + }); + } + + private void doTest(final Consumer> checkConflicts) { configureByFile(BASE_PATH + getTestName(false) + ".java"); final PsiElement targetElement = TargetElementUtil.findTargetElement(getEditor(), TargetElementUtil.ELEMENT_NAME_ACCEPTED); @@ -147,9 +169,7 @@ public class PushDownTest extends LightRefactoringTestCase { new DocCommentPolicy(DocCommentPolicy.ASIS)) { @Override protected boolean showConflicts(@NotNull MultiMap conflicts, UsageInfo[] usages) { - if (failure == conflicts.isEmpty()) { - fail(failure ? "Conflict was not detected" : "False conflict was detected"); - } + checkConflicts.accept(conflicts); return true; } }.run();