Fixed a bug that resulted in a false negative when assigning a `T | N…

…one` type to `object | None` in an invariant context. This same bug led to issues with the validation of TypeVar variance within a Protocol. This addresses microsoft/pylance-release#4613. (#5518) Co-authored-by: Eric Traut <[email protected]>
microsoft · Jul 17, 2023 · 7fcdc92 · 7fcdc92
1 parent bfe2128
commit 7fcdc92
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 34 deletions.
diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts
@@ -20904,6 +20904,10 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
             }
         });
 
+        if (!isAssignable) {
+            return false;
+        }
+
         // Now handle generic base classes.
         destType.details.baseClasses.forEach((baseClass) => {
             if (
@@ -21652,42 +21656,40 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
         }
 
         if (isUnion(destType)) {
+            // If both the source and dest are unions, use assignFromUnionType which has
+            // special-case logic to handle this case.
             if (isUnion(srcType)) {
-                if (
-                    assignFromUnionType(
-                        destType,
-                        srcType,
-                        /* diag */ undefined,
-                        destTypeVarContext,
-                        srcTypeVarContext,
-                        originalFlags,
-                        recursionCount
-                    )
-                ) {
-                    return true;
+                return assignFromUnionType(
+                    destType,
+                    srcType,
+                    /* diag */ undefined,
+                    destTypeVarContext,
+                    srcTypeVarContext,
+                    originalFlags,
+                    recursionCount
+                );
+            }
+
+            const clonedDestTypeVarContext = destTypeVarContext?.clone();
+            const clonedSrcTypeVarContext = srcTypeVarContext?.clone();
+            if (
+                assignToUnionType(
+                    destType,
+                    srcType,
+                    /* diag */ undefined,
+                    clonedDestTypeVarContext,
+                    clonedSrcTypeVarContext,
+                    originalFlags,
+                    recursionCount
+                )
+            ) {
+                if (destTypeVarContext && clonedDestTypeVarContext) {
+                    destTypeVarContext.copyFromClone(clonedDestTypeVarContext);
                 }
-            } else {
-                const clonedDestTypeVarContext = destTypeVarContext?.clone();
-                const clonedSrcTypeVarContext = srcTypeVarContext?.clone();
-                if (
-                    assignToUnionType(
-                        destType,
-                        srcType,
-                        /* diag */ undefined,
-                        clonedDestTypeVarContext,
-                        clonedSrcTypeVarContext,
-                        originalFlags,
-                        recursionCount
-                    )
-                ) {
-                    if (destTypeVarContext && clonedDestTypeVarContext) {
-                        destTypeVarContext.copyFromClone(clonedDestTypeVarContext);
-                    }
-                    if (srcTypeVarContext && clonedSrcTypeVarContext) {
-                        srcTypeVarContext.copyFromClone(clonedSrcTypeVarContext);
-                    }
-                    return true;
+                if (srcTypeVarContext && clonedSrcTypeVarContext) {
+                    srcTypeVarContext.copyFromClone(clonedSrcTypeVarContext);
                 }
+                return true;
             }
         }
 
@@ -22429,7 +22431,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
                     !isSubtypeSubsumed &&
                     !assignType(
                         destType,
-                        concreteSubtype,
+                        subtype,
                         diag?.createAddendum(),
                         destTypeVarContext,
                         srcTypeVarContext,

diff --git a/packages/pyright-internal/src/tests/samples/protocol17.py b/packages/pyright-internal/src/tests/samples/protocol17.py
@@ -97,3 +97,7 @@ def prop1(self) -> _T1_co:
 class Protocol10(Protocol[_T1_co]):
     def m1(self) -> type[_T1_co]:
         ...
+
+
+class Protocol11(Protocol[_T1]):
+    x: _T1 | None