pull up: do not rise a conflict if pulled method uses method which would be pulled as abstract (IDEADEV-40999)

This commit is contained in:
anna
2009-10-27 12:23:57 +03:00
parent d58e748cba
commit 6383243722
6 changed files with 90 additions and 35 deletions
@@ -84,7 +84,7 @@ public class PullUpConflictsUtil {
}
}
final MultiMap<PsiElement, String> conflicts = new MultiMap<PsiElement, String>();
RefactoringConflictsUtil.analyzeAccessibilityConflicts(movedMembers, superClass, conflicts, null, targetRepresentativeElement);
RefactoringConflictsUtil.analyzeAccessibilityConflicts(movedMembers, superClass, conflicts, null, targetRepresentativeElement, abstractMethods);
if (superClass != null) {
checkSuperclassMembers(superClass, infos, conflicts);
if (isInterfaceTarget) {
@@ -56,18 +56,19 @@ public class RefactoringConflictsUtil {
public static void analyzeAccessibilityConflicts(@NotNull Set<PsiMember> membersToMove,
@NotNull final PsiClass targetClass,
final MultiMap<PsiElement, String> conflicts, String newVisibility) {
analyzeAccessibilityConflicts(membersToMove, targetClass, conflicts, newVisibility, targetClass);
analyzeAccessibilityConflicts(membersToMove, targetClass, conflicts, newVisibility, targetClass, null);
}
public static void analyzeAccessibilityConflicts(@NotNull Set<PsiMember> membersToMove, @Nullable final PsiClass targetClass, final MultiMap<PsiElement, String> conflicts,
String newVisibility,
@NotNull PsiElement context) {
@NotNull PsiElement context,
@Nullable Set<PsiMethod> abstractMethods) {
if (VisibilityUtil.ESCALATE_VISIBILITY.equals(newVisibility)) { //Still need to check for access object
newVisibility = PsiModifier.PUBLIC;
}
for (PsiMember member : membersToMove) {
checkUsedElements(member, member, membersToMove, targetClass, context, conflicts);
checkUsedElements(member, member, membersToMove, abstractMethods, targetClass, context, conflicts);
PsiModifierList modifierList = member.getModifierList();
if (modifierList!=null) modifierList= (PsiModifierList)modifierList.copy();
@@ -107,14 +108,19 @@ public class RefactoringConflictsUtil {
}
}
public static void checkUsedElements(PsiMember member, PsiElement scope, @NotNull Set<PsiMember> membersToMove, @Nullable PsiClass targetClass,
public static void checkUsedElements(PsiMember member, PsiElement scope, @NotNull Set<PsiMember> membersToMove,
@Nullable Set<PsiMethod> abstractMethods, @Nullable PsiClass targetClass,
@NotNull PsiElement context,
MultiMap<PsiElement, String> conflicts) {
final Set<PsiMember> moving = new HashSet<PsiMember>(membersToMove);
if (abstractMethods != null) {
moving.addAll(abstractMethods);
}
if(scope instanceof PsiReferenceExpression) {
PsiReferenceExpression refExpr = (PsiReferenceExpression)scope;
PsiElement refElement = refExpr.resolve();
if (refElement instanceof PsiMember) {
if (!RefactoringHierarchyUtil.willBeInTargetClass(refElement, membersToMove, targetClass, false)){
if (!RefactoringHierarchyUtil.willBeInTargetClass(refElement, moving, targetClass, false)){
PsiExpression qualifier = refExpr.getQualifierExpression();
PsiClass accessClass = (PsiClass)(qualifier != null ? PsiUtil.getAccessObjectClass(qualifier).getElement() : null);
checkAccessibility((PsiMember)refElement, context, accessClass, member, conflicts);
@@ -125,13 +131,13 @@ public class RefactoringConflictsUtil {
final PsiNewExpression newExpression = (PsiNewExpression)scope;
final PsiAnonymousClass anonymousClass = newExpression.getAnonymousClass();
if (anonymousClass != null) {
if (!RefactoringHierarchyUtil.willBeInTargetClass(anonymousClass, membersToMove, targetClass, false)){
if (!RefactoringHierarchyUtil.willBeInTargetClass(anonymousClass, moving, targetClass, false)){
checkAccessibility(anonymousClass, context, anonymousClass, member, conflicts);
}
} else {
final PsiMethod refElement = newExpression.resolveConstructor();
if (refElement != null) {
if (!RefactoringHierarchyUtil.willBeInTargetClass(refElement, membersToMove, targetClass, false)) {
if (!RefactoringHierarchyUtil.willBeInTargetClass(refElement, moving, targetClass, false)) {
checkAccessibility(refElement, context, null, member, conflicts);
}
}
@@ -141,7 +147,7 @@ public class RefactoringConflictsUtil {
PsiJavaCodeReferenceElement refExpr = (PsiJavaCodeReferenceElement)scope;
PsiElement refElement = refExpr.resolve();
if (refElement instanceof PsiMember) {
if (!RefactoringHierarchyUtil.willBeInTargetClass(refElement, membersToMove, targetClass, false)){
if (!RefactoringHierarchyUtil.willBeInTargetClass(refElement, moving, targetClass, false)){
checkAccessibility((PsiMember)refElement, context, null, member, conflicts);
}
}
@@ -150,7 +156,7 @@ public class RefactoringConflictsUtil {
PsiElement[] children = scope.getChildren();
for (PsiElement child : children) {
if (!(child instanceof PsiWhiteSpace)) {
checkUsedElements(member, child, membersToMove, targetClass, context, conflicts);
checkUsedElements(member, child, membersToMove, abstractMethods, targetClass, context, conflicts);
}
}
}
@@ -0,0 +1,11 @@
public class A2 {
}
class B2 extends A2 {
public void <caret>a() {
b();
}
private void b() {
}
}
@@ -0,0 +1,14 @@
public abstract class A2 {
public void a() {
b();
}
protected abstract void b();
}
class B2 extends A2 {
@Override
protected void b() {
}
}
@@ -6,16 +6,18 @@ import com.intellij.openapi.fileEditor.FileDocumentManager;
import com.intellij.openapi.projectRoots.Sdk;
import com.intellij.openapi.projectRoots.impl.JavaSdkImpl;
import com.intellij.openapi.roots.LanguageLevelProjectExtension;
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.vfs.LocalFileSystem;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.pom.java.LanguageLevel;
import com.intellij.psi.*;
import com.intellij.psi.PsiClass;
import com.intellij.psi.PsiDocumentManager;
import com.intellij.psi.PsiField;
import com.intellij.psi.PsiMethod;
import com.intellij.psi.impl.source.PostprocessReformattingAspect;
import com.intellij.psi.search.ProjectScope;
import com.intellij.refactoring.extractSuperclass.ExtractSuperClassProcessor;
import com.intellij.refactoring.util.classMembers.MemberInfo;
import com.intellij.refactoring.util.DocCommentPolicy;
import com.intellij.refactoring.util.classMembers.MemberInfo;
import com.intellij.testFramework.IdeaTestUtil;
import com.intellij.testFramework.PsiTestUtil;
import org.jetbrains.annotations.NonNls;
@@ -27,16 +29,16 @@ import java.io.File;
*/
public class ExtractSuperClassTest extends CodeInsightTestCase {
public void testFinalFieldInitialization() throws Exception { // IDEADEV-19704
doTest("Test", "TestSubclass", new Pair<String, Class<? extends PsiMember>>("X", PsiClass.class),
new Pair<String, Class<? extends PsiMember>>("x", PsiField.class));
doTest("Test", "TestSubclass", new PullUpTest.MemberDescriptor("X", PsiClass.class),
new PullUpTest.MemberDescriptor("x", PsiField.class));
}
public void testFieldInitializationWithCast() throws Exception {
doTest("Test", "TestSubclass", new Pair<String, Class<? extends PsiMember>>("x", PsiField.class));
doTest("Test", "TestSubclass", new PullUpTest.MemberDescriptor("x", PsiField.class));
}
public void testParameterNameEqualsFieldName() throws Exception { // IDEADEV-10629
doTest("Test", "TestSubclass", new Pair<String, Class<? extends PsiMember>>("a", PsiField.class));
doTest("Test", "TestSubclass", new PullUpTest.MemberDescriptor("a", PsiField.class));
}
public void testExtendsLibraryClass() throws Exception {
@@ -44,7 +46,7 @@ public class ExtractSuperClassTest extends CodeInsightTestCase {
}
public void testRequiredImportRemoved() throws Exception {
doTest("foo.impl.B", "BImpl", new Pair<String, Class<? extends PsiMember>>("getInstance", PsiMethod.class));
doTest("foo.impl.B", "BImpl", new PullUpTest.MemberDescriptor("getInstance", PsiMethod.class));
}
public void testSubstituteGenerics() throws Exception {
@@ -68,7 +70,7 @@ public class ExtractSuperClassTest extends CodeInsightTestCase {
}
private void doTest(@NonNls final String className, @NonNls final String newClassName,
Pair<String, Class<? extends PsiMember>>... membersToFind) throws Exception {
PullUpTest.MemberDescriptor... membersToFind) throws Exception {
String rootBefore = getRoot() + "/before";
PsiTestUtil.removeAllRoots(myModule, JavaSdkImpl.getMockJdk("java 1.5"));
final VirtualFile rootDir = PsiTestUtil.createTestProjectStructure(myProject, myModule, rootBefore, myFilesToDelete);
@@ -6,7 +6,6 @@ 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.util.Pair;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.refactoring.listeners.JavaRefactoringListenerManager;
@@ -24,46 +23,51 @@ public class PullUpTest extends LightCodeInsightTestCase {
public void testQualifiedThis() throws Exception {
doTest(new Pair<String, Class<? extends PsiMember>> ("Inner", PsiClass.class));
doTest(new MemberDescriptor ("Inner", PsiClass.class));
}
public void testQualifiedSuper() throws Exception {
doTest(new Pair<String, Class<? extends PsiMember>> ("Inner", PsiClass.class));
doTest(new MemberDescriptor ("Inner", PsiClass.class));
}
public void testQualifiedReference() throws Exception { // IDEADEV-25008
doTest(new Pair<String, Class<? extends PsiMember>> ("x", PsiField.class),
new Pair<String, Class<? extends PsiMember>> ("getX", PsiMethod.class),
new Pair<String, Class<? extends PsiMember>> ("setX", PsiMethod.class));
doTest(new MemberDescriptor ("x", PsiField.class),
new MemberDescriptor ("getX", PsiMethod.class),
new MemberDescriptor ("setX", PsiMethod.class));
}
public void testPullUpAndAbstractize() throws Exception {
doTest(new MemberDescriptor("a", PsiMethod.class),
new MemberDescriptor("b", PsiMethod.class, true));
}
public void testTryCatchFieldInitializer() throws Exception {
doTest(new Pair<String, Class<? extends PsiMember>>("field", PsiField.class));
doTest(new MemberDescriptor("field", PsiField.class));
}
public void testIfFieldInitializationWithNonMovedField() throws Exception {
doTest(new Pair<String, Class<? extends PsiMember>>("f", PsiField.class));
doTest(new MemberDescriptor("f", PsiField.class));
}
public void testIfFieldMovedInitialization() throws Exception {
doTest(new Pair<String, Class<? extends PsiMember>>("f", PsiField.class));
doTest(new MemberDescriptor("f", PsiField.class));
}
public void testMultipleConstructorsFieldInitialization() throws Exception {
doTest(new Pair<String, Class<? extends PsiMember>>("f", PsiField.class));
doTest(new MemberDescriptor("f", PsiField.class));
}
public void testMultipleConstructorsFieldInitializationNoGood() throws Exception {
doTest(new Pair<String, Class<? extends PsiMember>>("f", PsiField.class));
doTest(new MemberDescriptor("f", PsiField.class));
}
public void testRemoveOverride() throws Exception {
doTest(new Pair<String, Class<? extends PsiMember>> ("get", PsiMethod.class));
doTest(new MemberDescriptor ("get", PsiMethod.class));
}
private void doTest(Pair<String, Class<? extends PsiMember>>... membersToFind) throws Exception {
private void doTest(MemberDescriptor... membersToFind) throws Exception {
configureByFile(BASE_PATH + getTestName(false) + ".java");
PsiElement elementAt = getFile().findElementAt(getEditor().getCaretModel().getOffset());
final PsiClass sourceClass = PsiTreeUtil.getParentOfType(elementAt, PsiClass.class);
@@ -94,11 +98,11 @@ public class PullUpTest extends LightCodeInsightTestCase {
checkResultByFile(BASE_PATH + getTestName(false) + "_after.java");
}
public static MemberInfo[] findMembers(final PsiClass sourceClass, final Pair<String, Class<? extends PsiMember>>... membersToFind) {
public static MemberInfo[] findMembers(final PsiClass sourceClass, final MemberDescriptor... membersToFind) {
MemberInfo[] infos = new MemberInfo[membersToFind.length];
for (int i = 0; i < membersToFind.length; i++) {
final Class<? extends PsiMember> clazz = membersToFind[i].getSecond();
final String name = membersToFind[i].getFirst();
final Class<? extends PsiMember> clazz = membersToFind[i].myClass;
final String name = membersToFind[i].myName;
PsiMember member = null;
if (PsiClass.class.isAssignableFrom(clazz)) {
member = sourceClass.findInnerClassByName(name, false);
@@ -112,6 +116,7 @@ public class PullUpTest extends LightCodeInsightTestCase {
assertNotNull(member);
infos[i] = new MemberInfo(member);
infos[i].setToAbstract(membersToFind[i].myAbstract);
}
return infos;
}
@@ -124,4 +129,21 @@ public class PullUpTest extends LightCodeInsightTestCase {
protected String getTestDataPath() {
return JavaTestUtil.getJavaTestDataPath();
}
public static class MemberDescriptor {
private String myName;
private Class<? extends PsiMember> myClass;
private boolean myAbstract;
public MemberDescriptor(String name, Class<? extends PsiMember> aClass, boolean isAbstract) {
myName = name;
myClass = aClass;
myAbstract = isAbstract;
}
public MemberDescriptor(String name, Class<? extends PsiMember> aClass) {
this(name, aClass, false);
}
}
}