GeezerVison

This looks great @filiphr. Given the JDK 21 failure, are you going to add a Java 25 (26 too - i.e. last non-LTS) into the CI matrix?

Given the JDK 21 failure, are you going to add a Java 25 (26 too - i.e. last non-LTS) into the CI matrix?

It's a silly error in the test code. Will be fixed. I'll add Java 25 and 26 separately into the CI matrix, it's not linked to this issue in particular.

I don't want to delay you merging what you have because it's already really valuable and will make a big difference for us.

However, I'm not clear if this does or doesn't cover type use on array elements. Some permutations of copies between the arrays below should fail.

@NullMarked
class JSpecifyArrays {

  public String @Nullable[] getNullableArrayOfNonNullStrings() { }

  public @Nullable String[] getNonNullArrayOfNullableStrings() { }

  public String[] getNonNullArrayOfNonNullStrings() { }
}

Thanks for looking into this @nealeu. I think that second 2 should be covered

  public @Nullable String[] getNonNullArrayOfNullableStrings() { }

  public String[] getNonNullArrayOfNonNullStrings() { }

However, I think that the first one would be treated the same as the getNonNullArrayOfNonNullStrings.

I'll create another issue for this, as that looks a bit more involved.

I would humbly ask that you also consider @lombok.NonNull, which is anecdotally very common as well (e.g. 272k github results for import lombok.NonNull vs. 58k for import org.jspecify.annotations.NonNull). Thank you so much for your hard work, I am a big fan of your project!

@vmckinney-cainc I would argue that you are doing an incorrect search for JSpecify. Perhaps a search for import org.jspecify.annotations with 260k results is more relevant. Especially considering the fact that maybe the most common use of JSpecify I would expect is the use of @NullMarked or @NullUmarked on package-info.java which would be applied to all classes in that package.

However, I'm not the biggest user of JSpecify yet, but what I've seen in the ecosystem is the package-info.java usage.

Having said that, I'm not sure that I would like to expand this support to all the other variations of Nullable / NonNull, especially due to the fact that it looks like JSpecify is becoming the industry standard for this.

First, thank you very much @filiphr for adding support for nullness annotations! This has been a long-expected feature that I’m happy to see.

I would like to support @vmckinney-cainc's feedback on considering additional nullness annotations (other than JSpecify, such as lombok.NonNull or jakarta.annotation.Nonnull), as this could improve the experience of working with third-party library code.

We have just tried out switching our codebase to the SNAPSHOT version of MapStruct built from this branch and seen new errors where source properties are annotated with non‑JSpecify @NonNull annotations (specifically jakarta.annotation.Nonnull) in third‑party code:

Can't map potentially nullable source property "foobar" to `@NonNull` constructor parameter "foobar". Consider adding a defaultValue or defaultExpression.

Because MapStruct currently only recognizes JSpecify annotations, those sources are treated as potentially nullable. For us, this results in many occurrences across mappers and would require adding defaultValue/defaultExpression for every affected property, even though the sources are annotated by their authors.

While I agree that JSpecify is becoming the industry standard, certain libraries may choose to not migrate to it from other annotation types. Adding support for non-JSpecify annotations would reduce the amount of errors an user of the MapStruct library would need to solve when transitioning to the new MapStruct release with improved nullness support.

Therefore, I would like to propose introducing an annotation processor option (e.g. mapstruct.nullness.nonNullAnnotations) to configure the list of considered annotation names. The default would remain JSpecify only, so the behavior is unchanged. For example:

-Amapstruct.nullness.nonNullAnnotations=org.jspecify.annotations.NonNull,jakarta.annotation.Nonnull,lombok.NonNull

This could improve the migration path for projects such as mine that depend on libraries unlikely to adopt JSpecify soon.

If this direction sounds reasonable, we could also discuss adding configurable support for variations of @NullMarked (e.g. JSR-305 @ParametersAreNonnullByDefault) and @Nullable annotations.

Thanks again for your time and for maintaining MapStruct.

First, thank you very much @filiphr for adding support for nullness annotations! This has been a long-expected feature that I’m happy to see.

I would like to support @vmckinney-cainc's feedback on considering additional nullness annotations (other than JSpecify, such as lombok.NonNull or jakarta.annotation.Nonnull), as this could improve the experience of working with third-party library code.

As lombok is a generation tool, isn't it supposed to generate the JSpecify annotation based on its annotations ?

@filiphr

I would argue that you are doing an incorrect search for JSpecify

Apologies! Didn't mean to misrepresent the popularity, I'm just not familiar with JSpecify.

JSpecify is becoming the industry standard

I can say at my company in my little niche that in our corporate git we have 1 single match for jspecify. That being said, MapStruct only supporting JSpecify is another reason for us to consider it. I understand giving bias towards open-source projects and don't want to challenge that or pressure you or anything. It's not like closed-source is doing anything to help you :)

@pkernevez

isn't [lombok] supposed to generate the JSpecify annotation[?]

With lombok.addNullAnnotations you can generate jspecify nullability annotations, but only for code which lombok generates. This means that anywhere where we have @lombok.NonNull stays @lombok.NonNull unless we manually migrate.

@rRgbMVg7tO

Therefore, I would like to propose introducing an annotation processor option

Caveat with regards to Lombok: @lombok.NonNull has CLASS retention, as opposed to JSpecify and Jakarta's RUNTIME. I think there'd have to be some bytecode introspection or something to look towards Lombok. Ugly stuff.

JSpecify is becoming the industry standard

I can say at my company in my little niche that in our corporate git we have 1 single match for jspecify. That being said, MapStruct only supporting JSpecify is another reason for us to consider it. I understand giving bias towards open-source projects and don't want to challenge that or pressure you or anything. It's not like closed-source is doing anything to help you :)

JSpecify is not just yet another nonNull annotation specification. It's a consortium with a lot of major Java actor https://jspecify.dev/about/
Regarding the adoption it's a quite new project (~1 year from its publication) and it's already adopted in open source like Spring, SpringBoot and Hibernate.
It was complex, ~4 years to create 4 annotations (and the behaviors).
Other annotations like JetBrains, lombok or Jakarta, will probably be deprecated.

Thanks @pkernevez for the clarification around JSpecify.

JSpecify is indeed something that took a really long time to get to, and I think the reason has been that there have been all those big players with the different existing annotations. I think with JSpecify 1.0.0 out there, it would become the standard. Considering the fact that large open source projects like Spring and Hibernate have already adopted them, I think it's a matter of fact until other libraries follow suit. IntelliJ also supports them, Kotlin also supports them for their interoperability for reading Java code from Kotlin. They have KT-47417 open for the other way arround, i.e. emit JSpecify annotation from Kotlin itself.

@vmckinney-cainc, I'm not sure which other dependencies you use, but I would be surprised if you do not have jspecify on the transitive path if you are on the latest versions of your dependencies.

@rRgbMVg7tO thanks for your comment.

Because MapStruct currently only recognizes JSpecify annotations, those sources are treated as potentially nullable. For us, this results in many occurrences across mappers and would require adding defaultValue/defaultExpression for every affected property, even though the sources are annotated by their authors.

This was a trigger for me to actually add an opt-out for this using a compiler flag -Amapstruct.disableJSpecify=true.

While I agree that JSpecify is becoming the industry standard, certain libraries may choose to not migrate to it from other annotation types.

To be honest I would rather spend my time in helping those libraries get to JSpecify then spend the time on supporting the different flavors and quirks of the other nullability annoations.

If this direction sounds reasonable, we could also discuss adding configurable support for variations of @NullMarked (e.g. JSR-305 @ParametersAreNonnullByDefault) and @Nullable annotations.

That's the pandora's box I do not want to open.

@vmckinney-cainc not sure what you mean with

Caveat with regards to Lombok: @lombok.NonNull has CLASS retention, as opposed to JSpecify and Jakarta's RUNTIME. I think there'd have to be some bytecode introspection or something to look towards Lombok. Ugly stuff.

If you are thinking that it would be hard for us to read CLASS retention annotation, then no need to think about that. We run during the compilation phase, which means that we can read SOURCE (only for your own code in the same compilation module), CLASS and RUNTIME annotations.

Thanks everyone for your feedback. It is really valuable. However, my decision for this is final. I really do not want to spend time on supporting that the major Java actors have converged on. I would merge this PR in this stance and I would try to do a 1.7.0.Beta2 release soonish so that the community can already start trying it out.

Thanks for all you do @filiphr!! I am very happy to have this feature in any form :)

@filiphr Are you planning a beta.2 release before final 1.7.0? I think it would be good for this to get a more widely visible release.
[Edit: I just spotted/remembered your comment above, so this is a gentle nudge ;-) ]