Support for NonNull / Nullable annotations

[PCasafont]

Currently, when we create an update mapping such as: User updateUserFromDto(@MappingTarget User user, UserDto userDto);

In order to have null checks, we must configure the Mapper annotation (in the class level) to have nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS.

But this generates null checks everywhere, including the creator mappers. It would be awesome if there was a property in @Mapping like "nullCheck=true/false", so that we can fully control that.

[filiphr]

Doing it on the @Mapping is one way to do it.

I think that it could be better if we can check for NotNull or Nullable annotations on the getters. This is what IntelliJ does for the code inspections. I also think that Kotlin uses those annotations as for interoperability with Java. Spring has also done it for their entire code base, see SPR-15540. I am not sure about other frameworks.

WDYT?

[PCasafont]

A check for nullable/notnull annotations would be awesome!

[brunomarquete]

Are there plans to start this feature? I am working with non-null objects in Kotlin and I would like to see this feature implemented in MapStruct.

[altarasyuk]

It would be good feature

[mdelmoral]

In my current company we use the library in small and not very critical services since we are very strict with the validation of nullity. This feature would be fundamental to adopt the library at a global level 👍🏻

[darioseidl]

For Java, JSR 303 support would be nice. Similarly for Kotlin, it would be great if the implementation could create null-checks depending on the nullability of the Kotlin types in the mapper interface.

For example, in the PersonConverter all types are non-nullable, but in the generated PersonConverterImpl, null would be passed through instead of throwing an exception.

[andye2004]

This would be an incredibly valuable feature and something that would encourage greater adoption of MapStruct. Are there any plans to start this work given the ticket has been open for close to four years now?

[anandbhaskaran]

This would be incredibly nice to have a feature!

[agentgt]

The fact that MapStruct does not handle @Nullable annotations particularly TYPE_USE aka JSR 308 annotations (e.g. JSpecify and Checker) is a major reason I have the generator run and just copy the code back into the abstract class (or interface) as a concrete inner class (I rely entirely on constructor based mapping so new fields easily become compile errors and thus the maintenance of adding a new field is not that painful).

This is because if you run null analysis like Eclipse (which arguably is kind of broken but mostly follows JSpecify) or Checkerframework you will get warnings all over the place for conditional checking for null and or worse doesn't force the user to deal with mapping something potentially nullable to nonnull.

What fundamentally needs to happen IMO is that MapStruct should treat e.g. java.lang. @Nullable String (n.b. how TYPE_USE annotations declared different) as a different type than java.lang.String because it actually is in terms of static analysis.

This becomes really complicated when dealing with code generation particularly generics and arrays.

MapStruct pulls all fields that need to be mapped out as variables:

public record SomeType(List<@Nullable String> list, @Nullable String string){}

The mapping code then should produce code like:

List<@Nullable String> list = someType.list();
@Nullable String string = someType.string();
//... snip the setting or calling of constructor

Instead of:

List<String> list = someType.list();
String string = someType.string();
//... snip the setting or calling of constructor

That is the annotations should follow the type regardless of whether MapStruct tries to do things based on the semantics of @Nullable. Unfortunately this is inherently difficult with the current JDK annotation processor framework as it will often either strip the annotations or worse incorrectly print them (a JDK bug that is kind of fixed in 19).

Furthermore mapping methods will need to be annotation aware. For example say I do have @Nullable String -> String.

String convertNullable(@Nullable String s) {
}

MapStruct should know to call that method to convert the nullable string type to a nonnull (or in this case stripped annotation) string type. After all it is a different type.

[filiphr]

Thanks for your examples @agentgt.

MapStruct should know to call that method to convert the nullable string type to a nonnull (or in this case stripped annotation) string type. After all it is a different type.

I am not sure that MapStruct should treat methods with types annotated differently as different types. I would rather have MapStruct properly detect NonNull / Nullable annotations and invoke the methods appropriately.

Regarding the code that we should generate, I don't see a reason why not to generate code with passing down the annotations on the return type itself. However, that topic is different than this issue and I would suggest creating a separate issue where we can discuss MapStruct generating such code (irregardless of the bugs in the compilers)

[agentgt]

I am not sure that MapStruct should treat methods with types annotated differently as different types. I would rather have MapStruct properly detect NonNull / Nullable annotations and invoke the methods appropriately

But they mostly are. It is like the difference between String? and String in kotlin. The reason I brought up the whole TYPE_USE annotation following support isn't as much about generating the code but rather force the user to convert through some mechanism (as a configurable option).

If the annotations follow then the static null analysis will fail (compile failure) and MapStruct does not have to do any magic on trying to figure out how to map nullables to nonnull. I would call that nullable support "Level 1" which would require MapStruct (as a configurable option) to stop doing if (x == null) checks. MapStruct just assumes everything is NonNull! I would be happy with just that option today (I haven't played with updated MapStruct but I assume its still not an option of disable null checking). e.g NullValueCheckStrategy.NEVER is missing.

"Level 2" support would be to have MapStruct fail (instead of the compiler or null analysis) when it sees: @Nullable T -> T (as well as the inverse) and force manual conversion. Whether that is for any TYPE_USE annotation or not is an exercise in configuration.

That is because basically it is like mapping Optional<T> -> T. MapStruct should fail here. I don't know what it currently does but if it makes T null that is bad.

"Level 3" would be knowing that @Nullable is a wider type. That is T -> @Nullable T is a an obvious NOOP mapping but @Nullable T -> T requires conversion.

that topic is different than this issue and I would suggest creating a separate issue where we can discuss MapStruct generating such code (irregardless of the bugs in the compilers)

I can but the two are highly related. @Nullable T should be treated like Optional<T>. If you don't think of it as types its going to get much harder as things like Valhalla and JSpecify. At some point you will have to check if the type supports if (t == null) or not just like it is a primitive.