pull up conflicts: do not warn if moved method would use super instead (IDEA-56212)

2026-09-27 10:03:11 +07:00 · 2010-07-05 14:55:32 +04:00
parent 17a9e105a8
commit 862c964a70
16 changed files with 208 additions and 1 deletions
@@ -25,7 +25,10 @@
 package com.intellij.refactoring.memberPullUp;

 import com.intellij.psi.*;
+import com.intellij.psi.search.LocalSearchScope;
 import com.intellij.psi.search.searches.ClassInheritorsSearch;
+import com.intellij.psi.search.searches.OverridingMethodsSearch;
+import com.intellij.psi.search.searches.SuperMethodsSearch;
 import com.intellij.psi.util.MethodSignature;
 import com.intellij.psi.util.MethodSignatureUtil;
 import com.intellij.psi.util.PsiUtil;
@@ -84,7 +87,17 @@ public class PullUpConflictsUtil {
      }
    }
    final MultiMap<PsiElement, String> conflicts = new MultiMap<PsiElement, String>();
-    RefactoringConflictsUtil.analyzeAccessibilityConflicts(movedMembers, superClass, conflicts, null, targetRepresentativeElement, abstractMethods);
+    final Set<PsiMethod> abstrMethods = new HashSet<PsiMethod>(abstractMethods);
+    if (superClass != null) {
+      for (PsiMethod method : subclass.getMethods()) {
+        if (!movedMembers.contains(method) && !method.hasModifierProperty(PsiModifier.PRIVATE)) {
+          if (method.findSuperMethods(superClass).length > 0) {
+            abstrMethods.add(method);
+          }
+        }
+      }
+    }
+    RefactoringConflictsUtil.analyzeAccessibilityConflicts(movedMembers, superClass, conflicts, null, targetRepresentativeElement, abstrMethods);
    if (superClass != null) {
      checkSuperclassMembers(superClass, infos, conflicts);
      if (isInterfaceTarget) {
@@ -0,0 +1,5 @@
+package a;
+import b.B;
+public class A extends B {
+    void foo(){}
+}
@@ -0,0 +1,6 @@
+package b;
+public class B {
+    void method2Move() {
+      foo();
+    }
+}
@@ -0,0 +1,8 @@
+package a;
+import b.B;
+public class A extends B {
+  void method2Move() {
+    foo();
+  }
+  void foo(){}
+}
@@ -0,0 +1,2 @@
+package b;
+public class B {}
@@ -0,0 +1,5 @@
+package a;
+import b.B;
+public class A extends B {
+    protected void foo(){}
+}
@@ -0,0 +1,8 @@
+package b;
+public class B {
+  protected void foo(){}
+
+    void method2Move() {
+      foo();
+    }
+}
@@ -0,0 +1,8 @@
+package a;
+import b.B;
+public class A extends B {
+  void method2Move() {
+    foo();
+  }
+  protected void foo(){}
+}
@@ -0,0 +1,4 @@
+package b;
+public class B {
+  protected void foo(){}
+}
@@ -0,0 +1,5 @@
+package a;
+import b.B;
+public class A extends B {
+    protected void foo(){}
+}
@@ -0,0 +1,7 @@
+package b;
+import c.C;
+public class B extends C {
+    void method2Move() {
+      foo();
+    }
+}
@@ -0,0 +1,4 @@
+package c;
+public class C {
+  protected void foo(){}
+}
@@ -0,0 +1,8 @@
+package a;
+import b.B;
+public class A extends B {
+  void method2Move() {
+    foo();
+  }
+  protected void foo(){}
+}
@@ -0,0 +1,4 @@
+package b;
+import c.C;
+public class B extends C {
+}
@@ -0,0 +1,4 @@
+package c;
+public class C {
+  protected void foo(){}
+}
@@ -0,0 +1,116 @@
+/*
+ * Copyright 2000-2010 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/*
+ * User: anna
+ * Date: 20-Aug-2008
+ */
+package com.intellij.refactoring;
+
+import com.intellij.JavaTestUtil;
+import com.intellij.openapi.projectRoots.Sdk;
+import com.intellij.openapi.projectRoots.impl.JavaSdkImpl;
+import com.intellij.openapi.vfs.VirtualFile;
+import com.intellij.psi.PsiClass;
+import com.intellij.psi.PsiElement;
+import com.intellij.psi.PsiField;
+import com.intellij.psi.PsiMethod;
+import com.intellij.psi.search.GlobalSearchScope;
+import com.intellij.refactoring.memberPullUp.PullUpConflictsUtil;
+import com.intellij.refactoring.memberPullUp.PullUpHelper;
+import com.intellij.refactoring.memberPushDown.PushDownProcessor;
+import com.intellij.refactoring.util.DocCommentPolicy;
+import com.intellij.refactoring.util.classMembers.InterfaceContainmentVerifier;
+import com.intellij.refactoring.util.classMembers.MemberInfo;
+import com.intellij.util.containers.MultiMap;
+import junit.framework.Assert;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+
+//pull first method from class a.A to class b.B
+public class PullUpMultifileTest extends MultiFileTestCase {
+  protected String getTestRoot() {
+    return "/refactoring/pullUp/";
+  }
+
+  @Override
+  protected String getTestDataPath() {
+    return JavaTestUtil.getJavaTestDataPath();
+  }
+
+  protected Sdk getTestProjectJdk() {
+    return JavaSdkImpl.getMockJdk15("java 1.5");
+  }
+
+
+  private void doTest(final String... conflicts) throws Exception {
+    final MultiMap<PsiElement, String> conflictsMap = new MultiMap<PsiElement, String>();
+    doTest(new PerformAction() {
+      public void performAction(final VirtualFile rootDir, final VirtualFile rootAfter) throws Exception {
+        final PsiClass srcClass = myJavaFacade.findClass("a.A", GlobalSearchScope.allScope(myProject));
+        assertTrue("Source class not found", srcClass != null);
+
+        final PsiClass targetClass = myJavaFacade.findClass("b.B", GlobalSearchScope.allScope(myProject));
+        assertTrue("Target class not found", targetClass != null);
+
+        final PsiMethod[] methods = srcClass.getMethods();
+        assertTrue("No methods found", methods.length > 0);
+        final MemberInfo[] membersToMove = new MemberInfo[1];
+        final MemberInfo memberInfo = new MemberInfo(methods[0]);
+        memberInfo.setChecked(true);
+        membersToMove[0] = memberInfo;
+
+        conflictsMap.putAllValues(
+          PullUpConflictsUtil.checkConflicts(membersToMove, srcClass, targetClass, null, null, new InterfaceContainmentVerifier() {
+            public boolean checkedInterfacesContain(PsiMethod psiMethod) {
+              return PullUpHelper.checkedInterfacesContain(Arrays.asList(membersToMove), psiMethod);
+            }
+          }));
+
+        new PullUpHelper(srcClass, targetClass, membersToMove, new DocCommentPolicy(DocCommentPolicy.ASIS)).run();
+      }
+    });
+
+    if (conflicts.length != 0 && conflictsMap.isEmpty()) {
+      fail("Conflict was not detected");
+    }
+    final HashSet<String> values = new HashSet<String>(conflictsMap.values());
+    final HashSet<String> expected = new HashSet<String>(Arrays.asList(conflicts));
+
+    assertEquals(expected.size(), values.size());
+    for (String value : values) {
+      if (!expected.contains(value)) {
+        fail("Conflict: " + value + " is unexpectedly reported");
+      }
+    }
+  }
+
+
+  public void testInaccessible() throws Exception {
+    doTest("Method <b><code>A.foo()</code></b> is package local and will not be accessible from method <b><code>method2Move()</code></b>.",
+           "Method <b><code>method2Move()</code></b> uses method <b><code>A.foo()</code></b>, which is not moved to the superclass");
+  }
+
+  public void testReuseSuperMethod() throws Exception {
+    doTest();
+  }
+
+   public void testReuseSuperSuperMethod() throws Exception {
+    doTest();
+  }
+}