From 63b2a44fd194d5b30a2e88e6e0e1ca2a91ec92cc Mon Sep 17 00:00:00 2001 From: jooohn Date: Sun, 25 Jun 2023 22:20:07 +0900 Subject: [PATCH] Fix warnings for single-subclass sealed class --- .../arrow/optics/plugin/internals/domain.kt | 38 +++++++++++++++---- .../arrow/optics/plugin/internals/prism.kt | 8 ++-- .../optics/plugin/internals/processor.kt | 11 ++++-- .../kotlin/arrow/optics/plugin/Compilation.kt | 11 +++--- .../kotlin/arrow/optics/plugin/PrismTests.kt | 20 ++++++++-- 5 files changed, 65 insertions(+), 23 deletions(-) diff --git a/arrow-libs/optics/arrow-optics-ksp-plugin/src/main/kotlin/arrow/optics/plugin/internals/domain.kt b/arrow-libs/optics/arrow-optics-ksp-plugin/src/main/kotlin/arrow/optics/plugin/internals/domain.kt index 159358715d1..2ce1a1cc692 100644 --- a/arrow-libs/optics/arrow-optics-ksp-plugin/src/main/kotlin/arrow/optics/plugin/internals/domain.kt +++ b/arrow-libs/optics/arrow-optics-ksp-plugin/src/main/kotlin/arrow/optics/plugin/internals/domain.kt @@ -75,11 +75,31 @@ typealias NullableFocus = Focus.Nullable sealed class Focus { companion object { - operator fun invoke(fullName: String, paramName: String, refinedType: KSType? = null): Focus = + operator fun invoke( + fullName: String, + paramName: String, + refinedType: KSType? = null, + onlyOneSealedSubclass: Boolean = false, + ): Focus = when { - fullName.endsWith("?") -> Nullable(fullName, paramName, refinedType) - fullName.startsWith("`arrow`.`core`.`Option`") -> Option(fullName, paramName, refinedType) - else -> NonNull(fullName, paramName, refinedType) + fullName.endsWith("?") -> Nullable( + fullName, + paramName, + refinedType, + onlyOneSealedSubclass = onlyOneSealedSubclass, + ) + fullName.startsWith("`arrow`.`core`.`Option`") -> Option( + fullName, + paramName, + refinedType, + onlyOneSealedSubclass = onlyOneSealedSubclass, + ) + else -> NonNull( + fullName, + paramName, + refinedType, + onlyOneSealedSubclass = onlyOneSealedSubclass, + ) } } @@ -87,6 +107,7 @@ sealed class Focus { abstract val paramName: String // only used for type-refining prisms abstract val refinedType: KSType? + abstract val onlyOneSealedSubclass: Boolean val refinedArguments: List get() = refinedType?.arguments?.filter { @@ -96,7 +117,8 @@ sealed class Focus { data class Nullable( override val className: String, override val paramName: String, - override val refinedType: KSType? + override val refinedType: KSType?, + override val onlyOneSealedSubclass: Boolean, ) : Focus() { val nonNullClassName = className.dropLast(1) } @@ -104,7 +126,8 @@ sealed class Focus { data class Option( override val className: String, override val paramName: String, - override val refinedType: KSType? + override val refinedType: KSType?, + override val onlyOneSealedSubclass: Boolean, ) : Focus() { val nestedClassName = Regex("`arrow`.`core`.`Option`<(.*)>$").matchEntire(className)!!.groupValues[1] @@ -113,7 +136,8 @@ sealed class Focus { data class NonNull( override val className: String, override val paramName: String, - override val refinedType: KSType? + override val refinedType: KSType?, + override val onlyOneSealedSubclass: Boolean, ) : Focus() } diff --git a/arrow-libs/optics/arrow-optics-ksp-plugin/src/main/kotlin/arrow/optics/plugin/internals/prism.kt b/arrow-libs/optics/arrow-optics-ksp-plugin/src/main/kotlin/arrow/optics/plugin/internals/prism.kt index be20c59c6f0..5210ca29c2c 100644 --- a/arrow-libs/optics/arrow-optics-ksp-plugin/src/main/kotlin/arrow/optics/plugin/internals/prism.kt +++ b/arrow-libs/optics/arrow-optics-ksp-plugin/src/main/kotlin/arrow/optics/plugin/internals/prism.kt @@ -1,7 +1,5 @@ package arrow.optics.plugin.internals -import com.google.devtools.ksp.symbol.KSTypeParameter - internal fun generatePrisms(ele: ADT, target: PrismTarget) = Snippet( `package` = ele.packageName, @@ -26,12 +24,16 @@ private fun processElement(ele: ADT, foci: List): String { "${ele.visibilityModifierName} inline fun $angledTypeParameters ${ele.sourceClassName}.Companion.${focus.paramName}(): $Prism<$sourceClassNameWithParams, ${focus.className}>" } + val elseBranch = if (focus.onlyOneSealedSubclass) "" else """ + | else -> ${ele.sourceName}.left() + """.trimMargin() + """ |$firstLine = $Prism( | getOrModify = { ${ele.sourceName}: $sourceClassNameWithParams -> | when (${ele.sourceName}) { | is ${focus.className} -> ${ele.sourceName}.right() - | else -> ${ele.sourceName}.left() + | $elseBranch | } | }, | reverseGet = ::identity diff --git a/arrow-libs/optics/arrow-optics-ksp-plugin/src/main/kotlin/arrow/optics/plugin/internals/processor.kt b/arrow-libs/optics/arrow-optics-ksp-plugin/src/main/kotlin/arrow/optics/plugin/internals/processor.kt index 3efd323d984..d2c9a20451a 100644 --- a/arrow-libs/optics/arrow-optics-ksp-plugin/src/main/kotlin/arrow/optics/plugin/internals/processor.kt +++ b/arrow-libs/optics/arrow-optics-ksp-plugin/src/main/kotlin/arrow/optics/plugin/internals/processor.kt @@ -75,14 +75,17 @@ internal fun evalAnnotatedPrismElement( logger: KSPLogger ): List = when { - element.isSealed -> - element.getSealedSubclasses().map { + element.isSealed -> { + val sealedSubclasses = element.getSealedSubclasses().toList() + sealedSubclasses.map { Focus( it.primaryConstructor?.returnType?.resolve()?.qualifiedString() ?: it.qualifiedNameOrSimpleName, it.simpleName.asString().replaceFirstChar { c -> c.lowercase(Locale.getDefault()) }, - it.superTypes.first().resolve() + it.superTypes.first().resolve(), + onlyOneSealedSubclass = sealedSubclasses.size == 1 ) - }.toList() + } + } else -> { logger.error(errorMessage, element) emptyList() diff --git a/arrow-libs/optics/arrow-optics-ksp-plugin/src/test/kotlin/arrow/optics/plugin/Compilation.kt b/arrow-libs/optics/arrow-optics-ksp-plugin/src/test/kotlin/arrow/optics/plugin/Compilation.kt index 0b14b296c44..0399c7c17da 100644 --- a/arrow-libs/optics/arrow-optics-ksp-plugin/src/test/kotlin/arrow/optics/plugin/Compilation.kt +++ b/arrow-libs/optics/arrow-optics-ksp-plugin/src/test/kotlin/arrow/optics/plugin/Compilation.kt @@ -26,8 +26,8 @@ fun String.compilationFails() { Assertions.assertThat(compilationResult.exitCode).isNotEqualTo(KotlinCompilation.ExitCode.OK) } -fun String.compilationSucceeds() { - val compilationResult = compile(this) +fun String.compilationSucceeds(allWarningsAsErrors: Boolean = false) { + val compilationResult = compile(this, allWarningsAsErrors = allWarningsAsErrors) Assertions.assertThat(compilationResult.exitCode).isEqualTo(KotlinCompilation.ExitCode.OK) } @@ -42,8 +42,8 @@ fun String.evals(thing: Pair) { // UTILITY FUNCTIONS COPIED FROM META-TEST // ======================================= -internal fun compile(text: String): KotlinCompilation.Result { - val compilation = buildCompilation(text) +internal fun compile(text: String, allWarningsAsErrors: Boolean = false): KotlinCompilation.Result { + val compilation = buildCompilation(text, allWarningsAsErrors = allWarningsAsErrors) // fix problems with double compilation and KSP // as stated in https://github.com/tschuchortdev/kotlin-compile-testing/issues/72 val pass1 = compilation.compile() @@ -58,7 +58,7 @@ internal fun compile(text: String): KotlinCompilation.Result { .compile() } -fun buildCompilation(text: String) = KotlinCompilation().apply { +fun buildCompilation(text: String, allWarningsAsErrors: Boolean = false) = KotlinCompilation().apply { classpaths = listOf( "arrow-annotations:$arrowVersion", "arrow-core:$arrowVersion", @@ -67,6 +67,7 @@ fun buildCompilation(text: String) = KotlinCompilation().apply { symbolProcessorProviders = listOf(OpticsProcessorProvider()) sources = listOf(SourceFile.kotlin(SOURCE_FILENAME, text.trimMargin())) verbose = false + this.allWarningsAsErrors = allWarningsAsErrors } private fun classpathOf(dependency: String): File { diff --git a/arrow-libs/optics/arrow-optics-ksp-plugin/src/test/kotlin/arrow/optics/plugin/PrismTests.kt b/arrow-libs/optics/arrow-optics-ksp-plugin/src/test/kotlin/arrow/optics/plugin/PrismTests.kt index feb264a4e2d..084865f0045 100755 --- a/arrow-libs/optics/arrow-optics-ksp-plugin/src/test/kotlin/arrow/optics/plugin/PrismTests.kt +++ b/arrow-libs/optics/arrow-optics-ksp-plugin/src/test/kotlin/arrow/optics/plugin/PrismTests.kt @@ -16,8 +16,7 @@ class PrismTests { | companion object |} |val i: Prism = PrismSealed.prismSealed1 - |val r = i != null - """.evals("r" to true) + """.compilationSucceeds(allWarningsAsErrors = true) } @Test @@ -32,8 +31,21 @@ class PrismTests { | companion object |} |val i: Prism, PrismSealed.PrismSealed1> = PrismSealed.prismSealed1() - |val r = i != null - """.evals("r" to true) + """.compilationSucceeds(allWarningsAsErrors = true) + } + + @Test + fun `Prism will be generated without warning for sealed class with only one subclass`() { + """ + |$`package` + |$imports + |@optics + |sealed class PrismSealed(val field: String, val nullable: String?) { + | data class PrismSealed1(private val a: String?) : PrismSealed("", a) + | companion object + |} + |val i: Prism = PrismSealed.prismSealed1 + """.compilationSucceeds(allWarningsAsErrors = true) } @Test