From afea7732d8b97d022911e12652fd42a6766f4aa5 Mon Sep 17 00:00:00 2001 From: "Anna.Kozlova" Date: Thu, 16 Aug 2018 17:55:31 +0200 Subject: [PATCH] extract field: allow to extract static field from super/this calls (IDEA-197438) --- .../BaseExpressionToFieldHandler.java | 87 ++++++++++++++++++- .../IntroduceConstantHandler.java | 83 ------------------ .../IntroduceFieldCentralPanel.java | 7 +- .../introduceField/IntroduceFieldHandler.java | 2 +- ...cceptIntroduceFieldFromExprInThisCall.java | 8 ++ ...cceptIntroduceFieldFromExprInThisCall.java | 6 ++ .../IntroduceFieldInSameClassTest.java | 6 ++ 7 files changed, 111 insertions(+), 88 deletions(-) create mode 100644 java/java-tests/testData/refactoring/introduceField/afterAcceptIntroduceFieldFromExprInThisCall.java create mode 100644 java/java-tests/testData/refactoring/introduceField/beforeAcceptIntroduceFieldFromExprInThisCall.java diff --git a/java/java-impl/src/com/intellij/refactoring/introduceField/BaseExpressionToFieldHandler.java b/java/java-impl/src/com/intellij/refactoring/introduceField/BaseExpressionToFieldHandler.java index 4cfe2edaa94b..525581432e4c 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceField/BaseExpressionToFieldHandler.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceField/BaseExpressionToFieldHandler.java @@ -18,6 +18,7 @@ package com.intellij.refactoring.introduceField; import com.intellij.codeInsight.AnnotationUtil; import com.intellij.codeInsight.ChangeContextUtil; +import com.intellij.codeInsight.ExceptionUtil; import com.intellij.codeInsight.TestFrameworks; import com.intellij.codeInsight.daemon.impl.quickfix.AnonymousTargetClassPreselectionUtil; import com.intellij.codeInsight.highlighting.HighlightManager; @@ -57,6 +58,7 @@ import com.intellij.refactoring.util.CommonRefactoringUtil; import com.intellij.refactoring.util.EnumConstantsUtil; import com.intellij.refactoring.util.RefactoringChangeUtil; import com.intellij.refactoring.util.RefactoringUtil; +import com.intellij.refactoring.util.classMembers.ClassMemberReferencesVisitor; import com.intellij.refactoring.util.occurrences.OccurrenceManager; import com.intellij.util.IncorrectOperationException; import com.intellij.util.VisibilityUtil; @@ -181,8 +183,12 @@ public abstract class BaseExpressionToFieldHandler extends IntroduceHandlerBase PsiElement tempAnchorElement = RefactoringUtil.getParentExpressionAnchorElement(selectedExpr); if (!Comparing.strEqual(IntroduceConstantHandler.REFACTORING_NAME, getRefactoringName()) && - IntroduceVariableBase.checkAnchorBeforeThisOrSuper(project, editor, tempAnchorElement, getRefactoringName(), getHelpID())) + IntroduceFieldHandler.isInSuperOrThis(selectedExpr) && + isStaticFinalInitializer(selectedExpr) != null) { + String message = RefactoringBundle.getCannotRefactorMessage(RefactoringBundle.message("invalid.expression.context")); + CommonRefactoringUtil.showErrorHint(project, editor, message, getRefactoringName(), getHelpID()); return true; + } final Settings settings = showRefactoringDialog(project, editor, myParentClass, selectedExpr, tempType, @@ -245,6 +251,15 @@ public abstract class BaseExpressionToFieldHandler extends IntroduceHandlerBase ); } + @Nullable + protected PsiElement isStaticFinalInitializer(PsiExpression expr) { + PsiClass parentClass = expr != null ? getParentClass(expr) : null; + if (parentClass == null) return null; + IsStaticFinalInitializerExpression visitor = new IsStaticFinalInitializerExpression(parentClass, expr); + expr.accept(visitor); + return visitor.getElementReference(); + } + protected abstract OccurrenceManager createOccurrenceManager(PsiExpression selectedExpr, PsiClass parentClass); protected final PsiClass getParentClass() { @@ -925,4 +940,74 @@ public abstract class BaseExpressionToFieldHandler extends IntroduceHandlerBase return myField; } } + + private static class IsStaticFinalInitializerExpression extends ClassMemberReferencesVisitor { + private PsiElement myElementReference; + private final PsiExpression myInitializer; + private boolean myCheckThrowables = true; + + public IsStaticFinalInitializerExpression(PsiClass aClass, PsiExpression initializer) { + super(aClass); + myInitializer = initializer; + } + + @Override + public void visitReferenceExpression(PsiReferenceExpression expression) { + final PsiElement psiElement = expression.resolve(); + if ((psiElement instanceof PsiLocalVariable || psiElement instanceof PsiParameter) && + !PsiTreeUtil.isAncestor(myInitializer, psiElement, false)) { + myElementReference = expression; + } + else { + super.visitReferenceExpression(expression); + } + } + + @Override + public void visitMethodReferenceExpression(PsiMethodReferenceExpression expression) { + if (!PsiMethodReferenceUtil.isResolvedBySecondSearch(expression)) { + super.visitMethodReferenceExpression(expression); + } + } + + @Override + public void visitCallExpression(PsiCallExpression callExpression) { + super.visitCallExpression(callExpression); + if (!myCheckThrowables) return; + final List checkedExceptions = ExceptionUtil.getThrownCheckedExceptions(callExpression); + if (!checkedExceptions.isEmpty()) { + myElementReference = callExpression; + } + } + + @Override + public void visitClass(PsiClass aClass) { + myCheckThrowables = false; + super.visitClass(aClass); + } + + @Override + public void visitLambdaExpression(PsiLambdaExpression expression) { + myCheckThrowables = false; + super.visitLambdaExpression(expression); + } + + @Override + protected void visitClassMemberReferenceElement(PsiMember classMember, PsiJavaCodeReferenceElement classMemberReference) { + if (!classMember.hasModifierProperty(PsiModifier.STATIC)) { + myElementReference = classMemberReference; + } + } + + @Override + public void visitElement(PsiElement element) { + if (myElementReference != null) return; + super.visitElement(element); + } + + @Nullable + public PsiElement getElementReference() { + return myElementReference; + } + } } diff --git a/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceConstantHandler.java b/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceConstantHandler.java index 796283d43546..c598f3e12e6d 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceConstantHandler.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceConstantHandler.java @@ -15,7 +15,6 @@ */ package com.intellij.refactoring.introduceField; -import com.intellij.codeInsight.ExceptionUtil; import com.intellij.codeInsight.highlighting.HighlightManager; import com.intellij.openapi.actionSystem.DataContext; import com.intellij.openapi.editor.Editor; @@ -34,14 +33,11 @@ import com.intellij.refactoring.introduce.inplace.AbstractInplaceIntroducer; import com.intellij.refactoring.ui.TypeSelectorManagerImpl; import com.intellij.refactoring.util.CommonRefactoringUtil; import com.intellij.refactoring.util.RefactoringUtil; -import com.intellij.refactoring.util.classMembers.ClassMemberReferencesVisitor; import com.intellij.refactoring.util.occurrences.ExpressionOccurrenceManager; import com.intellij.refactoring.util.occurrences.OccurrenceManager; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import java.util.ArrayList; -import java.util.List; public class IntroduceConstantHandler extends BaseExpressionToFieldHandler { public static final String REFACTORING_NAME = RefactoringBundle.message("introduce.constant.title"); @@ -212,90 +208,11 @@ public class IntroduceConstantHandler extends BaseExpressionToFieldHandler { return myInplaceIntroduceConstantPopup; } - @Nullable - private PsiElement isStaticFinalInitializer(PsiExpression expr) { - PsiClass parentClass = expr != null ? getParentClass(expr) : null; - if (parentClass == null) return null; - IsStaticFinalInitializerExpression visitor = new IsStaticFinalInitializerExpression(parentClass, expr); - expr.accept(visitor); - return visitor.getElementReference(); - } - @Override protected OccurrenceManager createOccurrenceManager(final PsiExpression selectedExpr, final PsiClass parentClass) { return new ExpressionOccurrenceManager(selectedExpr, parentClass, null); } - private static class IsStaticFinalInitializerExpression extends ClassMemberReferencesVisitor { - private PsiElement myElementReference; - private final PsiExpression myInitializer; - private boolean myCheckThrowables = true; - - public IsStaticFinalInitializerExpression(PsiClass aClass, PsiExpression initializer) { - super(aClass); - myInitializer = initializer; - } - - @Override - public void visitReferenceExpression(PsiReferenceExpression expression) { - final PsiElement psiElement = expression.resolve(); - if ((psiElement instanceof PsiLocalVariable || psiElement instanceof PsiParameter) && - !PsiTreeUtil.isAncestor(myInitializer, psiElement, false)) { - myElementReference = expression; - } - else { - super.visitReferenceExpression(expression); - } - } - - @Override - public void visitMethodReferenceExpression(PsiMethodReferenceExpression expression) { - if (!PsiMethodReferenceUtil.isResolvedBySecondSearch(expression)) { - super.visitMethodReferenceExpression(expression); - } - } - - @Override - public void visitCallExpression(PsiCallExpression callExpression) { - super.visitCallExpression(callExpression); - if (!myCheckThrowables) return; - final List checkedExceptions = ExceptionUtil.getThrownCheckedExceptions(callExpression); - if (!checkedExceptions.isEmpty()) { - myElementReference = callExpression; - } - } - - @Override - public void visitClass(PsiClass aClass) { - myCheckThrowables = false; - super.visitClass(aClass); - } - - @Override - public void visitLambdaExpression(PsiLambdaExpression expression) { - myCheckThrowables = false; - super.visitLambdaExpression(expression); - } - - @Override - protected void visitClassMemberReferenceElement(PsiMember classMember, PsiJavaCodeReferenceElement classMemberReference) { - if (!classMember.hasModifierProperty(PsiModifier.STATIC)) { - myElementReference = classMemberReference; - } - } - - @Override - public void visitElement(PsiElement element) { - if (myElementReference != null) return; - super.visitElement(element); - } - - @Nullable - public PsiElement getElementReference() { - return myElementReference; - } - } - @Override public PsiClass getParentClass(@NotNull PsiExpression initializerExpression) { final PsiType type = initializerExpression.getType(); diff --git a/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceFieldCentralPanel.java b/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceFieldCentralPanel.java index 3ebbacff42a3..4f9c69fa94a8 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceFieldCentralPanel.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceFieldCentralPanel.java @@ -87,7 +87,7 @@ public abstract class IntroduceFieldCentralPanel { myTypeSelectorManager = typeSelectorManager; } - protected boolean setEnabledInitializationPlaces(@NotNull final PsiElement initializer) { + protected boolean setEnabledInitializationPlaces(@NotNull final PsiExpression initializer) { final Set fields = new HashSet<>(); final Ref refsLocal = new Ref<>(false); initializer.accept(new JavaRecursiveElementWalkingVisitor() { @@ -113,11 +113,12 @@ public abstract class IntroduceFieldCentralPanel { }); final boolean locals = refsLocal.get(); - if (!locals && fields.isEmpty()) { + boolean superOrThis = IntroduceFieldHandler.isInSuperOrThis(initializer); + if (!locals && fields.isEmpty() && !superOrThis) { return true; } return updateInitializationPlaceModel(!locals && initializedInSetUp(fields), - !locals && initializedInConstructor(fields)); + !locals && !superOrThis && initializedInConstructor(fields)); } private static boolean initializedInConstructor(Set fields) { diff --git a/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceFieldHandler.java b/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceFieldHandler.java index c821e6ee3bc8..01899237c7e1 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceFieldHandler.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceFieldHandler.java @@ -174,7 +174,7 @@ public class IntroduceFieldHandler extends BaseExpressionToFieldHandler { return myInplaceIntroduceFieldPopup; } - private static boolean isInSuperOrThis(PsiExpression occurrence) { + static boolean isInSuperOrThis(PsiExpression occurrence) { return !NotInSuperCallOccurrenceFilter.INSTANCE.isOK(occurrence) || !NotInThisCallFilter.INSTANCE.isOK(occurrence); } diff --git a/java/java-tests/testData/refactoring/introduceField/afterAcceptIntroduceFieldFromExprInThisCall.java b/java/java-tests/testData/refactoring/introduceField/afterAcceptIntroduceFieldFromExprInThisCall.java new file mode 100644 index 000000000000..b6068d12e807 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceField/afterAcceptIntroduceFieldFromExprInThisCall.java @@ -0,0 +1,8 @@ +class Test { + public static final String foo = "foo"; + + Test(String s) {} + Test() { + this(foo); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceField/beforeAcceptIntroduceFieldFromExprInThisCall.java b/java/java-tests/testData/refactoring/introduceField/beforeAcceptIntroduceFieldFromExprInThisCall.java new file mode 100644 index 000000000000..9891c5c76034 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceField/beforeAcceptIntroduceFieldFromExprInThisCall.java @@ -0,0 +1,6 @@ +class Test { + Test(String s) {} + Test() { + this("foo"); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceFieldInSameClassTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceFieldInSameClassTest.java index cce2cc8d38f7..a865163d6277 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceFieldInSameClassTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceFieldInSameClassTest.java @@ -154,6 +154,12 @@ public class IntroduceFieldInSameClassTest extends LightCodeInsightTestCase { } } + public void testAcceptIntroduceFieldFromExprInThisCall() { + configureByFile("beforeAcceptIntroduceFieldFromExprInThisCall.java"); + performRefactoring(BaseExpressionToFieldHandler.InitializationPlace.IN_FIELD_DECLARATION, true); + checkResultByFile("afterAcceptIntroduceFieldFromExprInThisCall.java"); + } + public void testInConstructorEnclosingAnonymous() { configureByFile("beforeEnclosingAnonymous.java"); performRefactoring(BaseExpressionToFieldHandler.InitializationPlace.IN_CONSTRUCTOR, false);