Java inspection: Fixed catch finally in "Move return to computation" (IDEA-121153)

java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java

@@ -22,7 +22,6 @@ import com.intellij.psi.*;
 import com.intellij.psi.controlFlow.*;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.refactoring.util.RefactoringUtil;
-import com.siyeh.ig.psiutils.ControlFlowUtils;
 import com.siyeh.ig.psiutils.ExpressionUtils;
 import gnu.trove.THashMap;
 import gnu.trove.THashSet;
@@ -295,7 +294,7 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal
         return moveToForeach((PsiForeachStatement)targetStatement);
       }
       if (targetStatement instanceof PsiTryStatement) {
-        return moveToTry((PsiTryStatement)targetStatement);
+        return moveToTry((PsiTryStatement)targetStatement, returnAtTheEnd);
       }
       if (targetStatement instanceof PsiLabeledStatement) {
         return moveToLabeled((PsiLabeledStatement)targetStatement, returnAtTheEnd);
@@ -303,7 +302,7 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal
       if (targetStatement instanceof PsiExpressionStatement) {
         return inlineExpression((PsiExpressionStatement)targetStatement);
       }
-      if (targetStatement instanceof PsiThrowStatement) {
+      if (targetStatement instanceof PsiThrowStatement || targetStatement instanceof PsiReturnStatement) {
         return true;
       }
       return false;
@@ -317,7 +316,7 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal
       PsiJavaToken rBrace = codeBlock.getRBrace();
       if (rBrace != null) {
         PsiStatement lastNonEmptyStatement = getPrevNonEmptyStatement(rBrace, removeCompletely);
-        if (lastNonEmptyStatement == null || lastNonEmptyStatement instanceof PsiReturnStatement) {
+        if (lastNonEmptyStatement == null) {
           return false;
         }
         if (moveTo(lastNonEmptyStatement, returnAtTheEnd)) {
@@ -360,24 +359,24 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal
       return false;
     }
 
-    private boolean moveToTry(@NotNull PsiTryStatement targetStatement) {
+    private boolean moveToTry(@NotNull PsiTryStatement targetStatement, boolean returnAtTheEnd) {
       PsiCodeBlock tryBlock = targetStatement.getTryBlock();
       if (tryBlock == null) {
         return false;
       }
-      boolean result = true;
       PsiCodeBlock finallyBlock = targetStatement.getFinallyBlock();
-      if (finallyBlock != null && writesVariable(finallyBlock)) {
-        result = false;
+      if (finallyBlock != null && usesVariable(finallyBlock)) {
+        return false;
       }
+      boolean allCatchesReturn = true;
       PsiCatchSection[] catchSections = targetStatement.getCatchSections();
       for (PsiCatchSection catchSection : catchSections) {
         PsiCodeBlock catchBlock = catchSection.getCatchBlock();
         if (catchBlock == null || !moveToBlockBody(catchBlock, false)) {
-          result = false;
+          allCatchesReturn = false;
         }
       }
-      return moveToBlockBody(tryBlock, false) && result;
+      return moveToBlockBody(tryBlock, returnAtTheEnd && allCatchesReturn) && allCatchesReturn;
     }
 
     private boolean moveToLabeled(@NotNull PsiLabeledStatement targetStatement, boolean returnAtTheEnd) {
@@ -415,20 +414,13 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal
       }
     }
 
-    private boolean writesVariable(@NotNull PsiElement element) {
+    private boolean usesVariable(@NotNull PsiElement element) {
       int startOffset = flow.getStartOffset(element);
       int endOffset = flow.getEndOffset(element);
       if (startOffset < 0 || endOffset < 0) {
         return true;
       }
-      List<Instruction> instructions = flow.getInstructions();
-      for (int i = startOffset; i < endOffset; i++) {
-        Instruction instruction = instructions.get(i);
-        if (instruction instanceof WriteVariableInstruction && ((WriteVariableInstruction)instruction).variable == resultVariable) {
-          return true;
-        }
-      }
-      return false;
+      return ControlFlowUtil.isVariableUsed(flow, startOffset, endOffset, resultVariable);
     }
 
     private static boolean isAlwaysTrue(@Nullable PsiExpression condition, boolean nullIsTrue) {

java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java

@@ -238,6 +238,26 @@ public class ControlFlowUtil {
     return array;
   }
 
+  public static boolean isVariableUsed(ControlFlow flow, int start, int end, PsiVariable variable) {
+    List<Instruction> instructions = flow.getInstructions();
+    LOG.assertTrue(start >= 0, "flow start");
+    LOG.assertTrue(end <= instructions.size(), "flow end");
+    for (int i = start; i < end; i++) {
+      Instruction instruction = instructions.get(i);
+      if (instruction instanceof ReadVariableInstruction) {
+        if (((ReadVariableInstruction)instruction).variable == variable) {
+          return true;
+        }
+      }
+      else if (instruction instanceof WriteVariableInstruction) {
+        if (((WriteVariableInstruction)instruction).variable == variable) {
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
   public static List<PsiVariable> getInputVariables(ControlFlow flow, int start, int end) {
     List<PsiVariable> usedVariables = getUsedVariables(flow, start, end);
     ArrayList<PsiVariable> array = new ArrayList<PsiVariable>(usedVariables.size());

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInElse.java

@@ -4,6 +4,6 @@ class T {
         int n = 0;
         if (b) System.out.println("yes");
         else return 2;
-        return 0;
+        return n;
     }
 }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInIf.java

@@ -4,6 +4,6 @@ class T {
         int n = 0;
         if (b) return 1;
         else System.out.println("no");
-        return 0;
+        return n;
     }
 }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleFor.java

@@ -7,6 +7,6 @@ class T {
                 return i;
             }
         }
-        return -1;
+        return n;
     }
 }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIf.java

@@ -3,6 +3,6 @@ class T {
     int f(boolean b) {
         int n = 0;
         if (b) return 1;
-        return 0;
+        return n;
     }
 }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIfDefaultValueParameter.java

@@ -3,6 +3,6 @@ class T {
     int f(boolean b, int d) {
         int n = d;
         if (b) return 1;
-        return d;
+        return n;
     }
 }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleWhile.java

@@ -8,7 +8,7 @@ class T {
                 return s;
             }
         }
-        return "";
+        return r;
     }
 
     boolean hasNext() {

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterTryCatchAssign.java

@@ -0,0 +1,16 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+class T {
+    int f(boolean b) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) {
+                throw new RuntimeException();
+            }
+            return n;
+        }
+        catch (RuntimeException e) {
+            return 2;
+        }
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterTryCatchFinally1.java

@@ -0,0 +1,24 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+import java.io.*;
+
+class T {
+    int f(boolean b, boolean c) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) throw new IOException();
+            n = 2;
+            if (c) throw new RuntimeException();
+            return n;
+        }
+        catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        catch (RuntimeException e) {
+            return 3;
+        }
+        finally {
+            System.out.println();
+        }
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterTryCatchFinally2.java

@@ -0,0 +1,24 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+import java.io.*;
+
+class T {
+    int f(boolean b, boolean c) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) throw new IOException();
+            n = 2;
+            if (c) throw new RuntimeException();
+            return n;
+        }
+        catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        catch (RuntimeException e) {
+            return 3;
+        }
+        finally {
+            System.out.println();
+        }
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterTryCatchFinally5.java

@@ -0,0 +1,24 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+import java.io.*;
+
+class T {
+    int f(boolean b, boolean c) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) throw new IOException();
+            n = 2;
+            if (c) throw new RuntimeException();
+            return n;
+        }
+        catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        catch (RuntimeException e) {
+            return 3;
+        }
+        finally {
+            return 4;
+        }
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterTryCatchFinally6.java

@@ -0,0 +1,24 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+import java.io.*;
+
+class T {
+    int f(boolean b, boolean c, boolean d) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) throw new IOException();
+            n = 2;
+            if (c) throw new RuntimeException();
+            return n;
+        }
+        catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        catch (RuntimeException e) {
+            return 3;
+        }
+        finally {
+            if(d) return 4;
+        }
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterTryCatchRethrow.java

@@ -0,0 +1,16 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+class T {
+    int f(boolean b) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) {
+                throw new RuntimeException();
+            }
+            return n;
+        }
+        catch (RuntimeException e) {
+            throw new RuntimeException(e);
+        }
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterTryCatchReturn.java

@@ -0,0 +1,16 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+class T {
+    int f(boolean b) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) {
+                throw new RuntimeException();
+            }
+            return n;
+        }
+        catch (RuntimeException e) {
+            return 2;
+        }
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeTryCatchAssign.java

@@ -0,0 +1,16 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+class T {
+    int f(boolean b) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) {
+                throw new RuntimeException();
+            }
+        }
+        catch (RuntimeException e) {
+            n = 2;
+        }
+        re<caret>turn n;
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeTryCatchFinally1.java

@@ -0,0 +1,24 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+import java.io.*;
+
+class T {
+    int f(boolean b, boolean c) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) throw new IOException();
+            n = 2;
+            if (c) throw new RuntimeException();
+        }
+        catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        catch (RuntimeException e) {
+            return 3;
+        }
+        finally {
+            System.out.println();
+        }
+        re<caret>turn n;
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeTryCatchFinally2.java

@@ -0,0 +1,24 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+import java.io.*;
+
+class T {
+    int f(boolean b, boolean c) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) throw new IOException();
+            n = 2;
+            if (c) throw new RuntimeException();
+        }
+        catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        catch (RuntimeException e) {
+            n = 3;
+        }
+        finally {
+            System.out.println();
+        }
+        re<caret>turn n;
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeTryCatchFinally3.java

@@ -0,0 +1,24 @@
+// "Move 'return' to computation of the value of 'n'" "false"
+import java.io.*;
+
+class T {
+    int f(boolean b, boolean c) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) throw new IOException();
+            n = 2;
+            if (c) throw new RuntimeException();
+        }
+        catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        catch (RuntimeException e) {
+            n = 3;
+        }
+        finally {
+            n = 4;
+        }
+        re<caret>turn n;
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeTryCatchFinally4.java

@@ -0,0 +1,24 @@
+// "Move 'return' to computation of the value of 'n'" "false"
+import java.io.*;
+
+class T {
+    int f(boolean b, boolean c) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) throw new IOException();
+            n = 2;
+            if (c) throw new RuntimeException();
+        }
+        catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        catch (RuntimeException e) {
+            n = 3;
+        }
+        finally {
+            System.out.println(n);
+        }
+        re<caret>turn n;
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeTryCatchFinally5.java

@@ -0,0 +1,24 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+import java.io.*;
+
+class T {
+    int f(boolean b, boolean c) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) throw new IOException();
+            n = 2;
+            if (c) throw new RuntimeException();
+        }
+        catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        catch (RuntimeException e) {
+            n = 3;
+        }
+        finally {
+            return 4;
+        }
+        re<caret>turn n;
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeTryCatchFinally6.java

@@ -0,0 +1,24 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+import java.io.*;
+
+class T {
+    int f(boolean b, boolean c, boolean d) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) throw new IOException();
+            n = 2;
+            if (c) throw new RuntimeException();
+        }
+        catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        catch (RuntimeException e) {
+            n = 3;
+        }
+        finally {
+            if(d) return 4;
+        }
+        re<caret>turn n;
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeTryCatchRethrow.java

@@ -0,0 +1,16 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+class T {
+    int f(boolean b) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) {
+                throw new RuntimeException();
+            }
+        }
+        catch (RuntimeException e) {
+            throw new RuntimeException(e);
+        }
+        re<caret>turn n;
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeTryCatchReturn.java

@@ -0,0 +1,16 @@
+// "Move 'return' to computation of the value of 'n'" "true"
+class T {
+    int f(boolean b) {
+        int n = -1;
+        try {
+            n = 1;
+            if (b) {
+                throw new RuntimeException();
+            }
+        }
+        catch (RuntimeException e) {
+            return 2;
+        }
+        re<caret>turn n;
+    }
+}