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

feat: add common properties with different nullability to base mixin #740

Merged
merged 26 commits into from Nov 30, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
753c5a7
feat: add common properties with different nullability to base mixin
DevNico Aug 15, 2022
f7b3489
style: fix format
DevNico Aug 15, 2022
3de63e5
feat: common properties with different nullability are non-null in co…
DevNico Aug 16, 2022
78d8526
feat: properties with common super types have getters in base mixin
DevNico Aug 17, 2022
29012d0
feat: properties with common subtypes are available in copyWith
DevNico Aug 18, 2022
fe796dc
fix: fixed failing some failing cases
DevNico Aug 18, 2022
f4f7780
refactor: address pr comments
DevNico Aug 20, 2022
5fc858c
Remove dead code
rrousselGit Sep 27, 2022
5ea73b9
Remove more dead code
rrousselGit Sep 27, 2022
db001c0
check for correct getter types in tests
DevNico Sep 28, 2022
6d4ef80
Merge remote-tracking branch 'upstream/master'
DevNico Sep 28, 2022
98d294b
fix test for analyzer 5
DevNico Sep 28, 2022
0ea877a
small refactor
DevNico Sep 28, 2022
d52f6cb
chore: update sponsors.svg
github-actions[bot] Oct 2, 2022
230e586
Test pub downgrade via matrix (#776)
SunlightBro Oct 6, 2022
ac473e9
chore: Cache flutter sdk in the CI (#777)
SunlightBro Oct 7, 2022
94cae76
Merge remote-tracking branch 'upstream/master'
DevNico Oct 8, 2022
6f3bfe8
re-add expect_error dependency
DevNico Oct 8, 2022
400c517
Merge branch 'master' of https://github.com/rrousselGit/freezed into …
rrousselGit Nov 29, 2022
41aae94
Fix error
rrousselGit Nov 29, 2022
253ae77
Cleanup
rrousselGit Nov 29, 2022
eb731cc
Improve tests
rrousselGit Nov 29, 2022
4e798fb
Remove generated/typedef support
rrousselGit Nov 29, 2022
07e7ce2
Update tests
rrousselGit Nov 29, 2022
c578a5e
Fix tests and handle {int a}|{int? a}.copyWith({int a})
rrousselGit Nov 30, 2022
2f9c522
Remove dead code
rrousselGit Nov 30, 2022
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
86 changes: 51 additions & 35 deletions packages/freezed/lib/src/freezed_generator.dart
Expand Up @@ -128,27 +128,19 @@ class FreezedGenerator extends ParserGenerator<GlobalData, Data, Freezed> {
List<Property> _commonProperties(
List<ConstructorDetails> constructorsNeedsGeneration,
) {
final commonParameters =
_commonParametersBetweenAllConstructors(constructorsNeedsGeneration);

return [
for (final commonParameter in commonParameters)
Property(
decorators: commonParameter.decorators,
name: commonParameter.name,
isFinal: commonParameter.isFinal,
doc: commonParameter.doc,
type: commonParameter.type,
defaultValueSource: commonParameter.defaultValueSource,
isNullable: commonParameter.isNullable,
isDartList: commonParameter.isDartList,
isDartMap: commonParameter.isDartMap,
isDartSet: commonParameter.isDartSet,
isPossiblyDartCollection: commonParameter.isPossiblyDartCollection,
// TODO: support hasJsonKey
hasJsonKey: false,
),
];
return _commonParametersBetweenAllConstructors(
constructorsNeedsGeneration,
allowCommonSuperType: false,
).map(Property.fromParameter).toList();
}

List<Property> _commonGetters(
List<ConstructorDetails> constructorsNeedsGeneration,
) {
return _commonParametersBetweenAllConstructors(
constructorsNeedsGeneration,
allowCommonSuperType: true,
).map(Property.fromParameter).toList();
}

void _assertValidClassUsage(ClassElement element) {
Expand Down Expand Up @@ -248,26 +240,48 @@ Read here: https://github.com/rrousselGit/freezed/blob/master/packages/freezed/C
}

List<Parameter> _commonParametersBetweenAllConstructors(
List<ConstructorDetails> constructorsNeedsGeneration,
) {
List<ConstructorDetails> constructorsNeedsGeneration, {
required bool allowCommonSuperType,
}) {
return constructorsNeedsGeneration.first.parameters.allParameters
.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;
});
var anyMatchingPropertyIsFinal = false;
var isCommonWithDifferentNullability = false;

DartType? commonType;

for (final constructor in constructorsNeedsGeneration.skip(1)) {
final matchingParameter = constructor.parameters.allParameters
.firstWhereOrNull((p) => p.name == parameter.name);

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

if (hasAnyFinalProperty) {
return parameter.copyWith(isFinal: true);
final commonTypeWithMatch =
DevNico marked this conversation as resolved.
Show resolved Hide resolved
parameter.parameterElement!.library!.typeSystem.leastUpperBound(
parameter.parameterElement!.type,
DevNico marked this conversation as resolved.
Show resolved Hide resolved
matchingParameter.parameterElement!.type,
);

if (commonTypeWithMatch.isDartCoreObject) return null;
DevNico marked this conversation as resolved.
Show resolved Hide resolved

if (!allowCommonSuperType &&
commonTypeWithMatch.getDisplayString(withNullability: false) !=
parameter.type?.replaceAll(r'?$', '')) return null;
DevNico marked this conversation as resolved.
Show resolved Hide resolved

if (commonType != null && commonType != commonTypeWithMatch)
return null;

commonType = commonTypeWithMatch;
if (matchingParameter.isFinal) anyMatchingPropertyIsFinal = true;
if (parameter.isNullable != matchingParameter.isNullable)
isCommonWithDifferentNullability = true;
}

return parameter;
return parameter.copyWith(
isFinal: parameter.isFinal || anyMatchingPropertyIsFinal,
type: commonType?.getDisplayString(withNullability: true),
isCommonWithDifferentNullability: isCommonWithDifferentNullability,
);
})
.whereNotNull()
.toList();
Expand Down Expand Up @@ -311,7 +325,7 @@ Read here: https://github.com/rrousselGit/freezed/blob/master/packages/freezed/C
escapedName: _escapedName(element, constructor),
impliedProperties: [
for (final parameter in constructor.parameters)
await Property.fromParameter(
await Property.fromParameterElement(
parameter,
buildStep,
addImplicitFinal: options.addImplicitFinal,
Expand Down Expand Up @@ -595,6 +609,7 @@ Read here: https://github.com/rrousselGit/freezed/blob/master/packages/freezed/C
}

final commonProperties = _commonProperties(data.constructors);
final commonGetters = _commonGetters(data.constructors);

final commonCopyWith = !data.generateCopyWith
? null
Expand All @@ -613,6 +628,7 @@ Read here: https://github.com/rrousselGit/freezed/blob/master/packages/freezed/C
yield Abstract(
data: data,
copyWith: commonCopyWith,
commonGetters: commonGetters,
commonProperties: commonProperties,
);

Expand Down
4 changes: 3 additions & 1 deletion packages/freezed/lib/src/templates/abstract_template.dart
Expand Up @@ -9,15 +9,17 @@ class Abstract {
required this.data,
required this.copyWith,
required this.commonProperties,
required this.commonGetters,
});

final Data data;
final CopyWith? copyWith;
final List<Property> commonProperties;
final List<Property> commonGetters;

@override
String toString() {
final abstractProperties = commonProperties
final abstractProperties = commonGetters
.expand((e) => [
e.unimplementedGetter,
if (!e.isFinal) e.unimplementedSetter,
Expand Down
2 changes: 2 additions & 0 deletions packages/freezed/lib/src/templates/concrete_template.dart
Expand Up @@ -135,6 +135,8 @@ ${copyWith?.abstractCopyWithGetter ?? ''}
doc: '',
isPossiblyDartCollection: false,
showDefaultValue: false,
isCommonWithDifferentNullability: false,
parameterElement: null,
);

parameters = ParametersTemplate(
Expand Down
23 changes: 19 additions & 4 deletions packages/freezed/lib/src/templates/copy_with.dart
Expand Up @@ -80,6 +80,9 @@ ${_abstractDeepCopyMethods().join()}
type: e.type,
doc: e.doc,
isPossiblyDartCollection: e.isPossiblyDartCollection,
isCommonWithDifferentNullability:
e.isCommonWithDifferentNullability,
parameterElement: null,
);
}).toList(),
),
Expand Down Expand Up @@ -133,7 +136,11 @@ ${_deepCopyMethods().join()}
required List<Property> properties,
}) {
final parameters = properties.map((p) {
return '${p.decorators.join()} ${p.type} ${p.name}';
final type = p.isCommonWithDifferentNullability
? p.type.replaceFirst(RegExp(r'\?$'), '')
: p.type;

return '${p.decorators.join()} covariant $type ${p.name}';
}).join(',');

return _maybeOverride('''
Expand Down Expand Up @@ -197,10 +204,18 @@ $s''';
data.makeCollectionsImmutable) {
propertyName = '_$propertyName';
}
var ternary =
'${p.name} == freezed ? $accessor.$propertyName : ${p.name} ';
final type = p.isCommonWithDifferentNullability
? p.type!.replaceFirst(RegExp(r'\?$'), '')
: p.type;
var ternary = '${p.name} == freezed ? $accessor.$propertyName ';

if (p.isCommonWithDifferentNullability) {
ternary += 'as $type ';
DevNico marked this conversation as resolved.
Show resolved Hide resolved
}

ternary += ': ${p.name} ';
if (p.type != 'Object?' && p.type != null) {
ternary += _ignoreLints('as ${p.type}');
ternary += _ignoreLints('as $type');
}
return '$ternary,';
}
Expand Down
22 changes: 22 additions & 0 deletions packages/freezed/lib/src/templates/parameter_template.dart
Expand Up @@ -82,6 +82,8 @@ class ParametersTemplate {
doc: await documentationOfParameter(e, buildStep),
isPossiblyDartCollection: e.type.isPossiblyDartCollection,
showDefaultValue: true,
isCommonWithDifferentNullability: false,
parameterElement: e,
);

if (isAssignedToThis) return LocalParameter.fromParameter(value);
Expand Down Expand Up @@ -184,6 +186,8 @@ class Parameter {
required this.isFinal,
required this.isPossiblyDartCollection,
required this.showDefaultValue,
required this.isCommonWithDifferentNullability,
required this.parameterElement,
});

Parameter.fromParameter(Parameter p)
Expand All @@ -201,6 +205,8 @@ class Parameter {
showDefaultValue: p.showDefaultValue,
doc: p.doc,
isPossiblyDartCollection: p.isPossiblyDartCollection,
isCommonWithDifferentNullability: p.isCommonWithDifferentNullability,
parameterElement: p.parameterElement,
);

final String? type;
Expand All @@ -216,6 +222,8 @@ class Parameter {
final bool isPossiblyDartCollection;
final bool isFinal;
final String doc;
final bool isCommonWithDifferentNullability;
final ParameterElement? parameterElement;

Parameter copyWith({
String? type,
Expand All @@ -232,6 +240,7 @@ class Parameter {
bool? isDartMap,
bool? isDartSet,
bool? isFinal,
bool? isCommonWithDifferentNullability,
}) =>
Parameter(
type: type ?? this.type,
Expand All @@ -248,6 +257,9 @@ class Parameter {
isFinal: isFinal ?? this.isFinal,
isPossiblyDartCollection:
isPossiblyDartCollection ?? this.isPossiblyDartCollection,
isCommonWithDifferentNullability: isCommonWithDifferentNullability ??
this.isCommonWithDifferentNullability,
parameterElement: parameterElement,
);

@override
Expand Down Expand Up @@ -283,6 +295,8 @@ class LocalParameter extends Parameter {
required List<String> decorators,
required String doc,
required bool isPossiblyDartCollection,
required bool isCommonWithDifferentNullability,
required ParameterElement? parameterElement,
}) : super(
name: name,
type: type,
Expand All @@ -297,6 +311,8 @@ class LocalParameter extends Parameter {
defaultValueSource: defaultValueSource,
doc: doc,
isPossiblyDartCollection: isPossiblyDartCollection,
isCommonWithDifferentNullability: isCommonWithDifferentNullability,
parameterElement: parameterElement,
);

LocalParameter.fromParameter(Parameter p)
Expand All @@ -313,6 +329,8 @@ class LocalParameter extends Parameter {
decorators: p.decorators,
doc: p.doc,
isPossiblyDartCollection: p.isPossiblyDartCollection,
isCommonWithDifferentNullability: p.isCommonWithDifferentNullability,
parameterElement: p.parameterElement,
);

@override
Expand Down Expand Up @@ -346,6 +364,8 @@ class CallbackParameter extends Parameter {
required this.parameters,
required String doc,
required bool isPossiblyDartCollection,
required bool isCommonWithDifferentNullability,
required ParameterElement? parameterElement,
}) : super(
name: name,
type: type,
Expand All @@ -360,6 +380,8 @@ class CallbackParameter extends Parameter {
defaultValueSource: defaultValueSource,
doc: doc,
isPossiblyDartCollection: isPossiblyDartCollection,
isCommonWithDifferentNullability: isCommonWithDifferentNullability,
parameterElement: parameterElement,
);

final ParametersTemplate parameters;
Expand Down
33 changes: 32 additions & 1 deletion packages/freezed/lib/src/templates/properties.dart
Expand Up @@ -2,6 +2,7 @@ import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/nullability_suffix.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:build/build.dart';
import 'package:freezed/src/templates/parameter_template.dart';
import 'package:source_gen/source_gen.dart';

import '../utils.dart';
Expand Down Expand Up @@ -31,9 +32,30 @@ class Property {
required this.isDartMap,
required this.isDartSet,
required this.isPossiblyDartCollection,
required this.isCommonWithDifferentNullability,
required this.parameterElement,
}) : type = type ?? 'dynamic';

static Future<Property> fromParameter(
Property.fromParameter(Parameter p)
: this(
decorators: p.decorators,
name: p.name,
isFinal: p.isFinal,
doc: p.doc,
type: p.type,
defaultValueSource: p.defaultValueSource,
isNullable: p.isNullable,
isDartList: p.isDartList,
isDartMap: p.isDartMap,
isDartSet: p.isDartSet,
isPossiblyDartCollection: p.isPossiblyDartCollection,
// TODO: support hasJsonKey
hasJsonKey: false,
isCommonWithDifferentNullability: p.isCommonWithDifferentNullability,
parameterElement: p.parameterElement,
);

static Future<Property> fromParameterElement(
ParameterElement element,
BuildStep buildStep, {
required bool addImplicitFinal,
Expand All @@ -60,6 +82,8 @@ class Property {
defaultValueSource: defaultValue,
hasJsonKey: element.hasJsonKey,
isPossiblyDartCollection: element.type.isPossiblyDartCollection,
isCommonWithDifferentNullability: false,
parameterElement: element,
);
}

Expand All @@ -75,6 +99,8 @@ class Property {
final bool hasJsonKey;
final String doc;
final bool isPossiblyDartCollection;
final bool isCommonWithDifferentNullability;
final ParameterElement? parameterElement;

@override
String toString() {
Expand Down Expand Up @@ -135,6 +161,8 @@ class Property {
bool? hasJsonKey,
String? doc,
bool? isPossiblyDartCollection,
bool? isCommonWithDifferentNullability,
ParameterElement? parameterElement,
}) {
return Property(
type: type ?? this.type,
Expand All @@ -150,6 +178,9 @@ class Property {
isDartSet: isDartSet ?? this.isDartSet,
isPossiblyDartCollection:
isPossiblyDartCollection ?? this.isPossiblyDartCollection,
isCommonWithDifferentNullability: isCommonWithDifferentNullability ??
this.isCommonWithDifferentNullability,
parameterElement: parameterElement ?? this.parameterElement,
);
}
}
Expand Down
4 changes: 4 additions & 0 deletions packages/freezed/lib/src/templates/prototypes.dart
Expand Up @@ -127,6 +127,8 @@ String _mapPrototype(
// TODO: do we want to support freezed classes that implements MapView/ListView?
isPossiblyDartCollection: false,
showDefaultValue: false,
isCommonWithDifferentNullability: false,
parameterElement: null,
),
]);
},
Expand Down Expand Up @@ -191,6 +193,8 @@ String _unionPrototype(
defaultValueSource: '',
doc: '',
isPossiblyDartCollection: false,
isCommonWithDifferentNullability: false,
parameterElement: null,
);

if (constructor.isDefault) {
Expand Down