Improved detection of the use of variables used in type annotations. These are illegal according to the typing spec. (#8318)

This addresses #8316.
This commit is contained in:
Eric Traut 2024-07-06 10:18:21 -07:00 committed by GitHub
parent 598119b90f
commit 3ddf0ad705
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 150 additions and 109 deletions

View File

@ -4527,42 +4527,9 @@ export function createTypeEvaluator(
// Detect, report, and fill in missing type arguments if appropriate.
type = reportMissingTypeArguments(node, type, flags);
// Report inappropriate use of variables in type expressions.
if ((flags & EvalFlags.TypeExpression) !== 0) {
// Verify that the name does not refer to a (non type alias) variable.
if (effectiveTypeInfo.includesVariableDecl && !type.typeAliasInfo) {
let isAllowedTypeForVariable = isTypeVar(type) || isTypeAliasPlaceholder(type);
if (
isClass(type) &&
!type.includeSubclasses &&
!symbol.hasTypedDeclarations() &&
ClassType.isValidTypeAliasClass(type)
) {
// This check exempts class types that are created by calling
// NewType, NamedTuple, etc.
isAllowedTypeForVariable = true;
}
// Disable for assignments in the typings.pyi file, since it defines special forms.
if (!isAllowedTypeForVariable && !fileInfo.isTypingStubFile) {
// This might be a union that was previously a type alias
// but was reconstituted in such a way that we lost the
// typeAliasInfo. Avoid the false positive error by suppressing
// the error when it looks like a plausible type alias type.
if (
effectiveTypeInfo.includesIllegalTypeAliasDecl ||
!TypeBase.isInstantiable(type) ||
(flags & EvalFlags.NoSpecialize) !== 0
) {
addDiagnostic(
DiagnosticRule.reportInvalidTypeForm,
LocMessage.typeAnnotationVariable(),
node
);
type = UnknownType.create();
}
}
}
type = validateSymbolIsTypeExpression(node, type, !!effectiveTypeInfo.includesVariableDecl);
}
} else {
// Handle the special case of "reveal_type" and "reveal_locals".
@ -4607,6 +4574,33 @@ export function createTypeEvaluator(
return { type, isIncomplete };
}
// Reports diagnostics if type isn't valid within a type expression.
function validateSymbolIsTypeExpression(node: ExpressionNode, type: Type, includesVariableDecl: boolean): Type {
// Verify that the name does not refer to a (non type alias) variable.
if (!includesVariableDecl || type.typeAliasInfo) {
return type;
}
if (isTypeVar(type) || isTypeAliasPlaceholder(type)) {
return type;
}
// Exempts class types that are created by calling
// NewType, NamedTuple, etc.
if (isClass(type) && !type.includeSubclasses && ClassType.isValidTypeAliasClass(type)) {
return type;
}
// Disable for assignments in the typings.pyi file, since it defines special forms.
const fileInfo = AnalyzerNodeInfo.getFileInfo(node);
if (fileInfo.isTypingStubFile) {
return type;
}
addDiagnostic(DiagnosticRule.reportInvalidTypeForm, LocMessage.typeAnnotationVariable(), node);
return UnknownType.create();
}
// If the value is a special form (like a TypeVar or `Any`) and is being
// evaluated in a value expression context, convert it from its special
// meaning to its runtime value.
@ -5358,7 +5352,7 @@ export function createTypeEvaluator(
memberName,
usage,
diag,
/* memberAccessFlags */ undefined,
(flags & EvalFlags.TypeExpression) === 0 ? undefined : MemberAccessFlags.TypeExpression,
baseTypeResult.bindToSelfType
);
}
@ -5404,11 +5398,16 @@ export function createTypeEvaluator(
setSymbolAccessed(fileInfo, symbol, node.memberName);
}
type = getEffectiveTypeOfSymbolForUsage(
const typeResult = getEffectiveTypeOfSymbolForUsage(
symbol,
/* usageNode */ undefined,
/* useLastDecl */ true
).type;
);
type = typeResult.type;
if ((flags & EvalFlags.TypeExpression) !== 0) {
type = validateSymbolIsTypeExpression(node, type, !!typeResult.includesVariableDecl);
}
if (isTypeVar(type)) {
type = validateTypeVarUsage(node, type, flags);
@ -15022,7 +15021,7 @@ export function createTypeEvaluator(
if (!type) {
const exprType = getTypeOfExpression(
itemExpr,
(flags & EvalFlags.ForwardRefs) | EvalFlags.NoConvertSpecialForm
(flags & EvalFlags.ForwardRefs) | EvalFlags.NoConvertSpecialForm | EvalFlags.TypeExpression
);
// Is this an enum type?
@ -22318,61 +22317,74 @@ export function createTypeEvaluator(
selfClass: ClassType | TypeVarType | undefined,
flags: MemberAccessFlags
): TypeResult | undefined {
if (isInstantiableClass(member.classType)) {
const typeResult = getEffectiveTypeOfSymbolForUsage(member.symbol);
if (typeResult) {
// If the type is a function or overloaded function, infer
// and cache the return type if necessary. This needs to be done
// prior to specializing.
inferReturnTypeIfNecessary(typeResult.type);
// Check for ambiguous accesses to attributes with generic types?
if (
errorNode &&
selfClass &&
isClass(selfClass) &&
member.isInstanceMember &&
isClass(member.unspecializedClassType) &&
(flags & MemberAccessFlags.DisallowGenericInstanceVariableAccess) !== 0 &&
requiresSpecialization(typeResult.type, { ignoreSelf: true, ignoreImplicitTypeArgs: true })
) {
const specializedType = partiallySpecializeType(
typeResult.type,
member.unspecializedClassType,
selfSpecializeClass(selfClass, /* overrideTypeArgs */ true)
);
if (
findSubtype(
specializedType,
(subtype) =>
!isFunction(subtype) &&
!isOverloadedFunction(subtype) &&
requiresSpecialization(subtype, { ignoreSelf: true, ignoreImplicitTypeArgs: true })
)
) {
addDiagnostic(
DiagnosticRule.reportGeneralTypeIssues,
LocMessage.genericInstanceVariableAccess(),
errorNode
);
}
}
return {
type: partiallySpecializeType(typeResult.type, member.classType, selfClass),
isIncomplete: !!typeResult.isIncomplete,
};
}
} else if (isAnyOrUnknown(member.classType)) {
if (isAnyOrUnknown(member.classType)) {
return {
type: member.classType,
isIncomplete: false,
};
}
return undefined;
if (!isInstantiableClass(member.classType)) {
return undefined;
}
const typeResult = getEffectiveTypeOfSymbolForUsage(member.symbol);
if (!typeResult) {
return undefined;
}
// Report inappropriate use of variables in type expressions.
if ((flags & MemberAccessFlags.TypeExpression) !== 0 && errorNode) {
typeResult.type = validateSymbolIsTypeExpression(
errorNode,
typeResult.type,
!!typeResult.includesVariableDecl
);
}
// If the type is a function or overloaded function, infer
// and cache the return type if necessary. This needs to be done
// prior to specializing.
inferReturnTypeIfNecessary(typeResult.type);
// Check for ambiguous accesses to attributes with generic types?
if (
errorNode &&
selfClass &&
isClass(selfClass) &&
member.isInstanceMember &&
isClass(member.unspecializedClassType) &&
(flags & MemberAccessFlags.DisallowGenericInstanceVariableAccess) !== 0 &&
requiresSpecialization(typeResult.type, { ignoreSelf: true, ignoreImplicitTypeArgs: true })
) {
const specializedType = partiallySpecializeType(
typeResult.type,
member.unspecializedClassType,
selfSpecializeClass(selfClass, /* overrideTypeArgs */ true)
);
if (
findSubtype(
specializedType,
(subtype) =>
!isFunction(subtype) &&
!isOverloadedFunction(subtype) &&
requiresSpecialization(subtype, { ignoreSelf: true, ignoreImplicitTypeArgs: true })
)
) {
addDiagnostic(
DiagnosticRule.reportGeneralTypeIssues,
LocMessage.genericInstanceVariableAccess(),
errorNode
);
}
}
return {
type: partiallySpecializeType(typeResult.type, member.classType, selfClass),
isIncomplete: !!typeResult.isIncomplete,
};
}
function assignClass(
@ -27059,8 +27071,17 @@ export function createTypeEvaluator(
}
function isLegalImplicitTypeAliasType(type: Type) {
// We explicitly exclude "Unknown" and "...".
if (isUnknown(type) || isEllipsisType(type)) {
// We explicitly exclude "..." and "Unknown".
if (isEllipsisType(type)) {
return false;
}
if (isUnknown(type)) {
// If this is a union type, we'll assume that it was meant as a type
// alias even though all of the union subtypes are Unknown.
if (type.specialForm && ClassType.isBuiltIn(type.specialForm, 'UnionType')) {
return true;
}
return false;
}

View File

@ -143,6 +143,11 @@ export const enum MemberAccessFlags {
// Report an error if a symbol is an instance variable whose
// type is parameterized by a class TypeVar.
DisallowGenericInstanceVariableAccess = 1 << 10,
// The member access should be treated as if it's within a type
// expression, and errors should be reported if it doesn't conform
// with type expression rules.
TypeExpression = 1 << 11,
}
export const enum ClassIteratorFlags {

View File

@ -14,8 +14,8 @@ class Foo:
pass
# This should generate an error because Foo is not an
# allowed literal value.
# This should generate two errors because Foo() is not a valid
# type expression, and Foo is not an allowed literal value.
a: Literal["hi", Foo()]
# This should generate an error because SomeEnum is not an

View File

@ -1,18 +1,19 @@
# This sample tests various illegal forms of Literal.
from enum import Enum
from pathlib import Path
from typing import Any, Literal, TypeVar
# This should generate an error.
# This should generate two errors.
Wrong1 = Literal[3 + 4]
# This should generate an error.
# This should generate two errors.
Wrong2 = Literal["foo".replace("o", "b")]
# This should generate an error.
# This should generate two errors.
Wrong3 = Literal[4 + 3j]
# This should generate an error.
# This should generate three errors.
Wrong4 = Literal[-4 + 2j]
# This should generate an error.
@ -21,7 +22,7 @@ Wrong5 = Literal[(1, "foo", "bar")]
# This should generate an error.
Wrong6 = Literal[{"a": "b", "c": "d"}]
# This should generate an error.
# This should generate two errors.
Wrong7 = Literal[Path("abcd")]
T = TypeVar("T")
@ -46,20 +47,20 @@ def func():
Wrong12 = Literal[func]
some_variable = "foo"
# This should generate an error.
# This should generate two errors.
Wrong13 = Literal[some_variable]
# This should generate an error.
# This should generate two errors.
var1: Literal[3 + 4]
# This should generate an error.
# This should generate two errors.
var2: Literal["foo".replace("o", "b")]
# This should generate an error.
# This should generate two errors.
var3: Literal[4 + 3j]
# This should generate an error.
# This should generate three errors.
var4: Literal[-4 + 2j]
# This should generate an error.
@ -68,7 +69,7 @@ var5: Literal[(1, "foo", "bar")]
# This should generate an error.
var6: Literal[{"a": "b", "c": "d"}]
# This should generate an error.
# This should generate two errors.
var7: Literal[Path("abcd")]
# This should generate two errors.
@ -86,5 +87,21 @@ var11: Literal[...]
# This should generate an error.
var12: Literal[func]
# This should generate an error.
# This should generate two errors.
var13: Literal[some_variable]
class Enum1(Enum):
A = 1
B = 2
x: str
a = Enum1.A
# This should generate two errors.
var14: Literal[a]
# This should generate two errors.
var15: Literal[Enum1.x]

View File

@ -10,8 +10,6 @@ GenericClass0 = list[GenericClass0 | int]
# refers to itself.
RecursiveUnion = Union["RecursiveUnion", int]
a1: RecursiveUnion = 3
# This should generate an error because the type alias refers
# to itself through a mutually-referential type alias.
MutualReference1 = Union["MutualReference2", int]

View File

@ -560,7 +560,7 @@ test('Literals2', () => {
test('Literals3', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['literals3.py']);
TestUtils.validateResults(analysisResults, 4);
TestUtils.validateResults(analysisResults, 5);
});
test('Literals4', () => {
@ -578,7 +578,7 @@ test('Literals5', () => {
test('Literals6', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['literals6.py']);
TestUtils.validateResults(analysisResults, 27);
TestUtils.validateResults(analysisResults, 45);
});
test('Literals7', () => {

View File

@ -832,7 +832,7 @@ test('Annotated1', () => {
configOptions.defaultPythonVersion = pythonVersion3_8;
const analysisResults38 = TestUtils.typeAnalyzeSampleFiles(['annotated1.py'], configOptions);
TestUtils.validateResults(analysisResults38, 28);
TestUtils.validateResults(analysisResults38, 32);
configOptions.defaultPythonVersion = pythonVersion3_11;
const analysisResults39 = TestUtils.typeAnalyzeSampleFiles(['annotated1.py'], configOptions);