Diagnostic for mapping null mismatch

[latonz]

Emit a diagnostic if the source is nullable but the target isn't. Default severity should be info.
Proposed message: Mapping the nullable source property {0} of {1} to the target property {2} of {3} which is not nullable

Relates #375

[cremor]

If you add this, please make it possible to suppress this diagnostic for a single member. So that I can say "I know this, it's fine".

[marin-8]

Please, prioritize this issue. I would find it very useful and development without it is not compile time safe.

[latonz]

In my opinion, Mapperly is compile-safe without this feature, as all generated code is type-safe and provides diagnostics for invalid mappings. I can still understand your need for this feature, as it would definitely be a nice addition.
However, this is not currently high on our priority list. With only one 👍 reaction, it also doesn't seem to be one of the most important features missing by the community...
You are always welcome to contribute, we will support you with tips and reviews. If you cannot wait for this feature and do not have the time to implement it yourself, we are open to discussing sponsorship for specific features.

[marin-8]

Thanks for the quick reply!

I'm sorry, I didn't mean to say that "the code that Mapperly produces is not compile-time safe" in any way. What I meant is that:

• When a property in the source class is nullable but not in the target class, and the actual value is null, the resulting value assigned to the target property will be the default for it's type. This can produce unexpected behaviour (in the case of value types) or errors (in the case of reference types in a [nullable enable] context). That is why I think a diagnostic that warns you when this is happening is very important.

After writing my previous comment I discovered that in the 4.0.0-next.1 preview documentation there is a new diagnostic, RMG076, that seems to address this (?). It's description is "Cannot assign null to non-nullable member.". I will try it whenever I can, but what's your comment on that?

Thanks again!

[latonz]

We definitely want to implement this diagnostic, it's just not on top of our priority list 😊
RMG076 is only for MapValue...

[marin-8]

Okay, I understand. Thanks anyway! ❤️

[F1nn-T]

Hey @latonz, I would like to work on this, could you assign me? 😄

[latonz]

@F1nn-T are you still working on this?

[F1nn-T]

Yes, just didn't had the time yet to finish.

[jorisBarkema]

I don't mean to hijack the issue but this is something I would really like and progress seems to have stalled, so I have made an initial PR for this issue: #1592
The diagnostic is showing up where I want it to, but it is also showing up in place I think it would be best not to show up, such as in the ShouldPassNullValueToNullableUserMappingMethod test:

[MapProperty(nameof(A.Value), nameof(B.Value), Use = nameof(MapString)]
public partial B Map(A source);

[UserMapping(Default = false)]
private string MapString(string? source)
    => source ?? "(null)";
""",
"""
class A
{
    public string? Value { get; }
    public string? Value2 { get; }
}
""",
"""
class B
{
    public string Value { set; }
    public string Value2 { set; }
}

So before continuing, would it be possible to get a quick review just to see if I'm looking in the right place to add this feature? If so I will update the existing tests and try to see if I can add an option to disable this check per property too as requested earlier in this discussion

[latonz]

Feature got implemented by @jorisBarkema in #1592 / #1612. I created #1613 to support an API to disable the diagnostics for given members.