extract method: suggest non-void return type if final field is assigned inside (IDEA-93737)

This commit is contained in:
Anna Kozlova
2014-12-04 13:43:01 +01:00
parent ad726d0abc
commit e2e8384516
4 changed files with 66 additions and 17 deletions
@@ -47,10 +47,7 @@ import com.intellij.openapi.util.text.StringUtil;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.openapi.wm.WindowManager;
import com.intellij.psi.*;
import com.intellij.psi.codeStyle.CodeStyleManager;
import com.intellij.psi.codeStyle.CodeStyleSettingsManager;
import com.intellij.psi.codeStyle.JavaCodeStyleManager;
import com.intellij.psi.codeStyle.VariableKind;
import com.intellij.psi.codeStyle.*;
import com.intellij.psi.controlFlow.ControlFlowUtil;
import com.intellij.psi.impl.source.codeStyle.JavaCodeStyleManagerImpl;
import com.intellij.psi.scope.processor.VariablesProcessor;
@@ -345,6 +342,25 @@ public class ExtractMethodProcessor implements MatchProvider {
@Nullable
private PsiVariable getArtificialOutputVariable() {
if (myOutputVariables.length == 0) {
if (myCanBeChainedConstructor) {
final Set<PsiField> fields = new HashSet<PsiField>();
for (PsiElement element : myElements) {
element.accept(new JavaRecursiveElementWalkingVisitor() {
@Override
public void visitReferenceExpression(PsiReferenceExpression expression) {
super.visitReferenceExpression(expression);
final PsiElement resolve = expression.resolve();
if (resolve instanceof PsiField && ((PsiField)resolve).hasModifierProperty(PsiModifier.FINAL) &&
PsiUtil.isAccessedForWriting(expression)) {
fields.add((PsiField)resolve);
}
}
});
}
if (!fields.isEmpty()) {
return fields.size() == 1 ? fields.iterator().next() : null;
}
}
final VariablesProcessor processor = new VariablesProcessor(true) {
@Override
protected boolean check(PsiVariable var, ResolveState state) {
@@ -549,7 +565,7 @@ public class ExtractMethodProcessor implements MatchProvider {
}
protected boolean isVoidReturn() {
return myArtificialOutputVariable != null;
return myArtificialOutputVariable != null && !(myArtificialOutputVariable instanceof PsiField);
}
@Override
@@ -859,9 +875,9 @@ public class ExtractMethodProcessor implements MatchProvider {
myFirstExitStatementCopy = (PsiStatement)ifStatement.getThenBranch().replace(myFirstExitStatementCopy);
CodeStyleManager.getInstance(myProject).reformat(ifStatement);
}
else if (myOutputVariable != null) {
String name = myOutputVariable.getName();
boolean toDeclare = isDeclaredInside(myOutputVariable);
else if (myOutputVariable != null || isArtificialOutputUsed()) {
boolean toDeclare = isArtificialOutputUsed() ? !(myArtificialOutputVariable instanceof PsiField) : isDeclaredInside(myOutputVariable);
String name = isArtificialOutputUsed() ? myArtificialOutputVariable.getName() : myOutputVariable.getName();
if (!toDeclare) {
PsiExpressionStatement statement = (PsiExpressionStatement)myElementFactory.createStatementFromText(name + "=x;", null);
statement = (PsiExpressionStatement)myStyleManager.reformat(statement);
@@ -879,14 +895,9 @@ public class ExtractMethodProcessor implements MatchProvider {
myMethodCall = (PsiMethodCallExpression)((PsiReturnStatement)statement).getReturnValue().replace(myMethodCall);
}
else {
if (myArtificialOutputVariable != null && myReturnType != PsiType.VOID) {
declareVariableAtMethodCallLocation(myArtificialOutputVariable.getName());
}
else {
PsiStatement statement = myElementFactory.createStatementFromText("x();", null);
statement = (PsiStatement)addToMethodCallLocation(statement);
myMethodCall = (PsiMethodCallExpression)((PsiExpressionStatement)statement).getExpression().replace(myMethodCall);
}
PsiStatement statement = myElementFactory.createStatementFromText("x();", null);
statement = (PsiStatement)addToMethodCallLocation(statement);
myMethodCall = (PsiMethodCallExpression)((PsiExpressionStatement)statement).getExpression().replace(myMethodCall);
}
if (myHasReturnStatement && !myHasReturnStatementOutput && !hasNormalExit()) {
PsiStatement statement = myElementFactory.createStatementFromText("return;", null);
@@ -1046,12 +1057,16 @@ public class ExtractMethodProcessor implements MatchProvider {
}
}
}
else if (myArtificialOutputVariable != null && !PsiType.VOID.equals(myReturnType)) {
else if (isArtificialOutputUsed()) {
body.add(myElementFactory.createStatementFromText("return " + myArtificialOutputVariable.getName() + ";", null));
}
return exitStatementCopy;
}
private boolean isArtificialOutputUsed() {
return myArtificialOutputVariable != null && !PsiType.VOID.equals(myReturnType) && !myIsChainedConstructor;
}
private boolean hasNormalExit() {
boolean hasNormalExit = false;
PsiElement lastElement = myElements[myElements.length - 1];
@@ -1467,6 +1482,10 @@ public class ExtractMethodProcessor implements MatchProvider {
body.add(statement);
}
}
if (myArtificialOutputVariable instanceof PsiField && !myIsChainedConstructor) {
body.add(myElementFactory.createVariableDeclarationStatement(myArtificialOutputVariable.getName(), myArtificialOutputVariable.getType(), null));
}
}
protected void declareNecessaryVariablesAfterCall(PsiVariable outputVariable) throws IncorrectOperationException {
@@ -0,0 +1,10 @@
class X {
private final int i;
public X() {
<selection>
i = 0;
System.out.println(i);
</selection>
}
}
@@ -0,0 +1,16 @@
class X {
private final int i;
public X() {
i = newMethod();
}
private int newMethod() {
int i;
i = 0;
System.out.println(i);
return i;
}
}
@@ -638,6 +638,10 @@ public class ExtractMethodTest extends LightCodeInsightTestCase {
doTestReturnTypeChanged(PsiType.INT);
}
public void testReassignFinalFieldInside() throws Exception {
doTestReturnTypeChanged(PsiType.INT);
}
public void testPassFieldAsParameterAndMakeStatic() throws Exception {
doTestPassFieldsAsParams();
}