Skip to content

Commit

Permalink
Eliminated some differences between the treatment of type and `Type…
Browse files Browse the repository at this point in the history 
…`. These should be treated the same under all circumstances. This addresses #6252.
  • Loading branch information
@erictraut
erictraut committed Nov 26, 2023
1 parent 197604d commit 4fe4306
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 77 deletions.
11 changes: 0 additions & 11 deletions packages/pyright-internal/src/analyzer/constraintSolver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { DiagnosticAddendum } from '../common/diagnostic';
import { Localizer } from '../localization/localize';
import { maxSubtypesForInferredType, TypeEvaluator } from './typeEvaluatorTypes';
import {
AnyType,
ClassType,
combineTypes,
FunctionParameter,
Expand Down Expand Up @@ -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
Expand Down
115 changes: 52 additions & 63 deletions packages/pyright-internal/src/analyzer/typeEvaluator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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)) {
Expand All @@ -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".
Expand Down
12 changes: 11 additions & 1 deletion packages/pyright-internal/src/analyzer/typeGuards.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions packages/pyright-internal/src/tests/samples/annotations6.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]")

0 comments on commit 4fe4306

Please sign in to comment.