diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/ExitContext.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/ExitContext.java index 9f39b6df1a23..5da007c8e8ad 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/ExitContext.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/ExitContext.java @@ -33,7 +33,7 @@ class ExitContext { myFactory = JavaPsiFacade.getElementFactory(block.getProject()); myReturnType = returnType; myReturnVariable = - new VariableNameGenerator(block, VariableKind.LOCAL_VARIABLE).byName("result", "res").byType(returnType).generate(false); + new VariableNameGenerator(block, VariableKind.LOCAL_VARIABLE).byName("result", "res").byType(returnType).generate(true); myReturnVariableDefaultValue = marker.myDefaultValue; if (myReturnVariableDefaultValue != null && myReturnVariableDefaultValue.isPhysical()) { myReturnVariableDefaultValue = (PsiExpression)myReturnVariableDefaultValue.copy(); @@ -89,12 +89,13 @@ class ExitContext { if (myFinishMarkerType != FinishMarker.FinishMarkerType.SEPARATE_VAR) return; if (myFinishedVariable == null) { myFinishedVariable = - new VariableNameGenerator(myBlock, VariableKind.LOCAL_VARIABLE).byName("finished", "completed").generate(false); + new VariableNameGenerator(myBlock, VariableKind.LOCAL_VARIABLE).byName("finished", "completed").generate(true); } - String firstItem = ContainerUtil.getFirstItem(replacements); String assignment = myFinishedVariable + "=true;"; - if (!assignment.equals(firstItem)) { - replacements.add(0, assignment); + if (!replacements.contains(assignment)) { + String first = ContainerUtil.getFirstItem(replacements); + int index = first != null && first.startsWith(myReturnVariable + "=") ? 1 : 0; + replacements.add(index, assignment); } } diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/FinishMarker.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/FinishMarker.java index f60e261db089..a38397ff79bf 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/FinishMarker.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/FinishMarker.java @@ -70,14 +70,21 @@ class FinishMarker { */ static boolean mayNeedMarker(PsiReturnStatement returnStatement, PsiCodeBlock block) { PsiElement parent = returnStatement.getParent(); - if (parent instanceof PsiCodeBlock) { + while (parent instanceof PsiCodeBlock) { PsiElement grandParent = parent.getParent(); - if (grandParent instanceof PsiStatement) { + if (grandParent instanceof PsiBlockStatement) { parent = grandParent.getParent(); + continue; } - else { - return parent != block; + if (grandParent instanceof PsiCatchSection) { + parent = grandParent.getParent(); + break; } + if (grandParent instanceof PsiStatement) { + parent = grandParent; + break; + } + return parent != block; } if (!(parent instanceof PsiStatement)) return true; PsiStatement currentContext = (PsiStatement)parent; diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/ReturnReplacementContext.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/ReturnReplacementContext.java index 606ee7f6e7ad..850fba2f8530 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/ReturnReplacementContext.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/ReturnReplacementContext.java @@ -6,20 +6,17 @@ import com.intellij.openapi.diagnostic.Attachment; import com.intellij.openapi.diagnostic.RuntimeExceptionWithAttachments; import com.intellij.openapi.project.Project; import com.intellij.psi.*; +import com.intellij.psi.controlFlow.*; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiTypesUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.containers.ContainerUtil; -import com.siyeh.ig.psiutils.BoolUtils; -import com.siyeh.ig.psiutils.CommentTracker; -import com.siyeh.ig.psiutils.ControlFlowUtils; -import com.siyeh.ig.psiutils.SideEffectChecker; +import com.siyeh.ig.psiutils.*; import one.util.streamex.StreamEx; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; +import java.util.*; import static com.intellij.util.ObjectUtils.tryCast; import static java.util.Objects.requireNonNull; @@ -61,7 +58,7 @@ class ReturnReplacementContext { @NotNull private PsiStatement goUp() { PsiElement parent = myReturnStatement.getParent(); - if (parent instanceof PsiCodeBlock) { + while (parent instanceof PsiCodeBlock) { PsiElement grandParent = parent.getParent(); if (!(grandParent instanceof PsiSwitchStatement)) { PsiStatement[] statements = ((PsiCodeBlock)parent).getStatements(); @@ -76,15 +73,23 @@ class ReturnReplacementContext { } } } - if (grandParent instanceof PsiBlockStatement || grandParent instanceof PsiTryStatement || - grandParent instanceof PsiSwitchStatement) { + if (grandParent instanceof PsiBlockStatement) { parent = grandParent.getParent(); + continue; + } + if (grandParent instanceof PsiCatchSection) { + parent = grandParent.getParent(); + break; + } + if (grandParent instanceof PsiStatement) { + parent = grandParent; } else if (parent != myBlock) { throw new RuntimeExceptionWithAttachments("Unexpected structure: " + grandParent.getClass(), new Attachment("body.txt", myBlock.getText()), new Attachment("context.txt", grandParent.getText())); } + break; } if (!(parent instanceof PsiStatement)) { throw new RuntimeExceptionWithAttachments("Unexpected structure: " + parent.getClass(), @@ -154,7 +159,8 @@ class ReturnReplacementContext { PsiElement tailEnd = ArrayUtil.getLastElement(tail); ifBlock.addRangeAfter(tailStart, tailEnd, lBrace); contextParent.deleteChildRange(tailStart, tailEnd); - contextParent.addAfter(ifStatement, currentContext); + PsiElement insertedIf = contextParent.addAfter(ifStatement, currentContext); + fixNonInitializedVars(insertedIf); } } } @@ -164,6 +170,9 @@ class ReturnReplacementContext { else if (contextParent.getParent() instanceof PsiStatement) { currentContext = (PsiStatement)contextParent.getParent(); } + else if (contextParent.getParent() instanceof PsiCatchSection) { + currentContext = (PsiStatement)contextParent.getParent().getParent(); + } else { throw new RuntimeExceptionWithAttachments("Unexpected structure: " + contextParent.getParent().getClass(), new Attachment("body.txt", myBlock.getText()), @@ -181,6 +190,35 @@ class ReturnReplacementContext { return currentContext; } + private void fixNonInitializedVars(PsiElement element) { + Set locals = new HashSet<>(); + PsiTreeUtil.processElements(element, e -> { + if (e instanceof PsiReferenceExpression) { + PsiLocalVariable variable = ExpressionUtils.resolveLocalVariable((PsiExpression)e); + if (variable != null && variable.getInitializer() == null && PsiTreeUtil.isAncestor(myBlock, variable, true)) { + locals.add(variable); + } + } + return true; + }); + if (!locals.isEmpty()) { + ControlFlow flow; + try { + flow = ControlFlowFactory.getInstance(myProject).getControlFlow(myBlock, new LocalsControlFlowPolicy(myBlock), false, false); + } + catch (AnalysisCanceledException ignored) { + return; + } + int offset = flow.getStartOffset(element); + if (offset == -1) return; + for (PsiLocalVariable local : locals) { + if (ControlFlowUtil.getVariablePossiblyUnassignedOffsets(local, flow)[offset]) { + local.setInitializer(myFactory.createExpressionFromText(PsiTypesUtil.getDefaultValueOfType(local.getType()), null)); + } + } + } + } + @NotNull private static PsiElement[] extractTail(PsiStatement current, PsiCodeBlock block) { PsiElement[] children = block.getChildren(); @@ -236,7 +274,7 @@ class ReturnReplacementContext { } private void replace() { - if (!(myReturnStatement.getParent().getParent() instanceof PsiBlockStatement)) { + if (!(myReturnStatement.getParent() instanceof PsiCodeBlock)) { myReturnStatement = BlockUtils.expandSingleStatementToBlockStatement(myReturnStatement); } PsiStatement[] newStatements = ContainerUtil.map2Array( diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterNestedIfUnknownLocalUsed.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterNestedIfUnknownLocalUsed.java index f39ac4c64e47..416e7e5cc25b 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterNestedIfUnknownLocalUsed.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterNestedIfUnknownLocalUsed.java @@ -6,8 +6,8 @@ class Test { if (strings.length > 2) { String string = strings[0]; if (string.equals(strings[1])) { - finished = true; res = foo(string); + finished = true; } } if (!finished) { diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterNestedIfUnknownPlusCall.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterNestedIfUnknownPlusCall.java index d2273a7f2b0a..e9f7250264ed 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterNestedIfUnknownPlusCall.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterNestedIfUnknownPlusCall.java @@ -6,8 +6,8 @@ class Test { if (strings.length > 2) { String string = strings[0]; if (string.equals(strings[1])) { - finished = true; result = foo(string); + finished = true; } } if (!finished) { diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterNonBlockInLoop.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterNonBlockInLoop.java index c67b0c3b7a2d..f1d71da5c49f 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterNonBlockInLoop.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterNonBlockInLoop.java @@ -6,8 +6,8 @@ class Test { for (String s : list) { for (int i = 0; i < 10; i++) { if (s.length() == i) { - finished = true; result = foo; + finished = true; break; } } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterSwitch.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterSwitch.java new file mode 100644 index 000000000000..b047555c1a34 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterSwitch.java @@ -0,0 +1,20 @@ +// "Transform body to single exit-point form" "true" +class Test { + String test(int x) { + String result = "foo"; + switch (x) { + case 1: + break; + case 2: + result = "bar"; + break; + case 3: + result = "baz"; + break; + default: + result = "qux"; + break; + } + return result; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterSynchronized.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterSynchronized.java new file mode 100644 index 000000000000..061d8944ce8b --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterSynchronized.java @@ -0,0 +1,16 @@ +// "Transform body to single exit-point form" "true" +class Test { + String test(int x) { + String result = "foo"; + synchronized (this) { + if (x != 0) { + if (x == 1) { + result = "bar"; + } else { + result = "baz"; + } + } + } + return result; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterSynchronized2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterSynchronized2.java new file mode 100644 index 000000000000..0538a2303120 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterSynchronized2.java @@ -0,0 +1,19 @@ +// "Transform body to single exit-point form" "true" +class Test { + String test(int x) { + String result = null; + synchronized (this) { + if (x == 0) { + result = "foo"; + } else { + if (x == 1) { + result = "bar"; + } + } + } + if (result == null) { + result = "baz"; + } + return result; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterTryCatch.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterTryCatch.java new file mode 100644 index 000000000000..8b838bcbeaf1 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterTryCatch.java @@ -0,0 +1,11 @@ +// "Transform body to single exit-point form" "true" +class Test { + int test(String s) { + int result = -1; + try { + result = Integer.parseInt(s); + } catch (NumberFormatException ex) { + } + return result; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterTryCatch2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterTryCatch2.java new file mode 100644 index 000000000000..a53a237788bd --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterTryCatch2.java @@ -0,0 +1,21 @@ +// "Transform body to single exit-point form" "true" +class Test { + int test(String s) { + int res = -1; + boolean finished = false; + try { + res = Integer.parseInt(s); + finished = true; + } catch (NumberFormatException ex) { + boolean result = s.isEmpty(); + if (result) { + finished = true; + } + } + if (!finished) { + System.out.println("oops"); + res = -2; + } + return res; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterUninit.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterUninit.java new file mode 100644 index 000000000000..0fdcb3f336d3 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterUninit.java @@ -0,0 +1,19 @@ +// "Transform body to single exit-point form" "true" +class Test { + void test(String s) { + boolean finished = false; + if (s != null) { + int a = 0; + synchronized (this) { + if (s.isEmpty()) { + finished = true; + } else { + a = s.length(); + } + } + if (!finished) { + System.out.println(a); + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeSwitch.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeSwitch.java new file mode 100644 index 000000000000..e51278b04e30 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeSwitch.java @@ -0,0 +1,11 @@ +// "Transform body to single exit-point form" "true" +class Test { + String test(int x) { + switch (x) { + case 1:return "foo"; + case 2:return "bar"; + case 3:return "baz"; + default:return "qux"; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeSynchronized.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeSynchronized.java new file mode 100644 index 000000000000..3c3800dab683 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeSynchronized.java @@ -0,0 +1,10 @@ +// "Transform body to single exit-point form" "true" +class Test { + String test(int x) { + synchronized(this) { + if(x == 0) return "foo"; + if(x == 1) return "bar"; + return "baz"; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeSynchronized2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeSynchronized2.java new file mode 100644 index 000000000000..c8b8f56cb7c8 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeSynchronized2.java @@ -0,0 +1,10 @@ +// "Transform body to single exit-point form" "true" +class Test { + String test(int x) { + synchronized(this) { + if(x == 0) return "foo"; + if(x == 1) return "bar"; + } + return "baz"; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeTryCatch.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeTryCatch.java new file mode 100644 index 000000000000..4837d98d69e9 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeTryCatch.java @@ -0,0 +1,11 @@ +// "Transform body to single exit-point form" "true" +class Test { + int test(String s) { + try { + return Integer.parseInt(s); + } + catch(NumberFormatException ex) { + return -1; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeTryCatch2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeTryCatch2.java new file mode 100644 index 000000000000..b7c7d862b169 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeTryCatch2.java @@ -0,0 +1,14 @@ +// "Transform body to single exit-point form" "true" +class Test { + int test(String s) { + try { + return Integer.parseInt(s); + } + catch(NumberFormatException ex) { + boolean result = s.isEmpty(); + if (result) return -1; + } + System.out.println("oops"); + return -2; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeUninit.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeUninit.java new file mode 100644 index 000000000000..802e7a49b79c --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeUninit.java @@ -0,0 +1,13 @@ +// "Transform body to single exit-point form" "true" +class Test { + void test(String s) { + if (s != null) { + int a; + synchronized (this) { + if (s.isEmpty()) return; + a = s.length(); + } + System.out.println(a); + } + } +} \ No newline at end of file