Cannot map `Value` or `HasValue` Property of `Nullable<T>`

[onionhammer]

Describe the bug I upgraded from next-2 to next-3 and now I am receiving a build error

ModelMapper.g.cs(705,78): error CS1061: 'JsonElement' does not contain a definition for 'Value' and no accessible extension method 'Value' accepting a first argument of type 'JsonElement' could be found (are you missing a using directive or an assembly reference?)

It looks like the generated code is calling .Value twice:

// Generated code
        {
            var target = new global::SellerAmend()
            {
                Kind = source.Kind,
                SellerPartnerId = source.SellerPartnerId,
                SellerLoanId = source.SellerLoanId,
                DateCreated = source.DateCreated
            };
            if (source.ListingAmendment != null)
            {
                                                                   // .Value.Value should just be .Value
                target.ListingAmendmentValue = source.ListingAmendment.Value.Value.ToString();
            }

            return target;
        }

[latonz]

Thanks for the bug report, I'll look into it.

[onionhammer]

Here's a repro:

[Mapper(PropertyNameMappingStrategy = PropertyNameMappingStrategy.CaseInsensitive)]
public class MapperClass
{
    public static partial Car MapCar(CarDto input);
    public static partial CarDto MapCar(Car input);
}

public record CarDto()
{
    public JsonElement? Amendment { get; set; }
}

public record Car()
{
    [JsonIgnore, System.Runtime.Serialization.IgnoreDataMember]
    public OtherThing? AmendmentMetadata
    {
        get => Amendment?.Deserialize<OtherThing>(JsonExtensions.JsonOptions);
    }

    [JsonIgnore, System.Runtime.Serialization.DataMember]
    public string? AmendmentValue
    {
        get => Amendment?.GetRawText();
        set => Amendment = value != null ? JsonSerializer.Deserialize<JsonElement>(value) : null;
    }

    [JsonInclude, System.Runtime.Serialization.IgnoreDataMember]
    public JsonElement? Amendment { get; set; }
}

public class OtherThing
{
    public string? Value { get; set; }
}

[TimothyMakkison]

Away rn, but I'm guessing because Amendment is a Nullable<JsonElement>, mapperly sees an object with a Value field and attempts to map it? Because mapperly automatically accesses the Value field (because it knows its not null), it then attempts to now "access" the value member, and fails because it has already been accessed.

[onionhammer]

@TimothyMakkison I think its because of the Value suffix and some special case with Mapperly for nullables?

If you rename AmendmentValue to AmendmentVal it compiles.

[TimothyMakkison]

@TimothyMakkison I think its because of the Value suffix and some special case with Mapperly for nullables?

If you rename AmendmentValue to AmendmentVal it compiles.

Yeah I've updated my comment with more speculation. I'm guessing it only affects nullable value types because its an actual type. Does this not happen with V2?

[onionhammer]

@TimothyMakkison maybe it was just doing a throw new NotImplementedException on v2

[TimothyMakkison]

It's strange this issue didn't crop up when field mapping was introduced. Iirc one of my nit pick optimizations changed how nullable types were checked, perhaps that broke it. #555

[latonz]

One bug which I just found but also existed in v2.8.0 is that a nested nullable struct's flattened values cannot be resolved. I created another issue https://github.com/riok/mapperly/issues/572 since I'm not sure if it is related to this issue.

[TimothyMakkison]

I don't think #555 caused it I've been using the following

[Fact]
public void VaueTest()
{
    var source = TestSourceBuilder.MapperWithBodyAndTypes(
        "partial B Map(A src);",
        "public record A(int? P);",
        "public record B { public int PValue { get; set; } }"
    );
    TestHelper
        .GenerateMapper(source)
        .Should()
        .HaveSingleMethodBody(
            """
            var target = new global::B();
            if (src.P != null)
            {
                target.PValue = src.P.Value.Value;
            }

            return target;
            """
        );
}

[latonz]

@TimothyMakkison The test you provided, which should reproduce the bug described by OP, fails on v2.8.0. So this probably exists since a longer time and isn't a regression of v2.9.0-next.3

[TimothyMakkison]

The test passes when the bug occurs. Does this mean the bug doesn't happen with 2.8?

[latonz]

@TimothyMakkison sorry my bad, I adjusted the test to verify that the bug does not exist. If I use exact your version, it fails on v2.8.0 as well as on v2.9.0-next.3. The problem is that the target property has a auto-flattened name of ending with Value. When resolving the name, autoflattening resolves it to P.Value (the value property of Nullable<T>). Then when building the mapping, the builder encounters a nullable value type and adds another .Value to access the value inside the null if condition. It relates to https://github.com/riok/mapperly/issues/572. If Nullable<T> properties are treated as T while resolving mappable members, the problem shouldn't occur.

[onionhammer]

Name of this issue makes no sense anymore, should we close as duplicate?

[latonz]

I adjusted the title. I think this is a slightly different issue than https://github.com/riok/mapperly/issues/572. Here the name of the target property Value which matches the Value property of Nullable<T>, which is special cased, is a problem. While in #572 the problem is that the member resolution does take Nullable<T> into account, while the user certainly expects T. However, the solution could probably be the same.

[latonz]

Rel. https://github.com/riok/mapperly/issues/98