fix: improve nullable handling (#15)

src/Riok.Mapperly.Abstractions/MapperAttribute.cs

@@ -28,6 +28,23 @@ public sealed class MapperAttribute : Attribute
     /// </summary>
     public bool EnumMappingIgnoreCase { get; set; }
 
+    /// <summary>
+    /// Specifies the behaviour in the case when the mapper tries to return <c>null</c> in a mapping method with a non-nullable return type.
+    /// If set to <c>true</c> an <see cref="ArgumentNullException"/> is thrown.
+    /// If set to <c>false</c> the mapper tries to return a default value.
+    /// For a <see cref="string"/> this is <see cref="string.Empty"/>,
+    /// for value types <c>default</c>
+    /// and for reference types <c>new()</c> if a parameterless constructor exists or else an <see cref="ArgumentNullException"/> is thrown.
+    /// </summary>
+    public bool ThrowOnMappingNullMismatch { get; set; } = true;
+
+    /// <summary>
+    /// Specifies the behaviour in the case when the mapper tries to set a non-nullable property to a <c>null</c> value.
+    /// If set to <c>true</c> an <see cref="ArgumentNullException"/> is thrown.
+    /// If set to <c>false</c> the property assignment is ignored.
+    /// </summary>
+    public bool ThrowOnPropertyMappingNullMismatch { get; set; }
+
     /// <summary>
     /// Whether to always deep copy objects.
     /// Eg. when the type <c>Person[]</c> should be mapped to the same type <c>Person[]</c>,

src/Riok.Mapperly/Descriptors/DescriptorBuilder.cs

@@ -14,6 +14,7 @@ public class DescriptorBuilder
 
     private static readonly IReadOnlyCollection<MappingBuilder> _mappingBuilders = new MappingBuilder[]
     {
+        NullableMappingBuilder.TryBuildMapping,
         SpecialTypeMappingBuilder.TryBuildMapping,
         DirectAssignmentMappingBuilder.TryBuildMapping,
         DictionaryMappingBuilder.TryBuildMapping,
@@ -40,7 +41,7 @@ public class DescriptorBuilder
     private readonly Dictionary<Type, Attribute> _defaultConfigurations = new();
 
     // this includes mappings to build and already built mappings
-    private readonly Dictionary<(ITypeSymbol SourceType, ITypeSymbol TargetType), TypeMapping> _mappings = new();
+    private readonly Dictionary<(ITypeSymbol SourceType, ITypeSymbol TargetType), TypeMapping> _mappings = new(new MappingTupleEqualityComparer());
 
     // additional user defined mappings
     // (with same signature as already defined mappings but with different names)
@@ -131,24 +132,47 @@ public MapperDescriptor Build()
             _mapperDescriptor.AddTypeMapping(extraMapping);
         }
 
+        // set method names
+        foreach (var methodMapping in _mapperDescriptor.MethodTypeMappings)
+        {
+            methodMapping.SetMethodNameIfNeeded(_methodNameBuilder.Build);
+        }
+
         return _mapperDescriptor;
     }
 
-    internal TypeMapping? FindOrBuildMapping(
+    public TypeMapping? FindOrBuildMapping(
         ITypeSymbol sourceType,
         ITypeSymbol targetType)
     {
         if (FindMapping(sourceType, targetType) is { } foundMapping)
             return foundMapping;
 
-        if (TryBuildNewMapping(null, sourceType, targetType) is not { } mapping)
+        if (BuildDelegateMapping(null, sourceType, targetType) is not { } mapping)
             return null;
 
         AddMapping(mapping);
         return mapping;
     }
 
-    public TypeMapping? TryBuildNewMapping(
+    public TypeMapping? FindMapping(ITypeSymbol sourceType, ITypeSymbol targetType)
+    {
+        _mappings.TryGetValue((sourceType, targetType), out var mapping);
+        return mapping;
+    }
+
+    public TypeMapping? FindOrBuildDelegateMapping(
+        ISymbol? userSymbol,
+        ITypeSymbol sourceType,
+        ITypeSymbol targetType)
+    {
+        if (FindMapping(sourceType, targetType) is { } foundMapping)
+            return foundMapping;
+
+        return BuildDelegateMapping(userSymbol, sourceType, targetType);
+    }
+
+    public TypeMapping? BuildDelegateMapping(
         ISymbol? userSymbol,
         ITypeSymbol sourceType,
         ITypeSymbol targetType)
@@ -171,11 +195,6 @@ internal void ReportDiagnostic(DiagnosticDescriptor descriptor, Location? locati
 
     private void BuildMappingBody(MappingBuilderContext ctx, TypeMapping typeMapping)
     {
-        if (typeMapping is not MethodMapping methodMapping)
-            return;
-
-        methodMapping.SetMethodNameIfNeeded(_methodNameBuilder.Build);
-
         switch (typeMapping)
         {
             case ObjectPropertyMapping mapping:
@@ -192,7 +211,7 @@ private void AddMapping(TypeMapping mapping)
 
     private void AddUserMapping(TypeMapping mapping)
     {
-        _methodNameBuilder.Add(((IUserMapping)mapping).Method.Name);
+        _methodNameBuilder.Reserve(((IUserMapping)mapping).Method.Name);
         if (mapping.CallableByOtherMappings && FindMapping(mapping.SourceType, mapping.TargetType) is null)
         {
             AddMapping(mapping);
@@ -202,9 +221,23 @@ private void AddUserMapping(TypeMapping mapping)
         _extraMappings.Add(mapping);
     }
 
-    private TypeMapping? FindMapping(ITypeSymbol sourceType, ITypeSymbol targetType)
+    private class MappingTupleEqualityComparer : IEqualityComparer<(ITypeSymbol Source, ITypeSymbol Target)>
     {
-        _mappings.TryGetValue((sourceType, targetType), out var mapping);
-        return mapping;
+        public bool Equals((ITypeSymbol Source, ITypeSymbol Target) x, (ITypeSymbol Source, ITypeSymbol Target) y)
+        {
+            return Equals(x.Source, y.Source)
+                && Equals(x.Target, y.Target);
+        }
+
+        public int GetHashCode((ITypeSymbol Source, ITypeSymbol Target) obj)
+        {
+            unchecked
+            {
+                return (SymbolEqualityComparer.Default.GetHashCode(obj.Source) * 397) ^ SymbolEqualityComparer.Default.GetHashCode(obj.Target);
+            }
+        }
+
+        private bool Equals(ITypeSymbol x, ITypeSymbol y)
+            => SymbolEqualityComparer.IncludeNullability.Equals(x, y);
     }
 }

src/Riok.Mapperly/Descriptors/MappingBuilder/CtorMappingBuilder.cs

@@ -1,5 +1,6 @@
 using Microsoft.CodeAnalysis;
 using Riok.Mapperly.Descriptors.TypeMappings;
+using Riok.Mapperly.Helpers;
 
 namespace Riok.Mapperly.Descriptors.MappingBuilder;
 
@@ -14,7 +15,8 @@ public static class CtorMappingBuilder
         var ctorMethod = namedTarget.InstanceConstructors
             .FirstOrDefault(m =>
                 m.Parameters.Length == 1
-                && SymbolEqualityComparer.Default.Equals(m.Parameters[0].Type, ctx.Source));
+                && SymbolEqualityComparer.Default.Equals(m.Parameters[0].Type, ctx.Source)
+                && ctx.Source.HasSameOrStricterNullability(m.Parameters[0].Type));
 
         return ctorMethod == null
             ? null

src/Riok.Mapperly/Descriptors/MappingBuilder/EnumMappingBuilder.cs

@@ -26,15 +26,15 @@ public static class EnumMappingBuilder
         }
 
         // since enums are immutable they can be directly assigned if they are of the same type
-        if (SymbolEqualityComparer.Default.Equals(ctx.Source, ctx.Target))
-            return new NullDelegateMapping(ctx.Source, ctx.Target, new DirectAssignmentMapping(ctx.Source.NonNullable()));
+        if (SymbolEqualityComparer.IncludeNullability.Equals(ctx.Source, ctx.Target))
+            return new DirectAssignmentMapping(ctx.Source);
 
         // map enums by strategy
         var config = ctx.GetConfigurationOrDefault<MapEnumAttribute>();
         return config.Strategy switch
         {
             EnumMappingStrategy.ByName => BuildNameMapping(ctx, config.IgnoreCase),
-            _ => new NullDelegateMapping(ctx.Source, ctx.Target, new CastMapping(ctx.Source.NonNullable(), ctx.Target.NonNullable())),
+            _ => new CastMapping(ctx.Source, ctx.Target),
         };
     }
 

src/Riok.Mapperly/Descriptors/MappingBuilder/NullableMappingBuilder.cs

@@ -0,0 +1,57 @@
+using Microsoft.CodeAnalysis;
+using Riok.Mapperly.Descriptors.TypeMappings;
+using Riok.Mapperly.Diagnostics;
+using Riok.Mapperly.Helpers;
+
+namespace Riok.Mapperly.Descriptors.MappingBuilder;
+
+public static class NullableMappingBuilder
+{
+    public static TypeMapping? TryBuildMapping(MappingBuilderContext ctx)
+    {
+        var sourceIsNullable = ctx.Source.TryGetNonNullable(out var sourceNonNullable);
+        var targetIsNullable = ctx.Target.TryGetNonNullable(out var targetNonNullable);
+        if (!sourceIsNullable && !targetIsNullable)
+            return null;
+
+        var delegateMapping = ctx.BuildDelegateMapping(sourceNonNullable ?? ctx.Source, targetNonNullable ?? ctx.Target);
+        return delegateMapping == null
+            ? null
+            : BuildNullDelegateMapping(ctx, delegateMapping);
+    }
+
+    private static TypeMapping BuildNullDelegateMapping(MappingBuilderContext ctx, TypeMapping mapping)
+    {
+        var nullFallback = GetNullFallbackValue(ctx);
+        return mapping switch
+        {
+            MethodMapping methodMapping => new NullDelegateMethodMapping(
+                ctx.Source,
+                ctx.Target,
+                methodMapping,
+                nullFallback),
+            _ => new NullDelegateMapping(ctx.Source, ctx.Target, mapping, nullFallback),
+        };
+    }
+
+    private static NullFallbackValue GetNullFallbackValue(MappingBuilderContext ctx)
+    {
+        if (ctx.Target.IsNullable())
+            return NullFallbackValue.Default;
+
+        if (ctx.MapperConfiguration.ThrowOnMappingNullMismatch)
+            return NullFallbackValue.ThrowArgumentNullException;
+
+        if (!ctx.Target.IsReferenceType || ctx.Target.IsNullable())
+            return NullFallbackValue.Default;
+
+        if (ctx.Target.SpecialType == SpecialType.System_String)
+            return NullFallbackValue.EmptyString;
+
+        if (ctx.Target.HasAccessibleParameterlessConstructor())
+            return NullFallbackValue.CreateInstance;
+
+        ctx.ReportDiagnostic(DiagnosticDescriptors.NoParameterlessConstructorFound, ctx.Target);
+        return NullFallbackValue.ThrowArgumentNullException;
+    }
+}

src/Riok.Mapperly/Descriptors/MappingBuilder/ObjectPropertyMappingBuilder.cs

@@ -16,7 +16,7 @@ public static class ObjectPropertyMappingBuilder
         if (ctx.Target.SpecialType != SpecialType.None || ctx.Source.SpecialType != SpecialType.None)
             return null;
 
-        return new NewInstanceObjectPropertyMapping(ctx.Source, ctx.Target);
+        return new NewInstanceObjectPropertyMapping(ctx.Source, ctx.Target.NonNullable());
     }
 
     public static void BuildMappingBody(MappingBuilderContext ctx, ObjectPropertyMapping mapping)
@@ -99,7 +99,7 @@ private static void AddUnmatchedIgnoredPropertiesDiagnostics(
             .FirstOrDefault(p => !p.IsStatic);
     }
 
-    private static PropertyMappingDescriptor? BuildPropertyMapping(
+    private static PropertyMapping? BuildPropertyMapping(
         MappingBuilderContext ctx,
         ObjectPropertyMapping mapping,
         IPropertySymbol sourceProperty,
@@ -111,26 +111,20 @@ private static void AddUnmatchedIgnoredPropertiesDiagnostics(
         if (sourceProperty.IsWriteOnly)
             return null;
 
-        var propertyMapping = ctx.FindOrBuildMapping(sourceProperty.Type.NonNullable(), targetProperty.Type.NonNullable());
-        if (propertyMapping == null)
-        {
-            ctx.ReportDiagnostic(
-                DiagnosticDescriptors.CouldNotMapProperty,
-                mapping.SourceType,
-                sourceProperty.Name,
-                sourceProperty.Type,
-                mapping.TargetType,
-                targetProperty.Name,
-                targetProperty.Type);
-            return null;
-        }
+        // nullability is handled inside the property mapping
+        var delegateMapping = ctx.FindMapping(sourceProperty.Type.UpgradeNullable(), targetProperty.Type.UpgradeNullable())
+            ?? ctx.FindOrBuildMapping(sourceProperty.Type.NonNullable(), targetProperty.Type.NonNullable());
+        if (delegateMapping != null)
+            return new PropertyMapping(sourceProperty, targetProperty, delegateMapping, ctx.MapperConfiguration.ThrowOnPropertyMappingNullMismatch);
 
-        var nullDelegateMapping = new NullDelegateMapping(
-            sourceProperty.IsNullable(),
-            targetProperty.IsNullable(),
+        ctx.ReportDiagnostic(
+            DiagnosticDescriptors.CouldNotMapProperty,
+            mapping.SourceType,
+            sourceProperty.Name,
             sourceProperty.Type,
-            targetProperty.Type,
-            propertyMapping);
-        return new PropertyMappingDescriptor(sourceProperty, targetProperty, nullDelegateMapping);
+            mapping.TargetType,
+            targetProperty.Name,
+            targetProperty.Type);
+        return null;
     }
 }

src/Riok.Mapperly/Descriptors/MappingBuilder/UserMethodMappingBuilder.cs

@@ -24,7 +24,7 @@ public static IEnumerable<TypeMapping> ExtractUserMappings(
 
     public static void BuildMappingBody(MappingBuilderContext ctx, UserDefinedNewInstanceMethodMapping mapping)
     {
-        mapping.DelegateMapping = ctx.TryBuildNewMapping(mapping.SourceType, mapping.TargetType);
+        mapping.DelegateMapping = ctx.BuildDelegateMapping(mapping.SourceType, mapping.TargetType);
         if (mapping.DelegateMapping == null)
         {
             ctx.ReportDiagnostic(
@@ -80,7 +80,7 @@ private static IEnumerable<IMethodSymbol> ExtractMethods(INamedTypeSymbol object
         // and is accessible it is a user implemented method mapping
         if (!methodSymbol.IsAbstract)
         {
-            return methodSymbol.IsAccessible()
+            return methodSymbol.IsAccessible(true)
                 ? new UserImplementedMethodMapping(methodSymbol)
                 : null;
         }

src/Riok.Mapperly/Descriptors/MappingBuilderContext.cs

@@ -26,6 +26,9 @@ public MappingBuilderContext(
 
     public ITypeSymbol Target { get; }
 
+    public TypeMapping? FindMapping(ITypeSymbol sourceType, ITypeSymbol targetType)
+        => _builder.FindMapping(sourceType, targetType);
+
     /// <summary>
     /// Tries to find an existing mapping for the provided types.
     /// If none is found, a new one is created.
@@ -41,16 +44,30 @@ public MappingBuilderContext(
     public TypeMapping? FindOrBuildMapping(ITypeSymbol sourceType, ITypeSymbol targetType)
         => _builder.FindOrBuildMapping(sourceType, targetType);
 
+    /// <summary>
+    /// Tries to find an existing mapping for the provided types.
+    /// If none is found, a new one is created.
+    /// If a new mapping is created, it is not added to the mapping descriptor (should only be used as a delegate to another mapping)
+    /// and is therefore not accessible by other mappings.
+    /// Configuration / the user symbol is passed from the caller.
+    /// </summary>
+    /// <param name="source">The source type.</param>
+    /// <param name="target">The target type.</param>
+    /// <returns>The created mapping or <c>null</c> if none could be created.</returns>
+    public TypeMapping? FindOrBuildDelegateMapping(ITypeSymbol source, ITypeSymbol target)
+        => _builder.FindOrBuildDelegateMapping(_userSymbol, source, target);
+
     /// <summary>
     /// Tries to build a new mapping for the given types.
-    /// The built mapping is not added to the mapping descriptor.
+    /// The built mapping is not added to the mapping descriptor (should only be used as a delegate to another mapping)
+    /// and is therefore not accessible by other mappings.
     /// Configuration / the user symbol is passed from the caller.
     /// </summary>
     /// <param name="source">The source type.</param>
     /// <param name="target">The target type.</param>
     /// <returns>The created mapping or <c>null</c> if none could be created.</returns>
-    public TypeMapping? TryBuildNewMapping(ITypeSymbol source, ITypeSymbol target)
-        => _builder.TryBuildNewMapping(_userSymbol, source, target);
+    public TypeMapping? BuildDelegateMapping(ITypeSymbol source, ITypeSymbol target)
+        => _builder.BuildDelegateMapping(_userSymbol, source, target);
 
     public T GetConfigurationOrDefault<T>() where T : Attribute
     {

src/Riok.Mapperly/Descriptors/MethodNameBuilder.cs

@@ -8,7 +8,7 @@ internal class MethodNameBuilder
     private const string MethodNamePrefix = "MapTo";
     private readonly HashSet<string> _usedNames = new();
 
-    internal void Add(string name)
+    internal void Reserve(string name)
         => _usedNames.Add(name);
 
     internal string Build(MethodMapping mapping)