diff --git a/java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java b/java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java index 02d6fb790508..b8252734d98a 100644 --- a/java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java @@ -60,16 +60,19 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal final PsiCodeBlock returnScope = (PsiCodeBlock)returnParent; final PsiStatement[] statements = returnScope.getStatements(); if (statements.length != 0 && statements[statements.length - 1] == returnStatement) { - PsiStatement refactoredStatement = getPrevNonEmptyStatement(returnStatement, null); - if (refactoredStatement != null) { - final PsiExpression returnValue = returnStatement.getReturnValue(); - if (returnValue instanceof PsiReferenceExpression) { - final PsiElement resolved = ((PsiReferenceExpression)returnValue).resolve(); - if (resolved instanceof PsiVariable) { - final PsiVariable returnedVariable = (PsiVariable)resolved; - final PsiCodeBlock variableScope = getVariableScopeBlock(returnedVariable); - if (variableScope != null) { - return new ReturnContext(returnStatement, returnScope, refactoredStatement, returnedVariable, variableScope); + final PsiType returnType = getReturnType(returnScope); + if (returnType != null) { + PsiStatement refactoredStatement = getPrevNonEmptyStatement(returnStatement, null); + if (refactoredStatement != null) { + final PsiExpression returnValue = returnStatement.getReturnValue(); + if (returnValue instanceof PsiReferenceExpression) { + final PsiElement resolved = ((PsiReferenceExpression)returnValue).resolve(); + if (resolved instanceof PsiVariable) { + final PsiVariable returnedVariable = (PsiVariable)resolved; + final PsiCodeBlock variableScope = getVariableScopeBlock(returnedVariable); + if (variableScope != null) { + return new ReturnContext(returnStatement, returnScope, returnType, refactoredStatement, returnedVariable, variableScope); + } } } } @@ -79,6 +82,31 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal return null; } + @Nullable + private static PsiType getReturnType(PsiCodeBlock returnScope) { + NavigatablePsiElement returnFrom = PsiTreeUtil.getNonStrictParentOfType(returnScope, PsiMethod.class, PsiLambdaExpression.class); + if (returnFrom instanceof PsiMethod) { + return ((PsiMethod)returnFrom).getReturnType(); + } + if (returnFrom instanceof PsiLambdaExpression) { + return getNonParametrizedReturnType((PsiLambdaExpression)returnFrom); + } + return null; + } + + @Nullable + private static PsiType getNonParametrizedReturnType(PsiLambdaExpression lambdaExpression) { + final PsiMethod interfaceMethod = LambdaUtil.getFunctionalInterfaceMethod(lambdaExpression.getFunctionalInterfaceType()); + if (interfaceMethod != null) { + final PsiType returnType = interfaceMethod.getReturnType(); + if (returnType instanceof PsiPrimitiveType || + returnType instanceof PsiClassType && ((PsiClassType)returnType).getParameterCount() == 0) { + return returnType; + } + } + return null; + } + @Nullable private static PsiCodeBlock getVariableScopeBlock(@Nullable PsiVariable variable) { if (variable instanceof PsiLocalVariable) { @@ -176,7 +204,7 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal return false; } - Mover mover = new Mover(flow, context.refactoredStatement, context.returnedVariable, true); + Mover mover = new Mover(flow, context.refactoredStatement, context.returnedVariable, context.returnType, true); mover.moveTo(context.refactoredStatement, true); return !mover.isEmpty(); } @@ -186,7 +214,7 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal if (context != null) { ControlFlow flow = createControlFlow(context); if (flow != null) { - Mover mover = new Mover(flow, context.refactoredStatement, context.returnedVariable, false); + Mover mover = new Mover(flow, context.refactoredStatement, context.returnedVariable, context.returnType, false); boolean removeReturn = mover.moveTo(context.refactoredStatement, true); if (!mover.isEmpty()) { applyChanges(mover, context, removeReturn); @@ -277,6 +305,7 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal final ControlFlow flow; final PsiStatement enclosingStatement; final PsiVariable resultVariable; + final PsiType returnType; final boolean checkingApplicability; final Set insertBefore = new THashSet<>(); final Set replaceInline = new THashSet<>(); @@ -287,10 +316,11 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal private Mover(@NotNull ControlFlow flow, @NotNull PsiStatement enclosingStatement, @NotNull PsiVariable resultVariable, - boolean checkingApplicability) { + PsiType returnType, boolean checkingApplicability) { this.flow = flow; this.enclosingStatement = enclosingStatement; this.resultVariable = resultVariable; + this.returnType = returnType; this.checkingApplicability = checkingApplicability; } @@ -437,8 +467,14 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal PsiAssignmentExpression assignmentExpression = (PsiAssignmentExpression)expression; PsiExpression lExpression = assignmentExpression.getLExpression(); if (assignmentExpression.getOperationTokenType() == JavaTokenType.EQ && isReferenceTo(lExpression, resultVariable)) { - replaceInline.add(assignmentExpression); - return true; + PsiExpression rExpression = assignmentExpression.getRExpression(); + if (rExpression != null) { + PsiType rExpressionType = rExpression.getType(); + if (rExpressionType != null && returnType.isAssignableFrom(rExpressionType)) { + replaceInline.add(assignmentExpression); + return true; + } + } } } return false; @@ -564,20 +600,23 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal } private static class ReturnContext { - private final PsiReturnStatement returnStatement; - private final PsiCodeBlock returnScope; - private final PsiStatement refactoredStatement; - private final PsiVariable returnedVariable; - private final PsiCodeBlock variableScope; + final PsiReturnStatement returnStatement; + final PsiCodeBlock returnScope; + final PsiType returnType; + final PsiStatement refactoredStatement; + final PsiVariable returnedVariable; + final PsiCodeBlock variableScope; private ReturnContext(@NotNull PsiReturnStatement returnStatement, @NotNull PsiCodeBlock returnScope, + @NotNull PsiType returnType, @NotNull PsiStatement refactoredStatement, @NotNull PsiVariable returnedVariable, @NotNull PsiCodeBlock variableScope) { this.returnStatement = returnStatement; this.returnScope = returnScope; + this.returnType = returnType; this.refactoredStatement = refactoredStatement; this.returnedVariable = returnedVariable; this.variableScope = variableScope; diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfComment.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfComment.java index 668baee11030..9fa3c26e66c9 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfComment.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfComment.java @@ -1,6 +1,6 @@ // "Move 'return' closer to computation of the value of 's'" "true" class T { - int f(String a) { + String f(String a) { String s = a; if (s == null) { return ""; /* return comment */ // end of line diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfComment.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfComment.java index 9edae3e9e1f6..54c383287c4f 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfComment.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfComment.java @@ -1,6 +1,6 @@ // "Move 'return' closer to computation of the value of 's'" "true" class T { - int f(String a) { + String f(String a) { String s = a; if (s == null) { s = ""; // end of line diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/GenericTypeCompatible.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/GenericTypeCompatible.java new file mode 100644 index 000000000000..e07ce3aa3a26 --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/GenericTypeCompatible.java @@ -0,0 +1,15 @@ +import java.util.*; + +class T { + List f(boolean b) { + List raw = null; + if (b) { + raw = g(); + } + return raw; + } + + List g() { + return Collections.singletonList(""); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/GenericTypeIncompatible.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/GenericTypeIncompatible.java new file mode 100644 index 000000000000..788bc2bf0a9a --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/GenericTypeIncompatible.java @@ -0,0 +1,15 @@ +import java.util.*; + +class T { + List f(boolean b) { + List raw = null; + if (b) { + raw = g(); + } + return raw; + } + + List g() { + return Collections.singletonList(""); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/ReturnSeparatedFromComputationTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/ReturnSeparatedFromComputationTest.java index 2496444f6084..f0dfbd7e672a 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/ReturnSeparatedFromComputationTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/ReturnSeparatedFromComputationTest.java @@ -132,6 +132,14 @@ public class ReturnSeparatedFromComputationTest extends LightCodeInsightFixtureT doTest(); } + public void testGenericTypeCompatible() { + doTest(); + } + + public void testGenericTypeIncompatible() { + doTest(); + } + private void doTest() { myFixture.enableInspections(new ReturnSeparatedFromComputationInspection());