fix: Add null checks when mapping nullable sources to non-nullable targets with MayBeNull Attribute

docs/docs/breaking-changes/5-0.md

@@ -17,7 +17,8 @@ description: How to upgrade to Mapperly 5.0 and a list of all its breaking chang
 - `Stack<T>` deep cloning now preserves the order of elements by default.
 - Inaccessible members from other assemblies are now included in the mapping process if `IncludedMembers` or `IncludedConstructors` options are configured to include them.
 - Constructor mappings with nullable source types no longer generate null guards when the constructor parameter accepts null, see below.
-- `MaybeNull` attributes are now respected for fields, properties and constructor parameters.
+- `MaybeNull` attributes are now respected for fields, properties and constructor parameters when reading values.
+- `AllowNull` attributes are now respected for fields, properties and constructor parameters when writing values.
 - Copy constructors are no longer selected over constructors annotated with `[MapperConstructor]` or when `[MapProperty]` configurations are present.
 
 ## Inaccessible members from other assemblies included

src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersContainerBuilderContext.cs

@@ -35,7 +35,7 @@ public void AddNullDelegateMemberAssignmentMapping(IMemberAssignmentMapping memb
         // set target member to null if null assignments are allowed
         // and the source is null
         var setMemberToNull =
-            BuilderContext.Configuration.Mapper.AllowNullPropertyAssignment && memberMapping.MemberInfo.TargetMember.Member.IsNullable;
+            BuilderContext.Configuration.Mapper.AllowNullPropertyAssignment && memberMapping.MemberInfo.TargetMember.Member.IsWriteNullable;
 
         // if the member is explicitly set to null,
         // make sure the parent members are initialized/non-null,
@@ -50,7 +50,7 @@ public void AddNullDelegateMemberAssignmentMapping(IMemberAssignmentMapping memb
 
         var nullConditionSourcePath = new NonEmptyMemberPath(
             memberMapping.MemberInfo.SourceMember.MemberPath.RootType,
-            memberMapping.MemberInfo.SourceMember.MemberPath.PathWithoutTrailingNonNullable().ToList()
+            memberMapping.MemberInfo.SourceMember.MemberPath.ReadPathWithoutTrailingNonNullable().ToList()
         );
         var container = GetOrCreateNullDelegateMappingForPath(nullConditionSourcePath);
         AddMemberAssignmentMapping(container, memberMapping);
@@ -76,15 +76,15 @@ private void AddMemberAssignmentMapping(IMemberAssignmentMappingContainer contai
 
         // if the source value is a non-nullable value,
         // the target should be non-null after this assignment and can be set as initialized.
-        if (!mapping.MemberInfo.IsSourceNullable && mapping.MemberInfo.TargetMember.MemberType.IsNullable())
+        if (!mapping.MemberInfo.IsSourceNullable && mapping.MemberInfo.TargetMember.Member.IsWriteNullable)
         {
             _initializedNullableTargetPaths.Add((container, mapping.MemberInfo.TargetMember));
         }
     }
 
     private void AddNullMemberInitializers(IMemberAssignmentMappingContainer container, MemberPath path)
     {
-        foreach (var nullablePathList in path.ObjectPathNullableSubPaths())
+        foreach (var nullablePathList in path.ObjectReadPathNullableSubPaths())
         {
             var nullablePath = new NonEmptyMemberPath(path.RootType, nullablePathList);
             var type = nullablePath.Member.Type.NonNullable();
@@ -137,7 +137,7 @@ private IMemberAssignmentMappingContainer FindParentNonNullContainer(MemberPath
         // try to reuse parent path mappings and wrap inside them
         // if the parentMapping is the first nullable path, no need to access the path in the condition in a null-safe way.
         needsNullSafeAccess = false;
-        foreach (var nullablePathList in nullConditionSourcePath.ObjectPathNullableSubPaths().Reverse())
+        foreach (var nullablePathList in nullConditionSourcePath.ObjectReadPathNullableSubPaths().Reverse())
         {
             var nullablePath = new NonEmptyMemberPath(nullConditionSourcePath.RootType, nullablePathList);
             if (_nullDelegateMappings.TryGetValue(nullablePath, out var parentMappingContainer))

src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingState.cs

@@ -28,7 +28,8 @@ internal class MembersMappingState(
     Dictionary<string, List<MemberMappingConfiguration>> memberConfigsByRootTargetName,
     Dictionary<string, List<IMemberPathConfiguration>> configuredTargetMembersByRootName,
     HashSet<string> ignoredSourceMemberNames,
-    ParameterScope parameterScope
+    ParameterScope parameterScope,
+    SymbolAccessor symbolAccessor
 )
 {
     private readonly Dictionary<string, IMappableMember> _aliasedSourceMembers = new(StringComparer.OrdinalIgnoreCase);
@@ -58,7 +59,7 @@ ParameterScope parameterScope
     public IReadOnlyDictionary<string, IMappableMember> AdditionalSourceMembers =>
         field ??= parameterScope.Parameters.Values.ToDictionary<MethodParameter, string, IMappableMember>(
             x => x.NormalizedName,
-            x => new ParameterSourceMember(x),
+            x => new ParameterSourceMember(x, symbolAccessor),
             StringComparer.OrdinalIgnoreCase
         );
 

src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingStateBuilder.cs

@@ -68,7 +68,8 @@ public static MembersMappingState Build(MappingBuilderContext ctx, IMapping mapp
             memberConfigsByRootTargetName,
             configuredTargetMembersByRootName.AsDictionary(),
             ignoredSourceMemberNames,
-            parameterScope
+            parameterScope,
+            ctx.SymbolAccessor
         );
     }
 

src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/NestedMappingsContext.cs

@@ -70,7 +70,7 @@ private bool TryFindNestedSourcePath(
     {
         if (
             _context.SymbolAccessor.TryFindMemberPath(
-                nestedMemberPath.MemberType,
+                nestedMemberPath.MemberReadType,
                 pathCandidates,
                 // Use empty ignore list to support ignoring a property for normal search while flattening its properties
                 Array.Empty<string>(),

src/Riok.Mapperly/Descriptors/MappingBodyBuilders/MemberMappingBuilder.cs

@@ -85,15 +85,15 @@ public static bool TryBuild(
             return false;
         }
 
-        var memberTargetNullable = memberMappingInfo.TargetMember.MemberType.IsNullable();
+        var memberTargetAcceptsNull = memberMappingInfo.TargetMember.Member.IsWriteNullable;
         var delegateTargetNullable = delegateMapping.TargetType.IsNullable();
         var memberSourceNullable = memberMappingInfo.IsSourceNullable;
         var delegateSourceNullable = delegateMapping.SourceType.IsNullable();
 
         if (
             memberMappingInfo.Configuration?.SuppressNullMismatchDiagnostic != true
             && memberSourceNullable
-            && !memberTargetNullable
+            && !memberTargetAcceptsNull
             && !(delegateSourceNullable && !delegateTargetNullable)
         )
         {
@@ -107,8 +107,8 @@ public static bool TryBuild(
         }
 
         if (
-            (memberSourceNullable == delegateSourceNullable && memberTargetNullable == delegateTargetNullable)
-            || (memberSourceNullable && !memberTargetNullable && delegateSourceNullable && !delegateTargetNullable)
+            (memberSourceNullable == delegateSourceNullable && memberTargetAcceptsNull == delegateTargetNullable)
+            || (memberSourceNullable && !memberTargetAcceptsNull && delegateSourceNullable && !delegateTargetNullable)
         )
         {
             sourceValue = new MappedMemberSourceValue(
@@ -132,7 +132,7 @@ public static bool TryBuild(
             return false;
         }
 
-        sourceValue = BuildInlineNullHandlingMapping(ctx, delegateMapping, sourceMember.MemberPath, targetMember.MemberType);
+        sourceValue = BuildInlineNullHandlingMapping(ctx, delegateMapping, sourceMember.MemberPath, targetMember.MemberWriteType);
         return true;
     }
 
@@ -174,7 +174,7 @@ ITypeSymbol targetMemberType
     )
     {
         var nullFallback = NullFallbackValue.Default;
-        if (!delegateMapping.SourceType.IsNullable() && sourcePath.IsAnyNullable())
+        if (!delegateMapping.SourceType.IsNullable() && sourcePath.IsAnyReadNullable())
         {
             nullFallback = ctx.BuilderContext.GetNullFallbackValue(targetMemberType);
         }
@@ -198,7 +198,7 @@ NonEmptyMemberPath targetMember
         var sourceGetter = sourceMember.BuildGetter(ctx.BuilderContext);
 
         // no member of the source path is nullable, no null handling needed
-        if (!sourceMember.IsAnyNullable())
+        if (!sourceMember.IsAnyReadNullable())
         {
             return new MappedMemberSourceValue(delegateMapping, sourceGetter, false, true);
         }
@@ -209,7 +209,7 @@ NonEmptyMemberPath targetMember
         // access the source in a null save matter (via ?.) but no other special handling required.
         if (
             ctx.BuilderContext.Configuration.Mapper.AllowNullPropertyAssignment
-            && (delegateMapping.SourceType.IsNullable() || delegateMapping.IsSynthetic && targetMember.Member.IsNullable)
+            && (delegateMapping.SourceType.IsNullable() || delegateMapping.IsSynthetic && targetMember.Member.IsWriteNullable)
         )
         {
             return new MappedMemberSourceValue(delegateMapping, sourceGetter, true, false);

src/Riok.Mapperly/Descriptors/MappingBodyBuilders/SourceValueBuilder.cs

@@ -77,8 +77,8 @@ private static bool TryBuildConstantSourceValue(
         // but the provided value is null or default (for default IsNullable is also true)
         if (
             value.ConstantValue.IsNull
-            && memberMappingInfo.TargetMember.MemberType.IsReferenceType
-            && !memberMappingInfo.TargetMember.Member.IsNullable
+            && memberMappingInfo.TargetMember.MemberWriteType.IsReferenceType
+            && !memberMappingInfo.TargetMember.Member.IsWriteNullable
         )
         {
             ctx.BuilderContext.ReportDiagnostic(
@@ -92,8 +92,8 @@ private static bool TryBuildConstantSourceValue(
         // target is value type but value is null
         if (
             value.ConstantValue.IsNull
-            && memberMappingInfo.TargetMember.MemberType.IsValueType
-            && !memberMappingInfo.TargetMember.MemberType.IsNullableValueType()
+            && memberMappingInfo.TargetMember.MemberWriteType.IsValueType
+            && !memberMappingInfo.TargetMember.MemberWriteType.IsNullableValueType()
             && value.Expression.IsKind(SyntaxKind.NullLiteralExpression)
         )
         {
@@ -116,7 +116,7 @@ private static bool TryBuildConstantSourceValue(
 
         // use non-nullable target type to allow non-null value type assignments
         // to nullable value types
-        if (!SymbolEqualityComparer.Default.Equals(value.ConstantValue.Type, memberMappingInfo.TargetMember.MemberType.NonNullable()))
+        if (!SymbolEqualityComparer.Default.Equals(value.ConstantValue.Type, memberMappingInfo.TargetMember.MemberWriteType.NonNullable()))
         {
             ctx.BuilderContext.ReportDiagnostic(
                 DiagnosticDescriptors.MapValueTypeMismatch,
@@ -137,7 +137,7 @@ private static bool TryBuildConstantSourceValue(
                 // expand enum member access to fully qualified identifier
                 // use simple member name approach instead of slower visitor pattern on the expression
                 var enumMemberName = ((MemberAccessExpressionSyntax)value.Expression).Name.Identifier.Text;
-                var enumTypeFullName = FullyQualifiedIdentifier(memberMappingInfo.TargetMember.MemberType.NonNullable());
+                var enumTypeFullName = FullyQualifiedIdentifier(memberMappingInfo.TargetMember.MemberWriteType.NonNullable());
                 sourceValue = new ConstantSourceValue(MemberAccess(enumTypeFullName, enumMemberName));
                 return true;
             case TypedConstantKind.Type:
@@ -186,10 +186,10 @@ private static bool TryBuildMethodProvidedSourceValue(
         // to nullable value types
         // nullable is checked with nullable annotation
         var methodCandidates = namedMethodCandidates.Where(x =>
-            SymbolEqualityComparer.Default.Equals(x.ReturnType.NonNullable(), memberMappingInfo.TargetMember.MemberType.NonNullable())
+            SymbolEqualityComparer.Default.Equals(x.ReturnType.NonNullable(), memberMappingInfo.TargetMember.MemberWriteType.NonNullable())
         );
 
-        if (!memberMappingInfo.TargetMember.Member.IsNullable)
+        if (!memberMappingInfo.TargetMember.Member.IsWriteNullable)
         {
             // Filter out methods that may return null when the target is non-nullable.
             methodCandidates = methodCandidates.Where(m => !ctx.BuilderContext.SymbolAccessor.MayReturnNull(m, false));
@@ -201,7 +201,7 @@ private static bool TryBuildMethodProvidedSourceValue(
             ctx.BuilderContext.ReportDiagnostic(
                 DiagnosticDescriptors.MapValueMethodTypeMismatch,
                 methodReferenceConfiguration.Name,
-                namedMethodCandidates[0].ReturnType.ToDisplayString(),
+                ctx.BuilderContext.SymbolAccessor.UpgradeReturnNullable(namedMethodCandidates[0]).ToDisplayString(),
                 memberMappingInfo.TargetMember.ToDisplayString()
             );
             sourceValue = null;

src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/MemberMappingInfo.cs

@@ -18,7 +18,7 @@ public MemberMappingInfo(SourceMemberPath? sourceMember, NonEmptyMemberPath targ
     public MemberMappingInfo(NonEmptyMemberPath targetMember, MemberValueMappingConfiguration configuration)
         : this(null, targetMember, configuration) { }
 
-    public bool IsSourceNullable => SourceMember?.MemberPath.IsAnyNullable() ?? ValueConfiguration?.Value?.ConstantValue.IsNull ?? true;
+    public bool IsSourceNullable => SourceMember?.MemberPath.IsAnyReadNullable() ?? ValueConfiguration?.Value?.ConstantValue.IsNull ?? true;
 
     private string DebuggerDisplay =>
         $"{SourceMember?.MemberPath.FullName ?? ValueConfiguration?.DescribeValue()} => {TargetMember.FullName}";
@@ -34,7 +34,11 @@ public TypeMappingKey ToTypeMappingKey()
         if (SourceMember == null)
             throw new InvalidOperationException($"{SourceMember} and {TargetMember} need to be set to create a {nameof(TypeMappingKey)}");
 
-        return new TypeMappingKey(SourceMember.MemberPath.MemberType, TargetMember.MemberType, Configuration?.ToTypeMappingConfiguration());
+        return new TypeMappingKey(
+            SourceMember.MemberPath.MemberReadType,
+            TargetMember.MemberWriteType,
+            Configuration?.ToTypeMappingConfiguration()
+        );
     }
 
     public string DescribeSource()

src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/SourceValue/MappedMemberSourceValue.cs

@@ -19,7 +19,7 @@ bool addValuePropertyOnNullable
     private readonly MemberPathGetter _sourceMember = sourceMember;
     private readonly INewInstanceMapping _delegateMapping = delegateMapping;
 
-    public bool RequiresSourceNullCheck => !nullConditionalAccess && _sourceMember.MemberPath.IsAnyNullable();
+    public bool RequiresSourceNullCheck => !nullConditionalAccess && _sourceMember.MemberPath.IsAnyReadNullable();
 
     public ExpressionSyntax Build(TypeMappingBuildContext ctx)
     {

src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/SourceValue/NullMappedMemberSourceValue.cs

@@ -27,7 +27,7 @@ bool useNullConditionalAccess
     public ExpressionSyntax Build(TypeMappingBuildContext ctx)
     {
         // if the source is not nullable, return it directly.
-        if (!_sourceGetter.MemberPath.IsAnyNullable())
+        if (!_sourceGetter.MemberPath.IsAnyReadNullable())
         {
             ctx = ctx.WithSource(_sourceGetter.BuildAccess(ctx.Source));
             return _delegateMapping.Build(ctx);
@@ -49,7 +49,7 @@ public ExpressionSyntax Build(TypeMappingBuildContext ctx)
         // source.A?.B == null ? <null-substitute> : Map(source.A.B.Value)
         // use simplified coalesce expression for synthetic mappings:
         // source.A?.B ?? <null-substitute>
-        if (_delegateMapping.IsSynthetic && (useNullConditionalAccess || !_sourceGetter.MemberPath.IsAnyObjectPathNullable()))
+        if (_delegateMapping.IsSynthetic && (useNullConditionalAccess || !_sourceGetter.MemberPath.IsAnyObjectReadPathNullable()))
         {
             var nullConditionalSourceAccess = _sourceGetter.BuildAccess(ctx.Source, nullConditional: true);
             var nameofSourceAccess = _sourceGetter.BuildAccess(ctx.Source, nullConditional: false);

src/Riok.Mapperly/Descriptors/SymbolAccessor.cs

@@ -91,7 +91,7 @@ private bool IsAccessible(ISymbol symbol, MemberVisibility visibility)
         };
     }
 
-    public bool IsNullable(ISymbol symbol)
+    public bool IsReadNullable(ISymbol symbol)
     {
         return symbol switch
         {
@@ -103,6 +103,18 @@ public bool IsNullable(ISymbol symbol)
         };
     }
 
+    public bool IsWriteNullable(ISymbol symbol)
+    {
+        return symbol switch
+        {
+            ITypeSymbol t => t.IsNullable(),
+            IPropertySymbol p => p.Type.IsNullable() || TryHasAttribute<AllowNullAttribute>(p),
+            IFieldSymbol f => f.Type.IsNullable() || TryHasAttribute<AllowNullAttribute>(f),
+            IParameterSymbol p => p.Type.IsNullable() || TryHasAttribute<AllowNullAttribute>(p),
+            _ => false,
+        };
+    }
+
     public bool MayReturnNull(IMethodSymbol symbol, bool treatUnannotatedAsNullable = true)
     {
         // only treat annotated as nullable
@@ -134,6 +146,30 @@ public bool CanAssign(ITypeSymbol sourceType, ITypeSymbol targetType)
 
     public MethodParameter WrapMethodParameter(IParameterSymbol symbol) => new(symbol, UpgradeNullable(symbol.Type));
 
+    public ITypeSymbol UpgradeReturnNullable(IMethodSymbol methodSymbol)
+    {
+        TryUpgradeReturnNullable(methodSymbol, out var upgradedReturnSymbol);
+        return upgradedReturnSymbol ?? methodSymbol.ReturnType;
+    }
+
+    private bool TryUpgradeReturnNullable(IMethodSymbol methodSymbol, [NotNullWhen(true)] out ITypeSymbol? upgradedReturnSymbol)
+    {
+        upgradedReturnSymbol = default;
+
+        if (methodSymbol.ReturnsVoid || methodSymbol.ReturnType.IsValueType)
+        {
+            return false;
+        }
+
+        if (!TryHasAttribute<MaybeNullAttribute>(methodSymbol.GetReturnTypeAttributes()))
+        {
+            return false;
+        }
+
+        upgradedReturnSymbol = methodSymbol.ReturnType.WithNullableAnnotation(NullableAnnotation.Annotated);
+        return true;
+    }
+
     /// <summary>
     /// Upgrade the nullability of a symbol from <see cref="NullableAnnotation.None"/> to <see cref="NullableAnnotation.Annotated"/>.
     /// Value types are not upgraded.

src/Riok.Mapperly/Descriptors/UserMethodMappingExtractor.cs

@@ -267,7 +267,9 @@ private static (ITypeSymbol, UserImplementedMethodMapping.TargetNullability) Bui
         string sourceParameterName
     )
     {
-        var targetType = ctx.SymbolAccessor.UpgradeNullable(method.ReturnType);
+        var effectiveReturnType = ctx.SymbolAccessor.UpgradeReturnNullable(method);
+        var targetType = ctx.SymbolAccessor.UpgradeNullable(effectiveReturnType);
+
         if (!targetType.IsNullable() || ctx.SymbolAccessor.TryHasAttribute<NotNullAttribute>(method.GetReturnTypeAttributes()))
         {
             return (targetType, UserImplementedMethodMapping.TargetNullability.NeverNull);

src/Riok.Mapperly/Symbols/Members/ConstructorParameterMember.cs

@@ -20,7 +20,8 @@ public class ConstructorParameterMember(IParameterSymbol symbol, SymbolAccessor
 {
     public ITypeSymbol Type { get; } = accessor.UpgradeNullable(symbol.Type);
     public INamedTypeSymbol ContainingType { get; } = symbol.ContainingType;
-    public bool IsNullable => accessor.IsNullable(Symbol);
+    public bool IsReadNullable => accessor.IsReadNullable(Symbol);
+    public bool IsWriteNullable => accessor.IsWriteNullable(Symbol);
     public bool CanGet => false;
     public bool CanGetDirectly => false;
     public bool CanSet => false;

src/Riok.Mapperly/Symbols/Members/EmptyMemberPath.cs

@@ -8,7 +8,13 @@ public class EmptyMemberPath(ITypeSymbol rootType) : MemberPath(rootType, [])
 {
     public override IMappableMember? Member => null;
 
-    public override ITypeSymbol MemberType => RootType;
+    public override ITypeSymbol MemberReadType => RootType;
+
+    public override ITypeSymbol MemberWriteType => RootType;
+
+    public override bool IsAnyReadNullable() => false;
+
+    public override bool IsWriteNullable() => false;
 
     public override string ToDisplayString(bool includeRootType = true, bool includeMemberType = true) =>
         includeRootType ? RootType.ToDisplayString() : string.Empty;