Fixed bug that results in an intermittent false positive "circular dependency" error for fields within a dataclass. This addresses #7516. (#7625)

This commit is contained in:
Eric Traut 2024-04-05 22:03:03 -07:00 committed by GitHub
parent 39dfca8acd
commit 4149eb3eda
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 87 additions and 37 deletions

View File

@ -3490,13 +3490,15 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
// Check for an attempt to write to an instance variable that is
// not defined by __slots__.
if (isThisClass && isInstanceMember) {
if (memberClass?.details.inheritedSlotsNames && memberClass.details.localSlotsNames) {
if (isThisClass && isInstanceMember && memberClass) {
const inheritedSlotsNames = ClassType.getInheritedSlotsNames(memberClass);
if (inheritedSlotsNames && memberClass.details.localSlotsNames) {
// Skip this check if the local slots is specified but empty because this pattern
// is used in a legitimate manner for mix-in classes.
if (
memberClass.details.localSlotsNames.length > 0 &&
!memberClass.details.inheritedSlotsNames.some((name) => name === memberName)
!inheritedSlotsNames.some((name) => name === memberName)
) {
// Determine whether the assignment corresponds to a descriptor
// that was assigned as a class variable. If so, then slots will not
@ -16186,6 +16188,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
// all of the type parameters in the specified order.
let genericTypeParameters: TypeVarType[] | undefined;
let protocolTypeParameters: TypeVarType[] | undefined;
let isNamedTupleSubclass = false;
const initSubclassArgs: FunctionArgument[] = [];
let metaclassNode: ExpressionNode | undefined;
@ -16279,6 +16282,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
// newer), it's considered a (read-only) dataclass.
if (fileInfo.executionEnvironment.pythonVersion.isGreaterOrEqualTo(pythonVersion3_6)) {
if (ClassType.isBuiltIn(argType, 'NamedTuple')) {
isNamedTupleSubclass = true;
classType.details.flags |=
ClassTypeFlags.DataClass |
ClassTypeFlags.SkipSynthesizedDataClassEq |
@ -16787,59 +16791,77 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
// See if there's already a non-synthesized __init__ method.
// We shouldn't override it.
if (!skipSynthesizedInit) {
const initSymbol = lookUpClassMember(classType, '__init__', MemberAccessFlags.SkipBaseClasses);
if (initSymbol) {
const initSymbol = classType.details.fields.get('__init__');
if (initSymbol && initSymbol.isClassMember()) {
hasExistingInitMethod = true;
}
}
let skipSynthesizeHash = false;
const hashSymbol = lookUpClassMember(classType, '__hash__', MemberAccessFlags.SkipBaseClasses);
const hashSymbol = classType.details.fields.get('__hash__');
// If there is a hash symbol defined in the class (i.e. one that we didn't
// synthesize above), then we shouldn't synthesize a new one for the dataclass.
if (hashSymbol && !hashSymbol.symbol.getSynthesizedType()) {
if (hashSymbol && hashSymbol.isClassMember() && !hashSymbol.getSynthesizedType()) {
skipSynthesizeHash = true;
}
synthesizeDataClassMethods(
evaluatorInterface,
node,
classType,
skipSynthesizedInit,
hasExistingInitMethod,
skipSynthesizeHash
);
const synthesizeMethods = () =>
synthesizeDataClassMethods(
evaluatorInterface,
node,
classType,
skipSynthesizedInit,
hasExistingInitMethod,
skipSynthesizeHash
);
// If this is a NamedTuple subclass, immediately synthesize dataclass methods
// because we also need to update the MRO classes in this case. For regular
// dataclasses, we'll defer the method synthesis to avoid circular dependencies.
if (isNamedTupleSubclass) {
synthesizeMethods();
} else {
classType.details.synthesizeMethodsDeferred = () => {
delete classType.details.synthesizeMethodsDeferred;
synthesizeMethods();
};
}
}
// Build a complete list of all slots names defined by the class hierarchy.
// This needs to be done after dataclass processing.
if (classType.details.localSlotsNames) {
let isLimitedToSlots = true;
const extendedSlotsNames = Array.from(classType.details.localSlotsNames);
classType.details.calculateInheritedSlotsNamesDeferred = () => {
delete classType.details.calculateInheritedSlotsNamesDeferred;
classType.details.baseClasses.forEach((baseClass) => {
if (isInstantiableClass(baseClass)) {
if (
!ClassType.isBuiltIn(baseClass, 'object') &&
!ClassType.isBuiltIn(baseClass, 'type') &&
!ClassType.isBuiltIn(baseClass, 'Generic')
) {
if (baseClass.details.inheritedSlotsNames === undefined) {
isLimitedToSlots = false;
} else {
appendArray(extendedSlotsNames, baseClass.details.inheritedSlotsNames);
if (classType.details.localSlotsNames) {
let isLimitedToSlots = true;
const extendedSlotsNames = Array.from(classType.details.localSlotsNames);
classType.details.baseClasses.forEach((baseClass) => {
if (isInstantiableClass(baseClass)) {
if (
!ClassType.isBuiltIn(baseClass, 'object') &&
!ClassType.isBuiltIn(baseClass, 'type') &&
!ClassType.isBuiltIn(baseClass, 'Generic')
) {
const inheritedSlotsNames = ClassType.getInheritedSlotsNames(baseClass);
if (inheritedSlotsNames) {
appendArray(extendedSlotsNames, inheritedSlotsNames);
} else {
isLimitedToSlots = false;
}
}
} else {
isLimitedToSlots = false;
}
} else {
isLimitedToSlots = false;
}
});
});
if (isLimitedToSlots) {
classType.details.inheritedSlotsNames = extendedSlotsNames;
if (isLimitedToSlots) {
classType.details.inheritedSlotsNamesCached = extendedSlotsNames;
}
}
}
};
// Update the undecorated class type.
writeTypeCache(node.name, { type: classType }, EvaluatorFlags.None);

View File

@ -615,7 +615,6 @@ interface ClassDetails {
dataClassEntries?: DataClassEntry[] | undefined;
dataClassBehaviors?: DataClassBehaviors | undefined;
typedDictEntries?: TypedDictEntries | undefined;
inheritedSlotsNames?: string[];
localSlotsNames?: string[];
// A cache of protocol classes (indexed by the class full name)
@ -635,6 +634,13 @@ interface ClassDetails {
// A cached value that indicates whether an instance of this class
// is hashable (i.e. does not override "__hash__" with None).
isInstanceHashable?: boolean;
// Callback for deferred synthesis of methods in symbol table.
synthesizeMethodsDeferred?: () => void;
// Callback for calculating inherited slots names.
calculateInheritedSlotsNamesDeferred?: () => void;
inheritedSlotsNamesCached?: string[];
}
export interface TupleTypeArgument {
@ -1136,6 +1142,8 @@ export namespace ClassType {
}
export function getDataClassEntries(classType: ClassType): DataClassEntry[] {
classType.details.synthesizeMethodsDeferred?.();
return classType.details.dataClassEntries || [];
}
@ -1172,9 +1180,21 @@ export namespace ClassType {
}
export function getSymbolTable(classType: ClassType) {
classType.details.synthesizeMethodsDeferred?.();
return classType.details.fields;
}
export function getInheritedSlotsNames(classType: ClassType) {
// First synthesize methods if needed. The slots entries
// can depend on synthesized methods.
classType.details.synthesizeMethodsDeferred?.();
classType.details.calculateInheritedSlotsNamesDeferred?.();
return classType.details.inheritedSlotsNamesCached;
}
// Similar to isPartiallyEvaluated except that it also looks at all of the
// classes in the MRO list for this class to see if any of them are still
// partially evaluated.

View File

@ -2,6 +2,7 @@
# circular type references within dataclass definitions.
from dataclasses import dataclass
from pathlib import Path
@dataclass
@ -20,3 +21,10 @@ class ClassB:
def method1(self):
ChildA(b=self)
@dataclass()
class ClassC:
name: str = "sample"
dir_a: Path = Path.home().joinpath(f"source/{name}")
dir_b: Path = dir_a.joinpath("path/to/b")