[Arrow Optics] Generated optics code does not escape Kotlin keywords in package names

[hrafnthor]

Description:

Testing out using 'arrow-optics' in a project of mine, it immediatly caused compilation errors due to package names in the generated code containing unescaped Kotlin keywords from the originator package.

For instance the following code is from my project

package `is`.hth.projectname

import arrow.optics.optics

@optics
public data class Audio(
    val url: String = "",
    val secureUrl: String = "",
    val type: String = ""
) {
    public companion object
}

And it generates the following:

package is.hth.projectname

public inline val is.hth.projectname.Audio.Companion.iso: arrow.optics.Iso<is.hth.projectname.Audio, kotlin.Triple<kotlin.String, kotlin.String, kotlin.String>> inline get() = arrow.optics.Iso(
  get = { audio: is.hth.projectname.Audio -> kotlin.Triple(audio.`url`, audio.`secureUrl`, audio.`type`) },
  reverseGet = { triple: kotlin.Triple<kotlin.String, kotlin.String, kotlin.String> -> is.hth.projectname.Audio(triple.first, triple.second, triple.third) }
)

In this case the keyword is being used in the package name to reference a reverse TLD.

A list of TLD's that match Kotlin's reserved keywords:

• as: American Samoa
• by: Belarus
• do: Dominican Republic
• in: India
• is: Iceland
• it: Italy

Versions

arrow: 1.1.5
kotlin: 1.8.0
ksp: 1.8.0-1.0.8

[hrafnthor]

Searching through the issues I found this one where a similar problem is mentioned. It however was closed with the message that "[arrow] are no longer supporting this type of type class in Arrow in the latest versions".

Probably this is referring to the arrow.typeclasses.Show type class being unsupported in later versions, rather than not wanting to escape package names in generated code.

If my understanding is correct, would a pull request looking at solving this issue be appreciated?

[nomisRev]

If my understanding is correct, would a pull request looking at solving this issue be appreciated?

Hey @hrafnthor,
Thanks for opening an issue with details reproducible steps! A PR would absolutely by greatly appreciated 🙏

[hrafnthor]

I have staged a PR #2925 for consideration. As this is my first contribution to the arrow project, all constructive feedback is appreciated as any norms and coding styles within it are currenlty unknown to me.

After going over the contribution document, I noticed that use of spotless is required for contribution PRs. However it does not seem that spotless is configured for the project. Or perhaps I am missunderstanding?

I did notice that all the unit tests contained in DSLTests.kt, IsoTests.kt, OptionalTests.kt, PrimsTests.kt broke as soon they were given package declarations which contained Kotlin keywords. My local changes that remedy those issues were held back to keep the scope relevant to this issue.

Please let me know if you would want me to add those changes as well to the PR. They use the same functionality that the PR already adds, but in other locations.

[nomisRev]

Thank you so much for creating a PR fixing the issues you reported so fast 😍

I noticed that use of spotless is required for contribution PRs. However it does not seem that spotless is configured for the project. Or perhaps I am missunderstanding?

No, you're correct. We had some conflict between KotlinX Knit and Spotless and had to temporarily disable it. We're in the process of re-adding it to the project.

I did notice that all the unit tests contained in DSLTests.kt, IsoTests.kt, OptionalTests.kt, PrimsTests.kt broke as soon they were given package declarations which contained Kotlin keywords. My local changes that remedy those issues were held back to keep the scope relevant to this issue.

I am not sure why those tests would break 🤔 Feel free to add the fix to the PR, it sounds like the changes are related. Need to make some time to review your changes in Optics, it's been some time since I worked on it so need to take a closer look 😅

[hrafnthor]

I am not sure why those tests would break

Sorry, I was probably not too clear on what I meant 🙂

As can be seen in the changes made in the PR here there were mock package definitions added to the test cases for Lens to make sure the sanitization changes work.

If the same dummy package field is added to any of the other test cases in DSLTests.kt, IsoTests.kt, OptionalTests.kt, PrimsTests.kt then the generated code will be cause errors.

Feel free to add the fix to the PR, it sounds like the changes are related.

I've added those other changes to the PR now 👍

[nomisRev]

Closed by #2925