ConvertToSingleReturn: default return value tuned; define it always before actual conversion

2026-09-27 10:03:11 +07:00 · 2019-04-12 17:19:54 +03:00
parent 7526618b79
commit e0d1fab43d
12 changed files with 109 additions and 29 deletions
@@ -27,7 +27,7 @@ class ExitContext {
  private final @NotNull String myReturnVariable;
  private final @NotNull PsiElementFactory myFactory;
  boolean myReturnVariableUsed = false;
-  PsiExpression myReturnVariableDefaultValue;
+  final PsiExpression myReturnVariableDefaultValue;

  ExitContext(@NotNull PsiCodeBlock block, @NotNull PsiType returnType, @NotNull FinishMarker marker) {
    myBlock = block;
@@ -35,9 +35,10 @@ class ExitContext {
    myReturnType = returnType;
    myReturnVariable =
      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();
+    if (marker.myDefaultValue != null && marker.myDefaultValue.isPhysical()) {
+      myReturnVariableDefaultValue = (PsiExpression)marker.myDefaultValue.copy();
+    } else {
+      myReturnVariableDefaultValue = marker.myDefaultValue;
    }
    myFinishMarkerType = marker.myType;
  }
@@ -76,12 +77,7 @@ class ExitContext {

  void registerReturnValue(PsiExpression value, List<String> replacements) {
    myReturnVariableUsed = true;
-    if (FinishMarker.canMoveToStart(value) &&
-        (myReturnVariableDefaultValue == null ||
-         EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(myReturnVariableDefaultValue, value))) {
-      myReturnVariableDefaultValue = (PsiExpression)value.copy();
-    }
-    else {
+    if (!EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(myReturnVariableDefaultValue, value)) {
      replacements.add(0, myReturnVariable + "=" + value.getText() + ";");
    }
  }
@@ -115,11 +111,11 @@ class ExitContext {
    }
    if (myReturnVariableUsed) {
      PsiJavaToken start = requireNonNull(myBlock.getLBrace());
-      if (myReturnVariableDefaultValue == null && myFinishedVariable != null) {
-        myReturnVariableDefaultValue = myFactory.createExpressionFromText(PsiTypesUtil.getDefaultValueOfType(myReturnType), null);
+      PsiExpression initializer = myReturnVariableDefaultValue;
+      if (initializer == null && myFinishedVariable != null) {
+        initializer = myFactory.createExpressionFromText(PsiTypesUtil.getDefaultValueOfType(myReturnType), null);
      }
-      PsiDeclarationStatement declaration =
-        myFactory.createVariableDeclarationStatement(myReturnVariable, myReturnType, myReturnVariableDefaultValue);
+      PsiDeclarationStatement declaration = myFactory.createVariableDeclarationStatement(myReturnVariable, myReturnType, initializer);
      PsiLocalVariable var = (PsiLocalVariable)((PsiDeclarationStatement)myBlock.addAfter(declaration, start)).getDeclaredElements()[0];
      if (var.hasModifierProperty(PsiModifier.FINAL) && !RefactoringUtil.canBeDeclaredFinal(var)) {
        // Keep final when possible to respect code style setting "generate local variables as 'final'"
@@ -13,11 +13,12 @@ import com.intellij.util.ArrayUtil;
 import com.siyeh.ig.psiutils.ControlFlowUtils;
 import com.siyeh.ig.psiutils.ExpressionUtils;
 import one.util.streamex.StreamEx;
+import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;

-import java.util.List;
-import java.util.Set;
+import java.util.*;
+import java.util.stream.Collectors;

 import static com.intellij.util.ObjectUtils.NULL;
 import static com.intellij.util.ObjectUtils.tryCast;
@@ -146,7 +147,7 @@ class FinishMarker {
      if (nonTerminalReturnValues.size() == 1 && nonTerminalReturnValues.iterator().next() != NULL) {
        return new FinishMarker(FinishMarkerType.SEPARATE_VAR, nonTerminalReturns.iterator().next());
      }
-      return new FinishMarker(FinishMarkerType.SEPARATE_VAR, null);
+      return new FinishMarker(FinishMarkerType.SEPARATE_VAR, findBestExpression(terminalReturn, nonTerminalReturns));
    }
    if (PsiType.BOOLEAN.equals(returnType)) {
      if (nonTerminalReturnValues.size() == 1) {
@@ -173,7 +174,25 @@ class FinishMarker {
        return new FinishMarker(FinishMarkerType.SEPARATE_VAR, value);
      }
    }
-    return new FinishMarker(FinishMarkerType.SEPARATE_VAR, null);
+    return new FinishMarker(FinishMarkerType.SEPARATE_VAR, findBestExpression(terminalReturn, nonTerminalReturns));
+  }
+
+  @Nullable
+  private static PsiExpression findBestExpression(PsiReturnStatement terminalReturn, List<PsiExpression> nonTerminalReturns) {
+    List<PsiExpression> bestGroup = StreamEx.of(nonTerminalReturns)
+      .filter(FinishMarker::canMoveToStart)
+      .groupingBy(PsiExpression::getText, LinkedHashMap::new, Collectors.toList())
+      .values()
+      .stream()
+      .max(Comparator.comparingInt(List::size))
+      .orElse(Collections.emptyList());
+    if (bestGroup.size() >= 2) {
+      return bestGroup.get(0);
+    }
+    if (terminalReturn != null && canMoveToStart(terminalReturn.getReturnValue())) {
+      return terminalReturn.getReturnValue();
+    }
+    return null;
  }

  @NotNull
@@ -217,9 +236,10 @@ class FinishMarker {
        return new FinishMarker(FinishMarkerType.VALUE_NON_EQUAL, factory.createExpressionFromText(text, null));
      }
    }
-    return new FinishMarker(FinishMarkerType.SEPARATE_VAR, null);
+    return new FinishMarker(FinishMarkerType.SEPARATE_VAR, findBestExpression(terminalReturn, nonTerminalReturns));
  }

+  @Contract("null -> false")
  static boolean canMoveToStart(PsiExpression value) {
    if (!ExpressionUtils.isSafelyRecomputableExpression(value)) return false;
    PsiReferenceExpression ref = tryCast(PsiUtil.skipParenthesizedExprDown(value), PsiReferenceExpression.class);
@@ -1,12 +1,13 @@
 // "Transform body to single exit-point form" "true"
 class Test {
    String test2(List<String> list, String foo, String bar) {
-        String result = foo;
+        String result = null;
        boolean finished = false;
        for (String s : list) {
            for (int i = 0; i < 10; i++) {
                bar = s;
                if (s.length() == i) {
+                    result = foo;
                    finished = true;
                    break;
                }
@@ -1,9 +1,11 @@
 // "Transform body to single exit-point form" "true"
 class Test {
    int test(String s) {
-        int result = 2;
+        int result = 1;
        if (s == null) {
-            if (!(Math.random() > 0.5)) {
+            if (Math.random() > 0.5) {
+                result = 2;
+            } else {
                result = 4;
            }
        } else {
@@ -11,7 +13,6 @@ class Test {
                result = 3;
            } else {
                System.out.println(s);
-                result = 1;
            }
        }
        return result;
@@ -0,0 +1,15 @@
+// "Transform body to single exit-point form" "true"
+import java.util.Collections;
+
+class Test {
+    List<String> test(int x) {
+        List<String> result = Collections.emptyList();
+        if (x != 0) {
+            int rem = x % 3;
+            if (rem != 1) {
+                result = Collections.singletonList("foo");
+            }
+        }
+        return result;
+    }
+}
@@ -1,9 +1,10 @@
 // "Transform body to single exit-point form" "true"
 class Test {
    String test(int x) {
-        String result = "foo";
+        String result;
        switch (x) {
            case 1:
+                result = "foo";
                break;
            case 2:
                result = "bar";
@@ -1,9 +1,11 @@
 // "Transform body to single exit-point form" "true"
 class Test {
    String test(int x) {
-        String result = "foo";
+        String result;
        synchronized (this) {
-            if (x != 0) {
+            if (x == 0) {
+                result = "foo";
+            } else {
                if (x == 1) {
                    result = "bar";
                } else {
@@ -1,10 +1,11 @@
 // "Transform body to single exit-point form" "true"
 class Test {
    int test(String s) {
-        int result = -1;
+        int result;
        try {
            result = Integer.parseInt(s);
        } catch (NumberFormatException ex) {
+            result = -1;
        }
        return result;
    }
@@ -1,7 +1,7 @@
 // "Transform body to single exit-point form" "true"
 class Test {
    int test(String s) {
-        int res = -1;
+        int res = -2;
        boolean finished = false;
        try {
            res = Integer.parseInt(s);
@@ -9,12 +9,12 @@ class Test {
        } catch (NumberFormatException ex) {
            boolean result = s.isEmpty();
            if (result) {
+                res = -1;
                finished = true;
            }
        }
        if (!finished) {
            System.out.println("oops");
-            res = -2;
        }
        return res;
    }
@@ -0,0 +1,18 @@
+// "Transform body to single exit-point form" "true"
+class Test {
+    int test(String s) {
+        int res = -2;
+        try {
+            res = Math.abs(Integer.parseInt(s));
+        } catch (NumberFormatException ex) {
+            boolean result = s.isEmpty();
+            if (result) {
+                res = -1;
+            }
+        }
+        if (res == -2) {
+            System.out.println("oops");
+        }
+        return res;
+    }
+}
@@ -0,0 +1,11 @@
+// "Transform body to single exit-point form" "true"
+import java.util.Collections;
+
+class Test {
+    List<String> <caret>test(int x) {
+        if (x == 0) return Collections.emptyList();
+        int rem = x % 3;
+        if (rem == 1) return Collections.emptyList();
+        return Collections.singletonList("foo");
+    }
+}
@@ -0,0 +1,14 @@
+// "Transform body to single exit-point form" "true"
+class Test {
+    int <caret>test(String s) {
+        try {
+            return Math.abs(Integer.parseInt(s));
+        }
+        catch(NumberFormatException ex) {
+            boolean result = s.isEmpty();
+            if (result) return -1;
+        }
+        System.out.println("oops");
+        return -2;
+    }
+}