pull up conflicts: check package local/protected access

2026-09-18 09:34:34 +07:00 · 2010-07-05 14:55:34 +04:00
parent bf92826812
commit 2cbb2da5f6
16 changed files with 152 additions and 24 deletions
@@ -24,11 +24,10 @@
 */
 package com.intellij.refactoring.memberPullUp;

+import com.intellij.openapi.util.Comparing;
+import com.intellij.openapi.util.text.StringUtil;
 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;
@@ -108,20 +107,31 @@ public class PullUpConflictsUtil {
      }
    }
    RefactoringConflictsUtil.analyzeAccessibilityConflicts(movedMembers, superClass, conflicts, null, targetRepresentativeElement, abstrMethods);
-    if (superClass != null && movedMembers2Super) {
-      checkSuperclassMembers(superClass, infos, conflicts);
-      if (isInterfaceTarget) {
-        checkInterfaceTarget(infos, conflicts);
+    if (superClass != null) {
+      if (movedMembers2Super) {
+        checkSuperclassMembers(superClass, infos, conflicts);
+        if (isInterfaceTarget) {
+          checkInterfaceTarget(infos, conflicts);
+        }
+      } else {
+        final String qualifiedName = superClass.getQualifiedName();
+        assert qualifiedName != null;
+        if (superClass.hasModifierProperty(PsiModifier.PACKAGE_LOCAL)) {
+          if (!Comparing.strEqual(StringUtil.getPackageName(qualifiedName), targetPackage.getQualifiedName())) {
+            conflicts.putValue(superClass, RefactoringUIUtil.getDescription(superClass, true) + " won't be accessible from " +RefactoringUIUtil.getDescription(targetPackage, true));
+          }
+        }
      }
    }
    // check if moved methods use other members in the classes between Subclass and Superclass
    List<PsiElement> checkModuleConflictsList = new ArrayList<PsiElement>();
    for (PsiMember member : movedMembers) {
      if (member instanceof PsiMethod || member instanceof PsiClass && !(member instanceof PsiCompiledElement)) {
-        ConflictingUsagesOfSubClassMembers visitor =
-          new ConflictingUsagesOfSubClassMembers(member, movedMembers, abstractMethods, subclass, superClass,
+        ClassMemberReferencesVisitor visitor =
+          movedMembers2Super? new ConflictingUsagesOfSubClassMembers(member, movedMembers, abstractMethods, subclass, superClass,
                                                 superClass != null ? null : targetPackage, conflicts,
-                                                 interfaceContainmentVerifier);
+                                                 interfaceContainmentVerifier)
+                            : new ConflictingUsagesOfSuperClassMemebers(member, subclass, targetPackage, movedMembers, conflicts);
        member.accept(visitor);
      }
      checkModuleConflictsList.add(member);
@@ -205,6 +215,53 @@ public class PullUpConflictsUtil {

  }

+  private static boolean willBeMoved(PsiElement element, Set<PsiMember> movedMembers) {
+    PsiElement parent = element;
+    while (parent != null) {
+      if (movedMembers.contains(parent)) return true;
+      parent = parent.getParent();
+    }
+    return false;
+  }
+
+  private static class ConflictingUsagesOfSuperClassMemebers extends ClassMemberReferencesVisitor {
+
+    private PsiMember myMember;
+    private PsiClass mySubClass;
+    private PsiPackage myTargetPackage;
+    private Set<PsiMember> myMovedMembers;
+    private MultiMap<PsiElement, String> myConflicts;
+
+    public ConflictingUsagesOfSuperClassMemebers(PsiMember member, PsiClass aClass,
+                                                 PsiPackage targetPackage,
+                                                 Set<PsiMember> movedMembers,
+                                                 MultiMap<PsiElement, String> conflicts) {
+      super(aClass);
+      myMember = member;
+      mySubClass = aClass;
+      myTargetPackage = targetPackage;
+      myMovedMembers = movedMembers;
+      myConflicts = conflicts;
+    }
+
+    @Override
+    protected void visitClassMemberReferenceElement(PsiMember classMember, PsiJavaCodeReferenceElement classMemberReference) {
+      if (classMember != null && !willBeMoved(classMember, myMovedMembers)) {
+        final PsiClass containingClass = classMember.getContainingClass();
+        if (containingClass != null) {
+          if (!PsiUtil.isAccessibleFromPackage(classMember, myTargetPackage)) {
+            if (classMember.hasModifierProperty(PsiModifier.PACKAGE_LOCAL)) {
+              myConflicts.putValue(myMember, RefactoringUIUtil.getDescription(classMember, true) + " won't be accessible");
+            }
+            else if (classMember.hasModifierProperty(PsiModifier.PROTECTED) && !mySubClass.isInheritor(containingClass, true)) {
+              myConflicts.putValue(myMember, RefactoringUIUtil.getDescription(classMember, true) + " won't be accessible");
+            }
+          }
+        }
+      }
+    }
+  }
+
  private static class ConflictingUsagesOfSubClassMembers extends ClassMemberReferencesVisitor {
    private final PsiElement myScope;
    private final Set<PsiMember> myMovedMembers;
@@ -236,7 +293,7 @@ public class PullUpConflictsUtil {
      if (classMember != null
          && RefactoringHierarchyUtil.isMemberBetween(mySuperClass, mySubclass, classMember)) {
        if (classMember.hasModifierProperty(PsiModifier.STATIC)
-            && !willBeMoved(classMember)) {
+            && !willBeMoved(classMember, myMovedMembers)) {
          final boolean isAccessible;
          if (mySuperClass != null) {
            isAccessible = PsiUtil.isAccessible(classMember, mySuperClass, null);
@@ -257,7 +314,7 @@ public class PullUpConflictsUtil {
          }
          return;
        }
-        if (!myAbstractMethods.contains(classMember) && !willBeMoved(classMember)) {
+        if (!myAbstractMethods.contains(classMember) && !willBeMoved(classMember, myMovedMembers)) {
          if (!existsInSuperClass(classMember)) {
            String message = RefactoringBundle.message("0.uses.1.which.is.not.moved.to.the.superclass",
                                                       RefactoringUIUtil.getDescription(myScope, false),
@@ -269,14 +326,7 @@ public class PullUpConflictsUtil {
      }
    }

-    private boolean willBeMoved(PsiElement element) {
-      PsiElement parent = element;
-      while (parent != null) {
-        if (myMovedMembers.contains(parent)) return true;
-        parent = parent.getParent();
-      }
-      return false;
-    }
+

    private boolean existsInSuperClass(PsiElement classMember) {
      if (!(classMember instanceof PsiMethod)) return false;
@@ -0,0 +1,4 @@
+package a;
+public class Sup {
+  void foo(){}
+}
@@ -0,0 +1,7 @@
+package a;
+
+public class Test extends Sup {
+    void x() {
+      foo();
+    }
+}
@@ -0,0 +1,2 @@
+package b;
+class E{}
@@ -0,0 +1,6 @@
+package b;
+
+import a.Test;
+
+public class TestSubclass  extends Test {
+    }
@@ -0,0 +1,4 @@
+package a;
+public class Sup {
+  void foo(){}
+}
@@ -0,0 +1,6 @@
+package a;
+public class Test extends Sup {
+    void x() {
+      foo();
+    }
+}
@@ -0,0 +1,2 @@
+package b;
+class E{}
@@ -0,0 +1,2 @@
+package a;
+class Sup {}
@@ -0,0 +1,5 @@
+package a;
+
+public class Test extends Sup {
+    void foo(){}
+}
@@ -0,0 +1,6 @@
+package b;
+
+import a.Test;
+
+public class TestSubclass  extends Test {
+    }
@@ -0,0 +1,2 @@
+package a;
+class Sup {}
@@ -0,0 +1,4 @@
+package a;
+public class Test extends Sup {
+    void foo(){}
+}
@@ -22,11 +22,11 @@ import com.intellij.testFramework.IdeaTestUtil;
 import com.intellij.testFramework.PsiTestUtil;
 import com.intellij.util.containers.HashSet;
 import com.intellij.util.containers.MultiMap;
+import junit.framework.Assert;
 import org.jetbrains.annotations.NonNls;

 import java.io.File;
 import java.util.Arrays;
-import java.util.Collection;

 /**
 * @author yole
@@ -43,11 +43,24 @@ public class ExtractSuperClassTest extends CodeInsightTestCase {

  public void testConflictUsingPrivateMethod() throws Exception {
    doTest("Test", "TestSubclass",
-           new String[] {"Method <b><code>Test.foo()</code></b> is private and will not be accessible from method <b><code>x()</code></b>.",
-                         "Method <b><code>x()</code></b> uses method <b><code>Test.foo()</code></b>, which is not moved to the superclass"},
+           new String[] {"Method <b><code>Test.foo()</code></b> is private and will not be accessible from method <b><code>x()</code></b>."},
           new RefactoringTestUtil.MemberDescriptor("x", PsiMethod.class));
  }

+  public void testConflictUsingPackageLocalMethod() throws Exception {
+    doTest("a.Test", "TestSubclass",
+           new String[] {"method <b><code>Sup.foo()</code></b> won't be accessible"},
+           "b",
+           new RefactoringTestUtil.MemberDescriptor("x", PsiMethod.class));
+  }
+
+  public void testConflictUsingPackageLocalSuperClass() throws Exception {
+    doTest("a.Test", "TestSubclass",
+           new String[] {"class <b><code>a.Sup</code></b> won't be accessible from package <b><code>b</code></b>"},
+           "b",
+           new RefactoringTestUtil.MemberDescriptor("foo", PsiMethod.class));
+  }
+
  public void testNoConflictUsingProtectedMethodFromSuper() throws Exception {
    doTest("Test", "TestSubclass",
           new RefactoringTestUtil.MemberDescriptor("x", PsiMethod.class));
@@ -101,13 +114,28 @@ public class ExtractSuperClassTest extends CodeInsightTestCase {
  private void doTest(@NonNls final String className, @NonNls final String newClassName,
                      String[] conflicts,
                      RefactoringTestUtil.MemberDescriptor... membersToFind) throws Exception {
+    doTest(className, newClassName, conflicts, null, membersToFind);
+  }
+
+  private void doTest(@NonNls final String className,
+                      @NonNls final String newClassName,
+                      String[] conflicts,
+                      String targetPackageName,
+                      RefactoringTestUtil.MemberDescriptor... membersToFind) throws Exception {
    String rootBefore = getRoot() + "/before";
    PsiTestUtil.removeAllRoots(myModule, JavaSdkImpl.getMockJdk14());
    final VirtualFile rootDir = PsiTestUtil.createTestProjectStructure(myProject, myModule, rootBefore, myFilesToDelete);
    PsiClass psiClass = myJavaFacade.findClass(className, ProjectScope.getAllScope(myProject));
    assertNotNull(psiClass);
    final MemberInfo[] members = RefactoringTestUtil.findMembers(psiClass, membersToFind);
-    final PsiDirectory targetDirectory = psiClass.getContainingFile().getContainingDirectory();
+    PsiDirectory targetDirectory;
+    if (targetPackageName == null) {
+      targetDirectory = psiClass.getContainingFile().getContainingDirectory();
+    } else {
+      final PsiPackage aPackage = myJavaFacade.findPackage(targetPackageName);
+      assertNotNull(aPackage);
+      targetDirectory = aPackage.getDirectories()[0];
+    }
    ExtractSuperClassProcessor processor = new ExtractSuperClassProcessor(myProject,
                                                                          targetDirectory,
                                                                          newClassName,