From abb41a2607371229e238dea17ebbc2233d2a0abb Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Tue, 26 Jun 2018 14:55:19 +0300 Subject: [PATCH] Java: Fixed return type when duplicate code is replaced with a call to the extracted method (IDEA-160546) --- .../ConditionalReturnStatementValue.java | 5 +++- .../duplicates/ExpressionReturnValue.java | 5 +++- .../util/duplicates/FieldReturnValue.java | 5 +++- .../util/duplicates/GotoReturnValue.java | 4 +-- .../refactoring/util/duplicates/Match.java | 12 ++++++--- .../ReturnStatementReturnValue.java | 5 +++- .../util/duplicates/ReturnValue.java | 4 ++- .../util/duplicates/VariableReturnValue.java | 18 +++++++++---- .../extractMethod/ExtractMethodProcessor.java | 2 +- .../extractMethod/BoxedConditionalReturn.java | 18 +++++++++++++ .../BoxedConditionalReturn_after.java | 25 +++++++++++++++++++ .../java/refactoring/ExtractMethodTest.java | 4 +++ 12 files changed, 90 insertions(+), 17 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethod/BoxedConditionalReturn.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/BoxedConditionalReturn_after.java diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ConditionalReturnStatementValue.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ConditionalReturnStatementValue.java index 23fa7c3d8e62..c87442ab6244 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ConditionalReturnStatementValue.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ConditionalReturnStatementValue.java @@ -21,6 +21,8 @@ import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.util.PsiUtil; import com.intellij.util.IncorrectOperationException; import com.siyeh.ig.psiutils.ExpressionUtils; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** * @author ven @@ -39,7 +41,8 @@ public class ConditionalReturnStatementValue implements ReturnValue { return PsiEquivalenceUtil.areElementsEquivalent(myReturnValue, otherReturnValue); } - public PsiStatement createReplacement(final PsiMethod extractedMethod, PsiMethodCallExpression methodCallExpression) throws IncorrectOperationException { + @Nullable + public PsiStatement createReplacement(@NotNull final PsiMethod extractedMethod, @NotNull PsiMethodCallExpression methodCallExpression, @Nullable PsiType returnType) throws IncorrectOperationException { final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(methodCallExpression.getProject()).getElementFactory(); PsiIfStatement statement; if (myReturnValue == null) { diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ExpressionReturnValue.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ExpressionReturnValue.java index d8555bd75b67..cd63966e6c3e 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ExpressionReturnValue.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ExpressionReturnValue.java @@ -19,6 +19,8 @@ import com.intellij.codeInsight.PsiEquivalenceUtil; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.util.IncorrectOperationException; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** * @author dsl @@ -39,7 +41,8 @@ public class ExpressionReturnValue implements ReturnValue { return PsiEquivalenceUtil.areElementsEquivalent(myExpression, ((ExpressionReturnValue)other).myExpression); } - public PsiStatement createReplacement(final PsiMethod extractedMethod, final PsiMethodCallExpression methodCallExpression) + @Nullable + public PsiStatement createReplacement(@NotNull final PsiMethod extractedMethod, @NotNull final PsiMethodCallExpression methodCallExpression, @Nullable PsiType returnType) throws IncorrectOperationException { final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(methodCallExpression.getProject()).getElementFactory(); final CodeStyleManager styleManager = CodeStyleManager.getInstance(methodCallExpression.getProject()); diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/FieldReturnValue.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/FieldReturnValue.java index 6ddb9e6f403f..6c1a4f79d97f 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/FieldReturnValue.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/FieldReturnValue.java @@ -18,6 +18,8 @@ package com.intellij.refactoring.util.duplicates; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.util.IncorrectOperationException; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** * @author dsl @@ -38,7 +40,8 @@ public class FieldReturnValue implements ReturnValue { return myField; } - public PsiStatement createReplacement(final PsiMethod extractedMethod, final PsiMethodCallExpression methodCallExpression) throws IncorrectOperationException { + @Nullable + public PsiStatement createReplacement(@NotNull final PsiMethod extractedMethod, @NotNull final PsiMethodCallExpression methodCallExpression, @Nullable PsiType returnType) throws IncorrectOperationException { final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(methodCallExpression.getProject()).getElementFactory(); final CodeStyleManager styleManager = CodeStyleManager.getInstance(methodCallExpression.getProject()); diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/GotoReturnValue.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/GotoReturnValue.java index b92a1fd93e54..fde88b6afa97 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/GotoReturnValue.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/GotoReturnValue.java @@ -21,12 +21,12 @@ import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.util.TypeConversionUtil; import com.intellij.util.IncorrectOperationException; import org.jetbrains.annotations.NonNls; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; public abstract class GotoReturnValue implements ReturnValue { @Nullable - public PsiStatement createReplacement(final PsiMethod extractedMethod, final PsiMethodCallExpression methodCallExpression) throws - IncorrectOperationException { + public PsiStatement createReplacement(@NotNull final PsiMethod extractedMethod, @NotNull final PsiMethodCallExpression methodCallExpression, @Nullable PsiType returnType) throws IncorrectOperationException { if (!TypeConversionUtil.isBooleanType(extractedMethod.getReturnType())) return null; final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(methodCallExpression.getProject()).getElementFactory(); final PsiIfStatement statement = (PsiIfStatement)elementFactory.createStatementFromText(getGotoStatement(), null); diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/Match.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/Match.java index eab012a9409e..a264b268863c 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/Match.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/Match.java @@ -237,7 +237,7 @@ public final class Match { return element; } - public PsiElement replaceByStatement(final PsiMethod extractedMethod, final PsiMethodCallExpression methodCallExpression, final PsiVariable outputVariable) throws IncorrectOperationException { + public PsiElement replaceByStatement(final PsiMethod extractedMethod, final PsiMethodCallExpression methodCallExpression, final PsiVariable outputVariable, @Nullable PsiType returnType) throws IncorrectOperationException { PsiStatement statement = null; if (outputVariable != null) { ReturnValue returnValue = getOutputVariableValue(outputVariable); @@ -245,10 +245,10 @@ public final class Match { returnValue = new FieldReturnValue((PsiField)outputVariable); } if (returnValue == null) return null; - statement = returnValue.createReplacement(extractedMethod, methodCallExpression); + statement = returnValue.createReplacement(extractedMethod, methodCallExpression, returnType); } else if (getReturnValue() != null) { - statement = getReturnValue().createReplacement(extractedMethod, methodCallExpression); + statement = getReturnValue().createReplacement(extractedMethod, methodCallExpression, returnType); } if (statement == null) { final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(methodCallExpression.getProject()).getElementFactory(); @@ -271,12 +271,16 @@ public final class Match { } public PsiElement replace(final PsiMethod extractedMethod, final PsiMethodCallExpression methodCallExpression, PsiVariable outputVariable) throws IncorrectOperationException { + return replace(extractedMethod, methodCallExpression, outputVariable, null); + } + + public PsiElement replace(final PsiMethod extractedMethod, final PsiMethodCallExpression methodCallExpression, PsiVariable outputVariable, @Nullable PsiType returnType) throws IncorrectOperationException { declareLocalVariables(); if (getMatchStart() == getMatchEnd() && getMatchStart() instanceof PsiExpression) { return replaceWithExpression(methodCallExpression); } else { - return replaceByStatement(extractedMethod, methodCallExpression, outputVariable); + return replaceByStatement(extractedMethod, methodCallExpression, outputVariable, returnType); } } diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ReturnStatementReturnValue.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ReturnStatementReturnValue.java index ea66c8636463..887aaf9e1948 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ReturnStatementReturnValue.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ReturnStatementReturnValue.java @@ -18,6 +18,8 @@ package com.intellij.refactoring.util.duplicates; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.util.IncorrectOperationException; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** * @author dsl @@ -31,7 +33,8 @@ public class ReturnStatementReturnValue implements ReturnValue { return other instanceof ReturnStatementReturnValue; } - public PsiStatement createReplacement(final PsiMethod extractedMethod, final PsiMethodCallExpression methodCallExpression) throws IncorrectOperationException { + @Nullable + public PsiStatement createReplacement(@NotNull final PsiMethod extractedMethod, @NotNull final PsiMethodCallExpression methodCallExpression,@Nullable PsiType returnType) throws IncorrectOperationException { final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(methodCallExpression.getProject()).getElementFactory(); final CodeStyleManager styleManager = CodeStyleManager.getInstance(methodCallExpression.getProject()); PsiReturnStatement returnStatement = (PsiReturnStatement)elementFactory.createStatementFromText("return x;", null); diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ReturnValue.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ReturnValue.java index 896ea60c3f98..612ef9b61eea 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ReturnValue.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ReturnValue.java @@ -18,7 +18,9 @@ package com.intellij.refactoring.util.duplicates; import com.intellij.psi.PsiMethod; import com.intellij.psi.PsiMethodCallExpression; import com.intellij.psi.PsiStatement; +import com.intellij.psi.PsiType; import com.intellij.util.IncorrectOperationException; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; /** @@ -28,5 +30,5 @@ public interface ReturnValue { boolean isEquivalent(ReturnValue other); @Nullable - PsiStatement createReplacement(final PsiMethod extractedMethod, PsiMethodCallExpression methodCallExpression) throws IncorrectOperationException; + PsiStatement createReplacement(@NotNull PsiMethod extractedMethod, @NotNull PsiMethodCallExpression methodCallExpression, @Nullable PsiType returnType) throws IncorrectOperationException; } diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/VariableReturnValue.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/VariableReturnValue.java index 75edb10c4853..5c6e1e8040f3 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/VariableReturnValue.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/VariableReturnValue.java @@ -15,9 +15,13 @@ */ package com.intellij.refactoring.util.duplicates; +import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; +import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.util.IncorrectOperationException; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** * @author dsl @@ -38,14 +42,18 @@ public class VariableReturnValue implements ReturnValue { return myVariable; } - public PsiStatement createReplacement(final PsiMethod extractedMethod, final PsiMethodCallExpression methodCallExpression) throws IncorrectOperationException { + @Nullable + public PsiStatement createReplacement(@NotNull final PsiMethod extractedMethod, @NotNull final PsiMethodCallExpression methodCallExpression, @Nullable final PsiType returnType) throws IncorrectOperationException { final PsiDeclarationStatement statement; - final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(methodCallExpression.getProject()).getElementFactory(); - final CodeStyleManager styleManager = CodeStyleManager.getInstance(methodCallExpression.getProject()); + Project project = methodCallExpression.getProject(); + final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(project).getElementFactory(); + final CodeStyleManager styleManager = CodeStyleManager.getInstance(project); + JavaCodeStyleManager javaStyleManager = JavaCodeStyleManager.getInstance(project); + PsiType type = returnType != null && returnType.isValid() ? returnType : myVariable.getType(); statement = (PsiDeclarationStatement)styleManager.reformat( - elementFactory.createVariableDeclarationStatement(myVariable.getName(), myVariable.getType(), methodCallExpression) - ); + javaStyleManager.shortenClassReferences( + elementFactory.createVariableDeclarationStatement(myVariable.getName(), type, methodCallExpression))); ((PsiVariable)statement.getDeclaredElements()[0]).getModifierList().replace(myVariable.getModifierList()); return statement; } 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 97146c0d3c59..f613e1b077a2 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java @@ -1355,7 +1355,7 @@ public class ExtractMethodProcessor implements MatchProvider { } } List reusedVariables = findReusedVariables(match, myInputVariables, myOutputVariable); - PsiElement replacedMatch = match.replace(myExtractedMethod, methodCallExpression, myOutputVariable); + PsiElement replacedMatch = match.replace(myExtractedMethod, methodCallExpression, myOutputVariable, myReturnType); PsiElement appendLocation = addNotNullConditionalCheck(match, replacedMatch); declareReusedVariables(appendLocation, reusedVariables); diff --git a/java/java-tests/testData/refactoring/extractMethod/BoxedConditionalReturn.java b/java/java-tests/testData/refactoring/extractMethod/BoxedConditionalReturn.java new file mode 100644 index 000000000000..f9d35ac7b5f9 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/BoxedConditionalReturn.java @@ -0,0 +1,18 @@ +class Test { + public static Integer foo(Integer[] a) { + + if (a.length != 0) { + int n = a[0] != null ? a[0] : 0; + return n; + } + return null; + } + + public static Integer bar(Integer[] a) { + if (a.length != 0) { + int n = a[0] != null ? a[0] : 0; + return n; + } + return null; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/BoxedConditionalReturn_after.java b/java/java-tests/testData/refactoring/extractMethod/BoxedConditionalReturn_after.java new file mode 100644 index 000000000000..ca7e24ea0123 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/BoxedConditionalReturn_after.java @@ -0,0 +1,25 @@ +import org.jetbrains.annotations.Nullable; + +class Test { + public static Integer foo(Integer[] a) { + + Integer n = newMethod(a); + if (n != null) return n; + return null; + } + + @Nullable + private static Integer newMethod(Integer[] a) { + if (a.length != 0) { + int n = a[0] != null ? a[0] : 0; + return n; + } + return null; + } + + public static Integer bar(Integer[] a) { + Integer n = newMethod(a); + if (n != null) return n; + return null; + } +} \ No newline at end of file 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 da2541f0f6b7..3c780b600de8 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java @@ -1270,6 +1270,10 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { doDuplicatesTest(); } + public void testBoxedConditionalReturn() throws Exception { + doDuplicatesTest(); + } + public void testBeforeCommentAfterSelectedFragment() throws Exception { doTest(); }