microsoft/pyright
1
1
Fork 0
mirror of https://github.com/microsoft/pyright.git synced 2024-10-06 12:57:14 +03:00
Code Issues Projects Releases Wiki Activity

Fixed a bug that results in a false positive error during protocol matching because writable class variables defined in a named tuple or a frozen dataclass were considered read-only. This addresses #8829. (#8896)

Browse Source
This commit is contained in:
Eric Traut 2024-09-04 14:44:49 -07:00 committed by GitHub
parent 8ffc9cb20a
commit 9287bfa5f8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 81 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
packages/pyright-internal/src/analyzer/checker.ts
View File

@ -5029,6 +5029,7 @@ export class Checker extends ParseTreeWalker {
private _validateFinalMemberOverrides(classType: ClassType) {
ClassType.getSymbolTable(classType).forEach((localSymbol, name) => {
const parentSymbol = lookUpClassMember(classType, name, MemberAccessFlags.SkipOriginalClass);
if (parentSymbol && isInstantiableClass(parentSymbol.classType) && !SymbolNameUtils.isPrivateName(name)) {
// Did the parent class explicitly declare the variable as final?
if (this._evaluator.isFinalVariable(parentSymbol.symbol)) {
@ -5042,7 +5043,7 @@ export class Checker extends ParseTreeWalker {
decl.node
);
} else if (
ClassType.isReadOnlyInstanceVariables(parentSymbol.classType) &&
ClassType.hasNamedTupleEntry(parentSymbol.classType, name) &&
!SymbolNameUtils.isDunderName(name)
) {
// If the parent class is a named tuple, all instance variables
@ -5468,7 +5469,7 @@ export class Checker extends ParseTreeWalker {
// If this is part of a dataclass, a class handled by a dataclass_transform,
// or a NamedTuple, exempt it because the class variable will be transformed
// into an instance variable in this case.
if (ClassType.isDataClass(classType) || ClassType.isReadOnlyInstanceVariables(classType)) {
if (ClassType.isDataClass(classType) || ClassType.hasNamedTupleEntry(classType, name)) {
return true;
}

6
packages/pyright-internal/src/analyzer/dataClasses.ts
View File

@ -139,6 +139,7 @@ export function synthesizeDataClassMethods(
// entries added by this class.
const localDataClassEntries: DataClassEntry[] = [];
const fullDataClassEntries: DataClassEntry[] = [];
const namedTupleEntries = new Set<string>();
const allAncestorsKnown = addInheritedDataClassEntries(classType, fullDataClassEntries);
if (!allAncestorsKnown) {
@ -354,6 +355,7 @@ export function synthesizeDataClassMethods(
// Don't include class vars. PEP 557 indicates that they shouldn't
// be considered data class entries.
const variableSymbol = ClassType.getSymbolTable(classType).get(variableName);
namedTupleEntries.add(variableName);
if (variableSymbol?.isClassVar() && !variableSymbol?.isFinalVarInClassBody()) {
// If an ancestor class declared an instance variable but this dataclass
@ -490,7 +492,9 @@ export function synthesizeDataClassMethods(
}
});
if (!isNamedTuple) {
if (isNamedTuple) {
classType.shared.namedTupleEntries = namedTupleEntries;
} else {
classType.shared.dataClassEntries = localDataClassEntries;
}

6
packages/pyright-internal/src/analyzer/namedTuples.ts
View File

@ -57,6 +57,7 @@ export function createNamedTupleType(
): ClassType {
const fileInfo = getFileInfo(errorNode);
let className = 'namedtuple';
const namedTupleEntries = new Set<string>();
// The "rename" parameter is supported only in the untyped version.
let allowRename = false;
@ -117,7 +118,7 @@ export function createNamedTupleType(
ParseTreeUtils.getClassFullName(errorNode, fileInfo.moduleName, className),
fileInfo.moduleName,
fileInfo.fileUri,
ClassTypeFlags.ReadOnlyInstanceVariables | ClassTypeFlags.ValidTypeAliasClass,
ClassTypeFlags.ValidTypeAliasClass,
ParseTreeUtils.getTypeSourceId(errorNode),
/* declaredMetaclass */ undefined,
isInstantiableClass(namedTupleType) ? namedTupleType.shared.effectiveMetaclass : UnknownType.create()
@ -339,6 +340,7 @@ export function createNamedTupleType(
newSymbol.addDeclaration(declaration);
}
classFields.set(entryName, newSymbol);
namedTupleEntries.add(entryName);
});
// Set the type in the type cache for the dict node so it
@ -359,6 +361,8 @@ export function createNamedTupleType(
}
}
classType.shared.namedTupleEntries = namedTupleEntries;
if (addGenericGetAttribute) {
constructorType.shared.parameters = [];
FunctionType.addDefaultParams(constructorType);

11
packages/pyright-internal/src/analyzer/protocols.ts
View File

@ -490,11 +490,7 @@ function assignClassToProtocolInternal(
}
}
// Frozen dataclasses and named tuples should be treated as read-only.
if (
(ClassType.isDataClassFrozen(srcType) && srcMemberInfo.isInstanceMember) ||
ClassType.isReadOnlyInstanceVariables(srcType)
) {
if (srcMemberInfo.isReadOnly) {
isSrcReadOnly = true;
}
} else {
@ -718,10 +714,7 @@ function assignClassToProtocolInternal(
const isDestReadOnly = !!destPrimaryDecl.isConstant;
let isSrcReadOnly = !!srcPrimaryDecl.isConstant;
if (srcMemberInfo && isClass(srcMemberInfo.classType)) {
if (
ClassType.isReadOnlyInstanceVariables(srcMemberInfo.classType) ||
(ClassType.isDataClassFrozen(srcMemberInfo.classType) && srcMemberInfo.isInstanceMember)
) {
if (srcMemberInfo.isReadOnly) {
isSrcReadOnly = true;
}
}

11
packages/pyright-internal/src/analyzer/typeEvaluator.ts
View File

@ -315,6 +315,7 @@ import {
isLiteralLikeType,
isLiteralType,
isMaybeDescriptorInstance,
isMemberReadOnly,
isMetaclassInstance,
isNoneInstance,
isNoneTypeClass,
@ -6099,11 +6100,7 @@ export function createTypeEvaluator(
// Check for an attempt to overwrite or delete an instance variable that is
// read-only (e.g. in a named tuple).
if (
memberInfo?.isInstanceMember &&
isClass(memberInfo.classType) &&
ClassType.isReadOnlyInstanceVariables(memberInfo.classType)
) {
if (memberInfo?.isInstanceMember && isClass(memberInfo.classType) && memberInfo.isReadOnly) {
diag?.addMessage(LocAddendum.readOnlyAttribute().format({ name: memberName }));
isDescriptorError = true;
}
@ -17057,7 +17054,6 @@ export function createTypeEvaluator(
) {
if (ClassType.isBuiltIn(argType, 'NamedTuple')) {
isNamedTupleSubclass = true;
classType.shared.flags |= ClassTypeFlags.ReadOnlyInstanceVariables;
}
}
@ -23337,8 +23333,7 @@ export function createTypeEvaluator(
if (
primaryDecl?.type === DeclarationType.Variable &&
!isFinalVariableDeclaration(primaryDecl) &&
!ClassType.isReadOnlyInstanceVariables(destType) &&
!ClassType.isDataClassFrozen(destType)
!isMemberReadOnly(destType, name)
) {
// Class and instance variables that are mutable need to
// enforce invariance. We will exempt variables that are

27
packages/pyright-internal/src/analyzer/typeUtils.ts
View File

@ -92,6 +92,10 @@ export interface ClassMember {
// a type violation if it is overwritten by an instance variable
isClassVar: boolean;
// True if the member is read-only, such as with named tuples
// or frozen dataclasses.
isReadOnly: boolean;
// True if member has declared type, false if inferred
isTypeDeclared: boolean;
@ -1642,6 +1646,7 @@ export function getProtocolSymbolsRecursive(
isInstanceMember: symbol.isInstanceMember(),
isClassMember: symbol.isClassMember(),
isClassVar: isEffectivelyClassVar(symbol, /* isDataclass */ false),
isReadOnly: false,
isTypeDeclared: symbol.hasTypedDeclarations(),
skippedUndeclaredType: false,
});
@ -1782,6 +1787,7 @@ export function* getClassMemberIterator(
isClassVar: false,
classType,
unspecializedClassType: classType,
isReadOnly: false,
isTypeDeclared: false,
skippedUndeclaredType: false,
};
@ -1809,6 +1815,7 @@ export function* getClassMemberIterator(
isClassVar: isEffectivelyClassVar(symbol, ClassType.isDataClass(specializedMroClass)),
classType: specializedMroClass,
unspecializedClassType: mroClass,
isReadOnly: isMemberReadOnly(specializedMroClass, memberName),
isTypeDeclared: hasDeclaredType,
skippedUndeclaredType,
};
@ -1850,6 +1857,7 @@ export function* getClassMemberIterator(
isClassVar: isEffectivelyClassVar(symbol, isDataclass),
classType: specializedMroClass,
unspecializedClassType: mroClass,
isReadOnly: false,
isTypeDeclared: hasDeclaredType,
skippedUndeclaredType,
};
@ -1870,6 +1878,7 @@ export function* getClassMemberIterator(
isClassVar: false,
classType,
unspecializedClassType: classType,
isReadOnly: false,
isTypeDeclared: false,
skippedUndeclaredType: false,
};
@ -1879,6 +1888,23 @@ export function* getClassMemberIterator(
return undefined;
}
// Checks for whether the member is effectively read only because it
// belongs to a frozen dataclass or a named tuple.
export function isMemberReadOnly(classType: ClassType, name: string): boolean {
if (ClassType.hasNamedTupleEntry(classType, name)) {
return true;
}
if (ClassType.isDataClassFrozen(classType)) {
const dcEntries = classType.shared?.dataClassEntries;
if (dcEntries?.some((entry) => entry.name === name)) {
return true;
}
}
return false;
}
export function* getClassIterator(classType: Type, flags = ClassIteratorFlags.Default, skipMroClass?: ClassType) {
if (isClass(classType)) {
let foundSkipMroClass = skipMroClass === undefined;
@ -1946,6 +1972,7 @@ export function getClassFieldsRecursive(classType: ClassType): Map<string, Class
isInstanceMember: symbol.isInstanceMember(),
isClassMember: symbol.isClassMember(),
isClassVar: isEffectivelyClassVar(symbol, ClassType.isDataClass(specializedMroClass)),
isReadOnly: isMemberReadOnly(specializedMroClass, name),
isTypeDeclared: true,
skippedUndeclaredType: false,
});

17
packages/pyright-internal/src/analyzer/types.ts
View File

@ -634,10 +634,6 @@ export const enum ClassTypeFlags {
// Class is declared within a type stub file.
DefinedInStub = 1 << 18,
// Class does not allow writing or deleting its instance variables
// through a member access. Used with named tuples.
ReadOnlyInstanceVariables = 1 << 19,
// Decorated with @type_check_only.
TypeCheckOnly = 1 << 20,
@ -687,6 +683,7 @@ interface ClassDetailsShared {
docString?: string | undefined;
dataClassEntries?: DataClassEntry[] | undefined;
dataClassBehaviors?: DataClassBehaviors | undefined;
namedTupleEntries?: Set<string> | undefined;
typedDictEntries?: TypedDictEntries | undefined;
localSlotsNames?: string[];
@ -1252,10 +1249,6 @@ export namespace ClassType {
return !!(classType.shared.flags & ClassTypeFlags.TupleClass);
}
export function isReadOnlyInstanceVariables(classType: ClassType) {
return !!(classType.shared.flags & ClassTypeFlags.ReadOnlyInstanceVariables);
}
export function getTypeParams(classType: ClassType) {
return classType.shared.typeParams;
}
@ -1290,6 +1283,14 @@ export namespace ClassType {
);
}
export function hasNamedTupleEntry(classType: ClassType, name: string): boolean {
if (!classType.shared.namedTupleEntries) {
return false;
}
return classType.shared.namedTupleEntries.has(name);
}
// Same as isTypeSame except that it doesn't compare type arguments.
export function isSameGenericClass(classType: ClassType, type2: ClassType, recursionCount = 0) {
if (!classType.priv.isTypedDictPartial !== !type2.priv.isTypedDictPartial) {

1
packages/pyright-internal/src/commands/dumpFileDebugInfoCommand.ts
View File

@ -491,7 +491,6 @@ const ClassTypeFlagsToString: [ClassTypeFlags, string][] = [
[ClassTypeFlags.PropertyClass, 'PropertyClass'],
[ClassTypeFlags.ProtocolClass, 'ProtocolClass'],
[ClassTypeFlags.PseudoGenericClass, 'PseudoGenericClass'],
[ClassTypeFlags.ReadOnlyInstanceVariables, 'ReadOnlyInstanceVariables'],
[ClassTypeFlags.RuntimeCheckable, 'RuntimeCheckable'],
[ClassTypeFlags.SpecialBuiltIn, 'SpecialBuiltIn'],
[ClassTypeFlags.SupportsAbstractMethods, 'SupportsAbstractMethods'],

47
packages/pyright-internal/src/tests/samples/protocol3.py
View File

@ -112,14 +112,12 @@ _Self = TypeVar("_Self")
class Class5:
@property
def real(self: _Self) -> _Self:
...
def real(self: _Self) -> _Self: ...
class MockClass5(Protocol[_T_co]):
@property
def real(self) -> _T_co:
...
def real(self) -> _T_co: ...
foo5 = Class5()
@ -132,14 +130,12 @@ C6 = TypeVar("C6", bound="Class6")
class MockClass6(Protocol):
@property
def bar(self: P6) -> ContextManager[P6]:
...
def bar(self: P6) -> ContextManager[P6]: ...
class Class6:
@property
def bar(self: C6) -> ContextManager[C6]:
...
def bar(self: C6) -> ContextManager[C6]: ...
i: MockClass6 = Class6()
@ -160,8 +156,7 @@ a: Proto7 = Class7("")
class Proto8(Protocol):
@property
def x(self) -> str:
...
def x(self) -> str: ...
class Class8(NamedTuple):
@ -173,12 +168,10 @@ b: Proto8 = Class8("")
class Proto9(Protocol):
@property
def x(self) -> str:
...
def x(self) -> str: ...
@x.setter
def x(self, n: str) -> None:
...
def x(self, n: str) -> None: ...
class Proto10(Protocol):
@ -257,12 +250,10 @@ T13 = TypeVar("T13", covariant=True)
class Proto13(Protocol[T13]):
@property
def prop1(self) -> T13:
...
def prop1(self) -> T13: ...
class Proto14(Proto13[T13], Protocol):
...
class Proto14(Proto13[T13], Protocol): ...
class Concrete14(Generic[T13]):
@ -270,8 +261,7 @@ class Concrete14(Generic[T13]):
self.prop1 = val
def func14(val: Proto14[T13]):
...
def func14(val: Proto14[T13]): ...
func14(Concrete14(1))
@ -319,3 +309,20 @@ p15_4_2: DataclassInstance = Concrete15_4()
p15_5_1: Proto15 = Concrete15_5()
p15_5_2: DataclassInstance = Concrete15_5()
class Proto16(Protocol):
__name__: str
class Concrete16_1(NamedTuple):
other: int
@dataclass(frozen=True)
class Concrete16_2:
other: int
p16_1: Proto16 = Concrete16_1
p16_2: Proto16 = Concrete16_2