From 77517a3a3ac09fa96d0992a0af562d3d211e424a Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 12 Nov 2019 11:27:02 +0700 Subject: [PATCH] Fix Pull Up refactoring after removal of MethodElement#copyElement override In some scenarios Pull Up worked correctly only by chance, thanks to incorrect resolve on copied method. However there are more scenarios, presented in new tests ReferencedStaticGenericClassFromOuterClass and ReferencedStaticMethodFromOuterClass, which did not work correctly even before. The fix addresses these cases. GitOrigin-RevId: bd56ed851dd8bbf546dba2847535a98de1eea295 --- .../memberPullUp/JavaPullUpHelper.java | 36 +++++++++++-------- ...encedStaticGenericClassFromOuterClass.java | 13 +++++++ ...taticGenericClassFromOuterClass_after.java | 14 ++++++++ .../ReferencedStaticMethodFromOuterClass.java | 13 +++++++ ...encedStaticMethodFromOuterClass_after.java | 13 +++++++ .../intellij/java/refactoring/PullUpTest.java | 9 +++++ 6 files changed, 84 insertions(+), 14 deletions(-) create mode 100644 java/java-tests/testData/refactoring/pullUp/ReferencedStaticGenericClassFromOuterClass.java create mode 100644 java/java-tests/testData/refactoring/pullUp/ReferencedStaticGenericClassFromOuterClass_after.java create mode 100644 java/java-tests/testData/refactoring/pullUp/ReferencedStaticMethodFromOuterClass.java create mode 100644 java/java-tests/testData/refactoring/pullUp/ReferencedStaticMethodFromOuterClass_after.java diff --git a/java/java-impl/src/com/intellij/refactoring/memberPullUp/JavaPullUpHelper.java b/java/java-impl/src/com/intellij/refactoring/memberPullUp/JavaPullUpHelper.java index 607cbba2706d..b4cb68be5485 100644 --- a/java/java-impl/src/com/intellij/refactoring/memberPullUp/JavaPullUpHelper.java +++ b/java/java-impl/src/com/intellij/refactoring/memberPullUp/JavaPullUpHelper.java @@ -100,16 +100,23 @@ public class JavaPullUpHelper implements PullUpHelper { member.accept(new JavaRecursiveElementWalkingVisitor() { @Override - public void visitReferenceExpression(PsiReferenceExpression expression) { - final PsiExpression qualifierExpression = expression.getQualifierExpression(); + public void visitReferenceElement(PsiJavaCodeReferenceElement reference) { + final PsiElement qualifierExpression = reference.getQualifier(); if (qualifierExpression != null) { final Boolean preserveQualifier = qualifierExpression.getCopyableUserData(PRESERVE_QUALIFIER); if (preserveQualifier != null && !preserveQualifier) { - qualifierExpression.delete(); - return; + PsiElement target = reference.resolve(); + if (target != null) { + PsiJavaCodeReferenceElement copy = (PsiJavaCodeReferenceElement)reference.copy(); + Objects.requireNonNull(copy.getQualifier()).delete(); + if (copy.resolve() == target) { + qualifierExpression.delete(); + return; + } + } } } - super.visitReferenceExpression(expression); + super.visitReferenceElement(reference); } }); @@ -686,15 +693,16 @@ public class JavaPullUpHelper implements PullUpHelper { PsiClass aClass = classes.get(i); if (namedElement instanceof PsiNamedElement) { - PsiReferenceExpression newRef = - (PsiReferenceExpression) factory.createExpressionFromText - ("a." + ((PsiNamedElement) namedElement).getName(), - null); - PsiExpression qualifierExpression = newRef.getQualifierExpression(); - assert qualifierExpression != null; - qualifierExpression = (PsiExpression)qualifierExpression.replace(factory.createReferenceExpression(aClass)); - qualifierExpression.putCopyableUserData(PRESERVE_QUALIFIER, ref.isQualified()); - ref.replace(newRef); + PsiElement oldQualifier = ref.getQualifier(); + if (oldQualifier != null) { + oldQualifier.delete(); + } + String template = aClass.getQualifiedName() + "." + ref.getText(); + PsiJavaCodeReferenceElement newRef = ref instanceof PsiReferenceExpression ? + (PsiReferenceExpression)factory.createExpressionFromText(template, null) : + factory.createReferenceFromText(template, null); + ref = (PsiJavaCodeReferenceElement)ref.replace(newRef); + Objects.requireNonNull(ref.getQualifier()).putCopyableUserData(PRESERVE_QUALIFIER, oldQualifier != null); } } } diff --git a/java/java-tests/testData/refactoring/pullUp/ReferencedStaticGenericClassFromOuterClass.java b/java/java-tests/testData/refactoring/pullUp/ReferencedStaticGenericClassFromOuterClass.java new file mode 100644 index 000000000000..2419762a098a --- /dev/null +++ b/java/java-tests/testData/refactoring/pullUp/ReferencedStaticGenericClassFromOuterClass.java @@ -0,0 +1,13 @@ + +class A extends C { + void foo() { + A.D d = new A.D<>(); + } + + static class D { + } +} + +class C { + +} diff --git a/java/java-tests/testData/refactoring/pullUp/ReferencedStaticGenericClassFromOuterClass_after.java b/java/java-tests/testData/refactoring/pullUp/ReferencedStaticGenericClassFromOuterClass_after.java new file mode 100644 index 000000000000..4dacafabcbdf --- /dev/null +++ b/java/java-tests/testData/refactoring/pullUp/ReferencedStaticGenericClassFromOuterClass_after.java @@ -0,0 +1,14 @@ + +class A extends C { + +} + +class C { + + void foo() { + C.D d = new C.D<>(); + } + + static class D { + } +} diff --git a/java/java-tests/testData/refactoring/pullUp/ReferencedStaticMethodFromOuterClass.java b/java/java-tests/testData/refactoring/pullUp/ReferencedStaticMethodFromOuterClass.java new file mode 100644 index 000000000000..25198c70008e --- /dev/null +++ b/java/java-tests/testData/refactoring/pullUp/ReferencedStaticMethodFromOuterClass.java @@ -0,0 +1,13 @@ +class A { + static class B extends C { + void foo() { + bar(); + } + } + + static void bar() {} +} + +class C { + +} diff --git a/java/java-tests/testData/refactoring/pullUp/ReferencedStaticMethodFromOuterClass_after.java b/java/java-tests/testData/refactoring/pullUp/ReferencedStaticMethodFromOuterClass_after.java new file mode 100644 index 000000000000..46f2f27ef030 --- /dev/null +++ b/java/java-tests/testData/refactoring/pullUp/ReferencedStaticMethodFromOuterClass_after.java @@ -0,0 +1,13 @@ +class A { + static class B extends C { + } + + static void bar() {} +} + +class C { + + void foo() { + A.bar(); + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/PullUpTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/PullUpTest.java index 3ff616812eae..f9b5f925053e 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/PullUpTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/PullUpTest.java @@ -160,6 +160,15 @@ public class PullUpTest extends LightRefactoringTestCase { public void testTypeParamsConflictingNames() { doTest(false, new RefactoringTestUtil.MemberDescriptor("foo", PsiMethod.class, true)); } + + public void testReferencedStaticMethodFromOuterClass() { + doTest(false, new RefactoringTestUtil.MemberDescriptor("foo", PsiMethod.class, false)); + } + + public void testReferencedStaticGenericClassFromOuterClass() { + doTest(false, new RefactoringTestUtil.MemberDescriptor("foo", PsiMethod.class), + new RefactoringTestUtil.MemberDescriptor("D", PsiClass.class)); + } public void testConflictOnNewAbstractMethod() { doTest(false, "Concrete 'class C' would inherit a new abstract method", new RefactoringTestUtil.MemberDescriptor("foo", PsiMethod.class));