PY-80421 PY-80471 PY-80824 PY-80550 false "unused variable" with for nested in if/else

PY-80564 Fp "Local variable might be referenced before assignment" when returning a comprehension in `try/except`

PY-80733 Fp "Local variable might be referenced before assignment" for `try/except` with a `break` inside a loop


Merge-request: IJ-MR-162320
Merged-by: Aleksandr Govenko <aleksandr.govenko@jetbrains.com>

(cherry picked from commit f3e5d76e1fb15e2951d395fa27768269e4d0cb8f)

IJ-MR-162320

GitOrigin-RevId: 76322f34176e25dd5cf4bc7fd329a9bbc8c7abd5

python/python-psi-impl/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowBuilder.java

@@ -587,6 +587,8 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor {
     if (elsePart != null) {
       visitPyStatementPart(elsePart);
     }
+
+    collectInternalPendingEdges(node);
   }
 
   @Override
@@ -631,7 +633,10 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor {
       elsePart.accept(this);
       myBuilder.addPendingEdge(node, myBuilder.prevInstruction);
     }
+    
+
     myBuilder.flowAbrupted();
+    collectInternalPendingEdges(node);
   }
 
   private static boolean loopHasAtLeastOneIteration(@NotNull PyLoopStatement loopStatement) {
@@ -751,18 +756,21 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor {
     final Instruction finallyFailInstruction;
 
     // Store pending normal exit instructions from try-except-else parts
-    myBuilder.processPending((pendingScope, instruction) -> {
-      final PsiElement pendingElement = instruction.getElement();
-      final boolean isPending = pendingElement == null || 
-                                PsiTreeUtil.isAncestor(node, pendingElement, false) &&
-                                !PsiTreeUtil.isAncestor(finallyPart, pendingElement, false);
-      if (isPending && pendingScope != null) {
-        pendingNormalExits.add(Pair.createNonNull(pendingScope, instruction));
-      }
-      else {
-        myBuilder.addPendingEdge(pendingScope, instruction);
-      }
-    });
+    if (finallyPart != null) {
+      myBuilder.processPending((pendingScope, instruction) -> {
+        final PsiElement pendingElement = instruction.getElement();
+        if (pendingElement != null) {
+          final boolean isPending = PsiTreeUtil.isAncestor(node, pendingElement, false) &&
+                                    !PsiTreeUtil.isAncestor(finallyPart, pendingElement, false);
+          if (isPending && pendingScope != null) {
+            pendingNormalExits.add(Pair.createNonNull(pendingScope, instruction));
+          }
+          else {
+            myBuilder.addPendingEdge(pendingScope, instruction);
+          }
+        }
+      });
+    }
 
     // Finally-fail part handling
     if (finallyPart != null) {
@@ -806,7 +814,6 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor {
       }
     }
 
-    final Instruction exitInstruction;
     if (finallyPart != null) {
       myBuilder.processPending((pendingScope, instruction) -> {
         final PsiElement e = instruction.getElement();
@@ -827,6 +834,7 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor {
 
       // Duplicate CFG for finally (-fail and -success) only if there are some successful exits from the
       // try part. Otherwise, a single CFG for finally provides the correct control flow
+      final Instruction finallyInstruction;
       if (!pendingNormalExits.isEmpty()) {
         // Finally-success part handling
         pendingBackup = new ArrayList<>(myBuilder.pending);
@@ -837,30 +845,28 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor {
         for (Pair<PsiElement, Instruction> pair : pendingBackup) {
           myBuilder.addPendingEdge(pair.first, pair.second);
         }
-        exitInstruction = finallySuccessInstruction;
+        finallyInstruction = finallySuccessInstruction;
       }
       else {
-        exitInstruction = finallyFailInstruction;
+        finallyInstruction = finallyFailInstruction;
+      }
+
+      // Connect normal exits from try and else parts to the finally part
+      for (Pair<PsiElement, Instruction> pendingScopeAndInstruction : pendingNormalExits) {
+        final PsiElement pendingScope = pendingScopeAndInstruction.first;
+        final Instruction instruction = pendingScopeAndInstruction.second;
+
+        myBuilder.addEdge(instruction, finallyInstruction);
+
+        // When instruction continues outside try-except statement scope
+        // the last instruction in finally-block is marked as pointing to that continuation
+        if (PsiTreeUtil.isAncestor(pendingScope, node, true)) {
+          myBuilder.addPendingEdge(pendingScope, myBuilder.prevInstruction);
+        }
       }
     }
-    else {
-      exitInstruction = addTransparentInstruction();    
-      myBuilder.prevInstruction = exitInstruction;
-    }
 
-    // Connect normal exits from try and else parts to the finally part or exit instruction
-    for (Pair<PsiElement, Instruction> pendingScopeAndInstruction : pendingNormalExits) {
-      final PsiElement pendingScope = pendingScopeAndInstruction.first;
-      final Instruction instruction = pendingScopeAndInstruction.second;
-
-      myBuilder.addEdge(instruction, exitInstruction);
-
-      // When instruction continues outside try-except statement scope
-      // the last instruction in finally-block is marked as pointing to that continuation
-      if (PsiTreeUtil.isAncestor(pendingScope, node, true)) {
-        myBuilder.addPendingEdge(pendingScope, myBuilder.prevInstruction);
-      }
-    }
+    collectInternalPendingEdges(node);
   }
 
   @Override
@@ -934,6 +940,8 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor {
         myBuilder.addEdge(myBuilder.prevInstruction, i);
       }
     }
+    
+    collectInternalPendingEdges(node);
   }
 
   @Override
@@ -1104,4 +1112,27 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor {
     myBuilder.instructions.add(instruction);
     return instruction;
   }
+
+  /**
+   * Can be used to collect all pending edges  
+   * that we used to build CFG for `node`,
+   * but are not relevant to other elements.
+   * Is almost equivalent to this:
+   * 
+   * <pre>{@code
+   * visitPy...(node);
+   * myBuilder.startNode(node.nextSibling); // collectInternalPendingEdges does this, without needing nextSibling
+   * }</pre>
+   */
+  private void collectInternalPendingEdges(@NotNull PyElement node) {
+    myBuilder.addNode(new TransparentInstructionImpl(myBuilder, null, "")); // exit
+    myBuilder.processPending((pendingScope, instruction) -> {
+      if (pendingScope != null && PsiTreeUtil.isAncestor(node, pendingScope, false)) {
+        myBuilder.addEdge(instruction, myBuilder.prevInstruction); // to exit
+      }
+      else {
+        myBuilder.addPendingEdge(pendingScope, instruction);
+      }
+    });
+  }
 }

python/testData/codeInsight/controlflow/IfFor.py

@@ -0,0 +1,6 @@
+if True:
+    for _ in range(1):
+        print()
+else:
+    raise Exception()
+return True

python/testData/codeInsight/controlflow/IfFor.txt

@@ -0,0 +1,20 @@
+0(1) element: null
+1(2) element: PyIfStatement
+2(3,4) READ ACCESS: True
+3() element: null. Condition: True:false
+4(5) element: null. Condition: True:true
+5(6) ASSERTTYPE ACCESS: True
+6(7) element: PyStatementList
+7(8) element: PyForStatement
+8(9) READ ACCESS: range
+9(10,17) element: PyCallExpression: range
+10(11) element: PyTargetExpression: _
+11(12) WRITE ACCESS: _
+12(10,17) element: PyPrintStatement
+13(14) element: PyStatementList
+14(15) raise: PyRaiseStatement
+15(16) READ ACCESS: Exception
+16(19) element: PyCallExpression: Exception
+17(18) element: PyReturnStatement
+18(19) READ ACCESS: True
+19() element: null

python/testData/codeInsight/controlflow/ReturnComprehensionFromExcept.py

@@ -0,0 +1,4 @@
+try:
+    x = f.x
+except AttributeError:
+    return [abs(g) for g in f]

python/testData/codeInsight/controlflow/ReturnComprehensionFromExcept.txt

@@ -0,0 +1,19 @@
+0(1) element: null
+1(2) element: PyTryExceptStatement
+2(3,6) element: PyTryPart
+3(4,6) element: PyAssignmentStatement
+4(5,6) READ ACCESS: f
+5(6,18) WRITE ACCESS: x
+6(7) element: PyExceptPart
+7(8) READ ACCESS: AttributeError
+8(9) element: PyReturnStatement
+9(10) element: PyListCompExpression
+10(11) element: PyReferenceExpression: f
+11(12,18) READ ACCESS: f
+12(13) element: PyTargetExpression: g
+13(14) WRITE ACCESS: g
+14(15) element: PyCallExpression: abs
+15(16) READ ACCESS: abs
+16(17) READ ACCESS: g
+17(12,18) element: PyCallExpression: abs
+18() element: null

python/testData/codeInsight/controlflow/TryTry.txt

@@ -55,7 +55,7 @@
 54(60) finally fail exit
 55(56,60) element: PyFinallyPart
 56(57,60) element: PyAssignmentStatement
-57(58,60,64) WRITE ACCESS: f
+57(60,64,58) WRITE ACCESS: f
 58(59,60) element: PyAssignmentStatement
 59(60,64) WRITE ACCESS: g
 60(61,71) element: PyFinallyPart
@@ -64,9 +64,9 @@
 63(71) finally fail exit
 64(65,71) element: PyFinallyPart
 65(66,71) element: PyAssignmentStatement
-66(67,69,71) WRITE ACCESS: h
+66(71,67,69) WRITE ACCESS: h
 67(68,71) element: PyAssignmentStatement
-68(8,69,71) WRITE ACCESS: i
+68(8,71,69) WRITE ACCESS: i
 69(70,71) element: PyAssignmentStatement
 70(71,75) WRITE ACCESS: j
 71(72) element: PyFinallyPart

python/testData/codeInsight/controlflow/WhileInsideIfTrue.py

@@ -0,0 +1,5 @@
+if True:
+    while expr:
+        break
+else:
+    print("unreachable")

python/testData/codeInsight/controlflow/WhileInsideIfTrue.txt

@@ -0,0 +1,16 @@
+0(1) element: null
+1(2) element: PyIfStatement
+2(3,4) READ ACCESS: True
+3() element: null. Condition: True:false
+4(5) element: null. Condition: True:true
+5(6) ASSERTTYPE ACCESS: True
+6(7) element: PyStatementList
+7(8) element: PyWhileStatement
+8(9,10) READ ACCESS: expr
+9(15) element: null. Condition: expr:false
+10(11) element: null. Condition: expr:true
+11(12) element: PyStatementList
+12(15) element: PyBreakStatement
+13(14) element: PyStatementList
+14(15) element: PyPrintStatement
+15() element: null

python/testData/codeInsight/controlflow/WhileTrueBreakInsideExcept.py

@@ -0,0 +1,6 @@
+while True:
+    try:
+        foo = could_raise()
+    except IndexError:
+        break
+    print(foo)

python/testData/codeInsight/controlflow/WhileTrueBreakInsideExcept.txt

@@ -0,0 +1,18 @@
+0(1) element: null
+1(2) element: PyWhileStatement
+2(3,4) READ ACCESS: True
+3() element: null. Condition: True:false
+4(5) element: null. Condition: True:true
+5(6) element: PyStatementList
+6(7) element: PyTryExceptStatement
+7(8,12) element: PyTryPart
+8(9,12) element: PyAssignmentStatement
+9(10,12) READ ACCESS: could_raise
+10(11,12) element: PyCallExpression: could_raise
+11(12,15) WRITE ACCESS: foo
+12(13) element: PyExceptPart
+13(14) READ ACCESS: IndexError
+14(17) element: PyBreakStatement
+15(16) element: PyPrintStatement
+16(1) READ ACCESS: foo
+17() element: null

python/testSrc/com/jetbrains/python/Py3TypeTest.java

@@ -3466,7 +3466,7 @@ public class Py3TypeTest extends PyTestCase {
   }
 
   public void testNonShadowingReturnInsideFinally() {
-    doTest("int | str", """
+    doTest("str | int", """
       def f(p):
           try:
               return 42

python/testSrc/com/jetbrains/python/PyControlFlowBuilderTest.java

@@ -117,10 +117,20 @@ public class PyControlFlowBuilderTest extends LightMarkedTestCase {
   public void testForIf() {
     doTest();
   }
+  
+  // PY-80824
+  public void testIfFor() {
+    doTest();
+  }
 
   public void testForReturn() {
     doTest();
   }
+  
+  // PY-80564
+  public void testReturnComprehensionFromExcept() {
+    doTest();
+  }
 
   public void testForTryContinue() {
     doTest();
@@ -583,6 +593,16 @@ public class PyControlFlowBuilderTest extends LightMarkedTestCase {
     doTest();
   }
 
+  // PY-80471
+  public void testWhileInsideIfTrue() {
+    doTest();
+  }
+
+  // PY-80733
+  public void testWhileTrueBreakInsideExcept() {
+    doTest();
+  }
+
   private void doTestFirstStatement() {
     final String testName = getTestName(false);
     configureByFile(testName + ".py");

python/testSrc/com/jetbrains/python/inspections/PyUnboundLocalVariableInspectionTest.java

@@ -446,6 +446,18 @@ public class PyUnboundLocalVariableInspectionTest extends PyInspectionTestCase {
     });
   }
 
+  // PY-80733
+  public void testTryExceptDoesNotRedirectBreak() {
+    doTestByText("""
+      while True:
+          try:
+              foo = could_raise()
+          except IndexError:
+              break
+      
+          print(foo)""");
+  }
+
   @NotNull
   @Override
   protected Class<? extends PyInspection> getInspectionClass() {

python/testSrc/com/jetbrains/python/inspections/PyUnreachableCodeInspectionTest.java

@@ -35,6 +35,28 @@ def foo(param: int) -> int:
             return 41
                    """);
   }
+  
+  // PY-80471
+  public void testIfTrueForLoop() {
+    doTestByText("""
+if True:
+    for i in []:
+        pass
+else:
+    <warning descr="This code is unreachable">print("unreachable")</warning>
+                   """);
+  }
+
+  // PY-80471
+  public void testIfTrueWhileLoop() {
+    doTestByText("""
+if True:
+    while expr:
+        break
+else:
+    <warning descr="This code is unreachable">print("unreachable")</warning>
+                   """);
+  }
 
   // PY-51564
   public void testWithNotContext() {