Elide redundant null checks

[jnyrup]

Is your feature request related to a problem? Please describe. I noticed various null checks in the generated code that didn't need to be there or could be reduced from two checks to a single check. I'm aware of #1614 but since that seems to (only?) be about correctness I created a separate issue for these examples.

Describe the solution you'd like In each example below the generated code performs more null checks that necessary. Ideally we emit C# code that performs the minimum number of null checks.

Example 1

source is null checked twice, explicitly with == null and implicitly through .Value.

[Mapper]
public partial class A
{
    public partial int? Map(int? source);
}

Actual

// <auto-generated />
#nullable enable
public partial class A
{
    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.2.1.0")]
    public partial int? Map(int? source)
    {
        return source == null ? default(int?) : source.Value;
    }
}

Expected

// <auto-generated />
#nullable enable
public partial class A
{
    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.2.1.0")]
    public partial int? Map(int? source)
    {
        return source;
    }
}

Example 2

Like in example 1 source is null checked twice, but to extract the value from decimal? we can use Nullable<T>.GetValueOrDefault() to save the null check inside Nullable<T>.Value.

[Mapper]
public partial class E
{
    public partial int? Map(decimal? source);
}

Actual

// <auto-generated />
#nullable enable
public partial class E
{
    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.2.1.0")]
    public partial int? Map(decimal? source)
    {
        return source == null ? default(int?) : (int)source.Value;
    }
}

Expected

// <auto-generated />
#nullable enable
public partial class E
{
    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.2.1.0")]
    public partial int? Map(decimal? source)
    {
        return source == null ? default(int?) : (int)source.GetValueOrDefault();
    }
}

Example 3

Each item of source is null checked.

#nullable enable
[Mapper(UseDeepCloning = true)]
public partial class B
{
    public partial List<string?> Map(List<string?> source);
}
#nullable restore

Actual

// <auto-generated />
#nullable enable
public partial class B
{
    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.2.1.0")]
    public partial global::System.Collections.Generic.List<string?> Map(global::System.Collections.Generic.List<string?> source)
    {
        var target = new global::System.Collections.Generic.List<string?>(source.Count);
        foreach (var item in source)
        {
            target.Add(item == null ? default : item);
        }
        return target;
    }
}

Expected

// <auto-generated />
#nullable enable
public partial class B
{
    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.2.1.0")]
    public partial global::System.Collections.Generic.List<string?> Map(global::System.Collections.Generic.List<string?> source)
    {
        var target = new global::System.Collections.Generic.List<string?>(source.Count);
        foreach (var item in source)
        {
            target.Add(item);
        }
        return target;
    }
}

Example 4

Each item of source is null checked. I don't know if it's related but furthermore I would expect that we just called Enumerable.ToList<int?>(IEnumerable<int?> source) like a generated mapper from List<int> to List<int> does.

[Mapper(UseDeepCloning = true)]
public partial class C
{
    public partial List<int?> Map(List<int?> source);
}

Actual

// <auto-generated />
#nullable enable
public partial class C
{
    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.2.1.0")]
    public partial global::System.Collections.Generic.List<int?> Map(global::System.Collections.Generic.List<int?> source)
    {
        return global::System.Linq.Enumerable.ToList(source);
    }
}

Expected

// <auto-generated />
#nullable enable
public partial class C
{
    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.2.1.0")]
    public partial global::System.Collections.Generic.List<int?> Map(global::System.Collections.Generic.List<int?> source)
    {
        var target = new global::System.Collections.Generic.List<int?>(source.Count);
        foreach (var item in source)
        {
            target.Add(item);
        }
        return target;
    }
}

Example 5

Each item.Key of source in MapToDictionaryOfStringAndInt32 is null checked.

#nullable disable
[Mapper(UseDeepCloning = true)]
public partial class D
{
    public partial MyClass Map(MyClass source);
}

public class MyClass
{
    public Dictionary<string, int> MyProperty { get; set; }
}
#nullable enable

Actual

// <auto-generated />
#nullable enable
public partial class D
{
    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.2.1.0")]
    [return: global::System.Diagnostics.CodeAnalysis.NotNullIfNotNull(nameof(source))]
    public partial global::MyClass? Map(global::MyClass? source)
    {
        if (source == null)
            return default;
        var target = new global::MyClass();
        if (source.MyProperty != null)
        {
            target.MyProperty = MapToDictionaryOfStringAndInt32(source.MyProperty);
        }
        else
        {
            target.MyProperty = null;
        }
        return target;
    }

    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.2.1.0")]
    private global::System.Collections.Generic.Dictionary<string?, int> MapToDictionaryOfStringAndInt32(global::System.Collections.Generic.IReadOnlyDictionary<string?, int> source)
    {
        var target = new global::System.Collections.Generic.Dictionary<string?, int>(source.Count);
        foreach (var item in source)
        {
            target[item.Key == null ? default : item.Key] = item.Value;
        }
        return target;
    }
}

Expected

// <auto-generated />
#nullable enable
public partial class D
{
    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.2.1.0")]
    [return: global::System.Diagnostics.CodeAnalysis.NotNullIfNotNull(nameof(source))]
    public partial global::MyClass? Map(global::MyClass? source)
    {
        if (source == null)
            return default;
        var target = new global::MyClass();
        if (source.MyProperty != null)
        {
            target.MyProperty = MapToDictionaryOfStringAndInt32(source.MyProperty);
        }
        else
        {
            target.MyProperty = null;
        }
        return target;
    }

    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.2.1.0")]
    private global::System.Collections.Generic.Dictionary<string?, int> MapToDictionaryOfStringAndInt32(global::System.Collections.Generic.IReadOnlyDictionary<string?, int> source)
    {
        var target = new global::System.Collections.Generic.Dictionary<string?, int>(source.Count);
        foreach (var item in source)
        {
            target[item.Key] = item.Value;
        }
        return target;
    }
}

[latonz]

Thank you for bringing these up! These are all excellent observations and, as you mentioned, closely related to #1614.

Most — if not all — of the examples you've provided seem to fall back to the current null-handling approach we outlined in #1614:

[...] when nullables are involved, Mapperly incorporates null handling into the mapping process and continues the pipeline with non-nullable types. This approach centralizes null handling, making it simple and consistent.

That said, I agree it's time to revisit and improve our null-handling strategy. I’ll keep this issue open alongside #1614. I’d really appreciate any suggestions you might have on refining the current approach. One idea I’m considering is removing the NullableMappingBuilder entirely (or possibly moving it to the end of the mapping pipeline) and instead embedding the null-handling logic directly into each mapping builder.