spring-projects / spring-projects/spring-data-mongodb
Kotlin type-safe queries and updates are not type-safe at all
@mp911de is already working on this.
Since Sep 30, 2024.
- Dominant language
- Java
- Stars
- 1.7k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
Hi,
spring-data-mongodb supports in Kotlin typed queries and recently introduced with my help typed updates too (#3028). While this API is handy because there's no need to keep field names in constants, it has one hidden drawback or bug - it's not type-safe at any manner. Even though docs (that one I wrote too) and its construction suggest it undeniably should be.
Problem explanation
I will show it by an example:
data class Book(val title: String)
val typed = Book::title isEqualTo 1234 // it should be prohibited by the compiler and it's not for now
isEqualTo infix fun is defined in a way that as I said undeniably suggests that it should be type-safe:
infix fun <T> KProperty<T>.isEqualTo(value: T) =
Criteria(this.toDotPath()).isEqualTo(value)
Why the type of value is not checked properly by a compiler? The culprit is declaration-site variance Kotlin mechanism. And especially the fact that KProperty interface has its generic type marked with out variance annotation:
public expect interface KProperty<out V>
Possible solutions
While the problem seems to be complicated and rather lies on the Kotlin language side, there is a simple (but tricky) solution. JetBrains team has internal annotation (in kotlin.internal package) @OnlyInputTypes designed for cases like this. The solution would be as simple as just using it on a generic type:
infix fun <@OnlyInputTypes T> KProperty<T>.isEqualTo(value: T) =
Criteria(this.toDotPath()).isEqualTo(value)
data class Book(val title: String)
val typed = Book::title isEqualTo 1234 // now isEqualTo is type-safe so this code will not compile
However, this solution has one drawback - while this annotation is in kotlin.internal package it can't be used directly outside the Kotlin project. There are two workarounds to use it, but both are tricky.
First:
- create your own copy of
@OnlyInputTypesannotation which is identical to an original one, ie. the same package, the same name - add
-Xallow-kotlin-packagecompiler arg to bypassOnly the Kotlin standard library is allowed to use the 'kotlin' packageerror
Second:
@Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE")before each usage or on the whole file:@file:Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE")
Both of them are rather workarounds with some drawbacks. Another drawback of fixing this bug is the fact that it breaks the backward compatibility of this API - some code will stop compiling (but let's say very error-prone code that uses this API in the wrong way)
I have prepared code with these solutions and can post PRs later on.
Moreover, I did some research and saw that ex. KMongo uses @OnlyInputTypes too: https://github.com/Litote/kmongo/commit/413539e7f7364774d292f8a6332bd4e9a9d5bd14
What are your thoughts about that? Do you think that introducing a fix with @OnlyInputTypes would be OK?
Resources
https://youtrack.jetbrains.com/issue/KT-13198
https://discuss.kotlinlang.org/t/restrict-type-by-receiver/5492
https://kotlinlang.org/docs/generics.html
https://stackoverflow.com/questions/54779553/kotlin-generic-constraints-require-param-to-be-of-same-type-as-other <-- there is workaround with wrapping KProperty into own type, but I can't see a way to use it in a clean and seamless way
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.