Multiple @optics code gen issues with generic parameters

[magneticflux-]

Like #3380, #3381, #3384, and #3692, this deals with edge-cases with generic parameters. Unlike #3380 and #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 #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
    }
}

[kyay10]

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.

[serras]

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.

[magneticflux-]

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.

[serras]

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.

[kyay10]

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