don't do the variance narrowing thing if the type was originally `Any…

…`/Unknown
DetachHead · Oct 7, 2024 · 69fb6f1 · 69fb6f1
1 parent ec6055e
commit 69fb6f1
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 10 deletions.
diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts
@@ -3794,7 +3794,7 @@ export class Checker extends ParseTreeWalker {
             return;
         }
 
-        const classTypeList = getIsInstanceClassTypes(this._evaluator, arg1Type);
+        const classTypeList = getIsInstanceClassTypes(this._evaluator, arg1Type, arg0Type);
         if (!classTypeList) {
             return;
         }

diff --git a/packages/pyright-internal/src/analyzer/patternMatching.ts b/packages/pyright-internal/src/analyzer/patternMatching.ts
@@ -758,7 +758,13 @@ function narrowTypeBasedOnClassPattern(
     if (isClass(exprType) && !exprType.props?.typeAliasInfo) {
         exprType = ClassType.cloneRemoveTypePromotions(exprType);
         evaluator.inferVarianceForClass(exprType);
-        exprType = specializeWithUnknownTypeArgs(exprType, evaluator.getTupleClassType(), evaluator.getObjectType());
+        exprType = specializeWithUnknownTypeArgs(
+            exprType,
+            evaluator.getTupleClassType(),
+            // for backwards compatibility with bacly typed code, we don't specialize using variance if the type we're
+            // narrowing is Any/Unknown
+            isAnyOrUnknown(type) || isPartlyUnknown(type) ? undefined : evaluator.getObjectType()
+        );
     }
 
     // Are there any positional arguments? If so, try to get the mappings for

diff --git a/packages/pyright-internal/src/analyzer/typeGuards.ts b/packages/pyright-internal/src/analyzer/typeGuards.ts
@@ -82,6 +82,7 @@ import {
     isMetaclassInstance,
     isNoneInstance,
     isNoneTypeClass,
+    isPartlyUnknown,
     isProperty,
     isTupleClass,
     isTupleGradualForm,
@@ -623,7 +624,11 @@ export function getTypeNarrowingCallback(
                     const arg1TypeResult = evaluator.getTypeOfExpression(arg1Expr, EvalFlags.IsInstanceArgDefaults);
                     const arg1Type = arg1TypeResult.type;
 
-                    const classTypeList = getIsInstanceClassTypes(evaluator, arg1Type);
+                    const classTypeList = getIsInstanceClassTypes(
+                        evaluator,
+                        arg1Type,
+                        evaluator.getTypeOfExpression(arg0Expr).type
+                    );
                     const isIncomplete = !!callTypeResult.isIncomplete || !!arg1TypeResult.isIncomplete;
 
                     if (classTypeList) {
@@ -1125,11 +1130,16 @@ function narrowTypeForIsEllipsis(evaluator: TypeEvaluator, node: ExpressionNode,
 // which form and returns a list of classes or undefined.
 export function getIsInstanceClassTypes(
     evaluator: TypeEvaluator,
-    argType: Type
+    argType: Type,
+    typeToNarrow: Type
 ): (ClassType | TypeVarType | FunctionType)[] | undefined {
     let foundNonClassType = false;
     const classTypeList: (ClassType | TypeVarType | FunctionType)[] = [];
-
+    /**
+     * if the type we're narrowing is Any or Unknown, we don't want to specialize using the
+     * variance/bound for compatibility with less strictly typed code (cringe)
+     */
+    const useVarianceForSpecialization = !isAnyOrUnknown(typeToNarrow) && !isPartlyUnknown(typeToNarrow);
     // Create a helper function that returns a list of class types or
     // undefined if any of the types are not valid.
     const addClassTypesToList = (types: Type[]) => {
@@ -1139,7 +1149,7 @@ export function getIsInstanceClassTypes(
                 subtype = specializeWithUnknownTypeArgs(
                     subtype,
                     evaluator.getTupleClassType(),
-                    evaluator.getObjectType()
+                    useVarianceForSpecialization ? evaluator.getObjectType() : undefined
                 );
 
                 if (isInstantiableClass(subtype) && ClassType.isBuiltIn(subtype, 'Callable')) {

diff --git a/packages/pyright-internal/src/tests/samples/typeNarrowingIsinstance13.py b/packages/pyright-internal/src/tests/samples/typeNarrowingIsinstance13.py
@@ -8,8 +8,8 @@
 
 def func1(v: Any) -> bool:
     if isinstance(v, Iterable):
-        reveal_type(v, expected_text="Iterable[object]")
+        reveal_type(v, expected_text="Iterable[Unknown]")
         if isinstance(v, Sized):
-            reveal_type(v, expected_text="<subclass of Iterable[object] and Sized>")
+            reveal_type(v, expected_text="<subclass of Iterable and Sized>")
             return True
     return False
diff --git a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py
@@ -1,4 +1,4 @@
-from typing import Never, assert_type, Iterable
+from typing import Any, Never, assert_type, Iterable
 
 
 class Covariant[T]:
@@ -40,4 +40,19 @@ def bar(self, other: T): ...
 def foo(value: object):
     match value:
         case Iterable():
-            assert_type(value, Iterable[object])
+            assert_type(value, Iterable[object])
+
+class AnyOrUnknown:
+    """for backwards compatibility with badly typed code we keep the old functionality when narrowing `Any`/Unknown"""
+    def foo(self, value: Any):
+        if isinstance(value, Iterable):
+            assert_type(value, Iterable[Any])
+
+    def bar(self, value: Any):
+        match value:
+            case Iterable():
+                assert_type(value, Iterable[Any])
+
+    def partially_unknown(self, value=None):
+        if isinstance(value, Iterable):
+            assert_type(value, Iterable[Any])