Kotlin: Handle language-level nullability and default values as orthogonal to each other

[okarmazin]

The current behavior implemented in response to #716 is incorrect. Whenever a property is not nullable, it gets marked as required even if it has a default value, which is an error. A property which has a default value is not required.

I realize that the current behavior may be a sane default for the Java world due to the absence of type-level nullability and the absence of default values. You are forced to conflate the two orthogonal attributes.

Since Kotlin does have language-level support for both default values and nullability, SmallRye OpenAPI should be able to take these language features into consideration.

The presence or absence of a default value in a property declaration must only affect the required attribute.

Nullability or non-nullability of a property must only affect the nullable attribute.

val s: String should produce required = true; nullable = false
val s: String = "default" should produce required = false; nullable = false; default = "default"

val s: String? should produce required = true; nullable = true
val s: String? = null should produce required = false; nullable = true; default = null

[MikeEdgar]

@okarmazin I see your point and I don't disagree. I think the missing piece is how to get the default value from the class file. From what I can tell, it's set in a synthetic constructor and is not readily available to the annotation scanner.

Look at the below (abbreviated) Java class, decompiled from the bytecode resulting from this Kotlin class:

data class KotlinBean (
    val description: String = "the-default"
)

public final class KotlinBean {
   @NotNull
   private final String description;

   public KotlinBean(@NotNull String description) {
      Intrinsics.checkNotNullParameter(description, "description");
      super();
      this.description = description;
   }

   // $FF: synthetic method
   public KotlinBean(String var1, int var2, DefaultConstructorMarker var3) {
      if ((var2 & 1) != 0) {
         var1 = "the-default"; // <- constant value for the default
      }

      this(var1);
   }
}

[MikeEdgar]

This might be a good issue to raise with the Kotlin language/compile project. It would be much more useful if the field had an annotation with the default value so that it could be retrieved in cases like this.

[okarmazin]

Let's forget about the actual default value. I only included that in the original post in hopes that there would be a reasonable heuristic to extract the value when it's a compile time constant of primitive type. Apparently there isn't, let's move away from that idea.

Would you be able to incorporate Kotlin Symbol Processing in this project? KSValueParameter has a field hasDefault that can serve as the basis for the required / not required attribute.

[MikeEdgar]

I don't think the symbol processing approach will work. The annotation scanning functionality is based on a Jandex index, which itself is based on bytecode. This input this project depends on is several steps away from the Kotlin source code.

It appears the only thing that can be done to improve the situation would be to remove the code that sets required: true when @org.jetbrains.annotations.NotNull is present and require the application to provide the additional information about the default and requirement using @Schema.

[rgmz]

...and require the application to provide the additional information about the default and requirement using @Schema.

Interestingly @Parameter also works, but only if you explicitly declare required = false (even though required = false is the default value 😕 )

No @Parameter annotation

    @GET
    suspend fun list(
        @RestQuery
        columns: List<String> = emptyList(),
    ): RestResponse<String> {

@Parameter annotation

    @GET
    suspend fun list(
        @RestQuery
        @Parameter
        columns: List<String> = emptyList(),
    ): RestResponse<String> {

@Parameter annotation with required = false

    @GET
    suspend fun list(
        @RestQuery
        @Parameter(required = false)
        columns: List<String> = emptyList(),
    ): RestResponse<String> {