diff --git a/packages/freezed/lib/src/models.dart b/packages/freezed/lib/src/models.dart index 8adeca3e..56403668 100644 --- a/packages/freezed/lib/src/models.dart +++ b/packages/freezed/lib/src/models.dart @@ -547,7 +547,7 @@ class Class { final GenericsDefinitionTemplate genericsDefinitionTemplate; final GenericsParameterTemplate genericsParameterTemplate; final ConstructorInvocation? superCall; - final CopyWithTarget? copyWithTarget; + CopyWithTarget? copyWithTarget; final PropertyList properties; final ClassDeclaration _node; final Set parents = {}; @@ -610,45 +610,22 @@ class Class { ), ); + // Initial (local-only) copyWith target; rebuilt after superclass merge. final copyWithTarget = constructors.isNotEmpty ? null : declaration.copyWithTarget; - if (copyWithTarget != null) { - // Check for missing required parameters on the copyWith target - for (final param in copyWithTarget.parameters.parameters) { - if (param.isOptional) continue; - - final cloneableProperty = properties.cloneableProperties - .firstWhereOrNull((e) => e.name == param.name?.lexeme); - if (cloneableProperty == null) { - throw InvalidGenerationSourceError( - ''' -The class ${declaration.name.lexeme} requested a copyWith implementation, yet the parameter `${param.name}` is not cloneable. - -To fix, either: -- Disable copyWith using @Freezed(copyWith: false) -- Make `${param.name}` optional -- Make sure `this.${param.name}` is accessible from the copyWith method -''', - element: declaration.declaredFragment?.element, - node: declaration, - ); - } - } - } - - final copyWithInvocation = copyWithTarget == null + final initialCopyWithTarget = copyWithTarget == null ? null : CopyWithTarget( name: copyWithTarget.name?.lexeme, parameters: ParametersTemplate.fromParameterList( // Only include parameters that are cloneable - copyWithTarget.parameters.parameters.where((e) { - return properties.cloneableProperties - .map((e) => e.name) - .contains(e.name!.lexeme); - }), + copyWithTarget.parameters.parameters.where( + (parameter) => properties.cloneableProperties.any( + (p) => p.name == parameter.name!.lexeme, + ), + ), addImplicitFinal: configs.annotation.addImplicitFinal, ), ); @@ -670,7 +647,7 @@ To fix, either: return Class( node: declaration, name: declaration.name.lexeme, - copyWithTarget: copyWithInvocation, + copyWithTarget: initialCopyWithTarget, properties: properties, superCall: superCall, options: configs, @@ -720,6 +697,47 @@ To fix, either: library: library, ); + final probeConstructors = ConstructorDetails.parseAll( + declaration, + configs, + globalConfigs: globalConfigs, + unitsExcludingGeneratedFiles: unitsExcludingGeneratedFiles, + ); + + if (probeConstructors.isEmpty) { + final mergedProps = _collectMergedPropsForPrecheck( + declaration, + configs, + globalConfigs, + unitsExcludingGeneratedFiles, + ); + final target = declaration.copyWithTarget; + if (target != null) { + final cloneableNames = { + for (final p in mergedProps.cloneableProperties) p.name, + }; + for (final parameter in target.parameters.parameters) { + if (parameter.isOptional) continue; + final paramName = parameter.name?.lexeme; + if (paramName == null) continue; + if (!cloneableNames.contains(paramName)) { + throw InvalidGenerationSourceError( + ''' +The class ${declaration.name.lexeme} requested a copyWith implementation, yet the parameter `$paramName` is not cloneable. + +To fix, either: +- Disable copyWith using @Freezed(copyWith: false) +- Make `$paramName` optional +- Make sure `this.$paramName` is accessible from the copyWith method +''', + element: declaration.declaredFragment?.element, + node: declaration, + ); + } + } + } + } + return Class._from( declaration, configs, @@ -729,35 +747,142 @@ To fix, either: }); final classMap = {for (final c in classes) c.name: c}; - for (final clazz in classMap.values) { + for (final currentClass in classMap.values) { // If a Freezed class redirects to another Freezed class, mark it as a parent - for (final constructor in clazz.constructors) { + for (final constructor in currentClass.constructors) { if (constructor.isSynthetic) continue; - final target = classMap[constructor.redirectedName]; - if (target == null) continue; + final targetClass = classMap[constructor.redirectedName]; + if (targetClass == null) continue; - target.parents.add(clazz); + targetClass.parents.add(currentClass); } - // If a Freezed class extends another Freezed class, mark it as a parent - final superTypes = [ - if (clazz._node.extendsClause case final extend?) extend.superclass, - ...?clazz._node.implementsClause?.interfaces, - ...?clazz._node.withClause?.mixinTypes, + final superTypeNames = [ + if (currentClass._node.extendsClause case final extend?) + extend.superclass, + ...?currentClass._node.implementsClause?.interfaces, + ...?currentClass._node.withClause?.mixinTypes, ].map((e) => e.name2.lexeme); - for (final superType in superTypes) { - final superTypeClass = classMap[superType]; + for (final superTypeName in superTypeNames) { + final superTypeClass = classMap[superTypeName]; if (superTypeClass == null) continue; - clazz.parents.add(superTypeClass); + currentClass.parents.add(superTypeClass); } } + _mergeReadableAndCloneableFromSupers(classMap); + _rebuildCopyWithTargetsAndValidate(classMap); + return classMap.values; } + static bool _isAccessible( + String fieldName, + LibraryElement ownerLibrary, + LibraryElement userLibrary, + ) { + // Library-private identifiers start with '_' and cannot cross library boundaries. + return !(fieldName.startsWith('_') && ownerLibrary != userLibrary); + } + + static void _mergeReadableAndCloneableFromSupers( + Map classMap, + ) { + for (final currentClass in classMap.values) { + final currentDeclaration = currentClass._node; + final userLibrary = currentClass.library; + + final seenReadableNames = { + for (final p in currentClass.properties.readableProperties) p.name, + }; + final seenCloneableNames = { + for (final p in currentClass.properties.cloneableProperties) p.name, + }; + + var superName = currentDeclaration.extendsClause?.superclass.name.lexeme; + while (superName != null) { + final parentClass = classMap[superName]; + if (parentClass == null) break; + + final ownerLibrary = parentClass.library; + + // Merge readable so toString sees superclass fields + for (final superProperty in parentClass.properties.readableProperties) { + if (!_isAccessible(superProperty.name, ownerLibrary, userLibrary)) + continue; + if (seenReadableNames.add(superProperty.name)) { + currentClass.properties.readableProperties.add( + superProperty.copyWith(originClass: parentClass.name), + ); + } + } + + // Merge cloneable so copyWith can set superclass fields + for (final superProperty + in parentClass.properties.cloneableProperties) { + if (!_isAccessible(superProperty.name, ownerLibrary, userLibrary)) + continue; + if (seenCloneableNames.add(superProperty.name)) { + currentClass.properties.cloneableProperties.add( + superProperty.copyWith(originClass: parentClass.name), + ); + } + } + + superName = parentClass._node.extendsClause?.superclass.name.lexeme; + } + } + } + + static void _rebuildCopyWithTargetsAndValidate(Map classMap) { + for (final currentClass in classMap.values) { + // Unions don't use copyWithTarget here + final targetConstructor = currentClass.constructors.isNotEmpty + ? null + : currentClass._node.copyWithTarget; + if (targetConstructor == null) continue; + + final cloneableNames = { + for (final p in currentClass.properties.cloneableProperties) p.name, + }; + + // Validate: any required parameter must be cloneable (local or via super) + for (final parameter in targetConstructor.parameters.parameters) { + if (parameter.isOptional) continue; + final paramName = parameter.name?.lexeme; + if (paramName == null) continue; + if (!cloneableNames.contains(paramName)) { + throw InvalidGenerationSourceError( + ''' +The class ${currentClass.name} requested a copyWith implementation, yet the parameter `$paramName` is not cloneable. + +To fix, either: +- Disable copyWith using @Freezed(copyWith: false) +- Make `$paramName` optional +- Make sure `this.$paramName` is accessible from the copyWith method +''', + element: currentClass._node.declaredFragment?.element, + node: currentClass._node, + ); + } + } + + // Rebuild filtered parameter list using the merged cloneables + currentClass.copyWithTarget = CopyWithTarget( + name: targetConstructor.name?.lexeme, + parameters: ParametersTemplate.fromParameterList( + targetConstructor.parameters.parameters.where( + (e) => cloneableNames.contains(e.name!.lexeme), + ), + addImplicitFinal: currentClass.options.annotation.addImplicitFinal, + ), + ); + } + } + static Iterable _computeCloneableProperties( ClassDeclaration declaration, List constructorsNeedsGeneration, @@ -1109,6 +1234,110 @@ To fix, either: return '$escapedElementName$generics'; } + + static PropertyList _collectMergedPropsForPrecheck( + ClassDeclaration declaration, + ClassConfig configs, + Freezed globalConfigs, + List unitsExcludingGeneratedFiles, + ) { + final props = PropertyList(); + final userLibrary = declaration.declaredFragment!.element.library; + + final localConstructors = ConstructorDetails.parseAll( + declaration, + configs, + globalConfigs: globalConfigs, + unitsExcludingGeneratedFiles: unitsExcludingGeneratedFiles, + ); + + props.readableProperties.addAll( + _computeReadableProperties(declaration, localConstructors), + ); + props.cloneableProperties.addAll( + _computeCloneableProperties( + declaration, + localConstructors, + configs, + ).where( + (cloneable) => + props.readableProperties.any((e) => e.name == cloneable.name), + ), + ); + + final seenReadable = {for (final p in props.readableProperties) p.name}; + final seenCloneable = {for (final p in props.cloneableProperties) p.name}; + + var superName = declaration.extendsClause?.superclass.name.lexeme; + while (superName != null) { + final parentDecl = _findClassDeclaration( + unitsExcludingGeneratedFiles, + superName, + ); + if (parentDecl == null) break; + + final parentConstructors = ConstructorDetails.parseAll( + parentDecl, + configs, + globalConfigs: globalConfigs, + unitsExcludingGeneratedFiles: unitsExcludingGeneratedFiles, + ); + + final parentProps = PropertyList(); + parentProps.readableProperties.addAll( + _computeReadableProperties(parentDecl, parentConstructors), + ); + parentProps.cloneableProperties.addAll( + _computeCloneableProperties( + parentDecl, + parentConstructors, + configs, + ).where( + (cloneable) => parentProps.readableProperties.any( + (e) => e.name == cloneable.name, + ), + ), + ); + + final ownerLibrary = parentDecl.declaredFragment!.element.library; + + for (final sp in parentProps.readableProperties) { + if (!_isAccessible(sp.name, ownerLibrary, userLibrary)) continue; + if (seenReadable.add(sp.name)) { + props.readableProperties.add( + sp.copyWith(originClass: parentDecl.name.lexeme), + ); + } + } + for (final sp in parentProps.cloneableProperties) { + if (!_isAccessible(sp.name, ownerLibrary, userLibrary)) continue; + if (seenCloneable.add(sp.name)) { + props.cloneableProperties.add( + sp.copyWith(originClass: parentDecl.name.lexeme), + ); + } + } + + superName = parentDecl.extendsClause?.superclass.name.lexeme; + } + + return props; + } + + static ClassDeclaration? _findClassDeclaration( + List units, + String name, + ) { + for (final unit in units) { + for (final declaration in unit.declarations) { + if (declaration is ClassDeclaration && + declaration.name.lexeme == name) { + return declaration; + } + } + } + return null; + } } class PropertyList { diff --git a/packages/freezed/lib/src/templates/concrete_template.dart b/packages/freezed/lib/src/templates/concrete_template.dart index 19271f4a..e423a835 100644 --- a/packages/freezed/lib/src/templates/concrete_template.dart +++ b/packages/freezed/lib/src/templates/concrete_template.dart @@ -429,11 +429,18 @@ String operatorEqualMethod( }) { if (!data.options.equal) return ''; + // Only skip super-origin properties if super== is already emitted + final eqProperties = data.hasSuperEqual + ? properties.where( + (p) => p.originClass == null || p.originClass == data.name, + ) + : properties; + final comparisons = [ 'other.runtimeType == runtimeType', 'other is $className${data.genericsParameterTemplate}', if (data.hasSuperEqual) 'super == other', - ...properties.map((p) { + ...eqProperties.map((p) { var name = p.name; if (data.options.asUnmodifiableCollections && @@ -470,6 +477,13 @@ String hashCodeMethod( }) { if (!data.options.equal) return ''; + // Only skip super-origin properties if super.hashCode is already emitted + final hashProperties = data.hasSuperHashCode + ? properties.where( + (p) => p.originClass == null || p.originClass == data.name, + ) + : properties; + final jsonKey = data.options.fromJson || data.options.toJson ? '@JsonKey(includeFromJson: false, includeToJson: false)' : ''; @@ -477,7 +491,7 @@ String hashCodeMethod( final hashedProperties = [ 'runtimeType', if (data.hasSuperHashCode) 'super.hashCode', - for (final property in properties) + for (final property in hashProperties) if (property.type.isPossiblyDartCollection) if (data.options.asUnmodifiableCollections && source == Source.syntheticClass && diff --git a/packages/freezed/lib/src/templates/properties.dart b/packages/freezed/lib/src/templates/properties.dart index 863d74e0..5cb821c1 100644 --- a/packages/freezed/lib/src/templates/properties.dart +++ b/packages/freezed/lib/src/templates/properties.dart @@ -20,25 +20,31 @@ class Property { required this.doc, required this.isSynthetic, required this.isFinal, + this.originClass, }); - Property.fromParameter(Parameter p, {required bool isSynthetic}) - : this( - decorators: p.decorators, - name: p.name, - isFinal: p.isFinal, - doc: p.doc, - type: p.type, - typeDisplayString: p.typeDisplayString, - defaultValueSource: p.defaultValueSource, - isSynthetic: isSynthetic, - hasJsonKey: false, - ); + Property.fromParameter( + Parameter p, { + required bool isSynthetic, + String? originClass, + }) : this( + decorators: p.decorators, + name: p.name, + isFinal: p.isFinal, + doc: p.doc, + type: p.type, + typeDisplayString: p.typeDisplayString, + defaultValueSource: p.defaultValueSource, + isSynthetic: isSynthetic, + hasJsonKey: false, + originClass: originClass, + ); static Property fromFormalParameter( FormalParameter parameter, { required bool addImplicitFinal, required bool isSynthetic, + String? originClass, }) { final element = parameter.declaredFragment!.element; @@ -61,6 +67,7 @@ class Property { decorators: parseDecorators(element.metadata2.annotations), defaultValueSource: defaultValue, hasJsonKey: element.hasJsonKey, + originClass: originClass, ); } @@ -73,6 +80,7 @@ class Property { final String? defaultValueSource; final bool hasJsonKey; final String doc; + final String? originClass; @override String toString() { @@ -115,6 +123,7 @@ class Property { String? doc, bool? isPossiblyDartCollection, TypeParameterElement2? parameterElement, + String? originClass, }) { return Property( type: type ?? this.type, @@ -126,6 +135,7 @@ class Property { hasJsonKey: hasJsonKey ?? this.hasJsonKey, doc: doc ?? this.doc, isFinal: isFinal ?? this.isFinal, + originClass: originClass ?? this.originClass, ); } } diff --git a/packages/freezed/test/extend_test.dart b/packages/freezed/test/extend_test.dart index 1d625cff..87adfbbd 100644 --- a/packages/freezed/test/extend_test.dart +++ b/packages/freezed/test/extend_test.dart @@ -12,4 +12,159 @@ void main() { expect(Subclass2(1, 1).hashCode, isNot(Subclass2(1, 2).hashCode)); expect(Subclass2(2, 2).hashCode, isNot(Subclass2(1, 2).hashCode)); }); + + test('== calls super and compares child field; hashCode combines both', () { + final sameFieldsFirst = Subclass2(1, 2); + final sameFieldsSecond = Subclass2(1, 2); + + final differentSuperField = Subclass2(1, 3); + final differentChildField = Subclass2(9, 2); + + expect(sameFieldsFirst, sameFieldsSecond); + expect(sameFieldsFirst, isNot(differentSuperField)); + expect(sameFieldsFirst, isNot(differentChildField)); + + expect(sameFieldsFirst.hashCode, sameFieldsSecond.hashCode); + expect(sameFieldsFirst.hashCode == differentSuperField.hashCode, isFalse); + expect(sameFieldsFirst.hashCode == differentChildField.hashCode, isFalse); + }); + + test('== uses super== AND child field; hashCode uses child fields', () { + final equalFirst = Subclass3(1); + final equalSecond = Subclass3(1); + final differentChild = Subclass3(2); + + expect(equalFirst, equalSecond); + expect(equalFirst, isNot(differentChild)); + + expect(equalFirst.hashCode, equalSecond.hashCode); + expect(equalFirst.hashCode == differentChild.hashCode, isFalse); + }); + + test('copyWith/toString affect child field', () { + final original = Subclass3(3); + final updated = original.copyWith(value: 4); + expect(updated, Subclass3(4)); + expect(updated.toString(), contains('value: 4')); + }); + + test('== depends on child; hashCode includes super.hashCode and child', () { + final equalFirst = Subclass4(1); + final equalSecond = Subclass4(1); + final differentChild = Subclass4(2); + + expect(equalFirst, equalSecond); + expect(equalFirst, isNot(differentChild)); + + expect(equalFirst.hashCode, equalSecond.hashCode); + expect(equalFirst.hashCode == differentChild.hashCode, isFalse); + }); + + test('copyWith/toString affect child field', () { + final original = Subclass4(9); + final updated = original.copyWith(value: 11); + expect(updated, Subclass4(11)); + expect(updated.toString(), contains('value: 11')); + }); + + test( + 'copyWith includes root, mid, leaf, finalValue and updates independently', + () { + final initial = const SubclassFinal( + root: 1, + mid: 2, + leaf: 3, + finalValue: 4, + ); + final updatedRoot = initial.copyWith(root: 10); + final updatedMid = updatedRoot.copyWith(mid: 20); + final updatedLeaf = updatedMid.copyWith(leaf: 30); + final updatedFinalValue = updatedLeaf.copyWith(finalValue: 40); + + expect( + updatedRoot, + const SubclassFinal(root: 10, mid: 2, leaf: 3, finalValue: 4), + ); + expect( + updatedMid, + const SubclassFinal(root: 10, mid: 20, leaf: 3, finalValue: 4), + ); + expect( + updatedLeaf, + const SubclassFinal(root: 10, mid: 20, leaf: 30, finalValue: 4), + ); + expect( + updatedFinalValue, + const SubclassFinal(root: 10, mid: 20, leaf: 30, finalValue: 40), + ); + }, + ); + + test('toString shows root, mid, leaf, finalValue', () { + final stringified = const SubclassFinal( + root: 1, + mid: 2, + leaf: 3, + finalValue: 4, + ).toString(); + expect(stringified, contains('root: 1')); + expect(stringified, contains('mid: 2')); + expect(stringified, contains('leaf: 3')); + expect(stringified, contains('finalValue: 4')); + }); + + test('==/hashCode differentiate changes at any level', () { + final baseline = const SubclassFinal( + root: 1, + mid: 2, + leaf: 3, + finalValue: 4, + ); + expect( + baseline, + const SubclassFinal(root: 1, mid: 2, leaf: 3, finalValue: 4), + ); + expect( + baseline, + isNot(const SubclassFinal(root: 9, mid: 2, leaf: 3, finalValue: 4)), + ); + expect( + baseline, + isNot(const SubclassFinal(root: 1, mid: 9, leaf: 3, finalValue: 4)), + ); + expect( + baseline, + isNot(const SubclassFinal(root: 1, mid: 2, leaf: 9, finalValue: 4)), + ); + expect( + baseline, + isNot(const SubclassFinal(root: 1, mid: 2, leaf: 3, finalValue: 9)), + ); + + expect( + baseline.hashCode == + const SubclassFinal(root: 1, mid: 2, leaf: 3, finalValue: 4).hashCode, + isTrue, + ); + expect( + baseline.hashCode == + const SubclassFinal(root: 9, mid: 2, leaf: 3, finalValue: 4).hashCode, + isFalse, + ); + expect( + baseline.hashCode == + const SubclassFinal(root: 1, mid: 9, leaf: 3, finalValue: 4).hashCode, + isFalse, + ); + expect( + baseline.hashCode == + const SubclassFinal(root: 1, mid: 2, leaf: 9, finalValue: 4).hashCode, + isFalse, + ); + expect( + baseline.hashCode == + const SubclassFinal(root: 1, mid: 2, leaf: 3, finalValue: 9).hashCode, + isFalse, + ); + }); } diff --git a/packages/freezed/test/integration/extend.dart b/packages/freezed/test/integration/extend.dart index 81b084b8..8a4d2f63 100644 --- a/packages/freezed/test/integration/extend.dart +++ b/packages/freezed/test/integration/extend.dart @@ -67,3 +67,44 @@ class Subclass4 extends HashCodeWithoutEqual with _$Subclass4 { @override final int value; } + +@freezed +class SubclassRoot with _$SubclassRoot { + const SubclassRoot({required this.root}); + + @override + final int root; +} + +@freezed +class SubclassMid extends SubclassRoot with _$SubclassMid { + const SubclassMid({required super.root, required this.mid}) : super(); + + @override + final int mid; +} + +@freezed +class SubclassLeaf extends SubclassMid with _$SubclassLeaf { + const SubclassLeaf({ + required super.root, + required super.mid, + required this.leaf, + }) : super(); + + @override + final int leaf; +} + +@freezed +class SubclassFinal extends SubclassLeaf with _$SubclassFinal { + const SubclassFinal({ + required super.root, + required super.mid, + required super.leaf, + required this.finalValue, + }) : super(); + + @override + final int finalValue; +}