From 06a9b9bde58ec7b8740e669de5735d492fcd10d5 Mon Sep 17 00:00:00 2001 From: "Aleksandr.Govenko" Date: Wed, 14 May 2025 12:15:56 +0000 Subject: [PATCH] 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 GitOrigin-RevId: f3e5d76e1fb15e2951d395fa27768269e4d0cb8f --- .../controlflow/PyControlFlowBuilder.java | 95 ++++++++++++------- .../testData/codeInsight/controlflow/IfFor.py | 6 ++ .../codeInsight/controlflow/IfFor.txt | 20 ++++ .../ReturnComprehensionFromExcept.py | 4 + .../ReturnComprehensionFromExcept.txt | 19 ++++ .../codeInsight/controlflow/TryTry.txt | 6 +- .../controlflow/WhileInsideIfTrue.py | 5 + .../controlflow/WhileInsideIfTrue.txt | 16 ++++ .../controlflow/WhileTrueBreakInsideExcept.py | 6 ++ .../WhileTrueBreakInsideExcept.txt | 18 ++++ .../com/jetbrains/python/Py3TypeTest.java | 3 +- .../python/PyControlFlowBuilderTest.java | 20 ++++ .../PyUnboundLocalVariableInspectionTest.java | 12 +++ .../PyUnreachableCodeInspectionTest.java | 22 +++++ 14 files changed, 215 insertions(+), 37 deletions(-) create mode 100644 python/testData/codeInsight/controlflow/IfFor.py create mode 100644 python/testData/codeInsight/controlflow/IfFor.txt create mode 100644 python/testData/codeInsight/controlflow/ReturnComprehensionFromExcept.py create mode 100644 python/testData/codeInsight/controlflow/ReturnComprehensionFromExcept.txt create mode 100644 python/testData/codeInsight/controlflow/WhileInsideIfTrue.py create mode 100644 python/testData/codeInsight/controlflow/WhileInsideIfTrue.txt create mode 100644 python/testData/codeInsight/controlflow/WhileTrueBreakInsideExcept.py create mode 100644 python/testData/codeInsight/controlflow/WhileTrueBreakInsideExcept.txt diff --git a/python/python-psi-impl/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowBuilder.java b/python/python-psi-impl/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowBuilder.java index f8be1d52beda..a26c156e9082 100644 --- a/python/python-psi-impl/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowBuilder.java +++ b/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 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 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 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: + * + * 
{@code
+   * visitPy...(node);
+   * myBuilder.startNode(node.nextSibling); // collectInternalPendingEdges does this, without needing nextSibling
+   * }
+ */ + 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); + } + }); + } } diff --git a/python/testData/codeInsight/controlflow/IfFor.py b/python/testData/codeInsight/controlflow/IfFor.py new file mode 100644 index 000000000000..305ba7d5f126 --- /dev/null +++ b/python/testData/codeInsight/controlflow/IfFor.py @@ -0,0 +1,6 @@ +if True: + for _ in range(1): + print() +else: + raise Exception() +return True diff --git a/python/testData/codeInsight/controlflow/IfFor.txt b/python/testData/codeInsight/controlflow/IfFor.txt new file mode 100644 index 000000000000..0837094ec773 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/python/testData/codeInsight/controlflow/ReturnComprehensionFromExcept.py b/python/testData/codeInsight/controlflow/ReturnComprehensionFromExcept.py new file mode 100644 index 000000000000..7be4f3c9bfa0 --- /dev/null +++ b/python/testData/codeInsight/controlflow/ReturnComprehensionFromExcept.py @@ -0,0 +1,4 @@ +try: + x = f.x +except AttributeError: + return [abs(g) for g in f] \ No newline at end of file diff --git a/python/testData/codeInsight/controlflow/ReturnComprehensionFromExcept.txt b/python/testData/codeInsight/controlflow/ReturnComprehensionFromExcept.txt new file mode 100644 index 000000000000..01774ab4f22f --- /dev/null +++ b/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 \ No newline at end of file diff --git a/python/testData/codeInsight/controlflow/TryTry.txt b/python/testData/codeInsight/controlflow/TryTry.txt index 6abebea907f4..15068a6d33b8 100644 --- a/python/testData/codeInsight/controlflow/TryTry.txt +++ b/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 diff --git a/python/testData/codeInsight/controlflow/WhileInsideIfTrue.py b/python/testData/codeInsight/controlflow/WhileInsideIfTrue.py new file mode 100644 index 000000000000..df969d4d82b4 --- /dev/null +++ b/python/testData/codeInsight/controlflow/WhileInsideIfTrue.py @@ -0,0 +1,5 @@ +if True: + while expr: + break +else: + print("unreachable") \ No newline at end of file diff --git a/python/testData/codeInsight/controlflow/WhileInsideIfTrue.txt b/python/testData/codeInsight/controlflow/WhileInsideIfTrue.txt new file mode 100644 index 000000000000..cf90e225ff88 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/python/testData/codeInsight/controlflow/WhileTrueBreakInsideExcept.py b/python/testData/codeInsight/controlflow/WhileTrueBreakInsideExcept.py new file mode 100644 index 000000000000..7b65fad45496 --- /dev/null +++ b/python/testData/codeInsight/controlflow/WhileTrueBreakInsideExcept.py @@ -0,0 +1,6 @@ +while True: + try: + foo = could_raise() + except IndexError: + break + print(foo) \ No newline at end of file diff --git a/python/testData/codeInsight/controlflow/WhileTrueBreakInsideExcept.txt b/python/testData/codeInsight/controlflow/WhileTrueBreakInsideExcept.txt new file mode 100644 index 000000000000..efaf39d95465 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/Py3TypeTest.java b/python/testSrc/com/jetbrains/python/Py3TypeTest.java index becf7407d6d0..0d85cb5ae31c 100644 --- a/python/testSrc/com/jetbrains/python/Py3TypeTest.java +++ b/python/testSrc/com/jetbrains/python/Py3TypeTest.java @@ -9,7 +9,6 @@ import com.jetbrains.python.psi.LanguageLevel; import com.jetbrains.python.psi.PyExpression; import com.jetbrains.python.psi.types.*; import com.jetbrains.python.psi.types.PyTypeChecker.GenericSubstitutions; -import org.intellij.lang.annotations.Language; import org.jetbrains.annotations.NotNull; import java.util.List; @@ -3551,7 +3550,7 @@ public class Py3TypeTest extends PyTestCase { } public void testNonShadowingReturnInsideFinally() { - doTest("int | str", """ + doTest("str | int", """ def f(p): try: return 42 diff --git a/python/testSrc/com/jetbrains/python/PyControlFlowBuilderTest.java b/python/testSrc/com/jetbrains/python/PyControlFlowBuilderTest.java index d90f94de592e..11a595e087e7 100644 --- a/python/testSrc/com/jetbrains/python/PyControlFlowBuilderTest.java +++ b/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"); diff --git a/python/testSrc/com/jetbrains/python/inspections/PyUnboundLocalVariableInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/PyUnboundLocalVariableInspectionTest.java index db553bd8fb65..6dd626341ac5 100644 --- a/python/testSrc/com/jetbrains/python/inspections/PyUnboundLocalVariableInspectionTest.java +++ b/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 getInspectionClass() { diff --git a/python/testSrc/com/jetbrains/python/inspections/PyUnreachableCodeInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/PyUnreachableCodeInspectionTest.java index e2292d22ee20..a4da5dfe1892 100644 --- a/python/testSrc/com/jetbrains/python/inspections/PyUnreachableCodeInspectionTest.java +++ b/python/testSrc/com/jetbrains/python/inspections/PyUnreachableCodeInspectionTest.java @@ -47,6 +47,28 @@ def foo(param: int) -> int: return 41 """); } + + // PY-80471 + public void testIfTrueForLoop() { + doTestByText(""" +if True: + for i in []: + pass +else: + print("unreachable") + """); + } + + // PY-80471 + public void testIfTrueWhileLoop() { + doTestByText(""" +if True: + while expr: + break +else: + print("unreachable") + """); + } // PY-51564 public void testWithNotContext() {