don't report unreachable in not TYPE_CHECKING blocks

DetachHead · Jan 22, 2024 · b02003c · b02003c
1 parent 31b4d4a
commit b02003c
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 12 deletions.
diff --git a/packages/pyright-internal/src/analyzer/binder.ts b/packages/pyright-internal/src/analyzer/binder.ts
@@ -1325,6 +1325,14 @@ export class Binder extends ParseTreeWalker {
         const elseLabel = this._createBranchLabel();
         const postIfLabel = this._createBranchLabel(preIfFlowNode);
 
+        const notTypeCheckingNode: FlowNode = {
+            flags: FlowFlags.NotTypeChecking | FlowFlags.Unreachable,
+            id: getUniqueFlowNodeId(),
+        };
+
+        const isTypeCheckingNode = (node: ExpressionNode): node is NameNode =>
+            node.nodeType === ParseNodeType.Name && node.value === 'TYPE_CHECKING';
+
         postIfLabel.affectedExpressions = this._trackCodeFlowExpressions(() => {
             // Determine if the test condition is always true or always false. If so,
             // we can treat either the then or the else clause as unconditional.
@@ -1339,16 +1347,30 @@ export class Binder extends ParseTreeWalker {
             this._bindConditional(node.testExpression, thenLabel, elseLabel);
 
             // Handle the if clause.
-            this._currentFlowNode =
-                constExprValue === false ? Binder._unreachableFlowNode : this._finishFlowLabel(thenLabel);
+            if (constExprValue === false) {
+                this._currentFlowNode =
+                    node.testExpression.nodeType === ParseNodeType.UnaryOperation &&
+                    node.testExpression.operator === OperatorType.Not &&
+                    isTypeCheckingNode(node.testExpression.expression) &&
+                    constExprValue === false
+                        ? notTypeCheckingNode
+                        : Binder._unreachableFlowNode;
+            } else {
+                this._currentFlowNode = this._finishFlowLabel(thenLabel);
+            }
             this.walk(node.ifSuite);
             this._addAntecedent(postIfLabel, this._currentFlowNode);
 
             // Now handle the else clause if it's present. If there
             // are chained "else if" statements, they'll be handled
             // recursively here.
-            this._currentFlowNode =
-                constExprValue === true ? Binder._unreachableFlowNode : this._finishFlowLabel(elseLabel);
+            if (constExprValue === true) {
+                this._currentFlowNode = isTypeCheckingNode(node.testExpression)
+                    ? notTypeCheckingNode
+                    : Binder._unreachableFlowNode;
+            } else {
+                this._currentFlowNode = this._finishFlowLabel(elseLabel);
+            }
             if (node.elseSuite) {
                 this.walk(node.elseSuite);
             } else {

diff --git a/packages/pyright-internal/src/analyzer/codeFlowTypes.ts b/packages/pyright-internal/src/analyzer/codeFlowTypes.ts
@@ -50,6 +50,7 @@ export enum FlowFlags {
     FalseNeverCondition = 1 << 17, // Condition whose type evaluates to never when narrowed in negative test
     NarrowForPattern = 1 << 18, // Narrow the type of the subject expression within a case statement
     ExhaustedMatch = 1 << 19, // Control flow gate that is closed when match is provably exhaustive
+    NotTypeChecking = 1 << 20, // not TYPE_CHECKING block (ie. unreachable, but should not reportUnreachable)
 }
 
 let _nextFlowNodeId = 1;

diff --git a/packages/pyright-internal/src/analyzer/codeFlowUtils.ts b/packages/pyright-internal/src/analyzer/codeFlowUtils.ts
@@ -262,6 +262,7 @@ export function formatControlFlowGraph(flowNode: FlowNode) {
         if (flags & FlowFlags.FalseNeverCondition) return 'FalseNever';
         if (flags & FlowFlags.NarrowForPattern) return 'Pattern';
         if (flags & FlowFlags.ExhaustedMatch) return 'Exhaust';
+        if (flags & FlowFlags.NotTypeChecking) return 'NotTypeChecking';
         throw new Error();
     }
 

diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts
@@ -2997,6 +2997,18 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
         return codeFlowEngine.isFlowNodeReachable(flowNode, sourceFlowNode);
     }
 
+    function isNotTypeCheckingBlock(node: ParseNode): boolean {
+        //TODO: abstract this logic, its used in isNodeReachable as well
+        const flowNode = AnalyzerNodeInfo.getFlowNode(node);
+        if (!flowNode) {
+            if (node.parent) {
+                return isNotTypeCheckingBlock(node.parent);
+            }
+            return false;
+        }
+        return !!(flowNode.flags & FlowFlags.NotTypeChecking);
+    }
+
     function isAfterNodeReachable(node: ParseNode): boolean {
         const returnFlowNode = AnalyzerNodeInfo.getAfterFlowNode(node);
         if (!returnFlowNode) {
@@ -3090,7 +3102,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
         if (!isDiagnosticSuppressedForNode(node)) {
             const fileInfo = AnalyzerNodeInfo.getFileInfo(node);
             const message = LocMessage.unreachableCode();
-            if (fileInfo.diagnosticRuleSet.reportUnreachable === 'none') {
+            if (fileInfo.diagnosticRuleSet.reportUnreachable === 'none' || isNotTypeCheckingBlock(node)) {
                 fileInfo.diagnosticSink.addUnreachableCodeWithTextRange(message, textRange);
             } else {
                 addDiagnostic(DiagnosticRule.reportUnreachable, message, node, textRange);

diff --git a/packages/pyright-internal/src/tests/samples/unreachable2.py b/packages/pyright-internal/src/tests/samples/unreachable2.py
@@ -1,4 +1,9 @@
-from typing import TYPE_CHECKING:
+from typing import TYPE_CHECKING
 
 if not TYPE_CHECKING:
+    ...
+
+if TYPE_CHECKING:
+    ...
+else:
     ...
diff --git a/packages/pyright-internal/src/tests/typeEvaluator1.test.ts b/packages/pyright-internal/src/tests/typeEvaluator1.test.ts
@@ -31,13 +31,13 @@ test('Unreachable1 reportUnreachable', () => {
     TestUtils.validateResults(analysisResults, 4, 0, 2, 1, 0);
 });
 
-// test('Unreachable2 reportUnreachable (TYPE_CHECKING)', () => {
-//     const configOptions = new ConfigOptions(Uri.empty());
-//     configOptions.diagnosticRuleSet.reportUnreachable = 'error';
-//     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unreachable2.py'], configOptions);
+test('Unreachable2 reportUnreachable TYPE_CHECKING', () => {
+    const configOptions = new ConfigOptions(Uri.empty());
+    configOptions.diagnosticRuleSet.reportUnreachable = 'error';
+    const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unreachable2.py'], configOptions);
 
-//     TestUtils.validateResults(analysisResults, 0, 0, 0, 0, 0);
-// });
+    TestUtils.validateResults(analysisResults, 0, 0, 0, 0, 2);
+});
 
 test('Builtins1', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['builtins1.py']);