arrow
arrow copied to clipboard
arrow-kt
Multiple @optics code gen issues with generic parameters
Like https://github.com/arrow-kt/arrow/issues/3380, https://github.com/arrow-kt/arrow/issues/3381, https://github.com/arrow-kt/arrow/issues/3384, and https://github.com/arrow-kt/arrow/issues/3692, this deals with edge-cases with generic parameters. Unlike https://github.com/arrow-kt/arrow/issues/3380 and https://github.com/arrow-kt/arrow/issues/3381 however, the properties being focused are invariant (polymorphic-wise and generic-wise) and so are safe to get and set. I've submitted them as a group since I think they're all closely related, but I can split them up if needed!
These are the failing cases I've found so far, what I think is happening, and what I think should happen.
User code
// Apache-2.0
package org.example.project
import arrow.optics.optics
@optics
sealed interface Test<T> {
val value: String
/**
* Test 1: Parameter passthrough
*
* This coincidentally works right now because it:
* 1. Preserves the generic `T` through [Test1.copy], and
* 2. Can have all of its generic parameters inferred from context.
* 3. Does not have any extra generic parameters not used in its parent
*/
data class Test1<T>(override val value: String) : Test<T>
/**
* Test 2: No child parameters, fixed parent parameters
*
* We are given something with type `Test<T> & Test2` (from the `when`'s smart-cast),
* and we need to return something of type `Test<T>`.
* Returning it verbatim is OK because we retain that hidden intersection type,
* but using [Test2.copy] reduces it to just `Test2` (due to the lack of a self-type in Kotlin, etc.).
*
* This can be solved by added a cast `as Test<T>` to reintroduce that intersection type.
* This is safe because the generated `copy` functions for data classes preserve their type and cannot be overridden.
*/
data class Test2(override val value: String) : Test<Int>
/**
* Test 3: Extra child parameter, parameter passthrough
*
* The `is` statement (inside the `when`)
* requires explicit generics on classes _unless_ all generics can be inferred.
* The extra parameters are also missing from the [arrow.optics.Prism], etc. functions.
*
* This can be solved by explicitly writing out the generics with wildcards for types not present in the parent,
* like `is Test.Test3<T, *>` and by including the new parameters in the discriminator prism functions.
*/
data class Test3<T, A>(override val value: String) : Test<T>
/**
* Test 4: Indirect parameter passthrough
*
* This triggers both of the previous issues:
* It needs an explicit cast `as Test<T>`,
* and it also needs explicit generics in the `when` branch `is Test.Test4<*>` and for each function.
*/
data class Test4<B>(override val value: String) : Test<List<B>>
companion object
}
Cleaned-up generated code
package org.example.project
import arrow.optics.Lens
import arrow.optics.Optional
import arrow.optics.Prism
import arrow.optics.Traversal
fun <T> Test.Companion.value(): Lens<Test<T>, String> =
arrow.optics.Lens(
get = { test: Test<T> -> test.value },
set = { test: Test<T>, value: String ->
when (test) {
is Test.Test1 -> test.copy(value = value)
is Test.Test2 -> test.copy(value = value)
is Test.Test3 -> test.copy(value = value)
is Test.Test4 -> test.copy(value = value)
}
}
)
fun <T> Test.Companion.test1(): Prism<Test<T>, Test.Test1<T>> = Prism.instanceOf()
fun Test.Companion.test2(): Prism<Test<Int>, Test.Test2> = Prism.instanceOf()
fun <T> Test.Companion.test3(): Prism<Test<T>, Test.Test3<T, A>> = Prism.instanceOf()
fun Test.Companion.test4(): Prism<Test<List<B>>, Test.Test4<B>> = Prism.instanceOf()
fun <__S, T> Optional<__S, Test<T>>.test1(): Optional<__S, Test.Test1<T>> = this + Test.test1()
fun <__S, T> Prism<__S, Test<T>>.test1(): Prism<__S, Test.Test1<T>> = this + Test.test1()
fun <__S, T> Traversal<__S, Test<T>>.test1(): Traversal<__S, Test.Test1<T>> = this + Test.test1()
fun <__S> Optional<__S, Test<Int>>.test2(): Optional<__S, Test.Test2> = this + Test.test2()
fun <__S> Prism<__S, Test<Int>>.test2(): Prism<__S, Test.Test2> = this + Test.test2()
fun <__S> Traversal<__S, Test<Int>>.test2(): Traversal<__S, Test.Test2> = this + Test.test2()
fun <__S, T> Optional<__S, Test<T>>.test3(): Optional<__S, Test.Test3<T, A>> = this + Test.test3()
fun <__S, T> Prism<__S, Test<T>>.test3(): Prism<__S, Test.Test3<T, A>> = this + Test.test3()
fun <__S, T> Traversal<__S, Test<T>>.test3(): Traversal<__S, Test.Test3<T, A>> = this + Test.test3()
fun <__S> Optional<__S, Test<List<B>>>.test4(): Optional<__S, Test.Test4<B>> = this + Test.test4()
fun <__S> Prism<__S, Test<List<B>>>.test4(): Prism<__S, Test.Test4<B>> = this + Test.test4()
fun <__S> Traversal<__S, Test<List<B>>>.test4(): Traversal<__S, Test.Test4<B>> = this + Test.test4()
My imagined fixed generated code
package org.example.project
import arrow.optics.Lens
import arrow.optics.Optional
import arrow.optics.Prism
import arrow.optics.Traversal
fun <T> Test.Companion.value(): Lens<Test<T>, String> =
arrow.optics.Lens(
get = { test: Test<T> -> test.value },
set = { test: Test<T>, value: String ->
when (test) {
is Test.Test1<T> -> test.copy(value = value) as Test<T>
is Test.Test2 -> test.copy(value = value) as Test<T>
is Test.Test3<T, *> -> test.copy(value = value) as Test<T>
is Test.Test4<*> -> test.copy(value = value) as Test<T>
}
}
)
fun <T> Test.Companion.test1(): Prism<Test<T>, Test.Test1<T>> = Prism.instanceOf()
fun Test.Companion.test2(): Prism<Test<Int>, Test.Test2> = Prism.instanceOf()
fun <T, A> Test.Companion.test3(): Prism<Test<T>, Test.Test3<T, A>> = Prism.instanceOf()
fun <B> Test.Companion.test4(): Prism<Test<List<B>>, Test.Test4<B>> = Prism.instanceOf()
fun <__S, T> Optional<__S, Test<T>>.test1(): Optional<__S, Test.Test1<T>> = this + Test.test1()
fun <__S, T> Prism<__S, Test<T>>.test1(): Prism<__S, Test.Test1<T>> = this + Test.test1()
fun <__S, T> Traversal<__S, Test<T>>.test1(): Traversal<__S, Test.Test1<T>> = this + Test.test1()
fun <__S> Optional<__S, Test<Int>>.test2(): Optional<__S, Test.Test2> = this + Test.test2()
fun <__S> Prism<__S, Test<Int>>.test2(): Prism<__S, Test.Test2> = this + Test.test2()
fun <__S> Traversal<__S, Test<Int>>.test2(): Traversal<__S, Test.Test2> = this + Test.test2()
fun <__S, T, A> Optional<__S, Test<T>>.test3(): Optional<__S, Test.Test3<T, A>> = this + Test.test3()
fun <__S, T, A> Prism<__S, Test<T>>.test3(): Prism<__S, Test.Test3<T, A>> = this + Test.test3()
fun <__S, T, A> Traversal<__S, Test<T>>.test3(): Traversal<__S, Test.Test3<T, A>> = this + Test.test3()
fun <__S, B> Optional<__S, Test<List<B>>>.test4(): Optional<__S, Test.Test4<B>> = this + Test.test4()
fun <__S, B> Prism<__S, Test<List<B>>>.test4(): Prism<__S, Test.Test4<B>> = this + Test.test4()
fun <__S, B> Traversal<__S, Test<List<B>>>.test4(): Traversal<__S, Test.Test4<B>> = this + Test.test4()
Diff of generated code
diff --git a/generated.kt b/fixed.kt
index cc6c7fc..149201c 100644
--- a/generated.kt
+++ b/fixed.kt
@@ -10,31 +10,31 @@ fun <T> Test.Companion.value(): Lens<Test<T>, String> =
get = { test: Test<T> -> test.value },
set = { test: Test<T>, value: String ->
when (test) {
- is Test.Test1 -> test.copy(value = value)
- is Test.Test2 -> test.copy(value = value)
- is Test.Test3 -> test.copy(value = value)
- is Test.Test4 -> test.copy(value = value)
+ is Test.Test1<T> -> test.copy(value = value) as Test<T>
+ is Test.Test2 -> test.copy(value = value) as Test<T>
+ is Test.Test3<T, *> -> test.copy(value = value) as Test<T>
+ is Test.Test4<*> -> test.copy(value = value) as Test<T>
}
}
)
fun <T> Test.Companion.test1(): Prism<Test<T>, Test.Test1<T>> = Prism.instanceOf()
fun Test.Companion.test2(): Prism<Test<Int>, Test.Test2> = Prism.instanceOf()
-fun <T> Test.Companion.test3(): Prism<Test<T>, Test.Test3<T, A>> = Prism.instanceOf()
-fun Test.Companion.test4(): Prism<Test<List<B>>, Test.Test4<B>> = Prism.instanceOf()
+fun <T, A> Test.Companion.test3(): Prism<Test<T>, Test.Test3<T, A>> = Prism.instanceOf()
+fun <B> Test.Companion.test4(): Prism<Test<List<B>>, Test.Test4<B>> = Prism.instanceOf()
fun <__S, T> Optional<__S, Test<T>>.test1(): Optional<__S, Test.Test1<T>> = this + Test.test1()
fun <__S, T> Prism<__S, Test<T>>.test1(): Prism<__S, Test.Test1<T>> = this + Test.test1()
fun <__S, T> Traversal<__S, Test<T>>.test1(): Traversal<__S, Test.Test1<T>> = this + Test.test1()
fun <__S> Optional<__S, Test<Int>>.test2(): Optional<__S, Test.Test2> = this + Test.test2()
fun <__S> Prism<__S, Test<Int>>.test2(): Prism<__S, Test.Test2> = this + Test.test2()
fun <__S> Traversal<__S, Test<Int>>.test2(): Traversal<__S, Test.Test2> = this + Test.test2()
-fun <__S, T> Optional<__S, Test<T>>.test3(): Optional<__S, Test.Test3<T, A>> = this + Test.test3()
-fun <__S, T> Prism<__S, Test<T>>.test3(): Prism<__S, Test.Test3<T, A>> = this + Test.test3()
-fun <__S, T> Traversal<__S, Test<T>>.test3(): Traversal<__S, Test.Test3<T, A>> = this + Test.test3()
+fun <__S, T, A> Optional<__S, Test<T>>.test3(): Optional<__S, Test.Test3<T, A>> = this + Test.test3()
+fun <__S, T, A> Prism<__S, Test<T>>.test3(): Prism<__S, Test.Test3<T, A>> = this + Test.test3()
+fun <__S, T, A> Traversal<__S, Test<T>>.test3(): Traversal<__S, Test.Test3<T, A>> = this + Test.test3()
-fun <__S> Optional<__S, Test<List<B>>>.test4(): Optional<__S, Test.Test4<B>> = this + Test.test4()
-fun <__S> Prism<__S, Test<List<B>>>.test4(): Prism<__S, Test.Test4<B>> = this + Test.test4()
-fun <__S> Traversal<__S, Test<List<B>>>.test4(): Traversal<__S, Test.Test4<B>> = this + Test.test4()
+fun <__S, B> Optional<__S, Test<List<B>>>.test4(): Optional<__S, Test.Test4<B>> = this + Test.test4()
+fun <__S, B> Prism<__S, Test<List<B>>>.test4(): Prism<__S, Test.Test4<B>> = this + Test.test4()
+fun <__S, B> Traversal<__S, Test<List<B>>>.test4(): Traversal<__S, Test.Test4<B>> = this + Test.test4()
Along the way, I wrote some code demonstrating why property type variance is incompatible with naively generated lenses. @sindrenm mentioned this https://github.com/arrow-kt/arrow/issues/3381 "I'm not, however, sure how safe those casts are" and I think this makes it clear they aren't 😆.
Bonus
package org.example.project
import arrow.optics.Lens
interface Pet
class Dog : Pet
class Cat : Pet
sealed interface GenericPetHolder<out T : Pet> {
val pet: T
}
data class GenericDogHolder(override val pet: Dog) : GenericPetHolder<Dog>
data class GenericCatHolder(override val pet: Cat) : GenericPetHolder<Cat>
fun <T : Pet> genericPet(): Lens<GenericPetHolder<T>, T> = Lens(
{ base: GenericPetHolder<T> -> base.pet },
{ base: GenericPetHolder<T>, value: T ->
when (base) {
is GenericDogHolder -> base.copy(pet = value as Dog) as GenericPetHolder<T>
is GenericCatHolder -> base.copy(pet = value as Cat) as GenericPetHolder<T>
}
},
)
sealed interface ExplicitPetHolder {
val pet: Pet
}
data class ExplicitDogHolder(override val pet: Dog) : ExplicitPetHolder
data class ExplicitCatHolder(override val pet: Cat) : ExplicitPetHolder
fun explicitPet(): Lens<ExplicitPetHolder, Pet> = Lens(
{ base: ExplicitPetHolder -> base.pet },
{ base: ExplicitPetHolder, value: Pet ->
when (base) {
is ExplicitDogHolder -> base.copy(pet = value as Dog)
is ExplicitCatHolder -> base.copy(pet = value as Cat)
}
},
)
fun main() {
val a1 = genericPet<Pet>()
val b1 = a1.get(GenericDogHolder(Dog()))
val c1 = try {
a1.set(GenericDogHolder(Dog()), Cat())
} catch (e: ClassCastException) {
e.printStackTrace()
null
}
val a2 = explicitPet()
val b2 = a2.get(ExplicitDogHolder(Dog()))
val c2 = try {
a2.set(ExplicitDogHolder(Dog()), Cat())
} catch (e: ClassCastException) {
e.printStackTrace()
null
}
}
I'm not the most familiar with Optics, but I feel like we should just generate code that looks like:
fun <T> Test.Companion.value(): Lens<Test<T>, String> =
arrow.optics.Lens(
get = { test: Test<T> -> test.value },
set = { test: Test<T>, value: String ->
when (test) {
is Test.Test1<*> -> test.copy(value = value)
is Test.Test2 -> test.copy(value = value)
is Test.Test3<*, *> -> test.copy(value = value)
is Test.Test4<*> -> test.copy(value = value)
} as Test<T>
}
)
and be done with it. Maybe there's some edgecase I'm not seeing here though where value: T2 for some T2, and hence casting with star-projected types won't work well.
In my opinion, doing this in general is very hard without compiler support for subtyping reconstruction. I would really like not to add unchecked casts in generated code, since it may lead to problems at unexpected times.
Subtyping reconstruction would also solve everything, but in this case I think the casts are sound specifically because we already disallow variant property types. I don't know how allowing property type variance would work with the existing optics (it feels like it needs function-specific generic parameters passed through each layer?) so I don't see that restriction being lifted without a major restructuring.
The other problem (actually including all of the generic parameters) is already sound, the types just need to be written out with star-projections.
Interestingly, I found a corner in the Scala documentation that explains with the casts are problematic with out variance. But in any other cases, they seem to be safe.
This is mentioned in the subtype reconstruction keep. Scala allows such inconsistent type parameters, but Kotlin doesn't. That Weird example doesn't even compile in Kotlin:
interface Expr<out A>
interface IntList: Expr<List<Int>>
class Weird: IntList, Expr<Nothing> // Type parameter 'A' of 'interface Expr<out A> : Any' has inconsistent values: List<Int>, Nothing.
Meaning that the casts likely are safe in Kotlin