From dabc1931adaec40f9ef91860924f261b4bf3c9f4 Mon Sep 17 00:00:00 2001 From: Nicolas Schlecker Date: Tue, 2 May 2023 19:48:26 +0200 Subject: [PATCH] cleanup and bug fixes --- .../src/builder/value_assignment_builder.dart | 18 +++--- .../src/extensions/dart_object_extension.dart | 4 +- .../src/extensions/dart_type_extension.dart | 16 +++++- .../lib/src/models/type_conversion.dart | 56 ++++++++++++------- .../integration/fixture/type_converter.dart | 51 +++++++++-------- .../test/integration/type_converter_test.dart | 18 +++++- 6 files changed, 103 insertions(+), 60 deletions(-) diff --git a/packages/auto_mappr/lib/src/builder/value_assignment_builder.dart b/packages/auto_mappr/lib/src/builder/value_assignment_builder.dart index ef3e9b08..12b3e3a9 100644 --- a/packages/auto_mappr/lib/src/builder/value_assignment_builder.dart +++ b/packages/auto_mappr/lib/src/builder/value_assignment_builder.dart @@ -38,8 +38,6 @@ class ValueAssignmentBuilder { return fieldMapping.apply(assignment); } - final assignNestedObject = !assignment.targetType.isPrimitiveType; - if (assignment.shouldAssignIterable()) { return _assignIterableValue(assignment); } @@ -51,20 +49,12 @@ class ValueAssignmentBuilder { final rightSide = refer(sourceField.isStatic ? '${sourceField.enclosingElement.name}' : 'model').property(sourceField.name); - if (assignNestedObject) { return _assignNestedObject( source: assignment.sourceType!, target: assignment.targetType, assignment: assignment, convertMethodArgument: rightSide, ); - } - - if (fieldMapping?.hasWhenNullDefault() ?? false) { - return rightSide.ifNullThen(fieldMapping!.whenNullExpression!); - } - - return rightSide; } Expression _assignIterableValue(SourceAssignment assignment) { @@ -212,7 +202,13 @@ class ValueAssignmentBuilder { bool includeGenericTypes = false, }) { if (source.isSame(target)) { - return refer('model').property(assignment.sourceField!.displayName); + final expression = convertMethodArgument ?? refer('model').property(assignment.sourceField!.displayName); + + if(assignment.fieldMapping?.hasWhenNullDefault() ?? false) { + return expression.ifNullThen(assignment.fieldMapping!.whenNullExpression!); + } + + return expression; } final nestedMapping = mapperConfig.findMapping( diff --git a/packages/auto_mappr/lib/src/extensions/dart_object_extension.dart b/packages/auto_mappr/lib/src/extensions/dart_object_extension.dart index e99bf0d7..72ad9c73 100644 --- a/packages/auto_mappr/lib/src/extensions/dart_object_extension.dart +++ b/packages/auto_mappr/lib/src/extensions/dart_object_extension.dart @@ -49,8 +49,8 @@ extension DartObjectExtension on DartObject { final target = function.returnType; return TypeConversion( - source: source, - target: target, + sourceType: source, + targetType: target, convertExpression: refer(function.referCallString), field: getField('field')?.toStringValue(), ); diff --git a/packages/auto_mappr/lib/src/extensions/dart_type_extension.dart b/packages/auto_mappr/lib/src/extensions/dart_type_extension.dart index a972e25a..03813433 100644 --- a/packages/auto_mappr/lib/src/extensions/dart_type_extension.dart +++ b/packages/auto_mappr/lib/src/extensions/dart_type_extension.dart @@ -1,3 +1,4 @@ +import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/nullability_suffix.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:auto_mappr/src/extensions/element_extension.dart'; @@ -19,6 +20,7 @@ extension DartTypeExtension on DartType { bool isSame( DartType other, { bool withNullability = false, + bool allowSubtypes = false, }) { // Not the same type of type. if ((this is InterfaceType) ^ (other is InterfaceType)) { @@ -31,17 +33,25 @@ extension DartTypeExtension on DartType { // return element!.thisOrAncestorMatching((p0) => p0 == other.element) != null; // } - // Name matches. + // Type matches. final thisName = getDisplayString(withNullability: withNullability); final otherName = other.getDisplayString(withNullability: withNullability); - final isSameName = thisName == otherName; + var isTypeMatch = thisName == otherName; + + if(!isTypeMatch && allowSubtypes) { + if(other.element != null && other.element is InterfaceElement) { + final instanceOf = asInstanceOf(other.element! as InterfaceElement); + + isTypeMatch = instanceOf != null && (element?.library?.typeSystem.isAssignableTo(instanceOf, other) ?? false); + } + } // Library matches. final thisLibrary = element?.library?.identifier; final otherLibrary = other.element?.library?.identifier; final isSameLibrary = thisLibrary == otherLibrary; - final isSameExceptNullability = isSameName && isSameLibrary; + final isSameExceptNullability = isTypeMatch && isSameLibrary; if (!withNullability) { return isSameExceptNullability; diff --git a/packages/auto_mappr/lib/src/models/type_conversion.dart b/packages/auto_mappr/lib/src/models/type_conversion.dart index fa0dc08f..8bbc07d8 100644 --- a/packages/auto_mappr/lib/src/models/type_conversion.dart +++ b/packages/auto_mappr/lib/src/models/type_conversion.dart @@ -4,19 +4,20 @@ import 'package:auto_mappr/src/extensions/dart_type_extension.dart'; import 'package:auto_mappr/src/models/source_assignment.dart'; import 'package:code_builder/code_builder.dart'; import 'package:equatable/equatable.dart'; +import 'package:source_gen/source_gen.dart'; class TypeConversion extends Equatable { - final DartType source; - final DartType target; + final DartType sourceType; + final DartType targetType; final Expression convertExpression; final String? field; @override - List get props => [source, target, convertExpression, field]; + List get props => [sourceType, targetType, convertExpression, field]; const TypeConversion({ - required this.source, - required this.target, + required this.sourceType, + required this.targetType, required this.convertExpression, required this.field, }); @@ -24,29 +25,42 @@ class TypeConversion extends Equatable { /// Wether this type conversion applies to the the given [SourceAssignment]. bool matchesAssignment(SourceAssignment assignment) { final typeMatches = assignment.sourceType != null && - assignment.sourceType!.isSame(source) && - assignment.targetType.isSame(target); + assignment.sourceType!.isSame(sourceType, allowSubtypes: true) && + assignment.targetType.isSame(targetType); final nameMatches = field == null || assignment.sourceName == field; return typeMatches && nameMatches; } Expression apply(SourceAssignment assignment) { - if (assignment.sourceType?.nullabilitySuffix == NullabilitySuffix.question && - target.nullabilitySuffix == NullabilitySuffix.none) { - return refer('model') - .property(assignment.sourceName!) - .notEqualTo(literalNull) - .conditional( - convertExpression.call([ - refer('model').property(assignment.sourceName!).nullChecked, - ]), - literalNull, - ); + final assignmentSourceNullable = assignment.sourceType?.nullabilitySuffix != NullabilitySuffix.none; + final assignmentTargetNullable = assignment.targetType.nullabilitySuffix != NullabilitySuffix.none; + final sourceNullable = sourceType.nullabilitySuffix != NullabilitySuffix.none; + final targetNullable = targetType.nullabilitySuffix != NullabilitySuffix.none; + final whenNullExpression = assignment.fieldMapping?.whenNullExpression; + + final sourceExpression = refer('model').property(assignment.sourceName!); + + if((assignmentSourceNullable || targetNullable) && !assignmentTargetNullable && whenNullExpression == null) { + throw InvalidGenerationSourceError( + 'Cannot convert nullable source type to non-nullable target type requirement without a default value.', + element: assignment.sourceField, + ); } - return convertExpression.call([ - refer('model').property(assignment.sourceName!), - ]); + if(assignmentSourceNullable && !sourceNullable) { + return sourceExpression.notEqualTo(literalNull).conditional( + convertExpression.call([sourceExpression.nullChecked]), + whenNullExpression ?? literalNull, + ); + } else { + final expression = convertExpression.call([sourceExpression]); + + if(targetNullable) { + return expression.ifNullThen(whenNullExpression!); + } + + return expression; + } } } diff --git a/packages/auto_mappr/test/integration/fixture/type_converter.dart b/packages/auto_mappr/test/integration/fixture/type_converter.dart index d7c56a09..b52b24d1 100644 --- a/packages/auto_mappr/test/integration/fixture/type_converter.dart +++ b/packages/auto_mappr/test/integration/fixture/type_converter.dart @@ -37,36 +37,45 @@ class DateDto2 { }); } -class UserId extends Equatable { - final String value; +abstract class ValueId extends Equatable { + final T value; @override List get props => [value]; - const UserId(this.value); + const ValueId(this.value); + + static T toValue(ValueId id) => id.value; + + static String toValueString(ValueId id) => id.value; } -class AccountId extends Equatable { - final String value; +class UserId extends ValueId { + static const defaultUserId = UserId('defaultUserId'); - @override - List get props => [value]; + const UserId(super.value); - const AccountId(this.value); + static UserId? newNullable(String? value) => value == null ? null : UserId(value); +} + +class AccountId extends ValueId { + const AccountId(super.value); + + static AccountId? newNullable(String? value) => value == null ? null : AccountId(value); } class User { - final UserId? id; + final UserId id; final AccountId? accountId; - User(this.id, this.accountId); + const User(this.id, this.accountId); } class UserDto { final String? id; final String? accountId; - UserDto(this.id, this.accountId); + const UserDto(this.id, this.accountId); } @AutoMappr( @@ -85,13 +94,18 @@ class UserDto { MapType(), MapType( fields: [ - Field('id', type: UserId.new), + Field('id', type: UserId.newNullable, whenNull: UserId.defaultUserId), ], ), + MapType(), ], types: [ Mappr.parseISO8601, - TypeConverter(AccountId.new, field: 'accountId'), + UserId.new, + AccountId.new, + // This doesn't work currently, maybe analyzer bug? + ValueId.toValue, + ValueId.toValueString, ], ) class Mappr extends $Mappr { @@ -99,16 +113,14 @@ class Mappr extends $Mappr { /// /// Falls back to first of december in 1970 if parsing fails. static DateTime tryParseFirstOfDecemberInYear(String input) { - return int.tryParse(input)?.let((v) => DateTime(v, 12)) ?? - DateTime(1970, 12); + return DateTime(int.tryParse(input) ?? 1970, 12); } /// Expects a year as input and returns the second of december in that year. /// /// Falls back to second of december in 1970 if parsing fails. static DateTime parseSecondOfDecemberInYear(String input) { - return int.tryParse(input)?.let((v) => DateTime(v, 12, 2)) ?? - DateTime(1970, 12, 2); + return DateTime(int.tryParse(input) ?? 1970, 12, 2); } /// Expects a ISO8601 formatted string as input and returns a DateTime. @@ -118,8 +130,3 @@ class Mappr extends $Mappr { return DateTime.tryParse(input) ?? DateTime(1970); } } - -extension on T { - @pragma('vm:prefer-inline') - R let(R Function(T v) block) => block(this); -} diff --git a/packages/auto_mappr/test/integration/type_converter_test.dart b/packages/auto_mappr/test/integration/type_converter_test.dart index 08517e83..dc9ed937 100644 --- a/packages/auto_mappr/test/integration/type_converter_test.dart +++ b/packages/auto_mappr/test/integration/type_converter_test.dart @@ -35,10 +35,26 @@ void main() { }); test('nullable input type with non-nullable type converter input', () { - final userDto = fixture.UserDto('userId', 'accountId'); + const userDto = fixture.UserDto('userId', 'accountId'); final user = mappr.convert(userDto); expect(user.id, const fixture.UserId('userId')); expect(user.accountId, const fixture.AccountId('accountId')); }); + + test('converter allows subtypes as input', () { + const user = fixture.User(fixture.UserId('userId'), fixture.AccountId('accountId')); + final userDto = mappr.convert(user); + + expect(userDto.id, 'userId'); + expect(userDto.accountId, 'accountId'); + }); + + test('whenNull takes precedence', () { + const userDto = fixture.UserDto(null, null); + final user = mappr.convert(userDto); + + expect(user.id, fixture.UserId.defaultUserId); + expect(user.accountId, null); + }); }