codegen-java: Support generating @Nullable annotations

[odenix]

Motivation:
codegen-java already supports generating @NonNull annotations.
However, many projects prefer @Nullable over @NonNull annotations.
JSpecify also recommends @Nullable.

Changes:

• Add a --nullable-annotation CLI parameter that defaults to none.
• Add a nullableAnnotation Gradle/CliJavaCodeGenerator/JavaCodeGenerator property that defaults to null.
• Annotate nullable types with the configured annotation (if any).

As part of this feature, I propose to also make the following changes:

• Change the -non-null-annotation CLI parameter and nonNullAnnotation properties accordingly.
• Do not generate @NonNull annotations by default.
• Deprecate org.pkl.java.config.mapper.NonNull and encourage users to switch to JSpecify.
  Note that tools won't recognize org.pkl.java.config.mapper.NonNull unless explicitly configured.

Open question:
@Nullable must be accompanied by @NullMarked (JSpecify) or @NonnullByDefault (JSR 305).
I can think of two ways to go about this:

1. Introduce --null-marked-annotation and annotate config classes accordingly.
   This is the most consistent solution, but it requires yet another CLI parameter and property.
   It can be made more convenient by inferring the correct annotation based on --nullable-annotation and --non-null-annotation for popular libraries such as JSpecify and JSR 305.
2. Leave it to users to add a package-info.java with a @NullMarked annotation.
   This is a clean and simple solution. However, it is less convenient than (1).
   It could, in theory, cause problems if users compile generated and handwritten code separately and don't have a place where to put package-info.java.

Final thought: The Java ecosystem is converging on JSpecify. For many users, the best option would be --generate-jspecify-annotations.

[holzensp]

I agree with the overall approach. With JSpecify gaining traction, it's a reasonable choice (and any somewhat popular annotation framework is better than bespoke annotations, because of the "unless explicitly configured").

but it requires yet another CLI parameter and property.

It seems cleaner to me to end up with a single CLI parameter and property, that picks the flavor, rather than a parameter and property per flavor. I like --nullable-annotation (or --nullability-annotation to avoid the implication that it's only about annotating the nullable case, rather than the non-nullable case).

I agree with your cost balancing assessment between (1) and (2), and I do like clean and simple. However, I think it's more important that our generated code is correct by default and (2) does leave awkward ways to hold it wrong. I prefer (1).

Let's have @stackoverflow & @bioball chime in also.

[odenix]

It seems cleaner to me to end up with a single CLI parameter and property, that picks the flavor

Can you clarify what you’re proposing here?

[bioball]

I think maybe a better solution is to let users configure what library they want their nullness annotations to come from (this is maybe what @holzensp is talking about too). Something like: --nullable-annotation=jsr305|jspecify|pkl.

But, another issue here is that JSR305 doesn't actually have a NonnullByDefault annotation. The closest thing is ParametersAreNonnullByDefault, but that only covers method parameters, and not fields nor method return types. For the JSR305 case, I think we'd need to do something like: also generate a @NonnullByDefault annotation class that can applied to the class.

2. Leave it to users to add a package-info.java with a @NullMarked annotation.
   This is a clean and simple solution. However, it is less convenient than (1).
   It could, in theory, cause problems if users compile generated and handwritten code separately and don't have a place where to put package-info.java.

This doesn't seem like a great solution; it feels like we'd be generating incomplete code.

Also, FWIW: I'm not sure how big of a problem this is, and at the same time, adds a lot of complexity to our code generator. It might be good to hold off for some more +1's from the community.

[odenix]

Not supporting @Nullable/@NullMarked means not supporting the recommended way to use jspecify. I’ve yet to see a project that uses jspecify and doesn’t use @Nullable/@NullMarked. If you think a generic solution is too complex, we should at least support this mode for jspecify. jsr305 is dead, and I don’t think it needs to be considered other than keeping the existing support.

Pkl’s @NonNull annotation should be deprecated as tools won’t understand it.

[bioball]

Pkl’s @nonnull annotation should be deprecated as tools won’t understand it.

I don't think this needs to be deprecated; tools like IntelliJ let you configure nullability annotations. The intention here is that, by default, Pkl's generated code does not require any dependencies.

If you think a generic solution is too complex, we should at least support this mode for jspecify

Yeah, fair. I think we can have two flags; the nullability library to use, and whether to generate nullable-by-default annotations.

And we can throw if the nullability mode is non-null-by-default if targeting JSR305; e.g. throw here:

pkl-codegen-java --nullable-annotations=jsr305 --nullable-by-default=false

[odenix]

I don't think this needs to be deprecated; tools like IntelliJ let you configure nullability annotations. The intention here is that, by default, Pkl's generated code does not require any dependencies.

It doesn’t need to be, but users who care about nullness annotations should be strongly encouraged to use one of the widely supported libraries instead of configuring Pkl’s proprietary @NonNull. And users who don’t care about nullness annotations won’t miss Pkl’s @NonNull either.

pkl-codegen-java --nullable-annotations=jsr305 --nullable-by-default=false

Given that we already have —non-null-annotation=fqcn, the most consistent solution would be to support —nullable-annotation=fqcn and support combining them. If only —nullable-annotation=org.jspecify.annotations.Nullable is specified, we know that we must also add @NullMarked. We could support jspecify and jsr305 as shorthands for the fqcn’s.

[bioball]

It doesn’t need to be, but users who care about nullness annotations should be strongly encouraged to use one of the widely supported libraries instead of configuring Pkl’s proprietary @nonnull. And users who don’t care about nullness annotations won’t miss Pkl’s @nonnull either.

I'm okay with switching the default to be JSpecify, but I'd prefer to keep around the current annotations as an alternative. We have users that are sensitive to new dependencies, so it would be good to provide options to those that don't want to incur a new dependency here. But, yeah, in principle, I agree that people that care about nullable annotations would want to use a known library.

Given that we already have —non-null-annotation=fqcn, the most consistent solution would be to support —nullable-annotation=fqcn and support combining them. If only —nullable-annotation=org.jspecify.annotations.Nullable is specified, we know that we must also add @NullMarked. We could support jspecify and jsr305 as shorthands for the fqcn’s.

Your suggestion seems a little quirky to me; it allows for someone to choose --nullable-annotation=jspecify --non-null-annotation=jsr305, which there isn't a real-world use for.

I feel that deprecating --non-null-annotation and introducing two new flags feels like the better approach. But, I also don't feel that strongly about this, and I'm okay with accepting a PR for either approach.