From 62967da97742b072a814f9d091994957c59d742b Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Thu, 31 Aug 2017 17:10:12 +0300 Subject: [PATCH] Java: If a variable is defined in the extracted part and reused later in the code declare it after the extracted part (IDEA-178180) --- .../extractMethod/ControlFlowWrapper.java | 2 +- .../extractMethod/ExtractMethodProcessor.java | 14 ++++++++ .../ExtractMethodObjectProcessor.java | 2 +- .../psi/controlFlow/ControlFlowUtil.java | 6 ++-- .../ExtractedVariableReused.java | 22 +++++++++++++ .../ExtractedVariableReused_after.java | 32 +++++++++++++++++++ .../java/refactoring/ExtractMethodTest.java | 4 +++ 7 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethod/ExtractedVariableReused.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/ExtractedVariableReused_after.java diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/ControlFlowWrapper.java b/java/java-impl/src/com/intellij/refactoring/extractMethod/ControlFlowWrapper.java index 024f889e9858..dd3c50ca8583 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ControlFlowWrapper.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ControlFlowWrapper.java @@ -147,7 +147,7 @@ public class ControlFlowWrapper { return myExitStatements; } - public boolean isVariableUsedAfterEnd(PsiVariable variable) { + public boolean needVariableValueAfterEnd(PsiVariable variable) { return ControlFlowUtil.needVariableValueAt(variable, myControlFlow, myFlowEnd); } 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 31995ab9c579..9858fbce6bc0 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java @@ -944,6 +944,7 @@ public class ExtractMethodProcessor implements MatchProvider { String varName = myOutputVariable != null ? myOutputVariable.getName() : "x"; varName = declareVariableAtMethodCallLocation(varName, myReturnType instanceof PsiPrimitiveType ? ((PsiPrimitiveType)myReturnType).getBoxedType(myCodeFragmentMember) : myReturnType); addToMethodCallLocation(myElementFactory.createStatementFromText("if (" + varName + " != null) return " + varName + ";", null)); + declareVariableReusedAfterCall(myOutputVariable); } else if (myGenerateConditionalExit) { PsiIfStatement ifStatement = (PsiIfStatement)myElementFactory.createStatementFromText("if (a) b;", null); @@ -1882,6 +1883,19 @@ public class ExtractMethodProcessor implements MatchProvider { } } + private void declareVariableReusedAfterCall(PsiVariable variable) { + if (variable != null && + variable.getName() != null && + isDeclaredInside(variable) && + myControlFlowWrapper.getUsedVariables().contains(variable) && + !myControlFlowWrapper.needVariableValueAfterEnd(variable)) { + + PsiDeclarationStatement declaration = + myElementFactory.createVariableDeclarationStatement(variable.getName(), variable.getType(), null); + addToMethodCallLocation(declaration); + } + } + private void showMultipleExitPointsMessage() { if (myShowErrorDialogs) { HighlightManager highlightManager = HighlightManager.getInstance(myProject); diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethodObject/ExtractMethodObjectProcessor.java b/java/java-impl/src/com/intellij/refactoring/extractMethodObject/ExtractMethodObjectProcessor.java index 2039c9c8bf67..8860f67e1808 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethodObject/ExtractMethodObjectProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethodObject/ExtractMethodObjectProcessor.java @@ -846,7 +846,7 @@ public class ExtractMethodObjectProcessor extends BaseRefactoringProcessor { } PsiVariable[] usedVariables = myOutputVariables; - if (generatesConditionalExit() && myOutputVariable != null && !myControlFlowWrapper.isVariableUsedAfterEnd(myOutputVariable)) { + if (generatesConditionalExit() && myOutputVariable != null && !myControlFlowWrapper.needVariableValueAfterEnd(myOutputVariable)) { usedVariables = ArrayUtil.remove(usedVariables, myOutputVariable); } Collection reassigned = myControlFlowWrapper.getInitializedTwice(); diff --git a/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java b/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java index a0593d72ae7d..8c3f3f65fa99 100644 --- a/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java +++ b/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java @@ -1729,15 +1729,13 @@ public class ControlFlowUtil { } public CopyOnWriteList(Collection infos) { - list = new LinkedList<>(infos); + list = new SmartList<>(infos); } public CopyOnWriteList addAll(CopyOnWriteList addList) { CopyOnWriteList newList = new CopyOnWriteList(); List list = getList(); - for (final VariableInfo variableInfo : list) { - newList.list.add(variableInfo); - } + newList.list.addAll(list); List toAdd = addList.getList(); for (final VariableInfo variableInfo : toAdd) { if (!newList.list.contains(variableInfo)) { diff --git a/java/java-tests/testData/refactoring/extractMethod/ExtractedVariableReused.java b/java/java-tests/testData/refactoring/extractMethod/ExtractedVariableReused.java new file mode 100644 index 000000000000..072d871605e3 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/ExtractedVariableReused.java @@ -0,0 +1,22 @@ +public class OutputVariableReused { + + static class X { + X(String s) {} + } + + String convert(String s, String s1, String s2) { + return s + s1 + s2; + } + + public X test(String s, String left, String right) { + String res = convert(s, left, right); + if (res != null) { + return new X(res); + } + res = convert(s, right, left); + if (res != null) { + return new X(res); + } + return null; + } +} diff --git a/java/java-tests/testData/refactoring/extractMethod/ExtractedVariableReused_after.java b/java/java-tests/testData/refactoring/extractMethod/ExtractedVariableReused_after.java new file mode 100644 index 000000000000..c7a858b4de46 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/ExtractedVariableReused_after.java @@ -0,0 +1,32 @@ +import org.jetbrains.annotations.Nullable; + +public class OutputVariableReused { + + static class X { + X(String s) {} + } + + String convert(String s, String s1, String s2) { + return s + s1 + s2; + } + + public X test(String s, String left, String right) { + X res1 = newMethod(s, left, right); + if (res1 != null) return res1; + String res; + res = convert(s, right, left); + if (res != null) { + return new X(res); + } + return null; + } + + @Nullable + private X newMethod(String s, String left, String right) { + String res = convert(s, left, right); + if (res != null) { + return new X(res); + } + return null; + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java index 5a66eb403113..ab3261bfbedc 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java @@ -1047,6 +1047,10 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { doTest(); } + public void testExtractedVariableReused() throws Exception { + doTest(); + } + private void doTestDisabledParam() throws PrepareFailedException { final CodeStyleSettings settings = CodeStyleSettingsManager.getSettings(getProject()); settings.ELSE_ON_NEW_LINE = true;