From 16cbf0277412085a350bbe661d44866f67068c57 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Mon, 16 Oct 2017 12:52:16 +0300 Subject: [PATCH] Java: Add conditional return when replacing duplicate fragment (IDEA-180601) --- .../extractMethod/ExtractMethodProcessor.java | 21 ++++++++++++++- .../ConditionalReturnInDuplicate.java | 24 +++++++++++++++++ .../ConditionalReturnInDuplicate_after.java | 25 +++++++++++++++++ .../ConditionalReturnVsAssignDuplicate.java | 18 +++++++++++++ ...ditionalReturnVsAssignDuplicate_after.java | 27 +++++++++++++++++++ .../java/refactoring/ExtractMethodTest.java | 9 +++++++ 6 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 java/java-tests/testData/refactoring/extractMethod/ConditionalReturnInDuplicate.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/ConditionalReturnInDuplicate_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/ConditionalReturnVsAssignDuplicate.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/ConditionalReturnVsAssignDuplicate_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 a827c63aa40d..0739c7407ae2 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java @@ -1307,7 +1307,26 @@ public class ExtractMethodProcessor implements MatchProvider { methodCallExpression.getArgumentList().add(myElementFactory.createExpressionFromText(data.variable.getName(), methodCallExpression)); } } - return match.replace(myExtractedMethod, methodCallExpression, myOutputVariable); + PsiElement replacedMatch = match.replace(myExtractedMethod, methodCallExpression, myOutputVariable); + + addNotNullConditionalCheck(match, replacedMatch); + return replacedMatch; + } + + private void addNotNullConditionalCheck(Match match, PsiElement replacedMatch) { + if (myNotNullConditionalCheck && myOutputVariable != null) { + ReturnValue returnValue = match.getOutputVariableValue(myOutputVariable); + if (returnValue instanceof VariableReturnValue) { + String varName = ((VariableReturnValue)returnValue).getVariable().getName(); + LOG.assertTrue(varName != null, "returned variable name is null"); + PsiStatement statement = PsiTreeUtil.getParentOfType(replacedMatch, PsiStatement.class, false); + if (statement != null) { + PsiStatement conditionalReturn = + myElementFactory.createStatementFromText("if (" + varName + " != null) return " + varName + ";", null); + statement.getParent().addAfter(conditionalReturn, statement); + } + } + } } @Nullable diff --git a/java/java-tests/testData/refactoring/extractMethod/ConditionalReturnInDuplicate.java b/java/java-tests/testData/refactoring/extractMethod/ConditionalReturnInDuplicate.java new file mode 100644 index 000000000000..b5e4fb9d2df4 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/ConditionalReturnInDuplicate.java @@ -0,0 +1,24 @@ +class Conditional { + int[] bar(String[] s) { + + if (s != null) { + int[] n = new int[s.length]; + for (int i = 0; i < s.length; i++) { + n[i] = s[i].length(); + } + return n; + } + return new int[0]; + } + + int[] baz(String[] z) { + if (z != null) { + int[] n = new int[z.length]; + for (int i = 0; i < z.length; i++) { + n[i] = z[i].length(); + } + return n; + } + return new int[0]; + } +} diff --git a/java/java-tests/testData/refactoring/extractMethod/ConditionalReturnInDuplicate_after.java b/java/java-tests/testData/refactoring/extractMethod/ConditionalReturnInDuplicate_after.java new file mode 100644 index 000000000000..1c0a814f0d4a --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/ConditionalReturnInDuplicate_after.java @@ -0,0 +1,25 @@ +class Conditional { + int[] bar(String[] s) { + + int[] n = newMethod(s); + if (n != null) return n; + return new int[0]; + } + + private int[] newMethod(String[] s) { + if (s != null) { + int[] n = new int[s.length]; + for (int i = 0; i < s.length; i++) { + n[i] = s[i].length(); + } + return n; + } + return null; + } + + int[] baz(String[] z) { + int[] n = newMethod(z); + if (n != null) return n; + return new int[0]; + } +} diff --git a/java/java-tests/testData/refactoring/extractMethod/ConditionalReturnVsAssignDuplicate.java b/java/java-tests/testData/refactoring/extractMethod/ConditionalReturnVsAssignDuplicate.java new file mode 100644 index 000000000000..41c3c0d4f3ba --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/ConditionalReturnVsAssignDuplicate.java @@ -0,0 +1,18 @@ +class Conditional { + int bar(String s) { + if (s != null) { + int n = s.length; + return n; + } + return 0; + } + + int baz(String z) { + int x = -1; + if (z != null) { + int n = z.length; + x = n; + } + return 0; + } +} diff --git a/java/java-tests/testData/refactoring/extractMethod/ConditionalReturnVsAssignDuplicate_after.java b/java/java-tests/testData/refactoring/extractMethod/ConditionalReturnVsAssignDuplicate_after.java new file mode 100644 index 000000000000..866809c7145f --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/ConditionalReturnVsAssignDuplicate_after.java @@ -0,0 +1,27 @@ +import org.jetbrains.annotations.Nullable; + +class Conditional { + int bar(String s) { + Integer n = newMethod(s); + if (n != null) return n; + return 0; + } + + @Nullable + private Integer newMethod(String s) { + if (s != null) { + int n = s.length; + return n; + } + return null; + } + + int baz(String z) { + int x = -1; + if (z != null) { + int n = z.length; + x = n; + } + return 0; + } +} 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 3974c94802b4..739f94557022 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java @@ -843,6 +843,15 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { doDuplicatesTest(); } + public void testConditionalReturnInDuplicate() throws Exception { + doDuplicatesTest(); + } + + // todo DuplicatesFinder.canBeEquivalent() should see the difference between 'return' and assignment + public void _testConditionalReturnVsAssignDuplicate() throws Exception { + doDuplicatesTest(); + } + public void testSuggestChangeSignatureWithChangedParameterName() throws Exception { configureByFile(BASE_PATH + getTestName(false) + ".java"); boolean success = performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, null, false, "p");