Merge pull request #2450 from AutoMapper/CollectionBug

Collection bug

src/AutoMapper/AutoMapper.csproj

@@ -3,7 +3,7 @@
   <PropertyGroup>
     <Summary>A convention-based object-object mapper</Summary>
     <Description>A convention-based object-object mapper. AutoMapper uses a fluent configuration API to define an object-object mapping strategy. AutoMapper uses a convention-based matching algorithm to match up source to destination values. Currently, AutoMapper is designed for model projection scenarios to flatten complex object models to DTOs and other simple objects, whose design is better suited for serialization, communication, messaging, or simply an anti-corruption layer between the domain and application layer.</Description>
-    <VersionPrefix>6.2.1</VersionPrefix>
+    <VersionPrefix>6.2.2</VersionPrefix>
     <Authors>Jimmy Bogard</Authors>
     <TargetFrameworks>netstandard1.3;netstandard1.1;net45;net40</TargetFrameworks>
     <NoWarn>$(NoWarn);1591</NoWarn>

src/AutoMapper/Execution/DelegateFactory.cs

@@ -1,11 +1,15 @@
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using System.Linq.Expressions;
+using AutoMapper.Configuration;
+using AutoMapper.Mappers.Internal;
 
 namespace AutoMapper.Execution
 {
     using static Expression;
-    using static ExpressionBuilder;
+    using static Internal.ExpressionFactory;
+    using static ElementTypeHelper;
 
     public static class DelegateFactory
     {
@@ -44,6 +48,15 @@ public static Expression GenerateConstructorExpression(Type type)
                 return Constant(null, typeof(string));
             }
 
+            if (type.IsInterface())
+            {
+                return type.ImplementsGenericInterface(typeof(IDictionary<,>))
+                    ? CreateCollection(type, typeof(Dictionary<,>))
+                    : (type.ImplementsGenericInterface(typeof(ICollection<>))
+                        ? CreateCollection(type, typeof(List<>))
+                        : InvalidType(type, $"Cannot create an instance of interface type {type}."));
+            }
+
             if (type.IsAbstract())
             {
                 return InvalidType(type, $"Cannot create an instance of abstract type {type}.");
@@ -68,6 +81,15 @@ public static Expression GenerateConstructorExpression(Type type)
             return New(ctorWithOptionalArgs, args);
         }
 
+        private static Expression CreateCollection(Type type, Type collectionType)
+        {
+            var listType = collectionType.MakeGenericType(GetElementTypes(type, ElementTypeFlags.BreakKeyValuePair));
+            if (type.IsAssignableFrom(listType))
+                return ToType(New(listType), type);
+
+            return InvalidType(type, $"Cannot create an instance of interface type {type}.");
+        }
+
         private static Expression InvalidType(Type type, string message)
         {
             var ex = new ArgumentException(message, "type");

src/AutoMapper/Mappers/Internal/CollectionMapperExpressionFactory.cs

@@ -9,8 +9,9 @@
 namespace AutoMapper.Mappers.Internal
 {
     using static Expression;
-    using static AutoMapper.Execution.ExpressionBuilder;
+    using static ExpressionBuilder;
     using static ExpressionFactory;
+    using static ElementTypeHelper;
 
     public static class CollectionMapperExpressionFactory
     {
@@ -22,7 +23,7 @@ public static Expression MapCollectionExpression(IConfigurationProvider configur
         {
             var passedDestination = Variable(destExpression.Type, "passedDestination");
             var newExpression = Variable(passedDestination.Type, "collectionDestination");
-            var sourceElementType = ElementTypeHelper.GetElementType(sourceExpression.Type);
+            var sourceElementType = GetElementType(sourceExpression.Type);
 
             var itemExpr = mapItem(configurationProvider, profileMap, sourceExpression.Type, passedDestination.Type,
                 contextExpression, out ParameterExpression itemParam);
@@ -33,7 +34,7 @@ public static Expression MapCollectionExpression(IConfigurationProvider configur
                 destinationCollectionType = typeof(IList);
             var addMethod = destinationCollectionType.GetDeclaredMethod("Add");
 
-            Expression destination, createInstance, assignNewExpression;
+            Expression destination, assignNewExpression;
 
             UseDestinationValue();
 
@@ -46,7 +47,7 @@ public static Expression MapCollectionExpression(IConfigurationProvider configur
                     Assign(passedDestination, destExpression),
                     assignNewExpression,
                     Call(destination, clearMethod),
-                    ToType(mapExpr, createInstance.Type)
+                    mapExpr
                 );
             if (propertyMap != null)
                 return checkNull;
@@ -61,14 +62,13 @@ void UseDestinationValue()
             {
                 if(propertyMap?.UseDestinationValue == true)
                 {
-                    createInstance = passedDestination;
                     destination = passedDestination;
                     assignNewExpression = Empty();
                 }
                 else
                 {
                     destination = newExpression;
-                    createInstance = passedDestination.Type.NewExpr(ifInterfaceType);
+                    Expression createInstance = passedDestination.Type.NewExpr(ifInterfaceType);
                     var isReadOnly = Property(ToType(passedDestination, destinationCollectionType), "IsReadOnly");
                     assignNewExpression = Assign(newExpression,
                         Condition(OrElse(Equal(passedDestination, Constant(null)), isReadOnly), ToType(createInstance, passedDestination.Type), passedDestination));
@@ -80,16 +80,16 @@ private static Expression NewExpr(this Type baseType, Type ifInterfaceType)
         {
             var newExpr = baseType.IsInterface()
                 ? New(
-                    ifInterfaceType.MakeGenericType(ElementTypeHelper.GetElementTypes(baseType,
+                    ifInterfaceType.MakeGenericType(GetElementTypes(baseType,
                         ElementTypeFlags.BreakKeyValuePair)))
                 : DelegateFactory.GenerateConstructorExpression(baseType);
             return newExpr;
         }
 
         public static Expression MapItemExpr(IConfigurationProvider configurationProvider, ProfileMap profileMap, Type sourceType, Type destType, Expression contextParam, out ParameterExpression itemParam)
         {
-            var sourceElementType = ElementTypeHelper.GetElementType(sourceType);
-            var destElementType = ElementTypeHelper.GetElementType(destType);
+            var sourceElementType = GetElementType(sourceType);
+            var destElementType = GetElementType(destType);
             itemParam = Parameter(sourceElementType, "item");
 
             var typePair = new TypePair(sourceElementType, destElementType);
@@ -100,8 +100,8 @@ public static Expression MapItemExpr(IConfigurationProvider configurationProvide
 
         public static Expression MapKeyPairValueExpr(IConfigurationProvider configurationProvider, ProfileMap profileMap, Type sourceType, Type destType, Expression contextParam, out ParameterExpression itemParam)
         {
-            var sourceElementTypes = ElementTypeHelper.GetElementTypes(sourceType, ElementTypeFlags.BreakKeyValuePair);
-            var destElementTypes = ElementTypeHelper.GetElementTypes(destType, ElementTypeFlags.BreakKeyValuePair);
+            var sourceElementTypes = GetElementTypes(sourceType, ElementTypeFlags.BreakKeyValuePair);
+            var destElementTypes = GetElementTypes(destType, ElementTypeFlags.BreakKeyValuePair);
 
             var typePairKey = new TypePair(sourceElementTypes[0], destElementTypes[0]);
             var typePairValue = new TypePair(sourceElementTypes[1], destElementTypes[1]);

src/UnitTests/CollectionMapping.cs

@@ -616,6 +616,52 @@ public void Should_not_replace_destination_list()
         }
     }
 
+    public class When_mapping_from_ICollection_types_but_implementations_are_different : AutoMapperSpecBase
+    {
+        public class Source
+        {
+            public ICollection<Item> Items { get; set; }
+
+            public class Item
+            {
+                public int Value { get; set; }
+            }
+        }
+        public class Dest
+        {
+            public ICollection<Item> Items { get; set; } = new HashSet<Item>();
+
+            public class Item
+            {
+                public int Value { get; set; }
+            }
+        }
+
+        protected override MapperConfiguration Configuration { get; } = new MapperConfiguration(cfg =>
+        {
+            cfg.CreateMap<Source, Dest>();
+            cfg.CreateMap<Source.Item, Dest.Item>();
+        });
+
+        [Fact]
+        public void Should_map_items()
+        {
+            var source = new Source
+            {
+                Items = new List<Source.Item>
+                {
+                    new Source.Item { Value = 5 }
+                }
+            };
+            var dest = new Dest();
+
+            Mapper.Map(source, dest);
+
+            dest.Items.Count.ShouldBe(1);
+            dest.Items.First().Value.ShouldBe(5);
+        }
+    }
+
     public class When_mapping_enumerable_to_array : AutoMapperSpecBase
     {
         public class Source