From 664db78773972c7ffa287c098dc9ce0f7a5d325a Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 25 Nov 2023 17:52:39 -0800 Subject: [PATCH] Eliminated some differences between the treatment of `type` and `Type`. These should be treated the same under all circumstances. This addresses #6252. --- .../src/analyzer/constraintSolver.ts | 11 -- .../src/analyzer/typeEvaluator.ts | 115 ++++++++---------- .../src/analyzer/typeGuards.ts | 12 +- .../src/tests/samples/annotations6.py | 4 +- 4 files changed, 65 insertions(+), 77 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/constraintSolver.ts b/packages/pyright-internal/src/analyzer/constraintSolver.ts index cd321f697c4b..e83308ca1b7f 100644 --- a/packages/pyright-internal/src/analyzer/constraintSolver.ts +++ b/packages/pyright-internal/src/analyzer/constraintSolver.ts @@ -13,7 +13,6 @@ import { DiagnosticAddendum } from '../common/diagnostic'; import { Localizer } from '../localization/localize'; import { maxSubtypesForInferredType, TypeEvaluator } from './typeEvaluatorTypes'; import { - AnyType, ClassType, combineTypes, FunctionParameter, @@ -206,16 +205,6 @@ export function assignTypeToTypeVar( srcType = TypeVarType.cloneForUnpacked(srcType, /* isInUnion */ true); } - // If we're attempting to assign `type` to Type[T], transform `type` into `Type[Any]`. - if ( - TypeBase.isInstantiable(destType) && - isClassInstance(srcType) && - ClassType.isBuiltIn(srcType, 'type') && - !srcType.typeArguments - ) { - srcType = AnyType.create(); - } - // Handle the constrained case. This case needs to be handled specially // because type narrowing isn't used in this case. For example, if the // source type is "Literal[1]" and the constraint list includes the type diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts index aabf711b31de..6884a4481604 100644 --- a/packages/pyright-internal/src/analyzer/typeEvaluator.ts +++ b/packages/pyright-internal/src/analyzer/typeEvaluator.ts @@ -9421,9 +9421,22 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions } if (ClassType.isBuiltIn(expandedCallType)) { - const className = expandedCallType.aliasName || expandedCallType.details.name; + const className = expandedCallType.aliasName ?? expandedCallType.details.name; + + // Handle the 'type' call specially. + if (expandedCallType.details.name === 'type') { + if (expandedCallType.typeArguments && expandedCallType.isTypeArgumentExplicit) { + addDiagnostic( + AnalyzerNodeInfo.getFileInfo(errorNode).diagnosticRuleSet.reportGeneralTypeIssues, + DiagnosticRule.reportGeneralTypeIssues, + Localizer.Diagnostic.objectNotCallable().format({ + type: printType(expandedCallType), + }), + errorNode + ); + return { returnType: UnknownType.create(), argumentErrors: true }; + } - if (className === 'type') { // Validate the constructor arguments. validateConstructorArguments( evaluatorInterface, @@ -9434,7 +9447,6 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions inferenceContext ); - // Handle the 'type' call specially. if (argList.length === 1) { // The one-parameter form of "type" returns the class // for the specified object. @@ -9537,7 +9549,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions errorNode ); - return { returnType: AnyType.create() }; + return { returnType: AnyType.create(), argumentErrors: true }; } if (isClass(unexpandedCallType) && isKnownEnumType(className)) { @@ -9723,7 +9735,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions isClass(expandedCallType) && ClassType.isBuiltIn(expandedCallType, 'type') ) { - // Handle the case where a Type[T] is being called. We presume this + // Handle the case where a type[T] is being called. We presume this // will instantiate an object of type T. returnType = convertToInstance(unexpandedCallType); } @@ -19243,17 +19255,6 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions } case 'Type': { - // PEP 484 says that Type[Any] should be considered - // equivalent to type. - if ( - typeArgs?.length === 1 && - isAnyOrUnknown(typeArgs[0].type) && - typeClassType && - isInstantiableClass(typeClassType) - ) { - return { type: typeClassType }; - } - let typeType = createSpecialType(classType, typeArgs, 1); if (isInstantiableClass(typeType)) { typeType = explodeGenericClass(typeType); @@ -19371,12 +19372,6 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions // in Python 3.9 and newer. if (ClassType.isBuiltIn(classType, 'type') && typeArgs) { if (typeArgs.length >= 1) { - // PEP 484 says that type[Any] should be considered - // equivalent to type. - if (isAnyOrUnknown(typeArgs[0].type)) { - return { type: classType }; - } - // Treat type[function] as illegal. if (isFunction(typeArgs[0].type) || isOverloadedFunction(typeArgs[0].type)) { addDiagnostic( @@ -19390,10 +19385,9 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions } } - const typeClass = getTypingType(errorNode, 'Type'); - if (typeClass && isInstantiableClass(typeClass)) { + if (typeClassType && isInstantiableClass(typeClassType)) { let typeType = createSpecialType( - typeClass, + typeClassType, typeArgs, 1, /* allowParamSpec */ undefined, @@ -22759,46 +22753,42 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions // Is the src a specialized "type" object? if (isClassInstance(expandedSrcType) && ClassType.isBuiltIn(expandedSrcType, 'type')) { const srcTypeArgs = expandedSrcType.typeArguments; - let typeTypeArg: Type | undefined; - let instantiableType: Type | undefined; + let typeTypeArg: Type; if (srcTypeArgs && srcTypeArgs.length >= 1) { typeTypeArg = srcTypeArgs[0]; - if (isAnyOrUnknown(typeTypeArg)) { - if (isClassInstance(destType) && ClassType.isBuiltIn(expandedSrcType, 'type')) { - return true; - } - return TypeBase.isInstantiable(destType); + } else { + typeTypeArg = UnknownType.create(); + } + + if (isAnyOrUnknown(typeTypeArg)) { + if (isClassInstance(destType) && ClassType.isBuiltIn(expandedSrcType, 'type')) { + return true; } - instantiableType = convertToInstantiable(typeTypeArg); - } else if (TypeBase.isInstantiable(destType)) { - typeTypeArg = objectType ?? AnyType.create(); - instantiableType = expandedSrcType; + return TypeBase.isInstantiable(destType); } - if (instantiableType && typeTypeArg) { - if (isClassInstance(typeTypeArg) || isTypeVar(typeTypeArg)) { - if ( - assignType( - destType, - instantiableType, - diag?.createAddendum(), - destTypeVarContext, - srcTypeVarContext, - flags, - recursionCount - ) - ) { - return true; - } + const instantiableType = convertToInstantiable(typeTypeArg); - diag?.addMessage( - Localizer.DiagnosticAddendum.typeAssignmentMismatch().format( - printSrcDestTypes(srcType, destType) - ) - ); - return false; + if (isClassInstance(typeTypeArg) || isTypeVar(typeTypeArg)) { + if ( + assignType( + destType, + instantiableType, + diag?.createAddendum(), + destTypeVarContext, + srcTypeVarContext, + flags, + recursionCount + ) + ) { + return true; } + + diag?.addMessage( + Localizer.DiagnosticAddendum.typeAssignmentMismatch().format(printSrcDestTypes(srcType, destType)) + ); + return false; } } @@ -22876,8 +22866,11 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions } if (isClassInstance(destType)) { - // Is the dest a specialized "Type" object? - if (ClassType.isBuiltIn(destType, 'Type')) { + if (ClassType.isBuiltIn(destType, 'type')) { + if (isAnyOrUnknown(srcType) && (flags & AssignTypeFlags.OverloadOverlapCheck) !== 0) { + return false; + } + const destTypeArgs = destType.typeArguments; if (destTypeArgs && destTypeArgs.length >= 1) { if (TypeBase.isInstance(destTypeArgs[0]) && TypeBase.isInstantiable(srcType)) { @@ -22892,10 +22885,6 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions ); } } - } else if (ClassType.isBuiltIn(destType, 'type')) { - if (isAnyOrUnknown(srcType) && (flags & AssignTypeFlags.OverloadOverlapCheck) !== 0) { - return false; - } // Is the dest a "type" object? Assume that all instantiable // types are assignable to "type". diff --git a/packages/pyright-internal/src/analyzer/typeGuards.ts b/packages/pyright-internal/src/analyzer/typeGuards.ts index 18366581b306..ede5d65b9b2e 100644 --- a/packages/pyright-internal/src/analyzer/typeGuards.ts +++ b/packages/pyright-internal/src/analyzer/typeGuards.ts @@ -1631,11 +1631,21 @@ function narrowTypeForIsInstance( const filterMetaclass = concreteFilterType.details.effectiveMetaclass; if (filterMetaclass && isInstantiableClass(filterMetaclass)) { - const isMetaclassOverlap = evaluator.assignType( + let isMetaclassOverlap = evaluator.assignType( metaclassType, ClassType.cloneAsInstance(filterMetaclass) ); + // Handle the special case where the metaclass for the filter is type. + // This will normally be treated as type[Any], which is compatible with + // any metaclass, but we specifically want to treat type as the class + // type[object] in this case. + if (ClassType.isBuiltIn(filterMetaclass, 'type') && !filterMetaclass.isTypeArgumentExplicit) { + if (!ClassType.isBuiltIn(metaclassType, 'type')) { + isMetaclassOverlap = false; + } + } + if (isMetaclassOverlap) { if (isPositiveTest) { filteredTypes.push(filterType); diff --git a/packages/pyright-internal/src/tests/samples/annotations6.py b/packages/pyright-internal/src/tests/samples/annotations6.py index 62cbaff9106c..772846131df1 100644 --- a/packages/pyright-internal/src/tests/samples/annotations6.py +++ b/packages/pyright-internal/src/tests/samples/annotations6.py @@ -25,5 +25,5 @@ def is_type2(x: object, y: type[Any]) -> bool: def func1(v1: Type[Any], v2: type[Any]): - reveal_type(v1, expected_text="type") - reveal_type(v2, expected_text="type") + reveal_type(v1, expected_text="Type[Any]") + reveal_type(v2, expected_text="type[Any]")