From e2e838451657d493023d075e49905b9bf5df3292 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Thu, 4 Dec 2014 13:37:38 +0100 Subject: [PATCH] extract method: suggest non-void return type if final field is assigned inside (IDEA-93737) --- .../extractMethod/ExtractMethodProcessor.java | 53 +++++++++++++------ .../ReassignFinalFieldInside.java | 10 ++++ .../ReassignFinalFieldInside_after.java | 16 ++++++ .../refactoring/ExtractMethodTest.java | 4 ++ 4 files changed, 66 insertions(+), 17 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethod/ReassignFinalFieldInside.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/ReassignFinalFieldInside_after.java diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java index 5c9701ab8621..ba4c2471c7ce 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java @@ -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 fields = new HashSet(); + 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 { diff --git a/java/java-tests/testData/refactoring/extractMethod/ReassignFinalFieldInside.java b/java/java-tests/testData/refactoring/extractMethod/ReassignFinalFieldInside.java new file mode 100644 index 000000000000..77c23b0535c3 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/ReassignFinalFieldInside.java @@ -0,0 +1,10 @@ +class X { + private final int i; + + public X() { + + i = 0; + System.out.println(i); + + } +} diff --git a/java/java-tests/testData/refactoring/extractMethod/ReassignFinalFieldInside_after.java b/java/java-tests/testData/refactoring/extractMethod/ReassignFinalFieldInside_after.java new file mode 100644 index 000000000000..921634d5726e --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/ReassignFinalFieldInside_after.java @@ -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; + } +} diff --git a/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java b/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java index 97d58a12ae6d..58678084b5d0 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java @@ -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(); }