Correctly set possible targets for exit from finally block

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java

@@ -1049,7 +1049,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
       myTrapStack = myTrapStack.prepend(new InsideFinally(finallyBlock));
 
       finallyBlock.accept(this);
-      addInstruction(new ControlTransferInstruction(null)); // DfaControlTransferValue is on stack
+      controlTransfer(new ExitFinallyTransfer(finallyDescriptor), FList.emptyList());
 
       popTrap(InsideFinally.class);
     }
@@ -1088,7 +1088,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
       myTrapStack = myTrapStack.prepend(new InsideFinally(resourceList));
       startElement(resourceList);
       addThrows(null, closerExceptions.toArray(PsiClassType.EMPTY_ARRAY));
-      addInstruction(new ControlTransferInstruction(null)); // DfaControlTransferValue is on stack
+      controlTransfer(new ExitFinallyTransfer(twrFinallyDescriptor), FList.emptyList()); // DfaControlTransferValue is on stack
       finishElement(resourceList);
       popTrap(InsideFinally.class);
     }

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java

@@ -67,7 +67,7 @@ public abstract class InstructionVisitor {
   public DfaInstructionState[] visitControlTransfer(@NotNull ControlTransferInstruction controlTransferInstruction,
                                                     @NotNull DataFlowRunner runner, @NotNull DfaMemoryState state) {
     DfaControlTransferValue transferValue = controlTransferInstruction.getTransfer();
-    if (transferValue == null) {
+    if (transferValue.getTarget() instanceof ExitFinallyTransfer) {
       transferValue = (DfaControlTransferValue)state.pop();
     }
     return new ControlTransferHandler(state, runner, transferValue.getTarget()).iteration(transferValue.getTraps())

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/controlTransfer.kt

@@ -25,6 +25,7 @@ import com.intellij.codeInspection.dataFlow.value.DfaVariableValue
 import com.intellij.psi.*
 import com.intellij.util.containers.FList
 import java.util.*
+import kotlin.collections.ArrayList
 
 /**
  * @author peter
@@ -35,50 +36,61 @@ class DfaControlTransferValue(factory: DfaValueFactory,
   override fun toString() = target.toString() + (if (traps.isEmpty()) "" else " $traps")
 }
 
-interface TransferTarget
+interface TransferTarget {
+  fun getPossibleTargets() : Collection<Int> = emptyList()
+}
 data class ExceptionTransfer(val throwable: DfaPsiType?) : TransferTarget {
   override fun toString() = "Exception($throwable)"
 }
 data class InstructionTransfer(val offset: ControlFlow.ControlFlowOffset, val toFlush: List<DfaVariableValue>) : TransferTarget {
-  override fun toString(): String {
-    return "-> $offset" + (if (toFlush.isEmpty()) "" else "; flushing $toFlush")
-  }
+  override fun getPossibleTargets() = listOf(offset.instructionOffset)
+
+  override fun toString() = "-> $offset" + (if (toFlush.isEmpty()) "" else "; flushing $toFlush")
+}
+data class ExitFinallyTransfer(private val enterFinally: Trap.EnterFinally) : TransferTarget {
+  override fun getPossibleTargets() = enterFinally.backLinks.asIterable().flatMap { it.getPossibleTargetIndices() }
+    .filter { index -> index != enterFinally.jumpOffset.instructionOffset }.toSet()
+
+  override fun toString() = "ExitFinally"
 }
 object ReturnTransfer : TransferTarget {
   override fun toString(): String = "Return"
 }
 
-open class ControlTransferInstruction(val transfer: DfaControlTransferValue?) : Instruction() {
+open class ControlTransferInstruction(val transfer: DfaControlTransferValue) : Instruction() {
+  init {
+    transfer.traps.forEach { trap -> trap.link(this) }
+  }
+
   override fun accept(runner: DataFlowRunner, state: DfaMemoryState, visitor: InstructionVisitor): Array<out DfaInstructionState> {
     return visitor.visitControlTransfer(this, runner, state)
   }
 
-  fun getPossibleTargetIndices() : List<Int> {
-    if (transfer == null) return emptyList()
-
-    val result = ArrayList(transfer.traps.flatMap(Trap::getPossibleTargets))
-    if (transfer.target is InstructionTransfer) {
-      result.add(transfer.target.offset.instructionOffset)
-    }
-    return result
-  }
+  fun getPossibleTargetIndices() = transfer.traps.flatMap(Trap::getPossibleTargets) + transfer.target.getPossibleTargets()
 
   fun getPossibleTargetInstructions(allInstructions: Array<Instruction>) = getPossibleTargetIndices().map { allInstructions[it] }
 
-  override fun toString() = (if (transfer == null) "RET" else "TRANSFER $transfer")+" [TARGETS: "+getPossibleTargetIndices()+"]"
+  override fun toString() = "TRANSFER $transfer [targets: ${getPossibleTargetIndices()}]"
 }
 
 sealed class Trap(val anchor: PsiElement) {
+  open fun link(instruction: ControlTransferInstruction) {}
+
   class TryCatch(tryStatement: PsiTryStatement, val clauses: LinkedHashMap<PsiCatchSection, ControlFlow.ControlFlowOffset>)
     : Trap(tryStatement) {
     override fun toString() = "TryCatch -> ${clauses.values}"
   }
-  class TryFinally(finallyBlock: PsiCodeBlock, val jumpOffset: ControlFlow.ControlFlowOffset): Trap(finallyBlock) {
-    override fun toString() = "TryFinally -> $jumpOffset"
-  }
-  class TwrFinally(resourceList: PsiResourceList, val jumpOffset: ControlFlow.ControlFlowOffset): Trap(resourceList) {
-    override fun toString() = "TwrFinally -> $jumpOffset"
+  abstract class EnterFinally(anchor: PsiElement, val jumpOffset: ControlFlow.ControlFlowOffset): Trap(anchor) {
+    internal val backLinks = ArrayList<ControlTransferInstruction>()
+
+    override fun link(instruction: ControlTransferInstruction) {
+      backLinks.add(instruction)
+    }
+
+    override fun toString() = "${javaClass.simpleName} -> $jumpOffset"
   }
+  class TryFinally(finallyBlock: PsiCodeBlock, jumpOffset: ControlFlow.ControlFlowOffset): EnterFinally(finallyBlock, jumpOffset)
+  class TwrFinally(resourceList: PsiResourceList, jumpOffset: ControlFlow.ControlFlowOffset): EnterFinally(resourceList, jumpOffset)
   class InsideFinally(finallyBlock: PsiElement): Trap(finallyBlock) {
     override fun toString() = "InsideFinally"
   }
@@ -89,8 +101,7 @@ sealed class Trap(val anchor: PsiElement) {
   internal fun getPossibleTargets(): Collection<Int> {
     return when (this) {
       is TryCatch -> clauses.values.map { it.instructionOffset }
-      is TryFinally -> listOf(jumpOffset.instructionOffset)
-      is TwrFinally -> listOf(jumpOffset.instructionOffset)
+      is EnterFinally -> listOf(jumpOffset.instructionOffset)
       else -> emptyList()
     }
   }
@@ -112,6 +123,7 @@ private class ControlTransferHandler(val state: DfaMemoryState, val runner: Data
         assert((state.pop() as DfaControlTransferValue).target === ReturnTransfer)
         iteration(tail)
       }
+      else -> throw InternalError("Impossible")
     }
   }
 

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ReturnInstruction.java

@@ -16,7 +16,9 @@
 
 package com.intellij.codeInspection.dataFlow.instructions;
 
-import com.intellij.codeInspection.dataFlow.*;
+import com.intellij.codeInspection.dataFlow.ControlTransferInstruction;
+import com.intellij.codeInspection.dataFlow.DfaControlTransferValue;
+import com.intellij.codeInspection.dataFlow.ExceptionTransfer;
 import com.intellij.psi.PsiElement;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -36,7 +38,7 @@ public class ReturnInstruction extends ControlTransferInstruction {
 
   public boolean isViaException() {
     DfaControlTransferValue transfer = getTransfer();
-    return transfer != null && transfer.getTarget() instanceof ExceptionTransfer;
+    return transfer.getTarget() instanceof ExceptionTransfer;
   }
 
 }

java/java-tests/testData/inspection/dataFlow/fixture/TryFinallyInsideFinally.java

@@ -16,7 +16,7 @@ class Test {
       }
 
       if (!rangeMarkersDisposed) {
-        <warning descr="Variable is already assigned to this value">foo</warning> = "dd";
+        foo = "dd";
       }
     }
   }

java/java-tests/testData/inspection/dataFlow/fixture/TryFinallySimple.java

@@ -0,0 +1,51 @@
+// IDEA-191876
+import org.jetbrains.annotations.*;
+
+class Test {
+  public static void runXyz() {
+    String output = null;
+
+    try {
+      Runnable shutdownHook = () -> { };
+
+      try {
+        output = "foo";
+      }
+      finally {
+        Runtime.getRuntime().addShutdownHook(new Thread(shutdownHook));
+        shutdownHook.run();
+      }
+    }
+    finally {
+      System.out.println(<warning descr="Condition 'output != null' is always 'true'">output != null</warning> ? output.trim() : "");
+    }
+    System.out.println(output.trim());
+  }
+
+  @NotNull
+  public static String getString() {
+    return "foo";
+  }
+
+  public static void test() {
+    String s = null;
+    try {
+      s = getString();
+    }
+    finally {
+      System.out.println(s == null ? null : s.trim());
+    }
+    System.out.println(s.trim());
+  }
+
+  public static void test2(@Nullable String s) {
+    if (s == null) return;
+
+    try {
+      System.out.println("Hello");
+    }
+    finally {
+      System.out.println(s.trim());
+    }
+  }
+}

java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java

@@ -228,4 +228,5 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase {
   public void testQueuePeek() { doTest(); }
   public void testForeachCollectionElement() { doTest(); }
   public void testContractReturnValues() { doTest(); }
+  public void testTryFinallySimple() { doTest(); }
 }