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

Keyword sanitization for Lens code generation (#2924) #2925

Merged
merged 6 commits into from
Feb 9, 2023
Merged
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -6,10 +6,10 @@ import com.google.devtools.ksp.symbol.*
import java.util.Locale

data class ADT(val pckg: KSName, val declaration: KSClassDeclaration, val targets: List<Target>) {
val sourceClassName = declaration.qualifiedNameOrSimpleName
val sourceClassName = declaration.qualifiedNameOrSimpleName.sanitizeDelimited()
val sourceName = declaration.simpleName.asString().replaceFirstChar { it.lowercase(Locale.getDefault()) }
val simpleName = declaration.nameWithParentClass
val packageName = pckg.asString()
val packageName = pckg.asString().sanitizeDelimited()
val visibilityModifierName = when (declaration.companionObject?.getVisibility()) {
Visibility.INTERNAL -> "internal"
else -> "public"
Original file line number Diff line number Diff line change
@@ -135,10 +135,10 @@ internal fun KSClassDeclaration.getConstructorTypesNames(): List<String> =

internal fun KSType.qualifiedString(): String = when (declaration) {
is KSTypeParameter -> {
val n = declaration.simpleName.asString()
val n = declaration.simpleName.asString().sanitizeDelimited()
if (isMarkedNullable) "$n?" else n
}
else -> when (val qname = declaration.qualifiedName?.asString()) {
else -> when (val qname = declaration.qualifiedName?.asString()?.sanitizeDelimited()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might read better if asStringSanitized was an extension of Name (but if you prefer this way, it's also fine)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, it would be more readable as well as limit the scope of the function to relevant parts i.e KSName rather than any String. Will change the extension signature and apply it where applicable.

null -> toString()
else -> {
val withArgs = when {
Original file line number Diff line number Diff line change
@@ -6,3 +6,103 @@ package arrow.optics.plugin.internals
*/
fun String.plusIfNotBlank(prefix: String = "", postfix: String = "") =
if (isNotBlank()) "$prefix${this}$postfix" else this

/**
* Sanitizes each delimited section if it matches with Kotlin reserved keywords.
*/
fun String.sanitizeDelimited(delimiter: String = ".", separator: String = delimiter) =
if (isNotBlank() && contains(delimiter)) {
splitToSequence(delimiter).joinToString(separator) { if (kotlinKeywords.contains(it)) "`$it`" else it }
} else this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to consider empty and not containing delimiter as special cases, splitToSequence + joinToString should do the right thing



private val kotlinKeywords = setOf(
// Hard keywords
"as",
"break",
"class",
"continue",
"do",
"else",
"false",
"for",
"fun",
"if",
"in",
"interface",
"is",
"null",
"object",
"package",
"return",
"super",
"this",
"throw",
"true",
"try",
"typealias",
"typeof",
"val",
"var",
"when",
"while",

// Soft keywords
"by",
"catch",
"constructor",
"delegate",
"dynamic",
"field",
"file",
"finally",
"get",
"import",
"init",
"param",
"property",
"receiver",
"set",
"setparam",
"where",

// Modifier keywords
"actual",
"abstract",
"annotation",
"companion",
"const",
"crossinline",
"data",
"enum",
"expect",
"external",
"final",
"infix",
"inline",
"inner",
"internal",
"lateinit",
"noinline",
"open",
"operator",
"out",
"override",
"private",
"protected",
"public",
"reified",
"sealed",
"suspend",
"tailrec",
"value",
"vararg",

// These aren't keywords anymore but still break some code if unescaped.
// https://youtrack.jetbrains.com/issue/KT-52315
"header",
"impl",

// Other reserved keywords
"yield",
)
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ class DSLTests {
@Test
fun `DSL is generated for complex model with Every`() {
"""
|$`package`
|$imports
|$dslModel
|$dslValues
@@ -19,6 +20,7 @@ class DSLTests {
@Test
fun `DSL is generated for complex model with At`() {
"""
|$`package`
|$imports
|$dslModel
|$dslValues
@@ -32,6 +34,7 @@ class DSLTests {
// it's important to keep the 'Source' name for the class,
// because files in the test are named 'Source.kt'
"""
|$`package`
|$imports
|
|@optics
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ class IsoTests {
@Test
fun `Isos will be generated for data class`() {
"""
|$`package`
|$imports
|@optics
|data class IsoData(
@@ -23,6 +24,7 @@ class IsoTests {
@Test
fun `Isos will be generated for generic data class`() {
"""
|$`package`
|$imports
|@optics
|data class IsoData<A>(
@@ -37,6 +39,7 @@ class IsoTests {
@Test
fun `Isos will be generated for data class with secondary constructors`() {
"""
|$`package`
|$imports
|@optics
|data class IsoSecondaryConstructor(val fieldNumber: Int, val fieldString: String) {
@@ -52,6 +55,7 @@ class IsoTests {
@Test
fun `Iso generation requires companion object declaration`() {
"""
|$`package`
|$imports
|@optics
|data class IsoNoCompanion(
@@ -63,6 +67,7 @@ class IsoTests {
@Test
fun `Isos cannot be generated for huge classes`() {
"""
|$`package`
|$imports
|@optics
|data class IsoXXL(
@@ -92,6 +97,8 @@ class IsoTests {
|) {
| companion object
|}
""".failsWith { it.contains("IsoXXL".isoTooBigErrorMessage) }
""".failsWith {
it.contains("${`package`.removePrefix("package ").replace("`", "")}.IsoXXL".isoTooBigErrorMessage)
}
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package arrow.optics.plugin

import arrow.optics.plugin.internals.typeParametersErrorMessage
import org.junit.jupiter.api.Test

class LensTests {

@Test
fun `Lenses will be generated for data class`() {
"""
|$`package`
|$imports
|@optics
|data class LensData(
@@ -22,6 +22,7 @@ class LensTests {
@Test
fun `Lenses will be generated for data class with secondary constructors`() {
"""
|$`package`
|$imports
|@optics
|data class LensesSecondaryConstructor(val fieldNumber: Int, val fieldString: String) {
@@ -37,6 +38,7 @@ class LensTests {
@Test
fun `Lenses which mentions imported elements`() {
"""
|$`package`
|$imports
|
|@optics
@@ -52,6 +54,7 @@ class LensTests {
@Test
fun `Lenses which mentions type arguments`() {
"""
|$`package`
|$imports
|@optics
|data class OpticsTest<A>(val field: A) {
@@ -66,6 +69,7 @@ class LensTests {
@Test
fun `Lenses for nested classes`() {
"""
|$`package`
|$imports
|@optics
|data class LensData(val field1: String) {
@@ -84,6 +88,7 @@ class LensTests {
@Test
fun `Lenses for nested classes with repeated names (#2718)`() {
"""
|$`package`
|$imports
|@optics
|data class LensData(val field1: String) {
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ class OptionalTests {
@Test
fun `Optional will be generated for data class`() {
"""
|$`package`
|$imports
|@optics
|data class OptionalData(
@@ -21,6 +22,7 @@ class OptionalTests {
@Test
fun `Optional will be generated for generic data class`() {
"""
|$`package`
|$imports
|@optics
|data class OptionalData<A>(
@@ -35,6 +37,7 @@ class OptionalTests {
@Test
fun `Optional will be generated for data class with secondary constructors`() {
"""
|$`package`
|$imports
|@optics
|data class OptionalSecondaryConstructor(val fieldNumber: Int?, val fieldString: String?) {
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ class PrismTests {
@Test
fun `Prism will be generated for sealed class`() {
"""
|$`package`
|$imports
|@optics
|sealed class PrismSealed(val field: String, val nullable: String?) {
@@ -22,6 +23,7 @@ class PrismTests {
@Test
fun `Prism will be generated for generic sealed class`() {
"""
|$`package`
|$imports
|@optics
|sealed class PrismSealed<A,B>(val field: A, val nullable: B?) {
@@ -37,6 +39,7 @@ class PrismTests {
@Test
fun `Prism will not be generated for sealed class if DSL Target is specified`() {
"""
|$`package`
|$imports
|@optics([OpticsTarget.DSL])
|sealed class PrismSealed(val field: String, val nullable: String?) {
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package arrow.optics.plugin

const val `package` = "package `if`.`this`.`object`.`is`.`finally`.`null`.`expect`.`annotation`"

const val imports =
"""
import arrow.core.None