Fixed bug that resulted in unbound variable condition not being reported if it was in a loop and was assigned conditionally.

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

@@ -107,8 +107,10 @@ import {
     isNone,
     isOverloadedFunction,
     isParamSpec,
+    isPossiblyUnbound,
     isTypeSame,
     isTypeVar,
+    isUnbound,
     isUnion,
     isUnknown,
     isVariadicTypeVar,
@@ -200,6 +202,7 @@ export class Checker extends ParseTreeWalker {
     private readonly _moduleNode: ModuleNode;
     private readonly _fileInfo: AnalyzerFileInfo;
     private readonly _evaluator: TypeEvaluator;
+    private _isUnboundCheckSuppressed = false;
 
     // A list of all nodes that are defined within the module that
     // have their own scopes.
@@ -1079,23 +1082,38 @@ export class Checker extends ParseTreeWalker {
     }
 
     override visitGlobal(node: GlobalNode): boolean {
-        node.nameList.forEach((name) => {
-            this._evaluator.getType(name);
+        this._suppressUnboundCheck(() => {
+            node.nameList.forEach((name) => {
+                this._evaluator.getType(name);
+
+                this.walk(name);
+            });
         });
-        return true;
+
+        return false;
     }
 
     override visitNonlocal(node: NonlocalNode): boolean {
-        node.nameList.forEach((name) => {
-            this._evaluator.getType(name);
+        this._suppressUnboundCheck(() => {
+            node.nameList.forEach((name) => {
+                this._evaluator.getType(name);
+
+                this.walk(name);
+            });
         });
-        return true;
+
+        return false;
     }
 
     override visitName(node: NameNode) {
         // Determine if we should log information about private usage.
         this._conditionallyReportPrivateUsage(node);
 
+        // Determine if the name is possibly unbound.
+        if (!this._isUnboundCheckSuppressed) {
+            this._reportUnboundName(node);
+        }
+
         // Report the use of a deprecated symbol. For now, this functionality
         // is disabled. We'll leave it in place for the future.
         // this._reportDeprecatedUse(node);
@@ -1104,11 +1122,15 @@ export class Checker extends ParseTreeWalker {
     }
 
     override visitDel(node: DelNode) {
-        node.expressions.forEach((expr) => {
-            this._evaluator.verifyDeleteExpression(expr);
+        this._suppressUnboundCheck(() => {
+            node.expressions.forEach((expr) => {
+                this._evaluator.verifyDeleteExpression(expr);
+
+                this.walk(expr);
+            });
         });
 
-        return true;
+        return false;
     }
 
     override visitMemberAccess(node: MemberAccessNode) {
@@ -1182,6 +1204,17 @@ export class Checker extends ParseTreeWalker {
         return false;
     }
 
+    private _suppressUnboundCheck(callback: () => void) {
+        const wasSuppressed = this._isUnboundCheckSuppressed;
+        this._isUnboundCheckSuppressed = true;
+
+        try {
+            callback();
+        } finally {
+            this._isUnboundCheckSuppressed = wasSuppressed;
+        }
+    }
+
     private _validateIllegalDefaultParamInitializer(node: ParseNode) {
         if (this._fileInfo.diagnosticRuleSet.reportCallInDefaultInitializer !== 'none') {
             if (ParseTreeUtils.isWithinDefaultParamInitializer(node) && !this._fileInfo.isStubFile) {
@@ -2648,6 +2681,34 @@ export class Checker extends ParseTreeWalker {
         }
     }
 
+    private _reportUnboundName(node: NameNode) {
+        if (this._fileInfo.diagnosticRuleSet.reportUnboundVariable === 'none') {
+            return;
+        }
+
+        if (!AnalyzerNodeInfo.isCodeUnreachable(node)) {
+            const type = this._evaluator.getType(node);
+
+            if (type) {
+                if (isUnbound(type)) {
+                    this._evaluator.addDiagnostic(
+                        this._fileInfo.diagnosticRuleSet.reportUnboundVariable,
+                        DiagnosticRule.reportUnboundVariable,
+                        Localizer.Diagnostic.symbolIsUnbound().format({ name: node.value }),
+                        node
+                    );
+                } else if (isPossiblyUnbound(type)) {
+                    this._evaluator.addDiagnostic(
+                        this._fileInfo.diagnosticRuleSet.reportUnboundVariable,
+                        DiagnosticRule.reportUnboundVariable,
+                        Localizer.Diagnostic.symbolIsPossiblyUnbound().format({ name: node.value }),
+                        node
+                    );
+                }
+            }
+        }
+    }
+
     private _conditionallyReportPrivateUsage(node: NameNode) {
         if (this._fileInfo.diagnosticRuleSet.reportPrivateUsage === 'none') {
             return;

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

@@ -183,7 +183,6 @@ import {
     isNone,
     isOverloadedFunction,
     isParamSpec,
-    isPossiblyUnbound,
     isTypeSame,
     isTypeVar,
     isUnbound,
@@ -3025,7 +3024,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
             case ParseNodeType.Name: {
                 // Get the type to evaluate whether it's bound
                 // and to mark it accessed.
-                getTypeOfExpression(node, /* expectedType */ undefined, EvaluatorFlags.SkipUnboundCheck);
+                getTypeOfExpression(node);
                 break;
             }
 
@@ -3035,7 +3034,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
                     node,
                     baseTypeResult,
                     { method: 'del' },
-                    EvaluatorFlags.SkipUnboundCheck
+                    EvaluatorFlags.None
                 );
                 writeTypeCache(node.memberName, memberType.type, /* isIncomplete */ false);
                 writeTypeCache(node, memberType.type, /* isIncomplete */ false);
@@ -3048,12 +3047,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
                     undefined,
                     EvaluatorFlags.DoNotSpecialize
                 );
-                getTypeFromIndexWithBaseType(
-                    node,
-                    baseTypeResult.type,
-                    { method: 'del' },
-                    EvaluatorFlags.SkipUnboundCheck
-                );
+                getTypeFromIndexWithBaseType(node, baseTypeResult.type, { method: 'del' }, EvaluatorFlags.None);
                 writeTypeCache(node, UnboundType.create(), /* isIncomplete */ false);
                 break;
             }
@@ -3070,7 +3064,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
                 // type information is cached for the completion handler.
                 if (node.child) {
                     suppressDiagnostics(node.child, () => {
-                        getTypeOfExpression(node.child!, /* expectedType */ undefined, EvaluatorFlags.SkipUnboundCheck);
+                        getTypeOfExpression(node.child!, /* expectedType */ undefined);
                     });
                 }
                 break;
@@ -3246,31 +3240,6 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
             // Detect, report, and fill in missing type arguments if appropriate.
             type = reportMissingTypeArguments(node, type, flags);
 
-            // If there is a resolution cycle, don't report it as an unbound symbol
-            // at this time. It will be re-evaluated as the call stack unwinds, and
-            // its actual type will be known then. Also, if the node is unreachable
-            // but within a reachable statement (e.g. if False and <name>) then avoid
-            // reporting an unbound error.
-            if (!isIncomplete && !AnalyzerNodeInfo.isCodeUnreachable(node)) {
-                if ((flags & EvaluatorFlags.SkipUnboundCheck) === 0) {
-                    if (isUnbound(type)) {
-                        addDiagnostic(
-                            fileInfo.diagnosticRuleSet.reportUnboundVariable,
-                            DiagnosticRule.reportUnboundVariable,
-                            Localizer.Diagnostic.symbolIsUnbound().format({ name }),
-                            node
-                        );
-                    } else if (isPossiblyUnbound(type)) {
-                        addDiagnostic(
-                            fileInfo.diagnosticRuleSet.reportUnboundVariable,
-                            DiagnosticRule.reportUnboundVariable,
-                            Localizer.Diagnostic.symbolIsPossiblyUnbound().format({ name }),
-                            node
-                        );
-                    }
-                }
-            }
-
             setSymbolAccessed(fileInfo, symbol, node);
 
             if ((flags & EvaluatorFlags.ExpectingTypeAnnotation) !== 0) {
@@ -14291,11 +14260,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
             ) {
                 // For global and nonlocal statements, allow forward references so
                 // we don't use code flow during symbol lookups.
-                getTypeOfExpression(
-                    node,
-                    /* expectedType */ undefined,
-                    EvaluatorFlags.AllowForwardReferences | EvaluatorFlags.SkipUnboundCheck
-                );
+                getTypeOfExpression(node, /* expectedType */ undefined, EvaluatorFlags.AllowForwardReferences);
                 return;
             }
         }

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

@@ -96,9 +96,6 @@ export const enum EvaluatorFlags {
     // the containing function's scope.
     AssociateTypeVarsWithCurrentScope = 1 << 13,
 
-    // Do not emit an error if the symbol is potentially unbound
-    SkipUnboundCheck = 1 << 14,
-
     // Used for PEP 526-style variable type annotations
     VariableTypeAnnotation = 1 << 15,
 

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

@@ -0,0 +1,22 @@
+# This sample tests for the detection of unbound or partially-unbound
+# variables within loops.
+
+import random
+
+for a in [1, 2, 3]:
+    # This should generate an error because b is unbound.
+    if b == 1:
+        b = 2
+
+
+for a in [1, 2, 3]:
+    if random.random() > 0.5:
+        c = 2
+
+    # This should generate an error because c is potentially unbound.
+    print(c)
+
+while True:
+    # This should generate an error because d is unbound.
+    if d == 1:
+        d = 2

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

@@ -186,6 +186,12 @@ test('Loops10', () => {
     TestUtils.validateResults(analysisResults, 0);
 });
 
+test('Loops11', () => {
+    const analysisResults = TestUtils.typeAnalyzeSampleFiles(['loops11.py']);
+
+    TestUtils.validateResults(analysisResults, 3);
+});
+
 test('ForLoop1', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['forLoop1.py']);