From f0c13cc55c5ea93c9686199d1e3d0dd2faad181b Mon Sep 17 00:00:00 2001 From: lbargaoanu Date: Fri, 16 Jun 2017 15:25:38 +0300 Subject: [PATCH 1/4] better generated code --- src/AutoMapper/Execution/TypeMapPlanBuilder.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/AutoMapper/Execution/TypeMapPlanBuilder.cs b/src/AutoMapper/Execution/TypeMapPlanBuilder.cs index fc1adb88d7..a8381dfd49 100644 --- a/src/AutoMapper/Execution/TypeMapPlanBuilder.cs +++ b/src/AutoMapper/Execution/TypeMapPlanBuilder.cs @@ -587,18 +587,21 @@ public static Expression NullCheckSource(ProfileMap profileMap, Expression sourc { var destinationType = destinationParameter.Type; var defaultDestination = DefaultDestination(destinationType, profileMap); - var ifSourceNull = destinationType.IsCollectionType() ? Block(ClearDestinationCollection(destinationParameter), defaultDestination) : defaultDestination; + var ifSourceNull = destinationType.IsCollectionType() ? ClearDestinationCollection(destinationParameter, defaultDestination) : defaultDestination; return sourceParameter.IfNullElse(ifSourceNull, objectMapperExpression); } - private static Expression ClearDestinationCollection(Expression destinationParameter) - { + private static Expression ClearDestinationCollection(Expression destinationParameter, Expression defaultDestination) + { var destinationElementType = ElementTypeHelper.GetElementType(destinationParameter.Type); var destinationCollectionType = typeof(ICollection<>).MakeGenericType(destinationElementType); + var destinationVariable = Variable(destinationCollectionType, "collectionDestination"); var clearMethod = destinationCollectionType.GetDeclaredMethod("Clear"); - var collection = ToType(destinationParameter, destinationCollectionType); - var clear = Condition(Property(collection, "IsReadOnly"), Empty(), Call(collection, clearMethod)); - return collection.IfNullElse(Empty(), clear); + var clear = Condition(Property(destinationVariable, "IsReadOnly"), Empty(), Call(destinationVariable, clearMethod)); + return Block(new[] { destinationVariable }, + Assign(destinationVariable, ToType(destinationParameter, destinationCollectionType)), + destinationVariable.IfNullElse(Empty(), clear), + defaultDestination); } private static Expression DefaultDestination(Type destinationType, ProfileMap profileMap) From fac3f58479f73b2cb75e6ddaffe21d5cc59f3b77 Mon Sep 17 00:00:00 2001 From: lbargaoanu Date: Fri, 16 Jun 2017 17:35:11 +0300 Subject: [PATCH 2/4] trying to be consistent --- .../Configuration/PrimitiveExtensions.cs | 3 + .../Execution/TypeMapPlanBuilder.cs | 85 ++++++++++++------- src/AutoMapper/Mappers/HashSetMapper.cs | 2 +- .../Mappers/NameValueCollectionMapperTests.cs | 8 +- src/UnitTests/NullBehavior.cs | 22 +++++ 5 files changed, 83 insertions(+), 37 deletions(-) diff --git a/src/AutoMapper/Configuration/PrimitiveExtensions.cs b/src/AutoMapper/Configuration/PrimitiveExtensions.cs index 06590292ed..e3cfa0310d 100644 --- a/src/AutoMapper/Configuration/PrimitiveExtensions.cs +++ b/src/AutoMapper/Configuration/PrimitiveExtensions.cs @@ -7,6 +7,9 @@ namespace AutoMapper.Configuration { internal static class PrimitiveExtensions { + public static bool IsSetType(this Type type) + => type.ImplementsGenericInterface(typeof(ISet<>)); + public static TValue GetOrDefault(this IDictionary dictionary, TKey key) => PrimitiveHelper.GetOrDefault(dictionary, key); diff --git a/src/AutoMapper/Execution/TypeMapPlanBuilder.cs b/src/AutoMapper/Execution/TypeMapPlanBuilder.cs index a8381dfd49..b979f81a1c 100644 --- a/src/AutoMapper/Execution/TypeMapPlanBuilder.cs +++ b/src/AutoMapper/Execution/TypeMapPlanBuilder.cs @@ -580,52 +580,71 @@ public static Expression MapExpression(IConfigurationProvider configurationProvi return ContextMap(typePair, sourceParameter, contextParameter, destinationParameter); } var objectMapperExpression = ObjectMapperExpression(configurationProvider, profileMap, typePair, sourceParameter, contextParameter, propertyMap, destinationParameter); - return NullCheckSource(profileMap, sourceParameter, destinationParameter, objectMapperExpression); + return NullCheckSource(profileMap, sourceParameter, destinationParameter, objectMapperExpression, propertyMap); } - public static Expression NullCheckSource(ProfileMap profileMap, Expression sourceParameter, Expression destinationParameter, Expression objectMapperExpression) + public static Expression NullCheckSource(ProfileMap profileMap, Expression sourceParameter, Expression destinationParameter, Expression objectMapperExpression, PropertyMap propertyMap = null) { var destinationType = destinationParameter.Type; - var defaultDestination = DefaultDestination(destinationType, profileMap); - var ifSourceNull = destinationType.IsCollectionType() ? ClearDestinationCollection(destinationParameter, defaultDestination) : defaultDestination; + Expression defaultDestination; + if(propertyMap == null) + { + defaultDestination = destinationParameter.IfNullElse(DefaultDestination(destinationType, profileMap), destinationParameter); + } + else + { + defaultDestination = propertyMap.UseDestinationValue ? destinationParameter : DefaultDestination(destinationType, profileMap); + } + var ifSourceNull = destinationType.IsCollectionType() ? ClearDestinationCollection() : defaultDestination; return sourceParameter.IfNullElse(ifSourceNull, objectMapperExpression); - } - - private static Expression ClearDestinationCollection(Expression destinationParameter, Expression defaultDestination) - { - var destinationElementType = ElementTypeHelper.GetElementType(destinationParameter.Type); - var destinationCollectionType = typeof(ICollection<>).MakeGenericType(destinationElementType); - var destinationVariable = Variable(destinationCollectionType, "collectionDestination"); - var clearMethod = destinationCollectionType.GetDeclaredMethod("Clear"); - var clear = Condition(Property(destinationVariable, "IsReadOnly"), Empty(), Call(destinationVariable, clearMethod)); - return Block(new[] { destinationVariable }, - Assign(destinationVariable, ToType(destinationParameter, destinationCollectionType)), - destinationVariable.IfNullElse(Empty(), clear), - defaultDestination); + Expression ClearDestinationCollection() + { + var destinationElementType = ElementTypeHelper.GetElementType(destinationParameter.Type); + var destinationCollectionType = typeof(ICollection<>).MakeGenericType(destinationElementType); + var destinationVariable = Variable(destinationCollectionType, "collectionDestination"); + var clearMethod = destinationCollectionType.GetDeclaredMethod("Clear"); + var clear = Condition(Property(destinationVariable, "IsReadOnly"), Empty(), Call(destinationVariable, clearMethod)); + return Block(new[] { destinationVariable }, + Assign(destinationVariable, ToType(destinationParameter, destinationCollectionType)), + destinationVariable.IfNullElse(Empty(), clear), + defaultDestination); + } } private static Expression DefaultDestination(Type destinationType, ProfileMap profileMap) { var defaultValue = Default(destinationType); - if(!profileMap.AllowNullCollections) + if(profileMap.AllowNullCollections) { - if(destinationType.IsArray) - { - var destinationElementType = ElementTypeHelper.GetElementType(destinationType); - return NewArrayBounds(destinationElementType, Enumerable.Repeat(Constant(0), destinationType.GetArrayRank())); - } - if(destinationType.IsDictionaryType()) - { - return destinationType.IsInterface() ? - DelegateFactory.GenerateNonNullConstructorExpression(typeof(Dictionary<,>).MakeGenericType(destinationType.GetGenericArguments())) : - defaultValue; - } - if(destinationType.IsCollectionType() && !destinationType.IsInterface()) - { - return DelegateFactory.GenerateNonNullConstructorExpression(destinationType); - } + return defaultValue; + } + if(destinationType.IsArray) + { + var destinationElementType = ElementTypeHelper.GetElementType(destinationType); + return NewArrayBounds(destinationElementType, Enumerable.Repeat(Constant(0), destinationType.GetArrayRank())); + } + if(destinationType.IsDictionaryType()) + { + return CreateCollection(typeof(Dictionary<,>)); + } + if(destinationType.IsSetType()) + { + return CreateCollection(typeof(HashSet<>)); + } + if(destinationType.IsCollectionType()) + { + return CreateCollection(typeof(List<>)); } return defaultValue; + Expression CreateCollection(Type collectionType) + { + var concreteDestinationType = + destinationType.IsInterface() ? + collectionType.MakeGenericType(destinationType.GetGenericArguments()) : + destinationType; + var constructor = DelegateFactory.GenerateNonNullConstructorExpression(concreteDestinationType); + return ToType(constructor, destinationType); + } } private static Expression ObjectMapperExpression(IConfigurationProvider configurationProvider, ProfileMap profileMap, TypePair typePair, Expression sourceParameter, Expression contextParameter, PropertyMap propertyMap, Expression destinationParameter) diff --git a/src/AutoMapper/Mappers/HashSetMapper.cs b/src/AutoMapper/Mappers/HashSetMapper.cs index 77f0a2400d..dd90437fde 100644 --- a/src/AutoMapper/Mappers/HashSetMapper.cs +++ b/src/AutoMapper/Mappers/HashSetMapper.cs @@ -16,6 +16,6 @@ public bool IsMatch(TypePair context) public Expression MapExpression(IConfigurationProvider configurationProvider, ProfileMap profileMap, PropertyMap propertyMap, Expression sourceExpression, Expression destExpression, Expression contextExpression) => MapCollectionExpression(configurationProvider, profileMap, propertyMap, sourceExpression, destExpression, contextExpression, IfNotNull, typeof(HashSet<>), MapItemExpr); - private static bool IsSetType(Type type) => type.ImplementsGenericInterface(typeof(ISet<>)); + private static bool IsSetType(Type type) => type.IsSetType(); } } \ No newline at end of file diff --git a/src/UnitTests/Mappers/NameValueCollectionMapperTests.cs b/src/UnitTests/Mappers/NameValueCollectionMapperTests.cs index 03d61625d9..4f69c77165 100644 --- a/src/UnitTests/Mappers/NameValueCollectionMapperTests.cs +++ b/src/UnitTests/Mappers/NameValueCollectionMapperTests.cs @@ -46,14 +46,16 @@ public void ReturnsIsFalseWhenSourceTypeIsNotNameValueCollection() public class Map { [Fact] - public void ReturnsNullIfSourceValueIsNull() + public void ReturnsTheDestinationWhenPassedOne() { var config = new MapperConfiguration(_ => { }); IMapper mapper = new Mapper(config); - var result = mapper.Map((NameValueCollection)null, new NameValueCollection()); + var destination = new NameValueCollection(); - result.ShouldBeNull(); + var result = mapper.Map((NameValueCollection)null, destination); + + result.ShouldBeSameAs(destination); } [Fact] diff --git a/src/UnitTests/NullBehavior.cs b/src/UnitTests/NullBehavior.cs index 0c0292774e..1ca23228d2 100644 --- a/src/UnitTests/NullBehavior.cs +++ b/src/UnitTests/NullBehavior.cs @@ -4,9 +4,31 @@ using Should; using Xunit; using System.Collections; +using System; namespace AutoMapper.UnitTests.NullBehavior { + public class AllowNullDestinationsButNotColections : AutoMapperSpecBase + { + class Source + { + public List Collection { get; set; } + } + + class Destination + { + public ICollection Collection { get; set; } + } + + protected override MapperConfiguration Configuration => new MapperConfiguration(cfg => cfg.CreateMap()); + + [Fact] + public void Should_map_to_non_null() + { + Mapper.Map(new Source()).Collection.ShouldNotBeNull(); + } + } + public class When_resolving_untyped_null : AutoMapperSpecBase { class Source From 4a1ea0f28175d5b5c390865ab079eee50f7cb4d3 Mon Sep 17 00:00:00 2001 From: lbargaoanu Date: Fri, 16 Jun 2017 17:38:36 +0300 Subject: [PATCH 3/4] cosmetic --- src/UnitTests/NullBehavior.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/UnitTests/NullBehavior.cs b/src/UnitTests/NullBehavior.cs index 1ca23228d2..ce92b3a9cd 100644 --- a/src/UnitTests/NullBehavior.cs +++ b/src/UnitTests/NullBehavior.cs @@ -8,7 +8,7 @@ namespace AutoMapper.UnitTests.NullBehavior { - public class AllowNullDestinationsButNotColections : AutoMapperSpecBase + public class When_mappping_null_list_to_ICollection : AutoMapperSpecBase { class Source { From 78bcb68eb2ecd010457dc88978e49aaf1e3f5847 Mon Sep 17 00:00:00 2001 From: lbargaoanu Date: Fri, 16 Jun 2017 17:50:30 +0300 Subject: [PATCH 4/4] prevent useless casts --- src/AutoMapper/Internal/ExpressionFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AutoMapper/Internal/ExpressionFactory.cs b/src/AutoMapper/Internal/ExpressionFactory.cs index 1f6151f0de..d6cc5f210f 100644 --- a/src/AutoMapper/Internal/ExpressionFactory.cs +++ b/src/AutoMapper/Internal/ExpressionFactory.cs @@ -139,7 +139,7 @@ public static Expression IfNotNull(Expression expression, Type destinationType) public static Expression IfNullElse(Expression expression, Expression then, Expression @else = null) { var isNull = expression.Type.IsValueType() && !expression.Type.IsNullableType() ? (Expression) Constant(false) : Equal(expression, Constant(null)); - return Condition(isNull, then, Convert(@else ?? Default(then.Type), then.Type)); + return Condition(isNull, then, ToType(@else ?? Default(then.Type), then.Type)); } internal class IfNotNullVisitor : ExpressionVisitor