From f2cddf72019fa7225ea953959bfded3c460e121d Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Wed, 17 Jun 2015 19:05:20 +0300 Subject: [PATCH] extract method: don't skip vars from the exit statements if nullable checks would be added (IDEA-141562) --- .../extractMethod/ControlFlowWrapper.java | 16 ++++++++++---- .../extractMethod/ExtractMethodProcessor.java | 4 ++-- ...houldPreserveVarsUsedInExitStatements.java | 12 +++++++++++ ...reserveVarsUsedInExitStatements_after.java | 21 +++++++++++++++++++ .../GuardMethodDuplicates1_after.java | 1 - .../refactoring/ExtractMethodTest.java | 4 ++++ 6 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethod/ConditionalExitCombinedWithNullabilityShouldPreserveVarsUsedInExitStatements.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/ConditionalExitCombinedWithNullabilityShouldPreserveVarsUsedInExitStatements_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 eb16437e6a33..11df15fbd9ac 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ControlFlowWrapper.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ControlFlowWrapper.java @@ -242,10 +242,10 @@ public class ControlFlowWrapper { return true; } - public List getInputVariables(final PsiElement codeFragment, PsiElement[] elements) { + public List getInputVariables(final PsiElement codeFragment, PsiElement[] elements, PsiVariable[] outputVariables) { final List inputVariables = ControlFlowUtil.getInputVariables(myControlFlow, myFlowStart, myFlowEnd); List myInputVariables; - if (myGenerateConditionalExit) { + if (skipVariablesFromExitStatements(outputVariables)) { List inputVariableList = new ArrayList(inputVariables); removeParametersUsedInExitsOnly(codeFragment, inputVariableList); myInputVariables = inputVariableList; @@ -341,8 +341,16 @@ public class ControlFlowWrapper { return getUsedVariables(myFlowEnd); } - public List getUsedVariablesInBody() { - return getUsedVariables(myFlowStart, myFlowEnd); + public List getUsedVariablesInBody(PsiElement codeFragment, PsiVariable[] outputVariables) { + final List variables = getUsedVariables(myFlowStart, myFlowEnd); + if (skipVariablesFromExitStatements(outputVariables)) { + removeParametersUsedInExitsOnly(codeFragment, variables); + } + return variables; + } + + private boolean skipVariablesFromExitStatements(PsiVariable[] outputVariables) { + return myGenerateConditionalExit && outputVariables.length == 0; } public Collection getInitializedTwice() { 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 987e7157303c..61fb8703159d 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java @@ -1438,7 +1438,7 @@ public class ExtractMethodProcessor implements MatchProvider { } private boolean chooseTargetClass(PsiElement codeFragment, final Pass extractPass) throws PrepareFailedException { - final List inputVariables = myControlFlowWrapper.getInputVariables(codeFragment, myElements); + final List inputVariables = myControlFlowWrapper.getInputVariables(codeFragment, myElements, myOutputVariables); myNeedChangeContext = false; myTargetClass = myCodeFragmentMember instanceof PsiMember @@ -1513,7 +1513,7 @@ public class ExtractMethodProcessor implements MatchProvider { } private void declareNecessaryVariablesInsideBody(PsiCodeBlock body) throws IncorrectOperationException { - List usedVariables = myControlFlowWrapper.getUsedVariablesInBody(); + List usedVariables = myControlFlowWrapper.getUsedVariablesInBody(ControlFlowUtil.findCodeFragment(myElements[0]), myOutputVariables); for (PsiVariable variable : usedVariables) { boolean toDeclare = !isDeclaredInside(variable) && myInputVariables.toDeclareInsideBody(variable); if (toDeclare) { diff --git a/java/java-tests/testData/refactoring/extractMethod/ConditionalExitCombinedWithNullabilityShouldPreserveVarsUsedInExitStatements.java b/java/java-tests/testData/refactoring/extractMethod/ConditionalExitCombinedWithNullabilityShouldPreserveVarsUsedInExitStatements.java new file mode 100644 index 000000000000..831178c21123 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/ConditionalExitCombinedWithNullabilityShouldPreserveVarsUsedInExitStatements.java @@ -0,0 +1,12 @@ +class X { + static String guessTestDataName(String method, String testName, String[] methods) { + for (String psiMethod : methods) { + String strings = method; + if (strings != null && !strings.isEmpty()) { + return strings.substring(0) + testName; + } + + } + return null; + } +} diff --git a/java/java-tests/testData/refactoring/extractMethod/ConditionalExitCombinedWithNullabilityShouldPreserveVarsUsedInExitStatements_after.java b/java/java-tests/testData/refactoring/extractMethod/ConditionalExitCombinedWithNullabilityShouldPreserveVarsUsedInExitStatements_after.java new file mode 100644 index 000000000000..db3cef1ba79d --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/ConditionalExitCombinedWithNullabilityShouldPreserveVarsUsedInExitStatements_after.java @@ -0,0 +1,21 @@ +import org.jetbrains.annotations.Nullable; + +class X { + static String guessTestDataName(String method, String testName, String[] methods) { + for (String psiMethod : methods) { + String strings = newMethod(method, testName); + if (strings != null) return strings; + + } + return null; + } + + @Nullable + private static String newMethod(String method, String testName) { + String strings = method; + if (strings != null && !strings.isEmpty()) { + return strings.substring(0) + testName; + } + return null; + } +} diff --git a/java/java-tests/testData/refactoring/extractMethod/GuardMethodDuplicates1_after.java b/java/java-tests/testData/refactoring/extractMethod/GuardMethodDuplicates1_after.java index 6ad32250004a..37d1a9bdcca9 100644 --- a/java/java-tests/testData/refactoring/extractMethod/GuardMethodDuplicates1_after.java +++ b/java/java-tests/testData/refactoring/extractMethod/GuardMethodDuplicates1_after.java @@ -7,7 +7,6 @@ public class Test { } private boolean newMethod() { - Object result; if (test1()) return true; if (test2()) return true; return false; diff --git a/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java b/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java index cad137eae2ab..6a232ae4b830 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java @@ -749,6 +749,10 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { } } + public void testConditionalExitCombinedWithNullabilityShouldPreserveVarsUsedInExitStatements() throws Exception { + doTest(); + } + private void doTestDisabledParam() throws PrepareFailedException { final CodeStyleSettings settings = CodeStyleSettingsManager.getSettings(getProject()); settings.ELSE_ON_NEW_LINE = true;