Allow TYPE_CHECKING guards in match statements

packages/pyright-internal/src/analyzer/binder.ts

@@ -1375,36 +1375,16 @@ 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.d.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.
-            const constExprValue = StaticExpressions.evaluateStaticBoolLikeExpression(
-                node.d.testExpr,
-                this._fileInfo.executionEnvironment,
-                this._fileInfo.definedConstants,
-                this._typingImportAliases,
-                this._sysImportAliases
-            );
+            const constExprValue = this._evaluateStaticBoolExpr(node.d.testExpr);
 
             this._bindConditional(node.d.testExpr, thenLabel, elseLabel);
 
             // Handle the if clause.
             if (constExprValue === false) {
-                this._currentFlowNode =
-                    node.d.testExpr.nodeType === ParseNodeType.UnaryOperation &&
-                    node.d.testExpr.d.operator === OperatorType.Not &&
-                    isTypeCheckingNode(node.d.testExpr.d.expr) &&
-                    constExprValue === false
-                        ? notTypeCheckingNode
-                        : Binder._unreachableFlowNode;
+                this._currentFlowNode = this._isNotTypeCheckingExpr(node.d.testExpr)
+                    ? this._createNotTypeCheckingFlowNode()
+                    : Binder._unreachableFlowNode;
             } else {
                 this._currentFlowNode = this._finishFlowLabel(thenLabel);
             }
@@ -1415,8 +1395,8 @@ export class Binder extends ParseTreeWalker {
             // are chained "else if" statements, they'll be handled
             // recursively here.
             if (constExprValue === true) {
-                this._currentFlowNode = isTypeCheckingNode(node.d.testExpr)
-                    ? notTypeCheckingNode
+                this._currentFlowNode = this._isTypeCheckingExpr(node.d.testExpr)
+                    ? this._createNotTypeCheckingFlowNode()
                     : Binder._unreachableFlowNode;
             } else {
                 this._currentFlowNode = this._finishFlowLabel(elseLabel);
@@ -1438,15 +1418,7 @@ export class Binder extends ParseTreeWalker {
         const elseLabel = this._createBranchLabel();
         const postWhileLabel = this._createBranchLabel();
 
-        // Determine if the test condition is always true or always false. If so,
-        // we can treat either the while or the else clause as unconditional.
-        const constExprValue = StaticExpressions.evaluateStaticBoolLikeExpression(
-            node.d.testExpr,
-            this._fileInfo.executionEnvironment,
-            this._fileInfo.definedConstants,
-            this._typingImportAliases,
-            this._sysImportAliases
-        );
+        const constExprValue = this._evaluateStaticBoolExpr(node.d.testExpr);
 
         const preLoopLabel = this._createLoopLabel();
         this._addAntecedent(preLoopLabel, this._currentFlowNode!);
@@ -2389,6 +2361,17 @@ export class Binder extends ParseTreeWalker {
 
             this._currentFlowNode = this._finishFlowLabel(preSuiteLabel);
 
+            // If the guard is always false, mark the case body as unreachable.
+            // If the guard is `not TYPE_CHECKING`, suppress unreachable diagnostics.
+            if (caseStatement.d.guardExpr) {
+                const constExprValue = this._evaluateStaticBoolExpr(caseStatement.d.guardExpr);
+                if (constExprValue === false) {
+                    this._currentFlowNode = this._isNotTypeCheckingExpr(caseStatement.d.guardExpr)
+                        ? this._createNotTypeCheckingFlowNode()
+                        : Binder._unreachableFlowNode;
+                }
+            }
+
             // Bind the body of the case statement.
             this.walk(caseStatement.d.suite);
             this._addAntecedent(postMatchLabel, this._currentFlowNode);
@@ -3034,13 +3017,7 @@ export class Binder extends ParseTreeWalker {
         if (antecedent.flags & FlowFlags.Unreachable) {
             return antecedent;
         }
-        const staticValue = StaticExpressions.evaluateStaticBoolLikeExpression(
-            expression,
-            this._fileInfo.executionEnvironment,
-            this._fileInfo.definedConstants,
-            this._typingImportAliases,
-            this._sysImportAliases
-        );
+        const staticValue = this._evaluateStaticBoolExpr(expression);
         if (
             (staticValue === true && flags & FlowFlags.FalseCondition) ||
             (staticValue === false && flags & FlowFlags.TrueCondition)
@@ -4346,6 +4323,38 @@ export class Binder extends ParseTreeWalker {
         return getUniqueFlowNodeId();
     }
 
+    /**
+     * 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. */
+    private _evaluateStaticBoolExpr(expr: ExpressionNode): boolean | undefined {
+        return StaticExpressions.evaluateStaticBoolLikeExpression(
+            expr,
+            this._fileInfo.executionEnvironment,
+            this._fileInfo.definedConstants,
+            this._typingImportAliases,
+            this._sysImportAliases
+        );
+    }
+
+    private _isTypeCheckingExpr(expr: ExpressionNode): boolean {
+        return expr.nodeType === ParseNodeType.Name && expr.d.value === 'TYPE_CHECKING';
+    }
+
+    private _isNotTypeCheckingExpr(expr: ExpressionNode): boolean {
+        return (
+            expr.nodeType === ParseNodeType.UnaryOperation &&
+            expr.d.operator === OperatorType.Not &&
+            this._isTypeCheckingExpr(expr.d.expr)
+        );
+    }
+
+    private _createNotTypeCheckingFlowNode(): FlowNode {
+        return {
+            flags: FlowFlags.NotTypeChecking | FlowFlags.Unreachable,
+            id: this._getUniqueFlowNodeId(),
+        };
+    }
+
     private _addDiagnostic(rule: DiagnosticRule, message: string, textRange: TextRange) {
         const diagLevel = this._fileInfo.diagnosticRuleSet[rule] as DiagnosticLevel;
 

packages/pyright-internal/src/analyzer/typeEvaluator.ts

@@ -20600,9 +20600,10 @@ export function createTypeEvaluator(
         for (const caseStatement of node.parent.d.cases) {
             if (caseStatement === node) {
                 if (fileInfo.diagnosticRuleSet.reportUnnecessaryComparison !== 'none') {
-                    if (!subjectTypeResult.isIncomplete) {
-                        checkForUnusedPattern(evaluatorInterface, node.d.pattern, subjectType, node.parent.d.expr);
-                    }
+                    const suiteInNotTypeChecking =
+                        node.d.suite.d.statements.length > 0 && isNotTypeCheckingBlock(node.d.suite.d.statements[0]);
+                    if (subjectTypeResult.isIncomplete || suiteInNotTypeChecking) continue;
+                    checkForUnusedPattern(evaluatorInterface, node.d.pattern, subjectType, node.parent.d.expr);
                 }
                 break;
             }

packages/pyright-internal/src/tests/samples/unreachable1.py

@@ -125,3 +125,12 @@ def func12(obj: str) -> list:
 
     # This should be marked as unreachable.
     return obj
+
+
+def func13(value: int | str) -> None:
+    match value:
+        case int(): ...
+        case str(): ...
+        case _:
+            # This should be marked as unreachable.
+            ...

packages/pyright-internal/src/tests/samples/unreachable2.py

@@ -6,4 +6,10 @@
 if TYPE_CHECKING:
     ...
 else:
-    ...
+    ...
+
+def func(value: int | str) -> None:
+    match value:
+        case int(): ...
+        case str(): ...
+        case _ if not TYPE_CHECKING: ...

packages/pyright-internal/src/tests/typeEvaluator1.test.ts

@@ -27,7 +27,7 @@ import * as TestUtils from './testUtils';
 test('Unreachable1', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unreachable1.py']);
 
-    TestUtils.validateResults(analysisResults, 0, 0, 2, 7);
+    TestUtils.validateResults(analysisResults, 0, 0, 2, 8);
 });
 
 test('Builtins1', () => {

packages/pyright-internal/src/tests/typeEvaluatorBased.test.ts

@@ -10,7 +10,7 @@ test('reportUnreachable', () => {
     configOptions.diagnosticRuleSet.reportUnreachable = 'error';
     const analysisResults = typeAnalyzeSampleFiles(['unreachable1.py'], configOptions);
     validateResultsButBased(analysisResults, {
-        errors: [78, 89, 106, 110, 118, 126].map((line) => ({ code: DiagnosticRule.reportUnreachable, line })),
+        errors: [78, 89, 106, 110, 118, 126, 135].map((line) => ({ code: DiagnosticRule.reportUnreachable, line })),
         infos: [{ line: 95 }, { line: 98 }],
         hints: [{ line: 102 }],
     });
@@ -19,6 +19,7 @@ test('reportUnreachable', () => {
 test('reportUnreachable TYPE_CHECKING', () => {
     const configOptions = new ConfigOptions(Uri.empty());
     configOptions.diagnosticRuleSet.reportUnreachable = 'error';
+    configOptions.diagnosticRuleSet.reportUnnecessaryComparison = 'error';
     const analysisResults = typeAnalyzeSampleFiles(['unreachable2.py'], configOptions);
 
     //TODO: should type checking unreachable blocks still always be reported with the unreachable hint?????
@@ -50,7 +51,7 @@ test('default typeCheckingMode=recommended', () => {
     const analysisResults = typeAnalyzeSampleFiles(['unreachable1.py'], configOptions);
     validateResultsButBased(analysisResults, {
         warnings: [
-            ...[78, 89, 106, 110, 118, 126].map((line) => ({ code: DiagnosticRule.reportUnreachable, line })),
+            ...[78, 89, 106, 110, 118, 126, 135].map((line) => ({ code: DiagnosticRule.reportUnreachable, line })),
             { line: 19, code: DiagnosticRule.reportUnknownParameterType },
             { line: 33, code: DiagnosticRule.reportUnknownParameterType },
             { line: 33, code: DiagnosticRule.reportInvalidAbstractMethod },
@@ -61,6 +62,7 @@ test('default typeCheckingMode=recommended', () => {
             { line: 121, code: DiagnosticRule.reportUnknownParameterType },
             { line: 122, code: DiagnosticRule.reportUnnecessaryIsInstance },
             { line: 123, code: DiagnosticRule.reportUnknownVariableType },
+            { line: 133, code: DiagnosticRule.reportUnnecessaryComparison },
         ],
         errors: [
             { line: 16, code: DiagnosticRule.reportUninitializedInstanceVariable },