From c9f72daaff0e26bdfc7e740322b5e7edc756a1bb Mon Sep 17 00:00:00 2001
From: "Anna.Kozlova" <anna.kozlova@jetbrains.com>
Date: Wed, 4 May 2016 20:10:50 +0200
Subject: [PATCH] push down: preserve @Override if keep abstract was selected
 (IDEA-155600)

---
 .../memberPushDown/JavaPushDownDelegate.java  | 25 ++++++++-----------
 ...eserveOverrideAnnotationAfterConflict.java | 18 +++++++++++++
 ...OverrideAnnotationAfterConflict_after.java | 16 ++++++++++++
 .../intellij/refactoring/PushDownTest.java    | 13 +++++++++-
 4 files changed, 56 insertions(+), 16 deletions(-)
 create mode 100644 java/java-tests/testData/refactoring/pushDown/PreserveOverrideAnnotationAfterConflict.java
 create mode 100644 java/java-tests/testData/refactoring/pushDown/PreserveOverrideAnnotationAfterConflict_after.java

diff --git a/java/java-impl/src/com/intellij/refactoring/memberPushDown/JavaPushDownDelegate.java b/java/java-impl/src/com/intellij/refactoring/memberPushDown/JavaPushDownDelegate.java
index ac29992e1116..f103485b083e 100644
--- a/java/java-impl/src/com/intellij/refactoring/memberPushDown/JavaPushDownDelegate.java
+++ b/java/java-impl/src/com/intellij/refactoring/memberPushDown/JavaPushDownDelegate.java
@@ -229,28 +229,20 @@ public class JavaPushDownDelegate extends PushDownDelegate<MemberInfo, PsiMember
           }
 
           if (memberInfo.isToAbstract()) {
-            if (sourceClass.isInterface()) {
-              if (oldMethod.hasModifierProperty(PsiModifier.DEFAULT)) {
-                PsiUtil.setModifierProperty(oldMethod, PsiModifier.DEFAULT, false);
-                RefactoringUtil.makeMethodAbstract(sourceClass, oldMethod);
-              }
-            }
-            else {
-              if (newMember.hasModifierProperty(PsiModifier.PRIVATE)) {
-                PsiUtil.setModifierProperty(newMember, PsiModifier.PROTECTED, true);
-              }
+            if (newMember.hasModifierProperty(PsiModifier.PRIVATE)) {
+              PsiUtil.setModifierProperty(newMember, PsiModifier.PROTECTED, true);
             }
 
             pushDownData.getCommentPolicy().processNewJavaDoc(((PsiMethod)newMember).getDocComment());
-          }
-          if (memberInfo.isToAbstract()) {
             OverrideImplementUtil.annotateOnOverrideImplement((PsiMethod)newMember, targetClass, (PsiMethod)memberInfo.getMember());
           }
         }
         else { //abstract method: remove @Override
-          final PsiAnnotation annotation = AnnotationUtil.findAnnotation(methodBySignature, "java.lang.Override");
-          if (annotation != null && !leaveOverrideAnnotation(sourceClass, substitutor, method)) {
-            annotation.delete();
+          if (!memberInfo.isToAbstract()) {
+            final PsiAnnotation annotation = AnnotationUtil.findAnnotation(methodBySignature, "java.lang.Override");
+            if (annotation != null && !leaveOverrideAnnotation(sourceClass, substitutor, method)) {
+              annotation.delete();
+            }
           }
           final PsiDocComment oldDocComment = method.getDocComment();
           if (oldDocComment != null) {
@@ -318,6 +310,9 @@ public class JavaPushDownDelegate extends PushDownDelegate<MemberInfo, PsiMember
           if (method.hasModifierProperty(PsiModifier.PRIVATE)) {
             PsiUtil.setModifierProperty(method, PsiModifier.PROTECTED, true);
           }
+          if (method.hasModifierProperty(PsiModifier.DEFAULT)) {
+            PsiUtil.setModifierProperty(method, PsiModifier.DEFAULT, false);
+          }
           RefactoringUtil.makeMethodAbstract((PsiClass)pushDownData.getSourceClass(), method);
           pushDownData.getCommentPolicy().processOldJavaDoc(method.getDocComment());
         }
diff --git a/java/java-tests/testData/refactoring/pushDown/PreserveOverrideAnnotationAfterConflict.java b/java/java-tests/testData/refactoring/pushDown/PreserveOverrideAnnotationAfterConflict.java
new file mode 100644
index 000000000000..c73853adc807
--- /dev/null
+++ b/java/java-tests/testData/refactoring/pushDown/PreserveOverrideAnnotationAfterConflict.java
@@ -0,0 +1,18 @@
+interface Test {
+  /**
+   * some javadoc
+   */
+  default void foo() {
+    System.out.println("I");
+  }
+}
+
+class C implements Test {
+  /**
+   * another javadoc
+   */
+  @Override
+  public void foo() {
+    System.out.println("C");
+  }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/pushDown/PreserveOverrideAnnotationAfterConflict_after.java b/java/java-tests/testData/refactoring/pushDown/PreserveOverrideAnnotationAfterConflict_after.java
new file mode 100644
index 000000000000..bb0fcb1116e4
--- /dev/null
+++ b/java/java-tests/testData/refactoring/pushDown/PreserveOverrideAnnotationAfterConflict_after.java
@@ -0,0 +1,16 @@
+interface Test {
+  /**
+   * some javadoc
+   */
+  void foo();
+}
+
+class C implements Test {
+  /**
+   * another javadoc
+   */
+  @Override
+  public void foo() {
+    System.out.println("C");
+  }
+}
\ No newline at end of file
diff --git a/java/java-tests/testSrc/com/intellij/refactoring/PushDownTest.java b/java/java-tests/testSrc/com/intellij/refactoring/PushDownTest.java
index 5084773e67b5..84648a70edbc 100644
--- a/java/java-tests/testSrc/com/intellij/refactoring/PushDownTest.java
+++ b/java/java-tests/testSrc/com/intellij/refactoring/PushDownTest.java
@@ -95,6 +95,10 @@ public class PushDownTest extends LightRefactoringTestCase {
     doTestImplements(true);
   }
 
+  public void testPreserveOverrideAnnotationAfterConflict() throws Exception {
+    doTestImplements(true, true);
+  }
+
   private void doTest() {
     doTest(false);
   }
@@ -135,7 +139,7 @@ public class PushDownTest extends LightRefactoringTestCase {
                           new DocCommentPolicy(DocCommentPolicy.ASIS)) {
       @Override
       protected boolean showConflicts(@NotNull MultiMap<PsiElement, String> conflicts, UsageInfo[] usages) {
-        if (failure ? conflicts.isEmpty() : !conflicts.isEmpty()) {
+        if (failure == conflicts.isEmpty()) {
           fail(failure ? "Conflict was not detected" : "False conflict was detected");
         }
         return true;
@@ -150,6 +154,10 @@ public class PushDownTest extends LightRefactoringTestCase {
   }
 
   private void doTestImplements(boolean toAbstract) {
+    doTestImplements(toAbstract, false);
+  }
+
+  private void doTestImplements(boolean toAbstract, boolean failure) {
     configureByFile(BASE_PATH + getTestName(false) + ".java");
 
     PsiClass currentClass = JavaPsiFacade.getInstance(getProject()).findClass("Test", GlobalSearchScope.projectScope(getProject()));
@@ -166,6 +174,9 @@ public class PushDownTest extends LightRefactoringTestCase {
                           new DocCommentPolicy(DocCommentPolicy.ASIS)) {
       @Override
       protected boolean showConflicts(@NotNull MultiMap<PsiElement, String> conflicts, UsageInfo[] usages) {
+        if (failure == conflicts.isEmpty()) {
+          fail(failure ? "Conflict was not detected" : "False conflict was detected");
+        }
         return true;
       }
     }.run();