From 1e7fa66f841dcae3b43b0573dd9c605605bbbadd Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 20 Jul 2023 20:50:09 -0700 Subject: [PATCH] Fixed an inconsistency in reporting of unbound variables when the variable is captured by an inner scope. The new behavior correctly identifies unbound (or potentially unbound) variables in cases where the captured variable is narrowed. This happens when there are no assignments to the variable after it is captured. This addresses #5548. Fixed a bug in type narrowing for captured variables. Captured variables that are modified in other scopes using a `nonlocal` or `global` binding should be ineligible for type narrowing. --- docs/type-concepts-advanced.md | 19 ++++++++++ .../src/analyzer/typeEvaluator.ts | 35 ++++++++++++++----- .../src/tests/samples/capturedVariable1.py | 15 ++++++++ .../src/tests/samples/unbound5.py | 31 ++++++++++++++++ .../src/tests/typeEvaluator2.test.ts | 6 ++++ 5 files changed, 97 insertions(+), 9 deletions(-) create mode 100644 packages/pyright-internal/src/tests/samples/unbound5.py diff --git a/docs/type-concepts-advanced.md b/docs/type-concepts-advanced.md index 19c3097aa170..ba7bd89631e5 100644 --- a/docs/type-concepts-advanced.md +++ b/docs/type-concepts-advanced.md @@ -155,6 +155,7 @@ def func4(x: str | None): ``` ### Narrowing for Implied Else + When an “if” or “elif” clause is used without a corresponding “else”, Pyright will generally assume that the code can “fall through” without executing the “if” or “elif” block. However, there are cases where the analyzer can determine that a fall-through is not possible because the “if” or “elif” is guaranteed to be executed based on type analysis. ```python @@ -222,6 +223,24 @@ b = c reveal_type(b) # list[Any] ``` +### Narrowing for Captured Variables + +If a variable’s type is narrowed in an outer scope and the variable is subsequently captured by an inner-scoped function or lambda, Pyright retains the narrowed type if it can determine that the value of the captured variable is not modified on any code path after the inner-scope function or lambda is defined and is not modified in another scope via a `nonlocal` or `global` binding. + +```python +def func(val: int | None): + if val is not None: + + def inner_1() -> None: + reveal_type(val) # int + print(val + 1) + + inner_2 = lambda: reveal_type(val) + 1 # int + + inner_1() + inner_2() +``` + ### Constrained Type Variables When a TypeVar is defined, it can be constrained to two or more types. diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts index 33bb231c58a3..3a3b701b37ce 100644 --- a/packages/pyright-internal/src/analyzer/typeEvaluator.ts +++ b/packages/pyright-internal/src/analyzer/typeEvaluator.ts @@ -4329,15 +4329,27 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions ): FlowNodeTypeResult | undefined { // This function applies only to variables, parameters, and imports, not to other // types of symbols. + const decls = symbolWithScope.symbol.getDeclarations(); if ( - !symbolWithScope.symbol - .getDeclarations() - .every( - (decl) => - decl.type === DeclarationType.Variable || - decl.type === DeclarationType.Parameter || - decl.type === DeclarationType.Alias - ) + !decls.every( + (decl) => + decl.type === DeclarationType.Variable || + decl.type === DeclarationType.Parameter || + decl.type === DeclarationType.Alias + ) + ) { + return undefined; + } + + // If the symbol is modified in scopes other than the one in which it is + // declared (e.g. through a nonlocal or global binding), it is not eligible + // for code flow analysis. + if ( + !decls.every( + (decl) => + decl.type === DeclarationType.Parameter || + ScopeUtils.getScopeForNode(decl.node) === symbolWithScope.scope + ) ) { return undefined; } @@ -4390,7 +4402,12 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions ); }) ) { - return getFlowTypeOfReference(node, symbolWithScope.symbol.id, effectiveType, innerScopeNode); + let typeAtStart = effectiveType; + if (symbolWithScope.symbol.isInitiallyUnbound()) { + typeAtStart = UnboundType.create(); + } + + return getFlowTypeOfReference(node, symbolWithScope.symbol.id, typeAtStart, innerScopeNode); } } } diff --git a/packages/pyright-internal/src/tests/samples/capturedVariable1.py b/packages/pyright-internal/src/tests/samples/capturedVariable1.py index 1f025f3a51b2..768a31c45742 100644 --- a/packages/pyright-internal/src/tests/samples/capturedVariable1.py +++ b/packages/pyright-internal/src/tests/samples/capturedVariable1.py @@ -104,3 +104,18 @@ def foo() -> str: return x.upper() return x.upper() + + +def func10(cond: bool, val: str): + x: str | None = val if cond else None + y: str | None = val if cond else None + + def inner1(): + nonlocal x + x = None + + if x is not None and y is not None: + + def inner2(): + reveal_type(x, expected_text="str | None") + reveal_type(y, expected_text="str") diff --git a/packages/pyright-internal/src/tests/samples/unbound5.py b/packages/pyright-internal/src/tests/samples/unbound5.py new file mode 100644 index 000000000000..0a9c245a57ac --- /dev/null +++ b/packages/pyright-internal/src/tests/samples/unbound5.py @@ -0,0 +1,31 @@ +# This sample tests the interplay between unbound symbol detection and +# the code that handles conditional narrowing of captured variables. + +from random import random + + +if random() > 0.5: + from datetime import datetime + from math import cos + +# The following should generate an error because datetime +# is "narrowed" across execution scopes. +test0 = lambda: datetime + + +def test1(): + # The following should generate an error because datetime + # is "narrowed" across execution scopes. + return datetime + + +test2 = lambda: cos + + +def test2(): + return cos + + +# This modification means that cos will not be narrowed +# across execution scopes. +cos = None diff --git a/packages/pyright-internal/src/tests/typeEvaluator2.test.ts b/packages/pyright-internal/src/tests/typeEvaluator2.test.ts index 7aebf631964b..4f74c992f4f8 100644 --- a/packages/pyright-internal/src/tests/typeEvaluator2.test.ts +++ b/packages/pyright-internal/src/tests/typeEvaluator2.test.ts @@ -322,6 +322,12 @@ test('Unbound4', () => { TestUtils.validateResults(analysisResults, 2); }); +test('Unbound5', () => { + const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unbound5.py']); + + TestUtils.validateResults(analysisResults, 2); +}); + test('Assert1', () => { const configOptions = new ConfigOptions('.');