-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Kotlin nullability feature for typesafe client (client model) #2263
base: main
Are you sure you want to change the base?
Conversation
@robp94 would you be willing to give this a try and see if it suits your expectations? |
fb6b194
to
c899e34
Compare
Looks good to me. |
I played a bit with the MP @Mutation
fun testMatrix(working: List<@NonNull String>): Boolean {
println("hello world")
return true
} schema:
edit: I know it worked before the PR, but it could be substitute of an explicit nullability of Kotlin (inside generic arguments). Gonna try add some server-side tests. |
@mskacelik please add a little note (about having to add this flag) to our documentation, it will be useful to many developers |
c899e34
to
af9b182
Compare
Ok, it seems to have some problems regarding nested Lists, i.e.: But I think I found a bigger problem, in Quarkus
Very strange (once again AFAIK only Kotlin is doing this) |
/cc @jmartisk
Ok, so with nullabilities in Kotlin, it is a bit complicated; when not having an
?
nullable operator for types, Kotlin automatically adds its own Kotlin@NonNull
annotations, but not ALWAYS. For example, when having aList<String>
, the annotation is only applied on theList<>
part but not the argument String. So, making these two codes indistinguishable (via Jandex):On top of that, there are some weird behaviors like annotation is applied only on
MethodParameterInfo
and not theType
So, I only added a simple Kotlin nullability feature (see tests). So it can be consistent with the server side.
Java reflection implementation of typesafe client still does not have Kotlin nullability checking (todo?).
issue: #2142
@robp94