From 7231e3423329fa717e60f6eef17548a389571d9d Mon Sep 17 00:00:00 2001 From: Max Medvedev Date: Mon, 28 Oct 2013 15:17:30 +0400 Subject: [PATCH] IDEA-113139 Groovy: Introduce Variable refactoring: correct place to insert variable --- .../refactoring/GroovyRefactoringUtil.java | 20 --------- .../GrIntroduceLocalVariableProcessor.java | 41 +++++++++++++++---- .../IntroduceVariableTest.groovy | 23 +++++++++++ .../IntroduceVariableValidatorTest.java | 3 +- 4 files changed, 57 insertions(+), 30 deletions(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/GroovyRefactoringUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/GroovyRefactoringUtil.java index 4965dd87ee91..feca69ee9dfc 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/GroovyRefactoringUtil.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/GroovyRefactoringUtil.java @@ -53,7 +53,6 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrContinueSta import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrReturnStatement; import org.jetbrains.plugins.groovy.lang.psi.api.statements.clauses.GrCaseSection; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.*; -import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.literals.GrStringInjection; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrCallExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrMethodCallExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.params.GrParameter; @@ -63,7 +62,6 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrEn import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod; import org.jetbrains.plugins.groovy.lang.psi.api.types.GrCodeReferenceElement; import org.jetbrains.plugins.groovy.lang.psi.api.types.GrTypeArgumentList; -import org.jetbrains.plugins.groovy.lang.psi.api.util.GrDeclarationHolder; import org.jetbrains.plugins.groovy.lang.psi.api.util.GrStatementOwner; import org.jetbrains.plugins.groovy.lang.psi.api.util.GrVariableDeclarationOwner; @@ -84,24 +82,6 @@ public abstract class GroovyRefactoringUtil { private static final String[] finalModifiers = new String[]{PsiModifier.FINAL}; - @Nullable - public static PsiElement getEnclosingContainer(PsiElement place) { - PsiElement parent = place.getParent(); - while (true) { - if (parent == null) { - return null; - } - if (parent instanceof GrDeclarationHolder && !(parent instanceof GrClosableBlock && parent.getParent() instanceof GrStringInjection)) { - return parent; - } - if (parent instanceof GrLoopStatement) { - return parent; - } - - parent = parent.getParent(); - } - } - @Nullable public static T findElementInRange(final PsiFile file, int startOffset, diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/variable/GrIntroduceLocalVariableProcessor.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/variable/GrIntroduceLocalVariableProcessor.java index a43356d5dd18..3d01195b1c4b 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/variable/GrIntroduceLocalVariableProcessor.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/variable/GrIntroduceLocalVariableProcessor.java @@ -19,6 +19,7 @@ import com.intellij.openapi.diagnostic.Logger; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiReference; import com.intellij.psi.codeStyle.JavaCodeStyleManager; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtilCore; import com.intellij.refactoring.util.RefactoringUtil; import com.intellij.util.ArrayUtilRt; @@ -33,6 +34,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpres import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.literals.GrStringInjection; import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrAccessorMethod; +import org.jetbrains.plugins.groovy.lang.psi.api.util.GrDeclarationHolder; import org.jetbrains.plugins.groovy.lang.psi.api.util.GrStatementOwner; import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil; import org.jetbrains.plugins.groovy.refactoring.GroovyRefactoringUtil; @@ -75,7 +77,7 @@ public class GrIntroduceLocalVariableProcessor { int expressionIndex = ArrayUtilRt.find(myOccurrences, myExpression); final PsiElement[] replaced = processOccurrences(); PsiElement replacedExpression = replaced[expressionIndex]; - GrStatement anchor = getAnchor(replaced, replacedExpression); + GrStatement anchor = getAnchor(replaced); RefactoringUtil.highlightAllOccurrences(myContext.getProject(), replaced, myContext.getEditor()); @@ -104,8 +106,6 @@ public class GrIntroduceLocalVariableProcessor { throw new IncorrectOperationException("Expression occurrence to be replaced is not instance of GroovyPsiElement"); } - boolean isOriginal = myExpression == occurrence; - final GrExpression replaced = ((GrExpression)occurrence).replaceWithExpression(templateRef, true); result.add(replaced); } @@ -174,10 +174,10 @@ public class GrIntroduceLocalVariableProcessor { return variable; } - private GrVariableDeclaration doInsertDefinition(GrVariableDeclaration declaration, - GrStatement anchor, - boolean deleteExpression, - boolean anchorEqualsExpression) { + private static GrVariableDeclaration doInsertDefinition(GrVariableDeclaration declaration, + GrStatement anchor, + boolean deleteExpression, + boolean anchorEqualsExpression) { PsiElement realContainer = anchor.getParent(); GrStatementOwner block = (GrStatementOwner)realContainer; @@ -206,12 +206,35 @@ public class GrIntroduceLocalVariableProcessor { } @NotNull - private GrStatement getAnchor(PsiElement[] replaced, PsiElement replacedExpression) { - PsiElement anchor = GrIntroduceHandlerBase.findAnchor(replaced, GroovyRefactoringUtil.getEnclosingContainer(replacedExpression)); + private GrStatement getAnchor(PsiElement[] replaced) { + PsiElement parent = PsiTreeUtil.findCommonParent(replaced); + PsiElement container = getEnclosingContainer(parent); + assert container != null; + PsiElement anchor = GrIntroduceHandlerBase.findAnchor(replaced, container); + GrIntroduceHandlerBase.assertStatement(anchor, myOccurrences, myContext.getScope()); return (GrStatement)anchor; } + @Nullable + public static PsiElement getEnclosingContainer(PsiElement place) { + PsiElement parent = place; + while (true) { + if (parent == null) { + return null; + } + if (parent instanceof GrDeclarationHolder && !(parent instanceof GrClosableBlock && parent.getParent() instanceof GrStringInjection)) { + return parent; + } + if (parent instanceof GrLoopStatement) { + return parent; + } + + parent = parent.getParent(); + } + } + + @Nullable private static String getFieldName(@Nullable PsiElement element) { if (element instanceof GrAccessorMethod) element = ((GrAccessorMethod)element).getProperty(); diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceVariable/IntroduceVariableTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceVariable/IntroduceVariableTest.groovy index 71482d280202..60a96ef055d6 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceVariable/IntroduceVariableTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceVariable/IntroduceVariableTest.groovy @@ -96,6 +96,29 @@ print "a" + preved + "c" ''') } + void testAllUsages() { + doTest('''\ +def foo() { + println(123); // (1) + println(123); // (2) + if (true) { + println(123); // (3) + println(123); // (4) + } +} +''', '''\ +def foo() { + def preved = 123 + println(preved); // (1) + println(preved); // (2) + if (true) { + println(preved); // (3) + println(preved); // (4) + } +} +''') + } + protected static final String ALL_MARKER = "" private void processFile(String fileText, boolean explicitType) { diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceVariable/IntroduceVariableValidatorTest.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceVariable/IntroduceVariableValidatorTest.java index 3f14d117ab10..8771d06e3e32 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceVariable/IntroduceVariableValidatorTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceVariable/IntroduceVariableValidatorTest.java @@ -29,6 +29,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpres import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil; import org.jetbrains.plugins.groovy.refactoring.GroovyRefactoringUtil; import org.jetbrains.plugins.groovy.refactoring.introduce.GrIntroduceContextImpl; +import org.jetbrains.plugins.groovy.refactoring.introduce.variable.GrIntroduceLocalVariableProcessor; import org.jetbrains.plugins.groovy.refactoring.introduce.variable.GroovyVariableValidator; import org.jetbrains.plugins.groovy.util.TestUtils; @@ -83,7 +84,7 @@ public class IntroduceVariableValidatorTest extends LightCodeInsightFixtureTestC Assert.assertNotNull("Selected expression reference points to null", selectedExpr); - final PsiElement tempContainer = GroovyRefactoringUtil.getEnclosingContainer(selectedExpr); + final PsiElement tempContainer = GrIntroduceLocalVariableProcessor.getEnclosingContainer(selectedExpr); Assert.assertTrue(tempContainer instanceof GroovyPsiElement); PsiElement[] occurences = GroovyRefactoringUtil.getExpressionOccurrences(PsiUtil.skipParentheses(selectedExpr, false), tempContainer);