Improved consistency of unreachable code. Previously, unreachable cod…

…e was not supported for `if` or `else` suites when the condition type was narrowed to `Never`. This addresses microsoft/pylance-release#6028.
microsoft · Jun 20, 2024 · 5a90bbd · 5a90bbd
1 parent 98d68d5
commit 5a90bbd
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 18 deletions.
diff --git a/packages/pyright-internal/src/analyzer/codeFlowEngine.ts b/packages/pyright-internal/src/analyzer/codeFlowEngine.ts
@@ -513,11 +513,13 @@ export function getCodeFlowEngine(
                                     }
                                 }
 
-                                if (flowTypeResult && !isFlowNodeReachable(flowNode)) {
-                                    flowTypeResult = undefined;
+                                if (!flowTypeResult || isFlowNodeReachable(curFlowNode)) {
+                                    return setCacheEntry(
+                                        curFlowNode,
+                                        flowTypeResult?.type,
+                                        !!flowTypeResult?.isIncomplete
+                                    );
                                 }
-
-                                return setCacheEntry(curFlowNode, flowTypeResult?.type, !!flowTypeResult?.isIncomplete);
                             }
 
                             // Is this a simple assignment to an index expression? If so, it could
@@ -1226,24 +1228,26 @@ export function getCodeFlowEngine(
                     curFlowNode.flags &
                     (FlowFlags.VariableAnnotation |
                         FlowFlags.Assignment |
-                        FlowFlags.TrueCondition |
-                        FlowFlags.FalseCondition |
                         FlowFlags.WildcardImport |
                         FlowFlags.NarrowForPattern |
                         FlowFlags.ExhaustedMatch)
                 ) {
                     const typedFlowNode = curFlowNode as
                         | FlowVariableAnnotation
                         | FlowAssignment
-                        | FlowCondition
                         | FlowWildcardImport
-                        | FlowCondition
                         | FlowExhaustedMatch;
                     curFlowNode = typedFlowNode.antecedent;
                     continue;
                 }
 
-                if (curFlowNode.flags & (FlowFlags.TrueNeverCondition | FlowFlags.FalseNeverCondition)) {
+                if (
+                    curFlowNode.flags &
+                    (FlowFlags.TrueCondition |
+                        FlowFlags.FalseCondition |
+                        FlowFlags.TrueNeverCondition |
+                        FlowFlags.FalseNeverCondition)
+                ) {
                     const conditionalFlowNode = curFlowNode as FlowCondition;
                     if (conditionalFlowNode.reference) {
                         // Make sure the reference type has a declared type. If not,

diff --git a/packages/pyright-internal/src/tests/samples/constrainedTypeVar13.py b/packages/pyright-internal/src/tests/samples/constrainedTypeVar13.py
@@ -49,21 +49,21 @@ def meth1(
             # This should generate an error.
             return [0]
 
-        if cond:
+        if cond or 3 > 2:
             if isinstance(val1, str):
                 # This should generate an error.
                 return [0]
             else:
                 return [0]
 
-        if cond:
+        if cond or 3 > 2:
             if isinstance(val3, B):
                 return [B()]
             else:
                 # This should generate an error.
                 return [C()]
 
-        if cond:
+        if cond or 3 > 2:
             if not isinstance(val3, B) and not isinstance(val3, C):
                 return [A()]
 

diff --git a/packages/pyright-internal/src/tests/samples/tryExcept4.py b/packages/pyright-internal/src/tests/samples/tryExcept4.py
@@ -1,7 +1,10 @@
 # This sample validates that the exception type provided
 # within a raise statement is valid.
 
-a: bool = True
+from random import random
+
+
+a: bool = True if random() > 0.5 else False
 
 
 class CustomException1(BaseException):
@@ -11,10 +14,10 @@ def __init__(self, code: int):
 
 # This should generate an error because CustomException1
 # requires an argument to instantiate.
-if a:
+if a or 2 > 1:
     raise CustomException1
 
-if a:
+if a or 2 > 1:
     raise CustomException1(3)
 
 
@@ -24,5 +27,5 @@ class CustomException2:
 
 # This should generate an error because
 # the exception doesn't derive from BaseException.
-if a:
+if a or 2 > 1:
     raise CustomException2
diff --git a/packages/pyright-internal/src/tests/samples/unreachable1.py b/packages/pyright-internal/src/tests/samples/unreachable1.py
@@ -1,8 +1,8 @@
 # This sample tests the detection and reporting of unreachable code.
 
-from abc import abstractmethod
 import os
 import sys
+from abc import abstractmethod
 from typing import NoReturn
 
 
@@ -109,3 +109,19 @@ def func10():
     return
     # This should be marked unreachable.
     b = e.errno
+
+
+def func11(obj: str) -> list:
+    if isinstance(obj, str):
+        return []
+    else:
+        # This should be marked as unreachable.
+        return obj
+
+
+def func12(obj: str) -> list:
+    if isinstance(obj, str):
+        return []
+
+    # This should be marked as unreachable.
+    return obj
diff --git a/packages/pyright-internal/src/tests/typeEvaluator1.test.ts b/packages/pyright-internal/src/tests/typeEvaluator1.test.ts
@@ -26,7 +26,7 @@ import * as TestUtils from './testUtils';
 test('Unreachable1', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unreachable1.py']);
 
-    TestUtils.validateResults(analysisResults, 0, 0, 2, 1, 4);
+    TestUtils.validateResults(analysisResults, 0, 0, 2, 1, 6);
 });
 
 test('Builtins1', () => {