IDEA-64430 improved groovy extract method

This commit is contained in:
Maxim Medvedev
2011-01-26 00:30:42 +03:00
parent aff1af7be8
commit e501ff82f4
17 changed files with 161 additions and 86 deletions
@@ -19,6 +19,8 @@ import com.intellij.psi.PsiElement;
import com.intellij.psi.util.PsiTreeUtil;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.plugins.groovy.lang.psi.GrControlFlowOwner;
import org.jetbrains.plugins.groovy.lang.psi.GroovyFile;
import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement;
import org.jetbrains.plugins.groovy.lang.psi.GroovyRecursiveElementVisitor;
import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.GrCondition;
@@ -32,6 +34,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrReturnState
import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrThrowStatement;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.clauses.GrCaseSection;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod;
import org.jetbrains.plugins.groovy.lang.psi.controlFlow.Instruction;
import org.jetbrains.plugins.groovy.lang.psi.controlFlow.impl.ControlFlowBuilder;
import org.jetbrains.plugins.groovy.lang.psi.controlFlow.impl.IfEndInstruction;
@@ -455,7 +458,7 @@ public class ControlFlowUtils {
flow = ((GrCodeBlock)element).getControlFlow();
}
else {
flow = new ControlFlowBuilder(element.getProject()).buildControlFlow((GroovyPsiElement)element, null, null);
flow = new ControlFlowBuilder(element.getProject()).buildControlFlow((GroovyPsiElement)element);
}
boolean[] visited = new boolean[flow.length];
final List<GrStatement> res = new ArrayList<GrStatement>();
@@ -463,13 +466,8 @@ public class ControlFlowUtils {
@Override
public boolean visitExitPoint(Instruction instruction, @Nullable GrExpression returnValue) {
final PsiElement element = instruction.getElement();
if (allExitPoints) {
if (element instanceof GrStatement) {
res.add((GrStatement)element);
}
}
else if (element instanceof GrReturnStatement) {
res.add(((GrReturnStatement)element));
if (element instanceof GrReturnStatement || (allExitPoints && instruction instanceof MaybeReturnInstruction)) {
res.add((GrStatement)element);
}
return true;
}
@@ -623,4 +621,14 @@ public class ControlFlowUtils {
return true;
}
@Nullable
public static GrControlFlowOwner findControlFlowOwner(PsiElement place) {
while (place.getParent() != null) {
place = place.getParent();
if (place instanceof GrClosableBlock) return (GrClosableBlock)place;
if (place instanceof GrMethod) return ((GrMethod)place).getBlock();
if (place instanceof GroovyFile) return (GroovyFile)place;
}
return null;
}
}
@@ -25,6 +25,7 @@ import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiTreeUtil;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.plugins.groovy.lang.parser.GroovyElementTypes;
import org.jetbrains.plugins.groovy.lang.psi.GroovyFileBase;
import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement;
import org.jetbrains.plugins.groovy.lang.psi.GroovyRecursiveElementVisitor;
import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.GrCondition;
@@ -42,6 +43,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.params.GrParameter;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrAnonymousClassDefinition;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTypeDefinition;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod;
import org.jetbrains.plugins.groovy.lang.psi.api.toplevel.GrTopStatement;
import org.jetbrains.plugins.groovy.lang.psi.controlFlow.*;
import org.jetbrains.plugins.groovy.lang.psi.impl.statements.expressions.TypesUtil;
import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil;
@@ -75,25 +77,12 @@ public class ControlFlowBuilder extends GroovyRecursiveElementVisitor {
private InstructionImpl myHead;
private boolean myNegate;
private boolean myAssertionsOnly;
private GroovyPsiElement myLastInScope;
private List<Pair<InstructionImpl, GroovyPsiElement>> myPending;
private GroovyPsiElement myStartInScope;
private GroovyPsiElement myEndInScope;
private boolean myIsInScope;
private int myInstructionNumber;
public void visitElement(GroovyPsiElement element) {
if (element == myStartInScope) {
myIsInScope = true;
}
else if (element == myEndInScope) myIsInScope = false;
if (myIsInScope) {
super.visitElement(element);
}
}
public void visitOpenBlock(GrOpenBlock block) {
final PsiElement parent = block.getParent();
final PsiElement lbrace = block.getLBrace();
@@ -114,22 +103,37 @@ public class ControlFlowBuilder extends GroovyRecursiveElementVisitor {
}
private void handlePossibleReturn(GrStatement last) {
if (last instanceof GrExpression) {
if (last instanceof GrExpression && PsiTreeUtil.isAncestor(myLastInScope, last, false)) {
final MaybeReturnInstruction instruction = new MaybeReturnInstruction((GrExpression)last, myInstructionNumber++);
checkPending(instruction);
addNode(instruction);
}
}
public Instruction[] buildControlFlow(GroovyPsiElement scope, GroovyPsiElement startInScope, GroovyPsiElement endInScope) {
public Instruction[] buildControlFlow(GroovyPsiElement scope) {
myInstructions = new ArrayList<InstructionImpl>();
myProcessingStack = new Stack<InstructionImpl>();
myCatchedExceptionInfos = new Stack<ExceptionInfo>();
myPending = new ArrayList<Pair<InstructionImpl, GroovyPsiElement>>();
myInstructionNumber = 0;
myStartInScope = startInScope;
myEndInScope = endInScope;
myIsInScope = startInScope == null;
myLastInScope = null;
if (scope instanceof GrCodeBlock) {
GrStatement[] statements = ((GrCodeBlock)scope).getStatements();
if (statements.length > 0) {
myLastInScope = statements[statements.length - 1];
}
}
else if (scope instanceof GroovyFileBase) {
GrTopStatement[] topStatements = ((GroovyFileBase)scope).getTopStatements();
for (int i = topStatements.length - 1; i >= 0; i--) {
if (topStatements[i] instanceof GrStatement) {
myLastInScope = topStatements[i];
break;
}
}
}
startNode(null);
if (scope instanceof GrClosableBlock) {
@@ -196,7 +196,7 @@ public abstract class GroovyFileBaseImpl extends PsiFileBase implements GroovyFi
SoftReference<Instruction[]> flow = myControlFlow;
Instruction[] result = flow != null ? flow.get() : null;
if (result == null) {
result = new ControlFlowBuilder(getProject()).buildControlFlow(this, null, null);
result = new ControlFlowBuilder(getProject()).buildControlFlow(this);
myControlFlow = new SoftReference<Instruction[]>(result);
}
return result;
@@ -57,7 +57,7 @@ public abstract class GrBlockImpl extends GroovyPsiElementImpl implements GrCode
myControlFlow = controlFlow = CachedValuesManager.getManager(getProject()).createCachedValue(new CachedValueProvider<Instruction[]>() {
@Override
public Result<Instruction[]> compute() {
return Result.create(new ControlFlowBuilder(getProject()).buildControlFlow(GrBlockImpl.this, null, null), getContainingFile());
return Result.create(new ControlFlowBuilder(getProject()).buildControlFlow(GrBlockImpl.this), getContainingFile());
}
}, false);
}
@@ -46,6 +46,7 @@ import org.jetbrains.plugins.groovy.lang.psi.util.GroovyCommonClassNames;
import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil;
import org.jetbrains.plugins.groovy.lang.resolve.ResolveUtil;
import java.util.Iterator;
import java.util.Map;
import static com.intellij.psi.CommonClassNames.*;
@@ -375,6 +376,17 @@ public class TypesUtil {
return getLeastUpperBound(type1, type2, manager);
}
@Nullable
public static PsiType getLeastUpperBoundNullable(@NotNull Iterable<PsiType> collection, @NotNull PsiManager manager) {
Iterator<PsiType> iterator = collection.iterator();
if (!iterator.hasNext()) return null;
PsiType result = iterator.next();
while (iterator.hasNext()) {
result = getLeastUpperBoundNullable(result, iterator.next(), manager);
}
return result;
}
@Nullable
public static PsiType getLeastUpperBound(@NotNull PsiType type1, @NotNull PsiType type2, PsiManager manager) {
if (type1 instanceof GrTupleType && type2 instanceof GrTupleType) {
@@ -17,21 +17,22 @@
package org.jetbrains.plugins.groovy.refactoring.extractMethod;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Condition;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiModifier;
import com.intellij.psi.PsiType;
import com.intellij.psi.util.TypeConversionUtil;
import com.intellij.util.containers.ContainerUtil;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrStatement;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrCodeBlock;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrReturnStatement;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrMemberOwner;
import org.jetbrains.plugins.groovy.lang.psi.dataFlow.reachingDefs.VariableInfo;
import org.jetbrains.plugins.groovy.lang.psi.impl.statements.expressions.TypesUtil;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.*;
/**
* @author ilyas
@@ -56,12 +57,17 @@ public class ExtractMethodInfoHelper {
GrStatement[] statements,
GrMemberOwner targetClass,
boolean isStatic,
boolean isReturnStatement) {
ArrayList<GrStatement> returnStatements) {
myInnerElements = innerElements;
myStatements = statements;
myTargetClass = targetClass;
myIsStatic = isStatic;
myIsReturnStatement = isReturnStatement;
myIsReturnStatement = ContainerUtil.find(returnStatements, new Condition<GrStatement>() {
@Override
public boolean value(GrStatement statement) {
return statement instanceof GrReturnStatement && ((GrReturnStatement)statement).getReturnValue() != null;
}
}) != null;
myVisibility = PsiModifier.PRIVATE;
assert myStatements.length > 0;
myProject = myStatements[0].getProject();
@@ -87,15 +93,21 @@ public class ExtractMethodInfoHelper {
}
else {
myOutputName = null;
if (isReturnStatement) {
assert myStatements.length > 0;
GrStatement finalStatement = myStatements[myStatements.length - 1];
if (finalStatement instanceof GrExpression) {
outputType = ((GrExpression)finalStatement).getType();
if (outputType != null) {
outputType = TypeConversionUtil.erasure(outputType);
if (myIsReturnStatement) {
assert returnStatements.size() > 0;
List<PsiType> types = new ArrayList<PsiType>(returnStatements.size());
for (GrStatement statement : returnStatements) {
if (statement instanceof GrReturnStatement) {
GrExpression returnValue = ((GrReturnStatement)statement).getReturnValue();
if (returnValue != null) {
types.add(returnValue.getType());
}
}
else if (statement instanceof GrExpression){
types.add(((GrExpression)statement).getType());
}
}
outputType = TypesUtil.getLeastUpperBoundNullable(types, targetClass.getManager());
}
}
myOutputType = outputType != null ? outputType : PsiType.VOID;
@@ -419,6 +419,9 @@ public class ExtractMethodUtil {
return checked & returnStatements.size() == 0;
}
if (statement instanceof GrExpression) {
return returnStatements.contains(statement);
}
return false;
}
@@ -29,13 +29,16 @@ import com.intellij.psi.PsiDocumentManager;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiFile;
import com.intellij.psi.PsiType;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.refactoring.HelpID;
import com.intellij.refactoring.RefactoringActionHandler;
import com.intellij.refactoring.RefactoringBundle;
import com.intellij.refactoring.util.CommonRefactoringUtil;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.containers.HashSet;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.plugins.groovy.codeInspection.utils.ControlFlowUtils;
import org.jetbrains.plugins.groovy.lang.psi.GrControlFlowOwner;
import org.jetbrains.plugins.groovy.lang.psi.GroovyFileBase;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrStatement;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression;
@@ -51,6 +54,8 @@ import org.jetbrains.plugins.groovy.refactoring.GroovyRefactoringBundle;
import org.jetbrains.plugins.groovy.refactoring.GroovyRefactoringUtil;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
/**
* @author ilyas
@@ -116,8 +121,9 @@ public class GroovyExtractMethodHandler implements RefactoringActionHandler {
}
}
GrMemberOwner owner = ExtractMethodUtil.getMemberOwner(statements[0]);
GrStatementOwner declarationOwner = ExtractMethodUtil.getDeclarationOwner(statements[0]);
GrStatement statement0 = statements[0];
GrMemberOwner owner = ExtractMethodUtil.getMemberOwner(statement0);
GrStatementOwner declarationOwner = ExtractMethodUtil.getDeclarationOwner(statement0);
if (owner == null ||
(declarationOwner == null && !ExtractMethodUtil.isSingleExpression(statements))) {
String message = RefactoringBundle.getCannotRefactorMessage(GroovyRefactoringBundle.message("refactoring.is.not.supported.in.the.current.context"));
@@ -126,20 +132,32 @@ public class GroovyExtractMethodHandler implements RefactoringActionHandler {
}
if (declarationOwner == null &&
ExtractMethodUtil.isSingleExpression(statements) &&
statements[0] instanceof GrExpression && PsiType.VOID.equals(((GrExpression)statements[0]).getType())) {
statement0 instanceof GrExpression && PsiType.VOID.equals(((GrExpression)statement0).getType())) {
String message = RefactoringBundle.getCannotRefactorMessage(GroovyRefactoringBundle.message("selected.expression.has.void.type"));
showErrorMessage(message, project, editor);
return false;
}
// collect information about return statements in selected statement set
Set<GrStatement> allReturnStatements = new HashSet<GrStatement>();
GrControlFlowOwner controlFlowOwner = ControlFlowUtils.findControlFlowOwner(statement0);
assert controlFlowOwner != null;
allReturnStatements.addAll(ControlFlowUtils.collectReturns(controlFlowOwner, true));
ArrayList<GrStatement> returnStatements = new ArrayList<GrStatement>();
for (GrStatement statement : statements) {
returnStatements.addAll(ControlFlowUtils.collectReturns(statement, false));
for (GrStatement returnStatement : allReturnStatements) {
for (GrStatement statement : statements) {
if (PsiTreeUtil.isAncestor(statement, returnStatement, false)) {
returnStatements.add(returnStatement);
break;
}
}
}
// collect information about variables in selected block
FragmentVariableInfos fragmentVariableInfos = ReachingDefinitionsCollector.obtainVariableFlowInformation(statements[0], statements[statements.length - 1]);
FragmentVariableInfos fragmentVariableInfos = ReachingDefinitionsCollector.obtainVariableFlowInformation(statement0, statements[statements.length - 1]);
VariableInfo[] inputInfos = fragmentVariableInfos.getInputVariableNames();
VariableInfo[] outputInfos = fragmentVariableInfos.getOutputVariableNames();
if (outputInfos.length > 1 ||
@@ -160,7 +178,9 @@ public class GroovyExtractMethodHandler implements RefactoringActionHandler {
}
// must be replaced by return statement
boolean hasReturns = returnStatements.size() > 0;
boolean isReturnStatement = ExtractMethodUtil.isReturnStatement(statements[statements.length - 1], returnStatements);
List<GrStatement> returnStatementsCopy = new ArrayList<GrStatement>(returnStatements.size());
returnStatementsCopy.addAll(returnStatements);
boolean isReturnStatement = ExtractMethodUtil.isReturnStatement(statements[statements.length - 1], returnStatementsCopy);
if (!isReturnStatement && hasReturns || hasInterruptingStatements) {
String message = GroovyRefactoringBundle.message("refactoring.is.not.supported.when.return.statement.interrupts.the.execution.flow");
showErrorMessage(message, project, editor);
@@ -168,9 +188,9 @@ public class GroovyExtractMethodHandler implements RefactoringActionHandler {
}
VariableInfo outputInfo = outputInfos.length == 0 ? null : outputInfos[0];
boolean canBeStatic = ExtractMethodUtil.canBeStatic(statements[0]);
boolean canBeStatic = ExtractMethodUtil.canBeStatic(statement0);
ExtractMethodInfoHelper helper = new ExtractMethodInfoHelper(inputInfos, outputInfo, elements, statements, owner, canBeStatic, isReturnStatement);
ExtractMethodInfoHelper helper = new ExtractMethodInfoHelper(inputInfos, outputInfo, elements, statements, owner, canBeStatic, returnStatements);
final String methodName;
if (!ApplicationManager.getApplication().isUnitTestMode()) {
@@ -186,7 +206,7 @@ public class GroovyExtractMethodHandler implements RefactoringActionHandler {
}
assert methodName != null;
runRefactoring(methodName, helper, owner, declarationOwner, editor, statements[0]);
runRefactoring(methodName, helper, owner, declarationOwner, editor, statement0);
return true;
}
@@ -68,7 +68,7 @@ public class ControlFlowTest extends LightCodeInsightFixtureTestCase {
final PsiElement start = file.findElementAt(model.hasSelection() ? model.getSelectionStart() : 0);
final PsiElement end = file.findElementAt(model.hasSelection() ? model.getSelectionEnd() - 1 : file.getTextLength() - 1);
final GrControlFlowOwner owner = PsiTreeUtil.getParentOfType(PsiTreeUtil.findCommonParent(start, end), GrControlFlowOwner.class, false);
final Instruction[] instructions = new ControlFlowBuilder(getProject()).buildControlFlow(owner, null, null);
final Instruction[] instructions = new ControlFlowBuilder(getProject()).buildControlFlow(owner);
final String cf = dumpControlFlow(instructions);
assertEquals(input.get(1).trim(), cf.trim());
}
@@ -20,7 +20,6 @@ import org.jetbrains.plugins.groovy.GroovyFileType;
import org.jetbrains.plugins.groovy.LightGroovyTestCase;
import org.jetbrains.plugins.groovy.util.TestUtils;
import java.io.IOException;
import java.util.List;
/**
@@ -38,11 +37,11 @@ public class ExtractMethodTest extends LightGroovyTestCase {
assertEquals(errorMessage, handler.getInvokeResult());
}
private List<String> readInput() throws IOException {
private List<String> readInput() {
return TestUtils.readInput(getTestDataPath() + getTestName(true) + ".test");
}
private void doTest() throws Exception {
private void doTest() {
final List<String> data = readInput();
GroovyExtractMethodHandler handler = configureFromText(data.get(0));
assertTrue(handler.invokeOnEditor(getProject(), myFixture.getEditor(), myFixture.getFile()));
@@ -50,7 +49,7 @@ public class ExtractMethodTest extends LightGroovyTestCase {
myFixture.checkResult(data.get(1));
}
private GroovyExtractMethodHandler configureFromText(String fileText) throws IOException {
private GroovyExtractMethodHandler configureFromText(String fileText) {
int startOffset = fileText.indexOf(TestUtils.BEGIN_MARKER);
fileText = TestUtils.removeBeginMarker(fileText);
int endOffset = fileText.indexOf(TestUtils.END_MARKER);
@@ -89,5 +88,6 @@ public class ExtractMethodTest extends LightGroovyTestCase {
public void testForIn() throws Throwable { doTest(); }
public void testClosureIt() throws Throwable { doTest(); }
public void testImplicitReturn() {doTest();}
}
+7 -8
View File
@@ -5,13 +5,12 @@ println blah
-----
0(1) element: null
1(2) WRITE blah
2(3,6) element: IF statement
2(3,5) element: IF statement
3(4) READ blah
4(5) WRITE blah
5(6) element: Assignment expression MAYBE_RETURN
6(7) element: IF statement
7(8) READ blah
8(9) WRITE blah
9(10) READ println
10(11) READ blah
11() element: null
5(6) element: IF statement
6(7) READ blah
7(8) WRITE blah
8(9) READ println
9(10) READ blah
10() element: null
+6 -7
View File
@@ -13,12 +13,11 @@ print e
5(9) element: IF statement
6(11) element: Finally clause
7(6,8) CALL 6
8(16) AFTER CALL 7
8(15) AFTER CALL 7
9(6,10) CALL 6
10(14) AFTER CALL 9
10(13) AFTER CALL 9
11(12) READ e
12(13) element: Reference expression MAYBE_RETURN
13(8,10) RETURN
14(15) READ print
15(16) READ e
16() element: null
12(8,10) RETURN
13(14) READ print
14(15) READ e
15() element: null
+10 -11
View File
@@ -9,15 +9,14 @@ if (url == null) {
return url
-----
0(1) element: null
1(2,9) element: IF statement
1(2,8) element: IF statement
2(3) READ url
3(4,6) element: Open block
4(5) WRITE url
5(6,10) element: Assignment expression MAYBE_RETURN
6(7) element: Catch clause
7(8) READ e
8(12) element: THROW statement
9(10) element: IF statement
10(11) READ url
11(12) element: RETURN statement
12() element: null
3(4,5) element: Open block
4(5,9) WRITE url
5(6) element: Catch clause
6(7) READ e
7(11) element: THROW statement
8(9) element: IF statement
9(10) READ url
10(11) element: RETURN statement
11() element: null
@@ -0,0 +1,19 @@
def List getMethodArguments(def methodCall) {
<begin>def argumentsExpression = methodCall.arguments
if (respondsTo(argumentsExpression, 'getExpressions')) {
return [1, 2, 3]
}
[]<end>
}
-----
def List getMethodArguments(def methodCall) {
return testMethod(methodCall)
}
private List<Integer> testMethod(methodCall) {
def argumentsExpression = methodCall.arguments
if (respondsTo(argumentsExpression, 'getExpressions')) {
return [1, 2, 3]
}
[4]
}
@@ -12,6 +12,6 @@ static def foo(int i, int j, int k) {
<caret>return testMethod(v, j)
}
private static def testMethod(int v, int j) {
private static int testMethod(int v, int j) {
return v + j
}
@@ -17,7 +17,7 @@ static def foo(int i, int j, int k) {
<caret>return testMethod(i, v, j)
}
private static def testMethod(int i, int v, int j) {
private static int testMethod(int i, int v, int j) {
if (i > 42) {
println("preved!")
return v + i
@@ -17,7 +17,7 @@ static def foo(int i, int j, int k) {
<caret>return testMethod(i, v, j)
}
private static def testMethod(int i, int v, int j) {
private static int testMethod(int i, int v, int j) {
if (i > 42) {
println("preved!")
} else {