Changed behavior of symbol resolution involving a quoted (forward-dec…

…lared) type annotation that references a symbol in the global (module) or builtins namespaces. The previous implementation didn't match the runtime behavior of `typing.get_type_hints`.
microsoft · Dec 2, 2021 · 27bf8c1 · 27bf8c1
1 parent b4b7843
commit 27bf8c1
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 30 deletions.
diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts
@@ -3371,7 +3371,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
 
         // Look for the scope that contains the value definition and
         // see if it has a declared type.
-        const symbolWithScope = lookUpSymbolRecursive(node, name, !allowForwardReferences);
+        const symbolWithScope = lookUpSymbolRecursive(node, name, !allowForwardReferences, allowForwardReferences);
 
         if (symbolWithScope) {
             let useCodeFlowAnalysis = !allowForwardReferences;
@@ -15825,7 +15825,12 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
         return nameType;
     }
 
-    function lookUpSymbolRecursive(node: ParseNode, name: string, honorCodeFlow: boolean): SymbolWithScope | undefined {
+    function lookUpSymbolRecursive(
+        node: ParseNode,
+        name: string,
+        honorCodeFlow: boolean,
+        preferGlobalScope = false
+    ): SymbolWithScope | undefined {
         const scope = ScopeUtils.getScopeForNode(node);
         let symbolWithScope = scope?.lookUpSymbolRecursive(name);
 
@@ -15870,6 +15875,34 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
             }
         }
 
+        // PEP 563 indicates that if a forward reference can be resolved in the module
+        // scope (or, by implication, in the builtins scope), it should prefer that
+        // resolution over local resolutions.
+        if (symbolWithScope && preferGlobalScope) {
+            let curSymbolWithScope: SymbolWithScope | undefined = symbolWithScope;
+            while (
+                curSymbolWithScope.scope.type !== ScopeType.Module &&
+                curSymbolWithScope.scope.type !== ScopeType.Builtin &&
+                curSymbolWithScope.scope.parent
+            ) {
+                curSymbolWithScope = curSymbolWithScope.scope.parent.lookUpSymbolRecursive(
+                    name,
+                    curSymbolWithScope.isOutsideCallerModule,
+                    curSymbolWithScope.isBeyondExecutionScope || curSymbolWithScope.scope.isIndependentlyExecutable()
+                );
+                if (!curSymbolWithScope) {
+                    break;
+                }
+            }
+
+            if (
+                curSymbolWithScope?.scope.type === ScopeType.Module ||
+                curSymbolWithScope?.scope.type === ScopeType.Builtin
+            ) {
+                symbolWithScope = curSymbolWithScope;
+            }
+        }
+
         return symbolWithScope;
     }
 
@@ -16093,7 +16126,13 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
                 allowForwardReferences = true;
             }
 
-            const symbolWithScope = lookUpSymbolRecursive(node, node.value, !allowForwardReferences);
+            const symbolWithScope = lookUpSymbolRecursive(
+                node,
+                node.value,
+                !allowForwardReferences,
+                allowForwardReferences
+            );
+
             if (symbolWithScope) {
                 declarations.push(...symbolWithScope.symbol.getDeclarations());
             }

diff --git a/packages/pyright-internal/src/tests/samples/annotations1.py b/packages/pyright-internal/src/tests/samples/annotations1.py
@@ -50,4 +50,24 @@ def func10():
         int,
         str
     ]
-"""
+"""
+
+
+class ClassD:
+    ClassA: "ClassA"
+
+    # This should generate an error because ClassF refers
+    # to itself, and there is no ClassF declared at the module
+    # level.
+    ClassF: "ClassF"
+
+    str: "str"
+
+    def int(self):
+        ...
+
+    foo: "int"
+
+    # This should generate an error because it refers to the local
+    # "int" symbol rather than the builtins "int".
+    bar: int
diff --git a/packages/pyright-internal/src/tests/samples/annotations3.py b/packages/pyright-internal/src/tests/samples/annotations3.py
@@ -29,6 +29,15 @@ def func3(self) -> "Optional[ClassC]":
     def func4(self) -> Optional["ClassC"]:
         return None
 
+    def func5(self, x: ClassA):
+        x.func0()
+
+    class ClassA:
+        ...
+
+    def func6(self, x: ClassC):
+        x.my_int
+
 
 class ClassC:
-    pass
+    my_int: int
diff --git a/packages/pyright-internal/src/tests/samples/circular1.py b/packages/pyright-internal/src/tests/samples/circular1.py
@@ -5,13 +5,12 @@
 class Example1:
     # This should not generate an error because "int"
     # is not forward-declared.
-    str: str = 4
+    str: str = ""
 
     int = int
 
     test: int
 
 
 class Example2:
-    # This should generate an error because it's forward-declared.
     int: "int" = 4
diff --git a/packages/pyright-internal/src/tests/samples/function5.py b/packages/pyright-internal/src/tests/samples/function5.py
diff --git a/packages/pyright-internal/src/tests/typeEvaluator1.test.ts b/packages/pyright-internal/src/tests/typeEvaluator1.test.ts
@@ -605,12 +605,6 @@ test('Function4', () => {
     TestUtils.validateResults(analysisResults, 1);
 });
 
-test('Function5', () => {
-    const analysisResults = TestUtils.typeAnalyzeSampleFiles(['function5.py']);
-
-    TestUtils.validateResults(analysisResults, 1);
-});
-
 test('Function6', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['function6.py']);
 
@@ -656,7 +650,7 @@ test('FunctionMember2', () => {
 test('Annotations1', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['annotations1.py']);
 
-    TestUtils.validateResults(analysisResults, 3);
+    TestUtils.validateResults(analysisResults, 5);
 });
 
 test('Annotations2', () => {

diff --git a/packages/pyright-internal/src/tests/typeEvaluator4.test.ts b/packages/pyright-internal/src/tests/typeEvaluator4.test.ts
@@ -902,7 +902,7 @@ test('Annotated1', () => {
 test('Circular1', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['circular1.py']);
 
-    TestUtils.validateResults(analysisResults, 2);
+    TestUtils.validateResults(analysisResults, 0);
 });
 
 test('TryExcept1', () => {