From 19194dd089f5bb3d1d3fd748887e3d4cc7880560 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 20 Jul 2023 21:08:53 -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. (#5551) 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. Co-authored-by: Eric Traut --- 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 19c3097aa..ba7bd8963 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 33bb231c5..3a3b701b3 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 1f025f3a5..768a31c45 100644 --- a/packages/pyright-internal/src/tests/samples/capturedVariable1.py +++ b/packages/pyright-internal/src/tests/samples/capturedVariable1.py @@ -104,3 +104,18 @@ def func9(x: str | None): 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 000000000..0a9c245a5 --- /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 7aebf6319..4f74c992f 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('.');