Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminated some differences between the treatment of type and `Type… #6551

Merged
merged 1 commit into from
Nov 26, 2023
Merged

Eliminated some differences between the treatment of type and `Type… #6551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions packages/pyright-internal/src/analyzer/constraintSolver.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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]")
Loading