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

fixes #675 #699

Merged
merged 1 commit into from Jul 6, 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.0.4

Fixes a bug where using `@unfreezed` on unions somehow still generated immutable properties.

# 2.0.3+1

Update warning message when using abstract freezed classes
Expand Down
27 changes: 20 additions & 7 deletions packages/freezed/lib/src/freezed_generator.dart
Expand Up @@ -250,13 +250,26 @@ Read here: https://github.com/rrousselGit/freezed/blob/master/packages/freezed/C
List<ConstructorDetails> constructorsNeedsGeneration,
) {
return constructorsNeedsGeneration.first.parameters.allParameters
.where((parameter) {
return constructorsNeedsGeneration.every((constructor) {
return constructor.parameters.allParameters.any((p) {
return p.name == parameter.name && p.type == parameter.type;
});
});
}).toList();
.map((parameter) {
var hasAnyFinalProperty = false;
for (final constructor in constructorsNeedsGeneration) {
final matchingParameter =
constructor.parameters.allParameters.firstWhereOrNull((p) {
return p.name == parameter.name && p.type == parameter.type;
});

if (matchingParameter == null) return null;
if (matchingParameter.isFinal) hasAnyFinalProperty = true;
}

if (hasAnyFinalProperty) {
return parameter.copyWith(isFinal: true);
}

return parameter;
})
.whereNotNull()
.toList();
}

Future<List<ConstructorDetails>> _parseConstructorsNeedsGeneration(
Expand Down
14 changes: 8 additions & 6 deletions packages/freezed/lib/src/templates/concrete_template.dart
Expand Up @@ -394,12 +394,14 @@ ${whenOrNullPrototype(data.constructors)} {
}

String get _abstractProperties {
return constructor.impliedProperties.map((p) {
if (commonProperties.any((element) => element.name == p.name)) {
return '@override ${p.unimplementedGetter}';
} else {
return '${p.unimplementedGetter}';
}
return constructor.impliedProperties.expand((p) {
return [
if (commonProperties.any((element) => element.name == p.name))
'@override ${p.abstractGetter}'
else
'${p.abstractGetter}',
if (!p.isFinal) p.abstractSetter,
];
}).join();
}

Expand Down
28 changes: 23 additions & 5 deletions packages/freezed/lib/src/templates/properties.dart
Expand Up @@ -90,6 +90,14 @@ class Property {
body: ' => throw $privConstUsedErrorVarName;',
);

Getter get abstractGetter => Getter(
name: name,
type: type,
decorators: decorators,
doc: doc,
body: ';',
);

Getter asGetter(String body) => Getter(
name: name,
type: type,
Expand All @@ -98,11 +106,20 @@ class Property {
body: body,
);

UnimplementedSetter get unimplementedSetter => UnimplementedSetter(
Setter get unimplementedSetter => Setter(
name: name,
type: type,
decorators: decorators,
doc: doc,
body: ' => throw $privConstUsedErrorVarName;',
);

Setter get abstractSetter => Setter(
name: name,
type: type,
decorators: decorators,
doc: doc,
body: ';',
);

Property copyWith({
Expand Down Expand Up @@ -158,23 +175,24 @@ class Getter {
}
}

class UnimplementedSetter {
UnimplementedSetter({
class Setter {
Setter({
required String? type,
required this.name,
required this.decorators,
required this.doc,
required this.body,
}) : type = type ?? 'dynamic';

final String type;
final String name;
final List<String> decorators;
final String doc;
final String body;

@override
String toString() {
return '$doc${decorators.join()} set $name($type value) => '
'throw $privConstUsedErrorVarName;';
return '$doc${decorators.join()} set $name($type value)$body';
}
}

Expand Down
21 changes: 21 additions & 0 deletions packages/freezed/test/integration/multiple_constructors.dart
Expand Up @@ -2,6 +2,27 @@ import 'package:freezed_annotation/freezed_annotation.dart';

part 'multiple_constructors.freezed.dart';

@unfreezed
class UnfreezedImmutableUnion with _$UnfreezedImmutableUnion {
factory UnfreezedImmutableUnion(final String a) =
DirectUnfreezedImmutableUnion;
factory UnfreezedImmutableUnion.named(String a) =
DirectUnfreezedImmutableUnionNamed;
}

@unfreezed
class UnfreezedImmutableUnion2 with _$UnfreezedImmutableUnion2 {
factory UnfreezedImmutableUnion2(String a) = DirectUnfreezedImmutableUnion2;
factory UnfreezedImmutableUnion2.named(final String a) =
DirectUnfreezedImmutableUnionNamed2;
}

@unfreezed
class MutableUnion with _$MutableUnion {
factory MutableUnion(String a, int b) = MutableUnion0;
factory MutableUnion.named(String a, int c) = MutableUnion1;
}

@freezed
class DefaultValueNamedConstructor with _$DefaultValueNamedConstructor {
factory DefaultValueNamedConstructor.a([@Default(42) int value]) =
Expand Down
41 changes: 41 additions & 0 deletions packages/freezed/test/multiple_constructors_test.dart
Expand Up @@ -151,6 +151,47 @@ void main() {
expect(errorResult.errors, isEmpty);
});

test('can mutate unfreezed unions', () {
final unionFirst = MutableUnion0('a', 42);
final unionSecond = MutableUnion1('a', 42);
final MutableUnion downcast = unionFirst;

unionFirst
..a = 'b'
..b = 21;

unionSecond
..a = 'b'
..c = 21;

downcast.a = 'b';
});

test(
'cannot mutate shared property if one of the union has an immutable variant',
() async {
DirectUnfreezedImmutableUnionNamed('a').a = '';
DirectUnfreezedImmutableUnion2('a').a = '';

await expectLater(compile(r'''
import 'multiple_constructors.dart';

void main() {
UnfreezedImmutableUnion('').a;
UnfreezedImmutableUnion2('').a;
}
'''), completes);

await expectLater(compile(r'''
import 'multiple_constructors.dart';

void main() {
UnfreezedImmutableUnion('').a = '';
UnfreezedImmutableUnion2('').a = '';
}
'''), throwsCompileError);
});

test('when works on unnamed constructors', () {
expect(RequiredParams(a: 'a').when((a) => 21, second: (_) => 42), 21);
expect(
Expand Down