Skip to content
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 type-safe queries and updates are not type-safe at all #4798

Open
pmatysek opened this issue Sep 27, 2024 · 1 comment · May be fixed by #4799 or #4800
Open

Kotlin type-safe queries and updates are not type-safe at all #4798

pmatysek opened this issue Sep 27, 2024 · 1 comment · May be fixed by #4799 or #4800
Assignees
Labels
in: kotlin Kotlin support status: blocked An issue that's blocked on an external project change status: waiting-for-triage An issue we've not yet triaged

Comments

@pmatysek
Copy link
Contributor

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 @OnlyInputTypes annotation which is identical to an original one, ie. the same package, the same name
  • add -Xallow-kotlin-package compiler arg to bypass Only the Kotlin standard library is allowed to use the 'kotlin' package error

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: Litote/kmongo@413539e

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

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 27, 2024
pmatysek added a commit to pmatysek/spring-data-mongodb that referenced this issue Sep 27, 2024
pmatysek added a commit to pmatysek/spring-data-mongodb that referenced this issue Sep 27, 2024
@mp911de mp911de self-assigned this Sep 30, 2024
@christophstrobl christophstrobl added the in: kotlin Kotlin support label Oct 1, 2024
@mp911de
Copy link
Member

mp911de commented Oct 1, 2024

Thanks for reaching out. Since we decided to support Kotlin, we're constantly patching our code for the odd quirks that Kotlin requires other libraries to do in order to leverage Kotin functionality. By now, we have sprawled Kotlin functionality all over our code base.

Roman Elizarov: [The OnlyInputTypes annotation] is under-designed at the moment.

Focusing on the second part of Roman's reply is true for a lot of Kotlin aspects. While Kotlin provides a good experience on the user side, it creates a lot of damage to integrators that want to provide functionality for Kotlin users.

You might argue that adding another tiny bit to our already existing sprawl doesn't make a big difference – where do you draw the line? KT-13198 is 8 years old and the problem doesn't seem to be addressed soon.

For the time being, I would like not to introduce additional changes for Kotlin support until Kotlin sorts out these issues for itself.

@mp911de mp911de added the status: blocked An issue that's blocked on an external project change label Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: kotlin Kotlin support status: blocked An issue that's blocked on an external project change status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
4 participants