Configuring Default Behaviour For ImmutableArray

[jacob7395]

Hey, not sure if this is supported behaviour I can't spot anything in the docs. There is something I would consider an issue with ImmutableArray (may be the same with other immutable types) where ImmutableArray<int> x = default is not the same as setting it to empty, and has the tendency to throw runtime exceptions if the devs don't deal with it correctly.

This extra trick when using this type in an API request where the API model might be null but the internal can't be null. When mapping the code is generating something like this;

Packages = request.Packages != null ? global::System.Collections.Immutable.ImmutableArray.ToImmutableArray(
    global::System.Linq.Enumerable.Select(request.Packages, x => MapToPackageRequest(x))
) : default,

Is there a way to easily configure the default/null behaviour for a given types so it generates something like;

Packages = request.Packages != null ? global::System.Collections.Immutable.ImmutableArray.ToImmutableArray(
    global::System.Linq.Enumerable.Select(request.Packages, x => MapToPackageRequest(x))
) : ImmutableArray<Packages>.Empty,

This would save some potential explosions if our tests don't catch the issue.

I know this could possibly be caught with some validation on the incoming request but in this case it's not something we can do.

#250 is where this feature was implemented but this was not raised.

This could be to do with how we are dealing with nulls possible something similar to how Newtonsoft handles this with [JsonProperty(NullValueHandling = NullValueHandling.Ignore)]. Would you be open to expanding how the null behaviour is used so haveing something like;

[Mapper]
public partial class CarMapper
{
    [MapperIgnoreSourceWhenNull(nameof(Car car))]
    public partial CarDto ToDto(Car car);
}

Where the mapper would not set the value on the target if the source is null? This would allow our domain model to deal with it, in this case we set it to empty instead of default. This works fine with json but has an issue with the mapper as shown in the generated code using default.

[latonz]

In fact we didn't think about default being uninitialized for immutable collections.
Doesn't AllowNullPropertyAssignment work for your use-case (which AFAIK is similar to NullValueHandling.Ignore)? (See also docs).

[latonz]

Does the AllowNullPropertyAssignment approach work if the target property has not init but set?

With #1285 the user mapping approach would get a lot easier:

[UserMapping(Default = true)]
private static ImmutableArray<T> MapImmutableArray<T>(List<T>? items)
    => items != null
        ? ImmutableArray<T>.Empty
        : Map(items);

Do you have a proposal on how this feature could work exactly? How could the API look like? What cases are exactly affected?

A few starting points to dive into Roslyn source gens:

• source generators cookbook,
• incremental generators cookbook
• mapperly contributors docs

[jacob7395]

Ye looks like it works as expect when it's not init;

public record Test1
{
    public List<int>? Prop { get; set; }
}

public record Test2
{
    public ImmutableArray<int> Prop { get; set; }
}

public static partial class TestMappers
{
    internal static partial Test2 ToDomain(this Test1 source);
}

Generates

var target = new global::Arcanaloth.Server.Costs.Infrastructure.Accountants.Dtos.Test2();
if (source.Prop != null)
{
  target.Prop = global::System.Collections.Immutable.ImmutableArray.ToImmutableArray(source.Prop);
}
return target;

It generates the same with or without the null assignment.

[jacob7395]

Before saying saying how I think this could look it's worth clarify the behavior using, the example above, the defaul assignment happens when ThrowOnMappingNullMismatch is set to false. AllowNullPropertyAssignment doesn't appear to effect this. So when generating for these types;

public record Test1
{
    public List<int>? Prop { get; set; }
}

public record Test2
{
    public ImmutableArray<int> Prop { get;  init; }
}

[Mapper(ThrowOnMappingNullMismatch = false)]
public static partial class TestMappers
{
    internal static partial Test2 ToDomain(this Test1 source);
}

It currently produces;

            var target = new global::Arcanaloth.Server.Costs.Infrastructure.Accountants.Dtos.Test2()
            {
                Prop = source.Prop != null ? global::System.Collections.Immutable.ImmutableArray.ToImmutableArray(source.Prop) : default,
            };
            return target;

If ThrowOnMappingNullMismatch is not set of set to true, then instead of default it will thrown and exception.

The problem with this situation is the use of init as shown above when it's a get there are not issues but I like immutable types so would like so see them supported.

Proposed Options

In my opinion I would lean towards something explicit so either an addition to the MapProperty attribute like;

    [MapProperty(nameof(Test1.Prop), nameof(Test2.Prop), WithDefaultValue = ImmutableArray<int>.Empty)]
    internal static partial Test2 ToDomain(this Test1 source);

But this isn't a compile time constant so can't go in an attribute.

Maybe then using a property or method similar to how user maps work;

    [MapProperty(nameof(Test1.Prop), nameof(Test2.Prop), WithDefaultValue = nameof(DefaultValue))]
    internal static partial Test2 ToDomain(this Test1 source);
    
    internal static ImmutableArray<int> DefaultValue => ImmutableArray<int>.Empty;

Another option would be;

[Mapper(ThrowOnMappingNullMismatch = false)]
public static partial class TestMappers
{
    internal static partial Test2 ToDomain(this Test1 source);
    
    [UserDefaultValue]
    internal static ImmutableArray<int> DefaultValue => ImmutableArray<int>.Empty;
}

I am not sure if either of these are a good solution for the issue 🤷 I had a look through #1285 and that might work I need a bit more time to think it though. I feel like the above options are trying to solve a problem with init properties when there could be a null issue by introducing a feature that allows for users to set the default options.

Maybe the best approach would be for Mapperly to incorporate default assignment values. So given this;

public record Test1
{
    public List<int>? Prop { get; set; }
}

public record Test2
{
    public ImmutableArray<int> Prop { get;  init; } = ImmutableArray<int>.Empty;
}

[Mapper(UseDefaultValues = true)]
public static partial class TestMappers
{
    internal static partial Test2 ToDomain(this Test1 source);
}

Mapperly would pick up that Test2.Prop has got a default value of ImmutableArray<int>.Empty, when building the assignments it would use this default value instead of assigning it in this case to default.

This is only a special case for init as get will just not be assigned and use the default value, however support init/constructer properties like this seems to most intuitive.

Thoughts?

[latonz]

I like the

[UserDefaultValue]
internal static ImmutableArray<int> DefaultValue => ImmutableArray<int>.Empty;

approach.

I'm not sure if we should name the attribute UserDefaultValue or MappingNullSubstitute. NullSubstitute is probably more common among other well-established mapping libraries in .NET but is not 100% true for value types.

[jacob7395]

Hey, sorry for the slow reply on this.

[UserDefaultValue]
internal static ImmutableArray<int> DefaultValue => ImmutableArray<int>.Empty;

This looks pretty, nice. Would fix the issue for Immutable arrays.

I'm not sure if we should name the attribute UserDefaultValue or MappingNullSubstitute. NullSubstitute is probably more common among other well-established mapping libraries in .NET but is not 100% true for value types.

Personally UserDefaultValue seems more clear.

---

I have a few thoughs;

1. Would it be good for this to have follow the rules of, the AutoUserMappings or should a new rule be introduced to control auto use behaviour.
2. Similar to the above would a new attribute to have the same behaviour MapPropertyFromSource be useful, and the equivilent to use map Use for the MapProperty attribute.
3. Did we rule out trying to read the default from the target?