smallrye-open-api
smallrye-open-api copied to clipboard
Kotlin: Handle language-level nullability and default values as orthogonal to each other
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
@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);
}
}
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.
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.
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
.
...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 :confused: )
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> {