PY-7758, PY-23859 improvements in Control Flow analysis

Control flow now abrupts on `exit()` and `pytest.fail()` calls Control flow now abrupts only if class which contains `self.fail()` call contains case-insensitive "test" word in the name Merge-request: IJ-MR-96165 Merged-by: Daniil Kalinin <Daniil.Kalinin@jetbrains.com> GitOrigin-RevId: ea173fdb72a10a373cd95f266ea7589e36545f30
2026-09-27 10:03:11 +07:00 · 2022-11-01 09:54:20 +00:00
parent 78efbf8857
commit c245993809
18 changed files with 208 additions and 16 deletions
@@ -39,16 +39,15 @@ import one.util.streamex.StreamEx;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;

-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-import java.util.Set;
+import java.util.*;

 public class PyControlFlowBuilder extends PyRecursiveElementVisitor {

  @NotNull
  private static final Set<String> EXCEPTION_SUPPRESSORS = ImmutableSet.of("suppress", "assertRaises", "assertRaisesRegex");

+  private static final Set<String> KNOWN_NORETURNS = ImmutableSet.of("sys.exit", "exit", "pytest.fail");
+
  private final ControlFlowBuilder myBuilder = new ControlFlowBuilder();

  public ControlFlow buildControlFlow(@NotNull final ScopeOwner owner) {
@@ -136,9 +135,7 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor {
  public void visitPyCallExpression(final @NotNull PyCallExpression node) {
    final PyExpression callee = node.getCallee();
    // Flow abrupted
-    final String repr = PyUtil.getReadableRepr(callee, true);
-    if (callee != null && ("sys.exit".equals(repr) ||
-                           "self.fail".equals(repr))) {
+    if (callee != null && assumeDeadEnd(callee)) {
      callee.accept(this);
      for (PyExpression expression : node.getArguments()) {
        expression.accept(this);
@@ -997,6 +994,28 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor {
    if (target != null) target.accept(this);
  }

+  private static boolean assumeDeadEnd(final @NotNull PyExpression callee) {
+    String repr = PyUtil.getReadableRepr(callee, true);
+    if (KNOWN_NORETURNS.contains(repr)) {
+      return true;
+    }
+    /* Since we can't fully resolve the call during the building of the control flow graph,
+     * here we make an assumption that the class which contains self.fail() call is the real
+     * test class and self.fail() is actually unittest.TestCase.fail() call which leads to flow abruption (see PY-23859).
+     * This approach does not completely eliminate false positives, but it helps to reduce their number. */
+    if (repr.equals("self.fail")) {
+      PyClass clazz = PsiTreeUtil.getParentOfType(callee, PyClass.class);
+      if (clazz != null && clazz.getName() != null) {
+        String className = clazz.getName();
+        boolean classNameContainsTest = className.contains("Test");
+        if (classNameContainsTest) {
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
  private void abruptFlow(final PsiElement node) {
    // Here we process pending instructions!!!
    myBuilder.processPending((pendingScope, instruction) -> {
@@ -0,0 +1,7 @@
+try:
+    n = int(sys.argv[1])
+except ValueError:
+    print("both arguments should be numbers")
+    exit()
+
+print("Please, input " + str(n) + " file names")
@@ -0,0 +1,17 @@
+0(1) element: null
+1(2) element: PyTryExceptStatement
+2(3,8) element: PyTryPart
+3(4,8) element: PyAssignmentStatement
+4(5,8) READ ACCESS: int
+5(6,8) element: PySubscriptionExpression
+6(7,8) READ ACCESS: sys
+7(8,13) WRITE ACCESS: n
+8(9) element: PyExceptPart
+9(10) READ ACCESS: ValueError
+10(11) element: PyPrintStatement
+11(12) element: PyExpressionStatement
+12(16) READ ACCESS: exit
+13(14) element: PyPrintStatement
+14(15) READ ACCESS: str
+15(16) READ ACCESS: n
+16() element: null
@@ -0,0 +1,6 @@
+def test_fail():
+    if True == False:
+        pytest.fail()
+        print("should be reported as unreachable")
+    else:
+        return 1
@@ -0,0 +1,11 @@
+0(1) element: null
+1(2) element: PyIfStatement
+2(3) READ ACCESS: True
+3(4,8) READ ACCESS: False
+4(5) element: PyStatementList. Condition: True == False:true
+5(6) element: PyExpressionStatement
+6(10) READ ACCESS: pytest
+7(10) element: PyPrintStatement
+8(9) element: PyStatementList. Condition: True == False:false
+9(10) element: PyReturnStatement
+10() element: null
@@ -0,0 +1,10 @@
+import unittest
+
+class TestStringMethods(unittest.TestCase):
+
+    def test_is_ok(self):
+        str = get_response()
+        if isinstance(str, int):
+            self.fail()
+            print("Not a string!")
+        self.assert_(str, "OK")
@@ -0,0 +1,19 @@
+0(1) element: null
+1(2) WRITE ACCESS: self
+2(3) element: PyAssignmentStatement
+3(4) READ ACCESS: get_response
+4(5) WRITE ACCESS: str
+5(6) element: PyIfStatement
+6(7) READ ACCESS: isinstance
+7(8) READ ACCESS: str
+8(9,14) READ ACCESS: int
+9(10) element: PyStatementList. Condition: isinstance(str, int):true
+10(11) ASSERTTYPE ACCESS: str
+11(12) element: PyExpressionStatement
+12(18) READ ACCESS: self
+13(15) element: PyPrintStatement
+14(15) ASSERTTYPE ACCESS: str
+15(16) element: PyExpressionStatement
+16(17) READ ACCESS: self
+17(18) READ ACCESS: str
+18() element: null
@@ -0,0 +1,7 @@
+try:
+    n = int(sys.argv[1])
+except ValueError:
+    print("both arguments should be numbers")
+    sys.exit()
+
+print("Please, input " + str(n) + " file names")
@@ -0,0 +1,17 @@
+0(1) element: null
+1(2) element: PyTryExceptStatement
+2(3,8) element: PyTryPart
+3(4,8) element: PyAssignmentStatement
+4(5,8) READ ACCESS: int
+5(6,8) element: PySubscriptionExpression
+6(7,8) READ ACCESS: sys
+7(8,13) WRITE ACCESS: n
+8(9) element: PyExceptPart
+9(10) READ ACCESS: ValueError
+10(11) element: PyPrintStatement
+11(12) element: PyExpressionStatement
+12(16) READ ACCESS: sys
+13(14) element: PyPrintStatement
+14(15) READ ACCESS: str
+15(16) READ ACCESS: n
+16() element: null
@@ -0,0 +1,7 @@
+try:
+  n = int(sys.argv[1])
+except:
+  print("both arguments should be numbers")
+  exit()
+
+print("Please, input " + str(n) + " file names")
@@ -0,0 +1,8 @@
+# PY-23859
+
+from unittest import TestCase
+
+class C(TestCase):
+    def test_1(self):
+        self.fail()
+        return -42
@@ -68,15 +68,6 @@ def f():
    return f, foo


-# PY-3886
-def f():
-    from unittest import TestCase
-    class C(TestCase):
-        def test_1(self):
-            self.fail()
-            <warning descr="This code is unreachable">return -42</warning>
-
-
 # PY-4149
 def f():
    try:
@@ -0,0 +1,4 @@
+def test_exit():
+    exit()
+    <warning descr="This code is unreachable">print("should be reported as unreachable")</warning>
+    return True
@@ -0,0 +1,8 @@
+import pytest
+
+def test_fail():
+    if True == False:
+        pytest.fail()
+        <warning descr="This code is unreachable">print("should be reported as unreachable")</warning>
+    else:
+        return 1
@@ -0,0 +1,13 @@
+# PY-23859, PY-3886
+
+from unittest import TestCase
+
+class TestSomething(TestCase):
+    def test_1(self):
+        self.fail()
+        <warning descr="This code is unreachable">return -42</warning>
+
+class SomethingTest(TestCase):
+    def test_1(self):
+        self.fail()
+        <warning descr="This code is unreachable">return -42</warning>
@@ -478,6 +478,30 @@ public class PyControlFlowBuilderTest extends LightMarkedTestCase {
    doTest();
  }

+  // PY-7758
+  public void testControlFlowAbruptedOnExit() {
+    doTest();
+  }
+
+  // PY-7758
+  public void testControlFlowAbruptedOnSysExit() {
+    doTest();
+  }
+
+  // PY-23859
+  public void testControlFlowAbruptedOnRealSelfFailAssumedByClassName() {
+    final String testName = getTestName(false);
+    configureByFile(testName + ".py");
+    final String fullPath = getTestDataPath() + testName + ".txt";
+    final PyClass pyClass = ((PyFile)myFile).getTopLevelClasses().get(0);
+    final ControlFlow flow = ControlFlowCache.getControlFlow(pyClass.getMethods()[0]);
+    check(fullPath, flow);
+  }
+
+  public void testControlFlowAbruptedOnPytestFail() {
+    doTestFirstStatement();
+  }
+
  private void doTestFirstStatement() {
    final String testName = getTestName(false);
    configureByFile(testName + ".py");
@@ -382,6 +382,11 @@ public class PyUnboundLocalVariableInspectionTest extends PyInspectionTestCase {
    doTest();
  }

+  // PY-7758
+  public void testVariableNotReportedAfterBuiltinExit() {
+    doTest();
+  }
+
  @NotNull
  @Override
  protected Class<? extends PyInspection> getInspectionClass() {
@@ -216,6 +216,25 @@ public class PyUnreachableCodeInspectionTest extends PyInspectionTestCase {
    doTest();
  }

+  // PY-7758
+  public void testUnreachableCodeReportedAfterBuiltinExit() {
+    doTest();
+  }
+
+  // PY-23859
+  public void testUnreachableCodeReportedAfterSelfFailInClassContainingTestInName() {
+    doTest();
+  }
+
+  // PY-23859
+  public void testCodeNotReportedAsUnreachableAfterSelfFailInClassNotContainingTestInName() {
+    doTest();
+  }
+
+  public void testUnreachableCodeReportedAfterPytestFail() {
+    doTest();
+  }
+
  @NotNull
  @Override
  protected Class<? extends PyInspection> getInspectionClass() {