Added reportUnreachable diagnostic check. If enabled, it emits a diagnostic for code that is determined to be structurally unreachable or unreachable via type analysis. It does not report code that is within a code block that is gated by a conditional that is statically determined to be false, such as TYPE_CHECKING and version checks. This diagnostic check is off by default even in strict mode because there are legitimate reasons for unreachable code to exist in Python code. (#10581)

This change also modifies the messages that accompany tagged hints (the mechanism by which unreachable or unanalyzed code is displayed as "dimmed" in an editor). The message now distinguishes between code that is structurally unreachable (e.g. after a `return` or `raise` statement) and code that is not analyzed because it is in a conditional block that is statically evaluated as false.

This addresses #10284.

Author: erictraut

docs/configuration.md

@@ -58,7 +58,7 @@ The following settings determine how different types should be evaluated.
 
 - <a name="deprecateTypingAliases"></a> **deprecateTypingAliases** [boolean]: PEP 585 indicates that aliases to types in standard collections that were introduced solely to support generics are deprecated as of Python 3.9. This switch controls whether these are treated as deprecated. This applies only when pythonVersion is 3.9 or newer. The default value for this setting is `false` but may be switched to `true` in the future.
 
-- <a name="enableReachabilityAnalysis"></a> **enableReachabilityAnalysis** [boolean]: If enabled, code that is determined to be unreachable by type analysis is reported using a tagged hint. This setting does not affect code that is determined to be unreachable regardless of type analysis; such code is always reported as unreachable. This setting also has no effect when using the command-line version of pyright because it never emits tagged hints for unreachable code.
+- <a name="enableReachabilityAnalysis"></a> **enableReachabilityAnalysis** [boolean]: If enabled, code that is determined to be unreachable by type analysis is reported using a tagged hint. This setting does not affect code that is determined to be unreachable independent of type analysis; such code is always reported as unreachable using a tagged hint. This setting also has no effect when using the command-line version of pyright because it never emits tagged hints for unreachable code.
 
 - <a name="enableExperimentalFeatures"></a> **enableExperimentalFeatures** [boolean]: Enables a set of experimental (mostly undocumented) features that correspond to proposed or exploratory changes to the Python typing standard. These features will likely change or be removed, so they should not be used except for experimentation purposes. The default value for this setting is `false`.
 
@@ -233,6 +233,8 @@ The following settings allow more fine grained control over the **typeCheckingMo
 
 - <a name="reportMatchNotExhaustive"></a> **reportMatchNotExhaustive** [boolean or string, optional]: Generate or suppress diagnostics for a `match` statement that does not provide cases that exhaustively match against all potential types of the target expression. The default value for this setting is `"none"`.
 
+- <a name="reportUnreachable"></a> **reportUnreachable** [boolean or string, optional]: Generate or suppress diagnostics for code that is determined to be structurally unreachable or unreachable by type analysis. The default value for this setting is `"none"`.
+
 - <a name="reportImplicitOverride"></a> **reportImplicitOverride** [boolean or string, optional]: Generate or suppress diagnostics for overridden methods in a class that are missing an explicit `@override` decorator. The default value for this setting is `"none"`.
 
 - <a name="reportShadowedImports"></a> **reportShadowedImports** [boolean or string, optional]: Generate or suppress diagnostics for files that are overriding a module in the stdlib. The default value for this setting is `"none"`.
@@ -439,6 +441,7 @@ The following table lists the default severity levels for each diagnostic rule w
 | reportShadowedImports                     | "none"     | "none"     | "none"     | "none"     |
 | reportUninitializedInstanceVariable       | "none"     | "none"     | "none"     | "none"     |
 | reportUnnecessaryTypeIgnoreComment        | "none"     | "none"     | "none"     | "none"     |
+| reportUnreachable                         | "none"     | "none"     | "none"     | "none"     |
 | reportUnusedCallResult                    | "none"     | "none"     | "none"     | "none"     |
 
 ## Overriding settings (in VS Code)

docs/type-concepts-advanced.md

@@ -601,9 +601,9 @@ If one of these conditional expressions evaluates statically to false, pyright d
 
 
 ### Reachability
-Pyright performs “reachability analysis” to determine whether statements will be executed at runtime.
+Pyright performs “reachability analysis” to determine whether statements will be executed at runtime and whether it should analyze and report errors in code.
 
-Reachability analysis is based on both non-type and type information. Non-type information includes statements that unconditionally affect code flow such as `continue`, `raise` and `return`. It also includes conditional statements (`if`, `elif`, or `while`) where the conditional expression is one of these [supported expression forms](type-concepts-advanced#static-conditional-evaluation). Type analysis is not performed on code determined to be unreachable using non-type information. Therefore, language server features like completion suggestions are not available for this code.
+Reachability analysis is based on both non-type and type information. Non-type information includes statements that affect code structure such as `continue`, `raise` and `return`. It also includes conditional statements (`if`, `elif`, or `while`) where the conditional expression is one of these [supported expression forms](type-concepts-advanced#static-conditional-evaluation). Type analysis is not performed on code determined to be unreachable using non-type information. Therefore, language server features like completion suggestions are not available for this code.
 
 Here are some examples of code determined to be unreachable using non-type information.
 

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

@@ -212,7 +212,7 @@ export function isCodeUnreachable(node: ParseNode): boolean {
     while (curNode) {
         const flowNode = getFlowNode(curNode);
         if (flowNode) {
-            return !!(flowNode.flags & FlowFlags.Unreachable);
+            return (flowNode.flags & (FlowFlags.UnreachableStaticCondition | FlowFlags.UnreachableStructural)) !== 0;
         }
         curNode = curNode.parent;
     }

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

@@ -242,8 +242,13 @@ export class Binder extends ParseTreeWalker {
     private _dunderSlotsEntries: StringListNode[] | undefined;
 
     // Flow node that is used for unreachable code.
-    private static _unreachableFlowNode: FlowNode = {
-        flags: FlowFlags.Unreachable,
+    private static _unreachableStaticConditionFlowNode: FlowNode = {
+        flags: FlowFlags.UnreachableStaticCondition,
+        id: getUniqueFlowNodeId(),
+    };
+
+    private static _unreachableStructuralFlowNode: FlowNode = {
+        flags: FlowFlags.UnreachableStructural,
         id: getUniqueFlowNodeId(),
     };
 
@@ -1241,7 +1246,7 @@ export class Binder extends ParseTreeWalker {
         if (this._currentContinueTarget) {
             this._addAntecedent(this._currentContinueTarget, this._currentFlowNode!);
         }
-        this._currentFlowNode = Binder._unreachableFlowNode;
+        this._currentFlowNode = Binder._unreachableStructuralFlowNode;
 
         // Continue nodes don't have any children.
         return false;
@@ -1251,7 +1256,7 @@ export class Binder extends ParseTreeWalker {
         if (this._currentBreakTarget) {
             this._addAntecedent(this._currentBreakTarget, this._currentFlowNode!);
         }
-        this._currentFlowNode = Binder._unreachableFlowNode;
+        this._currentFlowNode = Binder._unreachableStructuralFlowNode;
 
         // Break nodes don't have any children.
         return false;
@@ -1277,7 +1282,7 @@ export class Binder extends ParseTreeWalker {
         this._finallyTargets.forEach((target) => {
             this._addAntecedent(target, this._currentFlowNode!);
         });
-        this._currentFlowNode = Binder._unreachableFlowNode;
+        this._currentFlowNode = Binder._unreachableStructuralFlowNode;
         return false;
     }
 
@@ -1351,15 +1356,17 @@ export class Binder extends ParseTreeWalker {
 
             // Handle the if clause.
             this._currentFlowNode =
-                constExprValue === false ? Binder._unreachableFlowNode : this._finishFlowLabel(thenLabel);
+                constExprValue === false
+                    ? Binder._unreachableStaticConditionFlowNode
+                    : this._finishFlowLabel(thenLabel);
             this.walk(node.d.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);
+                constExprValue === true ? Binder._unreachableStaticConditionFlowNode : this._finishFlowLabel(elseLabel);
             if (node.d.elseSuite) {
                 this.walk(node.d.elseSuite);
             } else {
@@ -1395,14 +1402,14 @@ export class Binder extends ParseTreeWalker {
 
         // Handle the while clause.
         this._currentFlowNode =
-            constExprValue === false ? Binder._unreachableFlowNode : this._finishFlowLabel(thenLabel);
+            constExprValue === false ? Binder._unreachableStaticConditionFlowNode : this._finishFlowLabel(thenLabel);
         this._bindLoopStatement(preLoopLabel, postWhileLabel, () => {
             this.walk(node.d.whileSuite);
         });
         this._addAntecedent(preLoopLabel, this._currentFlowNode);
 
         this._currentFlowNode =
-            constExprValue === true ? Binder._unreachableFlowNode : this._finishFlowLabel(elseLabel);
+            constExprValue === true ? Binder._unreachableStaticConditionFlowNode : this._finishFlowLabel(elseLabel);
         if (node.d.elseSuite) {
             this.walk(node.d.elseSuite);
         }
@@ -1489,7 +1496,7 @@ export class Binder extends ParseTreeWalker {
             this._addAntecedent(target, this._currentFlowNode!);
         });
 
-        this._currentFlowNode = Binder._unreachableFlowNode;
+        this._currentFlowNode = Binder._unreachableStructuralFlowNode;
         return false;
     }
 
@@ -1619,7 +1626,9 @@ export class Binder extends ParseTreeWalker {
                 antecedent: this._currentFlowNode!,
                 preFinallyGate,
             };
-            this._currentFlowNode = isAfterElseAndExceptsReachable ? postFinallyNode : Binder._unreachableFlowNode;
+            this._currentFlowNode = isAfterElseAndExceptsReachable
+                ? postFinallyNode
+                : Binder._unreachableStructuralFlowNode;
         }
 
         return false;
@@ -2868,7 +2877,7 @@ export class Binder extends ParseTreeWalker {
     private _finishFlowLabel(node: FlowLabel) {
         // If there were no antecedents, this is unreachable.
         if (node.antecedents.length === 0) {
-            return Binder._unreachableFlowNode;
+            return Binder._unreachableStructuralFlowNode;
         }
 
         // If there was only one antecedent and this is a simple
@@ -2989,7 +2998,7 @@ export class Binder extends ParseTreeWalker {
     }
 
     private _createFlowConditional(flags: FlowFlags, antecedent: FlowNode, expression: ExpressionNode): FlowNode {
-        if (antecedent.flags & FlowFlags.Unreachable) {
+        if (antecedent.flags & (FlowFlags.UnreachableStructural | FlowFlags.UnreachableStaticCondition)) {
             return antecedent;
         }
         const staticValue = StaticExpressions.evaluateStaticBoolLikeExpression(
@@ -3003,7 +3012,7 @@ export class Binder extends ParseTreeWalker {
             (staticValue === true && flags & FlowFlags.FalseCondition) ||
             (staticValue === false && flags & FlowFlags.TrueCondition)
         ) {
-            return Binder._unreachableFlowNode;
+            return Binder._unreachableStaticConditionFlowNode;
         }
 
         const expressionList: CodeFlowReferenceExpressionNode[] = [];
@@ -3442,7 +3451,10 @@ export class Binder extends ParseTreeWalker {
     }
 
     private _isCodeUnreachable() {
-        return !!(this._currentFlowNode!.flags & FlowFlags.Unreachable);
+        return !!(
+            this._currentFlowNode!.flags &
+            (FlowFlags.UnreachableStaticCondition | FlowFlags.UnreachableStructural)
+        );
     }
 
     private _addExceptTargets(flowNode: FlowNode) {
@@ -3487,7 +3499,9 @@ export class Binder extends ParseTreeWalker {
     }
 
     private _addAntecedent(label: FlowLabel, antecedent: FlowNode) {
-        if (!(this._currentFlowNode!.flags & FlowFlags.Unreachable)) {
+        if (
+            !(this._currentFlowNode!.flags & (FlowFlags.UnreachableStructural | FlowFlags.UnreachableStaticCondition))
+        ) {
             // Don't add the same antecedent twice.
             if (!label.antecedents.some((existing) => existing.id === antecedent.id)) {
                 label.antecedents.push(antecedent);

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

@@ -2846,7 +2846,23 @@ export class Checker extends ParseTreeWalker {
                     const start = statement.start;
                     const lastStatement = statements[statements.length - 1];
                     const end = TextRange.getEnd(lastStatement);
-                    this._evaluator.addUnreachableCode(statement, reachability, { start, length: end - start });
+                    const textRange: TextRange = { start, length: end - start };
+
+                    if (
+                        reachability === Reachability.UnreachableByAnalysis ||
+                        reachability === Reachability.UnreachableStructural
+                    ) {
+                        this._evaluator.addDiagnosticForTextRange(
+                            this._fileInfo,
+                            DiagnosticRule.reportUnreachable,
+                            reachability === Reachability.UnreachableStructural
+                                ? LocMessage.unreachableCodeStructure()
+                                : LocMessage.unreachableCodeType(),
+                            statement.nodeType === ParseNodeType.Error ? statement : statement.d.firstToken
+                        );
+                    }
+
+                    this._evaluator.addUnreachableCode(statement, reachability, textRange);
 
                     reportedUnreachable = true;
                 }
@@ -7682,6 +7698,14 @@ export class Checker extends ParseTreeWalker {
                         LocMessage.unreachableExcept() + diagAddendum.getString(),
                         except.d.typeExpr
                     );
+
+                    this._evaluator.addDiagnostic(
+                        DiagnosticRule.reportUnreachable,
+                        LocMessage.unreachableCodeType(),
+                        except.d.exceptSuite,
+                        except.d.exceptToken
+                    );
+
                     this._evaluator.addUnreachableCode(
                         except,
                         Reachability.UnreachableByAnalysis,

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

@@ -492,7 +492,7 @@ export function getCodeFlowEngine(
                         );
                     }
 
-                    if (curFlowNode.flags & FlowFlags.Unreachable) {
+                    if (curFlowNode.flags & (FlowFlags.UnreachableStaticCondition | FlowFlags.UnreachableStructural)) {
                         // We can get here if there are nodes in a compound logical expression
                         // (e.g. "False and x") that are never executed but are evaluated.
                         return setCacheEntry(curFlowNode, NeverType.createNever(), /* isIncomplete */ false);
@@ -1302,14 +1302,18 @@ export function getCodeFlowEngine(
                 // If we've already visited this node, we can assume
                 // it wasn't reachable.
                 if (visitedFlowNodeSet.has(curFlowNode.id)) {
-                    return cacheReachabilityResult(Reachability.UnreachableAlways);
+                    return cacheReachabilityResult(Reachability.UnreachableStructural);
                 }
 
                 // Note that we've been here before.
                 visitedFlowNodeSet.add(curFlowNode.id);
 
-                if (curFlowNode.flags & FlowFlags.Unreachable) {
-                    return cacheReachabilityResult(Reachability.UnreachableAlways);
+                if (curFlowNode.flags & FlowFlags.UnreachableStructural) {
+                    return cacheReachabilityResult(Reachability.UnreachableStructural);
+                }
+
+                if (curFlowNode.flags & FlowFlags.UnreachableStaticCondition) {
+                    return cacheReachabilityResult(Reachability.UnreachableStaticCondition);
                 }
 
                 if (curFlowNode === sourceFlowNode) {
@@ -1430,16 +1434,23 @@ export function getCodeFlowEngine(
 
                     const labelNode = curFlowNode as FlowLabel;
                     let unreachableByType = false;
+                    let unreachableByStaticCondition = false;
                     for (const antecedent of labelNode.antecedents) {
                         const reachability = getFlowNodeReachabilityRecursive(antecedent, recursionCount);
                         if (reachability === Reachability.Reachable) {
                             return cacheReachabilityResult(reachability);
                         } else if (reachability === Reachability.UnreachableByAnalysis) {
                             unreachableByType = true;
+                        } else if (reachability === Reachability.UnreachableStaticCondition) {
+                            unreachableByStaticCondition = true;
                         }
                     }
                     return cacheReachabilityResult(
-                        unreachableByType ? Reachability.UnreachableByAnalysis : Reachability.UnreachableAlways
+                        unreachableByType
+                            ? Reachability.UnreachableByAnalysis
+                            : unreachableByStaticCondition
+                            ? Reachability.UnreachableStaticCondition
+                            : Reachability.UnreachableStructural
                     );
                 }
 
@@ -1526,7 +1537,10 @@ export function getCodeFlowEngine(
                     return startingConstraints;
                 }
 
-                if (curFlowNode.flags & (FlowFlags.Unreachable | FlowFlags.Start)) {
+                if (
+                    curFlowNode.flags &
+                    (FlowFlags.UnreachableStaticCondition | FlowFlags.UnreachableStructural | FlowFlags.Start)
+                ) {
                     return startingConstraints;
                 }
 

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

@@ -33,14 +33,15 @@ import {
 import { OperatorType } from '../parser/tokenizerTypes';
 
 export enum FlowFlags {
-    Unreachable = 1 << 0, // Unreachable code
-    Start = 1 << 1, // Entry point
-    BranchLabel = 1 << 2, // Junction for forward control flow
-    LoopLabel = 1 << 3, // Junction for backward control flow
-    Assignment = 1 << 4, // Assignment statement
-    Unbind = 1 << 5, // Used with assignment to indicate target should be unbound
-    WildcardImport = 1 << 6, // For "from X import *" statements
-    TrueCondition = 1 << 7, // Condition known to be true
+    UnreachableStructural = 1 << 0, // Code that is structurally unreachable (e.g. following a return statement)
+    UnreachableStaticCondition = 1 << 1, // code that is unreachable due to a condition that the binder evaluates to False
+    Start = 1 << 2, // Entry point
+    BranchLabel = 1 << 3, // Junction for forward control flow
+    LoopLabel = 1 << 4, // Junction for backward control flow
+    Assignment = 1 << 5, // Assignment statement
+    Unbind = 1 << 6, // Used with assignment to indicate target should be unbound
+    WildcardImport = 1 << 7, // For "from X import *" statements
+    TrueCondition = 1 << 8, // Condition known to be true
     FalseCondition = 1 << 9, // Condition known to be false
     Call = 1 << 10, // Call node
     PreFinallyGate = 1 << 11, // Injected edge that links pre-finally label and pre-try flow