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.

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

More copyWith fix #819

Merged
merged 2 commits into from Dec 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/freezed/CHANGELOG.md
@@ -1,3 +1,7 @@
# 2.3.2

Fix more issues with `copyWith`

# 2.3.1

Fix various issues with `copyWith`
Expand Down
120 changes: 68 additions & 52 deletions packages/freezed/lib/src/freezed_generator.dart
Expand Up @@ -247,11 +247,6 @@ Read here: https://github.com/rrousselGit/freezed/blob/master/packages/freezed/C
in constructorsNeedsGeneration.first.parameters.allParameters) {
final library = parameter.parameterElement!.library!;

// Whether a property was downcasted other than through a nullability change.
// Such as changing int -> num, but not int -> int?
var didNonNullDowncast = false;
var didDowncast = false;
var anyMatchingPropertyIsFinal = parameter.isFinal;
var commonTypeBetweenAllUnionConstructors =
parameter.parameterElement!.type;

Expand All @@ -262,54 +257,49 @@ Read here: https://github.com/rrousselGit/freezed/blob/master/packages/freezed/C
// be present in the abstract class.
if (matchingParameter == null) continue parameterLoop;

final parameterType = matchingParameter.parameterElement!.type;

anyMatchingPropertyIsFinal =
anyMatchingPropertyIsFinal || matchingParameter.isFinal;

final previousCommonType = commonTypeBetweenAllUnionConstructors;
commonTypeBetweenAllUnionConstructors =
library.typeSystem.leastUpperBound(
commonTypeBetweenAllUnionConstructors,
parameterType,
matchingParameter.parameterElement!.type,
);

// Checking the previous vs new common type isn't enough to determine
// whether a property was downcasted.
// Depending on the constructor order, the common type could already
// be low enough to apply too all properties.
// For example:
// factory Example.first(Object? a);
// factory Example.second(int a);

// At the same time, we can't just check the current property type with
// the current common type, as when the common type changes, it may
// match the current property type. For example:
// factory Example.first(int a);
// factory Example.second(Object? a);
if (previousCommonType != commonTypeBetweenAllUnionConstructors ||
parameterType != commonTypeBetweenAllUnionConstructors) {
didDowncast = true;

final nonNullCommonType = library.typeSystem
.promoteToNonNull(commonTypeBetweenAllUnionConstructors);

didNonNullDowncast = didNonNullDowncast ||
(previousCommonType != commonTypeBetweenAllUnionConstructors &&
previousCommonType != nonNullCommonType) ||
(parameterType != commonTypeBetweenAllUnionConstructors &&
parameterType != nonNullCommonType);
}
}

final matchingParameters = constructorsNeedsGeneration
.expand((element) => element.parameters.allParameters)
.where((element) => element.name == parameter.name)
.toList();

final isFinal = matchingParameters.any(
(element) =>
element.isFinal ||
element.parameterElement?.type !=
commonTypeBetweenAllUnionConstructors,
);

final nonNullableCommonType = library.typeSystem
.promoteToNonNull(commonTypeBetweenAllUnionConstructors);

final didDowncast = matchingParameters.any(
(element) =>
element.parameterElement?.type !=
commonTypeBetweenAllUnionConstructors,
);
final didNonNullDowncast = matchingParameters.any(
(element) =>
element.parameterElement?.type !=
commonTypeBetweenAllUnionConstructors &&
element.parameterElement?.type != nonNullableCommonType,
);
final didNullDowncast = !didNonNullDowncast && didDowncast;

final commonTypeString = resolveFullTypeStringFrom(
library,
commonTypeBetweenAllUnionConstructors,
withNullability: true,
);

final commonProperty = Property(
isFinal: anyMatchingPropertyIsFinal || didDowncast,
isFinal: isFinal,
type: commonTypeString,
isNullable: commonTypeBetweenAllUnionConstructors.isNullable,
isDartList: commonTypeBetweenAllUnionConstructors.isDartCoreList,
Expand All @@ -333,9 +323,30 @@ Read here: https://github.com/rrousselGit/freezed/blob/master/packages/freezed/C
// first union case.
// - num c is not allowed because num is not assignable int/double
if (!didNonNullDowncast) {
final copyWithType = didNullDowncast
? nonNullableCommonType
: commonTypeBetweenAllUnionConstructors;

result.cloneableProperties.add(
// Let's not downcast copyWith parameters
commonProperty.copyWith(type: parameter.type),
Property(
isFinal: isFinal,
type: resolveFullTypeStringFrom(
library,
copyWithType,
withNullability: true,
),
isNullable: copyWithType.isNullable,
isDartList: copyWithType.isDartCoreList,
isDartMap: copyWithType.isDartCoreMap,
isDartSet: copyWithType.isDartCoreSet,
isPossiblyDartCollection: copyWithType.isPossiblyDartCollection,
name: parameter.name,
decorators: parameter.decorators,
defaultValueSource: parameter.defaultValueSource,
doc: parameter.doc,
// TODO support JsonKey
hasJsonKey: false,
),
);
}
}
Expand Down Expand Up @@ -523,16 +534,21 @@ Read here: https://github.com/rrousselGit/freezed/blob/master/packages/freezed/C

Iterable<DeepCloneableProperty> _getCommonDeepCloneableProperties(
List<ConstructorDetails> constructors,
List<Property> commonProperties,
CommonProperties commonProperties,
) sync* {
for (final cloneableProperty in constructors.first.cloneableProperties) {
for (final commonProperty in commonProperties) {
if (cloneableProperty.name == commonProperty.name) {
yield cloneableProperty.copyWith(
nullable: commonProperty.isNullable,
);
}
}
for (final commonProperty in commonProperties.cloneableProperties) {
final commonGetter = commonProperties.readableProperties
.firstWhereOrNull((e) => e.name == commonProperty.name);
final deepCopyProperty = constructors.first.cloneableProperties
.firstWhereOrNull((e) => e.name == commonProperty.name);

if (deepCopyProperty == null || commonGetter == null) continue;

yield deepCopyProperty.copyWith(
nullable: deepCopyProperty.nullable ||
commonProperty.isNullable ||
commonGetter.isNullable,
);
}
}

Expand Down Expand Up @@ -691,7 +707,7 @@ Read here: https://github.com/rrousselGit/freezed/blob/master/packages/freezed/C
cloneableProperties: commonProperties.cloneableProperties,
deepCloneableProperties: _getCommonDeepCloneableProperties(
data.constructors,
commonProperties.cloneableProperties,
commonProperties,
).toList(),
genericsDefinition: data.genericsDefinitionTemplate,
genericsParameter: data.genericsParameterTemplate,
Expand Down
2 changes: 1 addition & 1 deletion packages/freezed/pubspec.yaml
Expand Up @@ -2,7 +2,7 @@ name: freezed
description: >
Code generation for immutable classes that has a simple syntax/API without
compromising on the features.
version: 2.3.1
version: 2.3.2
repository: https://github.com/rrousselGit/freezed
issue_tracker: https://github.com/rrousselGit/freezed/issues

Expand Down
45 changes: 43 additions & 2 deletions packages/freezed/test/integration/common_types.dart
Expand Up @@ -2,10 +2,51 @@ import 'package:freezed_annotation/freezed_annotation.dart';

part 'common_types.freezed.dart';

@freezed
class Union with _$Union {
factory Union.foo({int? arg}) = _UnionFoo;
factory Union.bar({required int arg}) = _UnionBar;
}

@freezed
class Union2 with _$Union2 {
factory Union2.foo({required int arg}) = _Union2Foo;
factory Union2.bar({double? arg}) = _Union2Bar;
}

@freezed
class Union3 with _$Union3 {
factory Union3.bar({double? arg}) = _Union3Bar;
factory Union3.foo({required int arg}) = _Union3Foo;
}

@freezed
class Union4 with _$Union4 {
factory Union4.eventOne({
required int count,
required String? id,
required String? name,
}) = Union4One;

factory Union4.eventTwo({
required int? count,
required String id,
required String name,
}) = Union4Two;
}

@freezed
class Union5 with _$Union5 {
factory Union5.first(int value) = _Union5First;
factory Union5.second(double? value) = _Union5Second;
factory Union5.third(String value) = _Union5Third;
}

@freezed
class UnionDeepCopy with _$UnionDeepCopy {
factory UnionDeepCopy.first(CommonSuperSubtype value) = _UnionWrapperFirst;
factory UnionDeepCopy.second(CommonSuperSubtype? value) = _UnionWrapperSecond;
factory UnionDeepCopy.first(CommonSuperSubtype value42) = _UnionWrapperFirst;
factory UnionDeepCopy.second(CommonSuperSubtype? value42) =
_UnionWrapperSecond;
}

@freezed
Expand Down