Java control flow: Don't allow extracting method if there's a potential side effect... appcode/163.3195 clion/163.3196
authorPavel Dolgov <pavel.dolgov@jetbrains.com>
Wed, 24 Aug 2016 12:13:26 +0000 (15:13 +0300)
committerPavel Dolgov <pavel.dolgov@jetbrains.com>
Wed, 24 Aug 2016 12:24:43 +0000 (15:24 +0300)
23 files changed:
java/java-impl/src/com/intellij/refactoring/extractMethod/ControlFlowWrapper.java
java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java
java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowAnalyzer.java
java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowImpl.java
java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java
java/java-tests/testData/refactoring/extractMethod/IDEADEV33368_after.java [deleted file]
java/java-tests/testData/refactoring/extractMethod/UseParamInCatch.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseParamInFinally.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarAfterCatch.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarInCatch1.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarInCatch1_after.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarInCatch2.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarInCatchInvisible.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarInCatchInvisible_after.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarInCatchNested1.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarInCatchNested1_after.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarInCatchNested2.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarInFinally1.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarInFinally1_after.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarInFinally2.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarInOtherCatch.java [new file with mode: 0644]
java/java-tests/testData/refactoring/extractMethod/UseVarInOtherCatch_after.java [new file with mode: 0644]
java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java

index 4bac49523ea0ff06ea6afbc9532aa902f6c12dd5..564b53aedcf06bf9ac7f4912944a392141de39bc 100644 (file)
@@ -90,10 +90,15 @@ public class ControlFlowWrapper {
     return myFirstExitStatementCopy;
   }
 
-  public Collection<PsiStatement> prepareExitStatements(final PsiElement[] elements) throws ExitStatementsNotSameException {
+  public Collection<PsiStatement> prepareExitStatements(final @NotNull PsiElement[] elements,
+                                                        final @NotNull PsiElement enclosingCodeFragment)
+    throws ExitStatementsNotSameException {
     myExitPoints = new IntArrayList();
     myExitStatements = ControlFlowUtil
       .findExitPointsAndStatements(myControlFlow, myFlowStart, myFlowEnd, myExitPoints, ControlFlowUtil.DEFAULT_EXIT_STATEMENTS_CLASSES);
+    if (ControlFlowUtil.hasObservableThrowExitPoints(myControlFlow, myFlowStart, myFlowEnd, elements, enclosingCodeFragment)) {
+      throw new ExitStatementsNotSameException();
+    }
     if (LOG.isDebugEnabled()) {
       LOG.debug("exit points:");
       for (int i = 0; i < myExitPoints.size(); i++) {
index 36105911ae7d05f98ed583f83d57122c7e3ed61c..6eaed57e2aff59e51098598d8c0de40902d9c57b 100644 (file)
@@ -142,7 +142,7 @@ public class ExtractMethodProcessor implements MatchProvider {
     myProject = project;
     myEditor = editor;
     if (elements.length != 1 || !(elements[0] instanceof PsiBlockStatement)) {
-      myElements = elements.length == 1 && elements[0] instanceof PsiParenthesizedExpression 
+      myElements = elements.length == 1 && elements[0] instanceof PsiParenthesizedExpression
                    ? new PsiElement[] {PsiUtil.skipParenthesizedExprDown((PsiExpression)elements[0])} : elements;
       myEnclosingBlockStatement = null;
     }
@@ -229,7 +229,7 @@ public class ExtractMethodProcessor implements MatchProvider {
     myControlFlowWrapper = new ControlFlowWrapper(myProject, codeFragment, myElements);
 
     try {
-      myExitStatements = myControlFlowWrapper.prepareExitStatements(myElements);
+      myExitStatements = myControlFlowWrapper.prepareExitStatements(myElements, codeFragment);
       if (myControlFlowWrapper.isGenerateConditionalExit()) {
         myGenerateConditionalExit = true;
       } else {
index 9e8b58368a0ed1ff99d39c6c45218397c9a3f883..871277286088c80c5d1a55d768c2ac017e6143d8 100644 (file)
@@ -21,12 +21,10 @@ import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.progress.ProgressManager;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.util.Comparing;
-import com.intellij.openapi.util.Condition;
 import com.intellij.psi.*;
 import com.intellij.psi.tree.IElementType;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
-import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.containers.Stack;
 import gnu.trove.THashMap;
 import gnu.trove.TIntArrayList;
@@ -980,7 +978,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor {
       ProgressManager.checkCanceled();
       PsiParameter parameter = myCatchParameters.get(i);
       PsiType catchType = parameter.getType();
-      if (catchType.isAssignableFrom(throwType) || mightBeAssignableFromSubclass(throwType, catchType)) {
+      if (ControlFlowUtil.isCaughtExceptionType(throwType, catchType)) {
         blocks.add(myCatchBlocks.get(i));
       }
     }
@@ -991,18 +989,6 @@ class ControlFlowAnalyzer extends JavaElementVisitor {
     return blocks;
   }
 
-  private static boolean mightBeAssignableFromSubclass(@NotNull final PsiClassType throwType, @NotNull PsiType catchType) {
-    if (catchType instanceof PsiDisjunctionType) {
-      return ContainerUtil.exists(((PsiDisjunctionType)catchType).getDisjunctions(), new Condition<PsiType>() {
-        @Override
-        public boolean value(PsiType catchDisjunction) {
-          return throwType.isAssignableFrom(catchDisjunction);
-        }
-      });
-    }
-    return throwType.isAssignableFrom(catchType);
-  }
-
   @Override
   public void visitAssertStatement(PsiAssertStatement statement) {
     startElement(statement);
index 58d522a54e2c2bcea4fdaf4716df2ab84421b6c9..42dcc3e8acbe8181726ec8bc8a90d07dbbc89deb 100644 (file)
@@ -18,8 +18,8 @@ package com.intellij.psi.controlFlow;
 
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.psi.PsiElement;
+import com.intellij.util.containers.ObjectIntHashMap;
 import com.intellij.util.containers.Stack;
-import gnu.trove.TObjectIntHashMap;
 import org.jetbrains.annotations.NotNull;
 
 import java.util.ArrayList;
@@ -29,8 +29,8 @@ class ControlFlowImpl implements ControlFlow {
   private static final Logger LOG = Logger.getInstance("#com.intellij.psi.controlFlow.ControlFlowImpl");
 
   private final List<Instruction> myInstructions = new ArrayList<Instruction>();
-  private final TObjectIntHashMap<PsiElement> myElementToStartOffsetMap = new TObjectIntHashMap<PsiElement>();
-  private final TObjectIntHashMap<PsiElement> myElementToEndOffsetMap = new TObjectIntHashMap<PsiElement>();
+  private final ObjectIntHashMap<PsiElement> myElementToStartOffsetMap = new ObjectIntHashMap<PsiElement>();
+  private final ObjectIntHashMap<PsiElement> myElementToEndOffsetMap = new ObjectIntHashMap<PsiElement>();
   private final List<PsiElement> myElementsForInstructions = new ArrayList<PsiElement>();
   private boolean myConstantConditionOccurred;
 
@@ -64,20 +64,12 @@ class ControlFlowImpl implements ControlFlow {
 
   @Override
   public int getStartOffset(@NotNull PsiElement element) {
-    int value = myElementToStartOffsetMap.get(element);
-    if (value == 0){
-      if (!myElementToStartOffsetMap.containsKey(element)) return -1;
-    }
-    return value;
+    return myElementToStartOffsetMap.get(element, -1);
   }
 
   @Override
   public int getEndOffset(@NotNull PsiElement element) {
-    int value = myElementToEndOffsetMap.get(element);
-    if (value == 0){
-      if (!myElementToEndOffsetMap.containsKey(element)) return -1;
-    }
-    return value;
+    return myElementToEndOffsetMap.get(element, -1);
   }
 
   @Override
index d997bcaadac341db1a3ad7d04e705a6c78fc3aee..179a2d907a923d613e91676b562312d52a24f7a0 100644 (file)
  */
 package com.intellij.psi.controlFlow;
 
+import com.intellij.codeInsight.ExceptionUtil;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.psi.*;
 import com.intellij.psi.impl.source.DummyHolder;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.util.ArrayUtil;
+import com.intellij.util.Function;
 import com.intellij.util.IncorrectOperationException;
 import com.intellij.util.ReflectionUtil;
 import com.intellij.util.containers.IntArrayList;
 import com.intellij.util.containers.IntStack;
+import gnu.trove.THashMap;
 import gnu.trove.THashSet;
 import gnu.trove.TIntHashSet;
 import org.jetbrains.annotations.NotNull;
@@ -380,6 +383,162 @@ public class ControlFlowUtil {
     return PsiTreeUtil.getParentOfType(element, PsiStatement.class, false);
   }
 
+  /**
+   * Detect throw instructions which might affect observable control flow via side effects with local variables
+   * The side effect of exception thrown occurs when a local variable is written in the try block, and then accessed
+   * in the finally section or in/after a catch section.
+   */
+  public static boolean hasObservableThrowExitPoints(@NotNull ControlFlow flow,
+                                                     int flowStart,
+                                                     int flowEnd,
+                                                     @NotNull PsiElement[] elements,
+                                                     @NotNull PsiElement enclosingCodeFragment) {
+    final List<Instruction> instructions = flow.getInstructions();
+    final Map<PsiVariable, IntArrayList> writeOffsets = new THashMap<PsiVariable, IntArrayList>();
+    for (int i = flowStart; i < flowEnd; i++) {
+      Instruction instruction = instructions.get(i);
+      if (instruction instanceof WriteVariableInstruction) {
+        final PsiVariable variable = ((WriteVariableInstruction)instruction).variable;
+        if (variable instanceof PsiLocalVariable || variable instanceof PsiParameter) {
+          IntArrayList offsets = writeOffsets.get(variable);
+          if (offsets == null) writeOffsets.put(variable, offsets = new IntArrayList());
+          offsets.add(i);
+        }
+      }
+    }
+    if (writeOffsets.isEmpty()) return false;
+    LOG.debug("minWriteOffsets:", writeOffsets);
+
+    final PsiElement commonParent = elements.length != 1 ? PsiTreeUtil.findCommonParent(elements) : elements[0].getParent();
+    final List<PsiTryStatement> tryStatements = collectTryStatementStack(commonParent, enclosingCodeFragment);
+    if (tryStatements.isEmpty()) return false;
+    final PsiCodeBlock tryBlock = tryStatements.get(0).getTryBlock();
+    if (tryBlock == null) return false;
+
+    final Map<PsiVariable, IntArrayList> visibleReadOffsets = new THashMap<PsiVariable, IntArrayList>();
+    for (PsiVariable variable : writeOffsets.keySet()) {
+      if (!PsiTreeUtil.isAncestor(tryBlock, variable, true)) {
+        visibleReadOffsets.put(variable, new IntArrayList());
+      }
+    }
+    if (visibleReadOffsets.isEmpty()) return false;
+
+    for (int i = 0; i < instructions.size(); i++) {
+      final Instruction instruction = instructions.get(i);
+      if (instruction instanceof ReadVariableInstruction) {
+        final PsiVariable variable = ((ReadVariableInstruction)instruction).variable;
+        final IntArrayList readOffsets = visibleReadOffsets.get(variable);
+        if (readOffsets != null) {
+          readOffsets.add(i);
+        }
+      }
+    }
+    if (visibleReadOffsets.isEmpty()) return false;
+    LOG.debug("visibleReadOffsets:", visibleReadOffsets);
+
+    final Map<PsiVariable, Set<PsiElement>> afterWrite = new THashMap<PsiVariable, Set<PsiElement>>();
+    for (PsiVariable variable : visibleReadOffsets.keySet()) {
+      final Function<Integer, BitSet> calculator = getReachableInstructionsCalculator(flow, flowStart, flowEnd);
+      final BitSet collectedOffsets = new BitSet(flowEnd);
+      for (final int writeOffset : writeOffsets.get(variable).toArray()) {
+        LOG.assertTrue(writeOffset >= flowStart, "writeOffset");
+        final BitSet reachableOffsets = calculator.fun(writeOffset);
+        collectedOffsets.or(reachableOffsets);
+      }
+      Set<PsiElement> throwSources = afterWrite.get(variable);
+      if (throwSources == null) afterWrite.put(variable, throwSources = new THashSet<PsiElement>());
+      for (int i = flowStart; i < flowEnd; i++) {
+        if (collectedOffsets.get(i)) {
+          throwSources.add(flow.getElement(i));
+        }
+      }
+      final List<PsiElement> subordinates = new ArrayList<PsiElement>();
+      for (PsiElement element : throwSources) {
+        if (throwSources.contains(element.getParent())) {
+          subordinates.add(element);
+        }
+      }
+      throwSources.removeAll(subordinates);
+    }
+    LOG.debug("afterWrite:", afterWrite);
+
+    for (Map.Entry<PsiVariable, Set<PsiElement>> entry : afterWrite.entrySet()) {
+      final PsiVariable variable = entry.getKey();
+      final PsiElement[] psiElements = entry.getValue().toArray(PsiElement.EMPTY_ARRAY);
+      final List<PsiClassType> thrownExceptions = ExceptionUtil.getThrownExceptions(psiElements);
+
+      if (!thrownExceptions.isEmpty()) {
+        final IntArrayList catchOrFinallyOffsets = new IntArrayList();
+        for (PsiTryStatement tryStatement : tryStatements) {
+          final PsiCodeBlock finallyBlock = tryStatement.getFinallyBlock();
+          if (finallyBlock != null) {
+            int offset = flow.getStartOffset(finallyBlock);
+            if (offset >= 0) {
+              catchOrFinallyOffsets.add(offset - 2); // -2 is an adjustment for rethrow-after-finally
+            }
+          }
+          for (PsiCatchSection catchSection : tryStatement.getCatchSections()) {
+            final PsiCodeBlock catchBlock = catchSection.getCatchBlock();
+            final PsiParameter parameter = catchSection.getParameter();
+            if (catchBlock != null && parameter != null) {
+              for (PsiClassType throwType : thrownExceptions) {
+                if (ControlFlowUtil.isCaughtExceptionType(throwType, parameter.getType())) {
+                  int offset = flow.getStartOffset(catchBlock);
+                  if (offset >= 0) {
+                    catchOrFinallyOffsets.add(offset - 1); // -1 is an adjustment for catch block initialization
+                  }
+                }
+              }
+            }
+          }
+        }
+        if (!catchOrFinallyOffsets.isEmpty()) {
+          final IntArrayList readOffsets = visibleReadOffsets.get(variable);
+          if (readOffsets != null && !readOffsets.isEmpty()) {
+            for (int j = 0; j < catchOrFinallyOffsets.size(); j++) {
+              int catchOrFinallyOffset = catchOrFinallyOffsets.get(j);
+              if (ControlFlowUtil.areInstructionsReachable(flow, readOffsets.toArray(), catchOrFinallyOffset)) {
+                LOG.debug("catchOrFinallyOffset:", catchOrFinallyOffset);
+                return true;
+              }
+            }
+          }
+        }
+      }
+    }
+    return false;
+  }
+
+  @Nullable
+  private static PsiTryStatement getEnclosingTryStatementHavingCatchOrFinally(@Nullable PsiElement startElement,
+                                                                              @NotNull PsiElement enclosingCodeFragment) {
+    for (PsiElement element = startElement; element != null && element != enclosingCodeFragment; element = element.getParent()) {
+      if (element instanceof PsiCodeBlock) {
+        final PsiElement parent = element.getParent();
+        if (parent instanceof PsiTryStatement) {
+          final PsiTryStatement tryStatement = (PsiTryStatement)parent;
+          if (tryStatement.getTryBlock() == element &&
+              (tryStatement.getFinallyBlock() != null || tryStatement.getCatchBlocks().length != 0)) {
+            return tryStatement;
+          }
+        }
+      }
+    }
+    return null;
+  }
+
+  @NotNull
+  private static List<PsiTryStatement> collectTryStatementStack(@Nullable PsiElement startElement,
+                                                                @NotNull PsiElement enclosingCodeFragment) {
+    final List<PsiTryStatement> stack = new ArrayList<PsiTryStatement>();
+    for (PsiTryStatement tryStatement = getEnclosingTryStatementHavingCatchOrFinally(startElement, enclosingCodeFragment);
+         tryStatement != null;
+         tryStatement = getEnclosingTryStatementHavingCatchOrFinally(tryStatement, enclosingCodeFragment)) {
+      stack.add(tryStatement);
+    }
+    return stack;
+  }
+
   @NotNull
   public static PsiElement findCodeFragment(@NotNull PsiElement element) {
     PsiElement codeFragment = element;
@@ -1540,13 +1699,19 @@ public class ControlFlowUtil {
   /**
    * @return true if instruction at 'instructionOffset' is reachable from offset 'startOffset'
    */
-  public static boolean isInstructionReachable(final ControlFlow flow, final int instructionOffset, final int startOffset) {
+  public static boolean isInstructionReachable(@NotNull final ControlFlow flow, final int instructionOffset, final int startOffset) {
+    return areInstructionsReachable(flow, new int[]{instructionOffset}, startOffset);
+  }
+
+  private static boolean areInstructionsReachable(@NotNull final ControlFlow flow,
+                                                 @NotNull final int[] instructionOffsets,
+                                                 final int startOffset) {
     class MyVisitor extends InstructionClientVisitor<Boolean> {
       boolean reachable;
 
       @Override
       public void visitInstruction(Instruction instruction, int offset, int nextOffset) {
-        if (nextOffset == instructionOffset) reachable = true;
+        reachable |= ArrayUtil.indexOf(instructionOffsets, nextOffset) >= 0;
       }
 
       @Override
@@ -1559,7 +1724,7 @@ public class ControlFlowUtil {
       // Additional computations are required to take into account CALL and RETURN instructions in the case where
       // the start offset isn't the beginning of the control flow, because we couldn't know the correct state
       // of the call stack if we started traversal of the control flow from an offset in the middle.
-      return isInstructionReachableConsideringCalls(flow, instructionOffset, startOffset);
+      return areInstructionsReachableWithCalls(flow, instructionOffsets, startOffset);
     }
     MyVisitor visitor = new MyVisitor();
     depthFirstSearch(flow, visitor, startOffset, flow.getSize());
@@ -1576,99 +1741,138 @@ public class ControlFlowUtil {
     return false;
   }
 
-  public static boolean isInstructionReachableConsideringCalls(final ControlFlow flow, final int instructionOffset, final int startOffset) {
-    class ControlFlowGraph {
-      // The graph is sparse: simple instructions have 1 next offset, branching - 2 next offsets, RETURN may have many (one per call)
-      final int[][] nextOffsets;
+  private abstract static class ControlFlowGraph extends InstructionClientVisitor<Void> {
+    // The graph is sparse: simple instructions have 1 next offset, branching - 2 next offsets, RETURN may have many (one per call)
+    final int[][] nextOffsets;
 
-      ControlFlowGraph(int size) {
-        nextOffsets = new int[size][];
-      }
+    ControlFlowGraph(int size) {
+      nextOffsets = new int[size][];
+    }
 
-      void addArc(int offset, int nextOffset) {
-        if (nextOffsets[offset] == null) {
-          nextOffsets[offset] = new int[]{nextOffset, -1};
-        }
-        else {
-          int[] targets = nextOffsets[offset];
-          if (ArrayUtil.indexOf(targets, nextOffset) < 0) {
-            int freeIndex = ArrayUtil.indexOf(targets, -1);
-            if (freeIndex >= 0) {
-              targets[freeIndex] = nextOffset;
-            }
-            else {
-              int oldLength = targets.length;
-              nextOffsets[offset] = targets = ArrayUtil.realloc(targets, oldLength * 3 / 2);
-              Arrays.fill(targets, oldLength, targets.length, -1);
-              targets[oldLength] = nextOffset;
-            }
+    @Override
+    public void visitInstruction(Instruction instruction, int offset, int nextOffset) {
+      if (nextOffset > size()) nextOffset = size();
+      addArc(offset, nextOffset);
+    }
+
+    void addArc(int offset, int nextOffset) {
+      if (nextOffsets[offset] == null) {
+        nextOffsets[offset] = new int[]{nextOffset, -1};
+      }
+      else {
+        int[] targets = nextOffsets[offset];
+        if (ArrayUtil.indexOf(targets, nextOffset) < 0) {
+          int freeIndex = ArrayUtil.indexOf(targets, -1);
+          if (freeIndex >= 0) {
+            targets[freeIndex] = nextOffset;
+          }
+          else {
+            int oldLength = targets.length;
+            nextOffsets[offset] = targets = ArrayUtil.realloc(targets, oldLength * 3 / 2);
+            Arrays.fill(targets, oldLength, targets.length, -1);
+            targets[oldLength] = nextOffset;
           }
         }
       }
+    }
 
-      int[] getNextOffsets(int offset) {
-        return nextOffsets[offset] != null ? nextOffsets[offset] : ArrayUtil.EMPTY_INT_ARRAY;
-      }
+    int[] getNextOffsets(int offset) {
+      return nextOffsets[offset] != null ? nextOffsets[offset] : ArrayUtil.EMPTY_INT_ARRAY;
+    }
+
+    int size() {
+      return nextOffsets.length;
+    }
 
-      int size() {
-        return nextOffsets.length;
+    @Override
+    public String toString() {
+      StringBuilder s = new StringBuilder();
+      for (int i = 0; i < nextOffsets.length; i++) {
+        int[] targets = nextOffsets[i];
+        if (targets != null && targets.length != 0 && targets[0] != -1) {
+          if (s.length() != 0) s.append(' ');
+          s.append('(').append(i).append("->");
+          for (int j = 0; j < targets.length && targets[j] != -1; j++) {
+            if (j != 0) s.append(",");
+            s.append(targets[j]);
+          }
+          s.append(')');
+        }
       }
+      return s.toString();
+    }
 
-      @Override
-      public String toString() {
-        StringBuilder s = new StringBuilder();
-        for (int i = 0; i < nextOffsets.length; i++) {
-          int[] targets = nextOffsets[i];
-          if (targets != null && targets.length != 0 && targets[0] != -1) {
-            if (s.length() != 0) s.append(' ');
-            s.append('(').append(i).append("->");
-            for (int j = 0; j < targets.length && targets[j] != -1; j++) {
-              if (j != 0) s.append(",");
-              s.append(targets[j]);
+    boolean depthFirstSearch(final int startOffset, final BitSet visitedOffsets) {
+      // traverse the graph starting with the startOffset
+      IntStack walkThroughStack = new IntStack(Math.max(size() / 2, 2));
+      visitedOffsets.clear();
+      walkThroughStack.push(startOffset);
+      while (!walkThroughStack.empty()) {
+        int currentOffset = walkThroughStack.pop();
+        if (currentOffset < size() && !visitedOffsets.get(currentOffset)) {
+          visitedOffsets.set(currentOffset);
+          int[] nextOffsets = getNextOffsets(currentOffset);
+          for (int nextOffset : nextOffsets) {
+            if (nextOffset == -1) break;
+            if (isComplete(currentOffset, nextOffset)) {
+              return true;
             }
-            s.append(')');
+            walkThroughStack.push(nextOffset);
           }
         }
-        return s.toString();
       }
+      return false;
     }
 
-    class MyVisitor extends InstructionClientVisitor<Boolean> {
-      final ControlFlowGraph graph = new ControlFlowGraph(flow.getInstructions().size());
+    @Override
+    public Void getResult() {
+      return null;
+    }
 
-      @Override
-      public void visitInstruction(Instruction instruction, int offset, int nextOffset) {
-        if (nextOffset > graph.size()) nextOffset = graph.size();
-        graph.addArc(offset, nextOffset);
+    boolean isComplete(int offset, int nextOffset) {
+      return false;
+    }
+
+    void buildFrom(ControlFlow flow) {
+      // traverse the whole flow in order to collect the graph edges
+      ControlFlowUtil.depthFirstSearch(flow, this, 0, flow.getSize());
+    }
+  }
+
+  private static boolean areInstructionsReachableWithCalls(@NotNull final ControlFlow flow,
+                                                           @NotNull final int[] instructionOffsets,
+                                                           final int startOffset) {
+    ControlFlowGraph graph = new ControlFlowGraph(flow.getSize()) {
+      boolean isComplete(int offset, int nextOffset) {
+        return ArrayUtil.indexOf(instructionOffsets, nextOffset) >= 0;
       }
+    };
+    graph.buildFrom(flow);
+    return graph.depthFirstSearch(startOffset, new BitSet(flow.getSize()));
+  }
 
+  private static Function<Integer, BitSet> getReachableInstructionsCalculator(@NotNull final ControlFlow flow,
+                                                                              final int flowStart,
+                                                                              final int flowEnd) {
+    final ControlFlowGraph graph = new ControlFlowGraph(flow.getSize()) {
       @Override
-      public Boolean getResult() {
-        // The same logic as in depthFirstSearch(), just more lightweight implementation
-        Arrays.fill(processedInstructions, false);
-        IntStack walkThroughStack = new IntStack(Math.max(graph.size() / 2, 2));
-        walkThroughStack.push(startOffset);
-        while (!walkThroughStack.empty()) {
-          int currentOffset = walkThroughStack.pop();
-          if (currentOffset < graph.size() && !processedInstructions[currentOffset]) {
-            processedInstructions[currentOffset] = true;
-            int[] nextOffsets = graph.getNextOffsets(currentOffset);
-            for (int nextOffset : nextOffsets) {
-              if (nextOffset == -1) break;
-              if (nextOffset == instructionOffset) {
-                return true;
-              }
-              walkThroughStack.push(nextOffset);
-            }
-          }
+      void addArc(int offset, int nextOffset) {
+        nextOffset = promoteThroughGotoChain(flow, nextOffset);
+        if (nextOffset >= flowStart && nextOffset < flowEnd) {
+          super.addArc(offset, nextOffset);
         }
-        return false;
       }
-    }
+    };
+    graph.buildFrom(flow);
 
-    MyVisitor visitor = new MyVisitor();
-    depthFirstSearch(flow, visitor, 0, flow.getSize()); // traverse the graph, store the graph edges in visitor.graph
-    return visitor.getResult().booleanValue(); // traverse the visitor.graph starting with the startOffset
+    return new Function<Integer, BitSet>() {
+      @Override
+      public BitSet fun(Integer startOffset) {
+        BitSet visitedOffsets = new BitSet(flowEnd);
+        graph.depthFirstSearch(startOffset, visitedOffsets);
+        return visitedOffsets;
+      }
+    };
   }
 
   public static boolean isVariableAssignedInLoop(@NotNull PsiReferenceExpression expression, PsiElement resolved) {
@@ -1695,4 +1899,20 @@ public class ControlFlowUtil {
     int startOffset = flow.getStartOffset(assignmentExpression);
     return startOffset != -1 && isInstructionReachable(flow, startOffset, startOffset);
   }
+
+  public static boolean isCaughtExceptionType(@NotNull PsiClassType throwType, @NotNull PsiType catchType) {
+    return catchType.isAssignableFrom(throwType) || mightBeAssignableFromSubclass(throwType, catchType);
+  }
+
+  private static boolean mightBeAssignableFromSubclass(@NotNull final PsiClassType throwType, @NotNull PsiType catchType) {
+    if (catchType instanceof PsiDisjunctionType) {
+      for (PsiType catchDisjunction : ((PsiDisjunctionType)catchType).getDisjunctions()) {
+        if (throwType.isAssignableFrom(catchDisjunction)) {
+          return true;
+        }
+      }
+      return false;
+    }
+    return throwType.isAssignableFrom(catchType);
+  }
 }
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/IDEADEV33368_after.java b/java/java-tests/testData/refactoring/extractMethod/IDEADEV33368_after.java
deleted file mode 100644 (file)
index c05d3dc..0000000
+++ /dev/null
@@ -1,21 +0,0 @@
-class Test {
-   void m(boolean b) {
-           int x = 42;
-           try {
-
-               x = newMethod(b, x);
-
-           } catch(Exception e) {
-               System.out.println(x);
-           }
-       }
-
-    private int newMethod(boolean b, int x) throws Exception {
-        if(b) {
-            x = 23;
-            throw new Exception();
-        }
-        return x;
-    }
-
-}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseParamInCatch.java b/java/java-tests/testData/refactoring/extractMethod/UseParamInCatch.java
new file mode 100644 (file)
index 0000000..28086f1
--- /dev/null
@@ -0,0 +1,11 @@
+class C {
+    void f(boolean b, String s) {
+        try {
+            <selection>s = "a";
+            if (b) throw new RuntimeException();</selection>
+            s = "b";
+        } catch (RuntimeException e) {
+            System.out.println(s);
+        }
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseParamInFinally.java b/java/java-tests/testData/refactoring/extractMethod/UseParamInFinally.java
new file mode 100644 (file)
index 0000000..2e7737c
--- /dev/null
@@ -0,0 +1,11 @@
+class C {
+    void f(boolean b, String s) {
+        try {
+            <selection>s = "a";
+            if (b) throw new RuntimeException();</selection>
+            s = "b";
+        } finally {
+            System.out.println(s);
+        }
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarAfterCatch.java b/java/java-tests/testData/refactoring/extractMethod/UseVarAfterCatch.java
new file mode 100644 (file)
index 0000000..b44ef06
--- /dev/null
@@ -0,0 +1,17 @@
+class A {
+    void foo() {
+        String s = "";
+        try {
+            <selection>s = "a";
+            bar();</selection>
+            s = "b";
+        } catch (Exception e) {
+            System.out.println(e);
+        }
+        System.out.println(s);
+    }
+
+    void bar() throws Exception {
+        if (true) throw new Exception();
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarInCatch1.java b/java/java-tests/testData/refactoring/extractMethod/UseVarInCatch1.java
new file mode 100644 (file)
index 0000000..9cbc666
--- /dev/null
@@ -0,0 +1,19 @@
+class A {
+    static class MyException extends Exception{ MyException(){ super(); } }
+
+    void foo() {
+        String s = "";
+        try {
+            s = "a";
+            <selection>bar();
+            s = "b";</selection>
+            bar();
+        } catch (MyException e) {
+            throw new RuntimeException(s, e);
+        }
+    }
+
+    void bar() throws MyException {
+        if (true) throw new MyException();
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarInCatch1_after.java b/java/java-tests/testData/refactoring/extractMethod/UseVarInCatch1_after.java
new file mode 100644 (file)
index 0000000..de99c81
--- /dev/null
@@ -0,0 +1,27 @@
+import org.jetbrains.annotations.NotNull;
+
+class A {
+    static class MyException extends Exception{ MyException(){ super(); } }
+
+    void foo() {
+        String s = "";
+        try {
+            s = "a";
+            s = newMethod(s);
+            bar();
+        } catch (MyException e) {
+            throw new RuntimeException(s, e);
+        }
+    }
+
+    @NotNull
+    private String newMethod(String s) throws MyException {
+        bar();
+        s = "b";
+        return s;
+    }
+
+    void bar() throws MyException {
+        if (true) throw new MyException();
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarInCatch2.java b/java/java-tests/testData/refactoring/extractMethod/UseVarInCatch2.java
new file mode 100644 (file)
index 0000000..1f975ab
--- /dev/null
@@ -0,0 +1,19 @@
+class A {
+    static class MyException extends Exception{ MyException(){ super(); } }
+
+    void foo() {
+        String s = "";
+        try {
+            <selection>s = "a";
+            bar();</selection>
+            s = "b";
+            bar();
+        } catch (MyException e) {
+            throw new RuntimeException(s, e);
+        }
+    }
+
+    void bar() throws MyException {
+        if (true) throw new MyException();
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarInCatchInvisible.java b/java/java-tests/testData/refactoring/extractMethod/UseVarInCatchInvisible.java
new file mode 100644 (file)
index 0000000..add4cbc
--- /dev/null
@@ -0,0 +1,18 @@
+class A {
+    void foo() {
+        String s = null;
+        try {
+            s = "1";
+            <selection>int invisibleOutsideTry = 1 + 1;
+            bar();</selection>
+            s = s + invisibleOutsideTry;
+        } catch (Exception e) {
+            System.out.println("ex " + s);
+        }
+        System.out.println("ok " + s);
+    }
+
+    private void bar() throws Exception {
+        throw new Exception();
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarInCatchInvisible_after.java b/java/java-tests/testData/refactoring/extractMethod/UseVarInCatchInvisible_after.java
new file mode 100644 (file)
index 0000000..d5f13a6
--- /dev/null
@@ -0,0 +1,23 @@
+class A {
+    void foo() {
+        String s = null;
+        try {
+            s = "1";
+            int invisibleOutsideTry = newMethod();
+            s = s + invisibleOutsideTry;
+        } catch (Exception e) {
+            System.out.println("ex " + s);
+        }
+        System.out.println("ok " + s);
+    }
+
+    private int newMethod() throws Exception {
+        int invisibleOutsideTry = 1 + 1;
+        bar();
+        return invisibleOutsideTry;
+    }
+
+    private void bar() throws Exception {
+        throw new Exception();
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarInCatchNested1.java b/java/java-tests/testData/refactoring/extractMethod/UseVarInCatchNested1.java
new file mode 100644 (file)
index 0000000..16126db
--- /dev/null
@@ -0,0 +1,28 @@
+import java.io.IOException;
+import java.sql.SQLException;
+import java.util.concurrent.ThreadLocalRandom;
+
+class A {
+    void f() {
+        int i = 0;
+        try {
+            i = 1;
+            try {
+                i = 2;
+                if (r()) throw new IOException();
+                <selection>i = 3;
+                if (r()) throw new SQLException();
+                i = 4;</selection>
+                System.out.println("ok");
+            } catch (IOException e) {
+                System.out.println("io " + i);
+            }
+        } catch (SQLException e) {
+            System.out.println("sql");
+        }
+    }
+
+    private boolean r() {
+        return ThreadLocalRandom.current().nextBoolean();
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarInCatchNested1_after.java b/java/java-tests/testData/refactoring/extractMethod/UseVarInCatchNested1_after.java
new file mode 100644 (file)
index 0000000..edcb22b
--- /dev/null
@@ -0,0 +1,34 @@
+import java.io.IOException;
+import java.sql.SQLException;
+import java.util.concurrent.ThreadLocalRandom;
+
+class A {
+    void f() {
+        int i = 0;
+        try {
+            i = 1;
+            try {
+                i = 2;
+                if (r()) throw new IOException();
+                i = newMethod();
+                System.out.println("ok");
+            } catch (IOException e) {
+                System.out.println("io " + i);
+            }
+        } catch (SQLException e) {
+            System.out.println("sql");
+        }
+    }
+
+    private int newMethod() throws SQLException {
+        int i;
+        i = 3;
+        if (r()) throw new SQLException();
+        i = 4;
+        return i;
+    }
+
+    private boolean r() {
+        return ThreadLocalRandom.current().nextBoolean();
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarInCatchNested2.java b/java/java-tests/testData/refactoring/extractMethod/UseVarInCatchNested2.java
new file mode 100644 (file)
index 0000000..2fdb97d
--- /dev/null
@@ -0,0 +1,28 @@
+import java.io.IOException;
+import java.sql.SQLException;
+import java.util.concurrent.ThreadLocalRandom;
+
+class A {
+    void f() {
+        int i = 0;
+        try {
+            i = 1;
+            try {
+                i = 2;
+                if (r()) throw new IOException();
+                <selection>i = 3;
+                if (r()) throw new SQLException();
+                i = 4;</selection>
+                System.out.println("ok");
+            } catch (IOException e) {
+                System.out.println("io " + i);
+            }
+        } catch (SQLException e) {
+            System.out.println("sql " + i);
+        }
+    }
+
+    private boolean r() {
+        return ThreadLocalRandom.current().nextBoolean();
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarInFinally1.java b/java/java-tests/testData/refactoring/extractMethod/UseVarInFinally1.java
new file mode 100644 (file)
index 0000000..baea4a5
--- /dev/null
@@ -0,0 +1,18 @@
+import java.util.concurrent.ThreadLocalRandom;
+
+public class A {
+    void f() {
+        String s = "";
+        try {
+            s = "a";
+            <selection>if (r()) throw new RuntimeException();
+            s = "b";</selection>
+        } finally {
+            System.out.println(s);
+        }
+    }
+
+    private boolean r() {
+        return ThreadLocalRandom.current().nextBoolean();
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarInFinally1_after.java b/java/java-tests/testData/refactoring/extractMethod/UseVarInFinally1_after.java
new file mode 100644 (file)
index 0000000..307f0db
--- /dev/null
@@ -0,0 +1,26 @@
+import org.jetbrains.annotations.NotNull;
+
+import java.util.concurrent.ThreadLocalRandom;
+
+public class A {
+    void f() {
+        String s = "";
+        try {
+            s = "a";
+            s = newMethod(s);
+        } finally {
+            System.out.println(s);
+        }
+    }
+
+    @NotNull
+    private String newMethod(String s) {
+        if (r()) throw new RuntimeException();
+        s = "b";
+        return s;
+    }
+
+    private boolean r() {
+        return ThreadLocalRandom.current().nextBoolean();
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarInFinally2.java b/java/java-tests/testData/refactoring/extractMethod/UseVarInFinally2.java
new file mode 100644 (file)
index 0000000..22fb9ad
--- /dev/null
@@ -0,0 +1,18 @@
+import java.util.concurrent.ThreadLocalRandom;
+
+public class A {
+    void f() {
+        String s = "";
+        try {
+            <selection>s = "a";
+            if (r()) throw new RuntimeException();</selection>
+            s = "b";
+        } finally {
+            System.out.println(s);
+        }
+    }
+
+    private boolean r() {
+        return ThreadLocalRandom.current().nextBoolean();
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarInOtherCatch.java b/java/java-tests/testData/refactoring/extractMethod/UseVarInOtherCatch.java
new file mode 100644 (file)
index 0000000..9db1360
--- /dev/null
@@ -0,0 +1,20 @@
+class A {
+    static class ExceptionA extends Exception{ ExceptionA(){ super(); } }
+    static class ExceptionB extends Exception{ ExceptionB(){ super(); } }
+
+    void foo(boolean a, boolean b) {
+        String s = "";
+        try {
+            s = "a";
+            if (a) throw new ExceptionA();
+            <selection>s = "b";
+            if (b) throw new ExceptionB();
+            s = "c";</selection>
+            System.out.println(s);
+        } catch (ExceptionA ea) {
+            throw new RuntimeException(s, ea);
+        } catch (ExceptionB eb) {
+            System.out.println(eb);
+        }
+    }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/refactoring/extractMethod/UseVarInOtherCatch_after.java b/java/java-tests/testData/refactoring/extractMethod/UseVarInOtherCatch_after.java
new file mode 100644 (file)
index 0000000..756b94f
--- /dev/null
@@ -0,0 +1,29 @@
+import org.jetbrains.annotations.NotNull;
+
+class A {
+    static class ExceptionA extends Exception{ ExceptionA(){ super(); } }
+    static class ExceptionB extends Exception{ ExceptionB(){ super(); } }
+
+    void foo(boolean a, boolean b) {
+        String s = "";
+        try {
+            s = "a";
+            if (a) throw new ExceptionA();
+            s = newMethod(b);
+            System.out.println(s);
+        } catch (ExceptionA ea) {
+            throw new RuntimeException(s, ea);
+        } catch (ExceptionB eb) {
+            System.out.println(eb);
+        }
+    }
+
+    @NotNull
+    private String newMethod(boolean b) throws ExceptionB {
+        String s;
+        s = "b";
+        if (b) throw new ExceptionB();
+        s = "c";
+        return s;
+    }
+}
\ No newline at end of file
index 00d81e6a7582904289b6e5737aad869d5a65dcc0..de258ffd3b9a253e3a338a0b3830d7a11da8e97f 100644 (file)
@@ -125,6 +125,50 @@ public class ExtractMethodTest extends LightCodeInsightTestCase {
     doTest();
   }
 
+  public void testUseParamInCatch() throws Exception {
+    doExitPointsTest(false);
+  }
+
+  public void testUseParamInFinally() throws Exception {
+    doExitPointsTest(false);
+  }
+
+  public void testUseVarAfterCatch() throws Exception {
+    doExitPointsTest(false);
+  }
+
+  public void testUseVarInCatch1() throws Exception {
+    doTest();
+  }
+
+  public void testUseVarInCatch2() throws Exception {
+    doExitPointsTest(false);
+  }
+
+  public void testUseVarInCatchInvisible() throws Exception {
+    doTest();
+  }
+
+  public void testUseVarInCatchNested1() throws Exception {
+    doTest();
+  }
+
+  public void testUseVarInCatchNested2() throws Exception {
+    doExitPointsTest(false);
+  }
+
+  public void testUseVarInOtherCatch() throws Exception {
+    doTest();
+  }
+
+  public void testUseVarInFinally1() throws Exception {
+    doTest();
+  }
+
+  public void testUseVarInFinally2() throws Exception {
+    doExitPointsTest(false);
+  }
+
   public void testOneBranchAssignment() throws Exception {
     doTest();
   }
@@ -185,7 +229,7 @@ public class ExtractMethodTest extends LightCodeInsightTestCase {
     doTest();
   }
 
-  public void _testExtractFromTryFinally2() throws Exception {  // IDEADEV-11844
+  public void testExtractFromTryFinally2() throws Exception {
     doTest();
   }
 
@@ -322,7 +366,7 @@ public class ExtractMethodTest extends LightCodeInsightTestCase {
   }
 
   public void testIDEADEV33368() throws Exception {
-    doTest();
+    doExitPointsTest(false);
   }
 
   public void testInlineCreated2ReturnLocalVariablesOnly() throws Exception {