Add ability to skip properties (#446)

* Introduce Poko.Skip annotation * Add SkipSupport opt-in annotation * Implement warning for consumers of custom Skip annotation * Use custom annotation in sample:jvm module, ensuring compilation works if no custom Skip annotation is provided
drewhamilton · Dec 12, 2024 · 18b3b4b · 18b3b4b
1 parent 0867931
commit 18b3b4b
Show file tree

Hide file tree

Showing 15 changed files with 196 additions and 16 deletions.
diff --git a/build-support/src/main/java/dev/drewhamilton/poko/build/PokoBuildPlugin.kt b/build-support/src/main/java/dev/drewhamilton/poko/build/PokoBuildPlugin.kt
@@ -109,6 +109,7 @@ class PokoBuildPlugin : Plugin<Project> {
                 buildConfigField("COMPILER_PLUGIN_ARTIFACT", "poko-compiler-plugin")
                 buildConfigField("DEFAULT_POKO_ENABLED", true)
                 buildConfigField("DEFAULT_POKO_ANNOTATION", "dev/drewhamilton/poko/Poko")
+                buildConfigField("SKIP_ANNOTATION_SHORT_NAME", "Skip")
             }
         }
     }

diff --git a/poko-annotations/api/poko-annotations.api b/poko-annotations/api/poko-annotations.api
@@ -7,3 +7,9 @@ public abstract interface annotation class dev/drewhamilton/poko/ArrayContentSup
 public abstract interface annotation class dev/drewhamilton/poko/Poko : java/lang/annotation/Annotation {
 }
 
+public abstract interface annotation class dev/drewhamilton/poko/Poko$Skip : java/lang/annotation/Annotation {
+}
+
+public abstract interface annotation class dev/drewhamilton/poko/SkipSupport : java/lang/annotation/Annotation {
+}
+
diff --git a/poko-annotations/src/commonMain/kotlin/dev/drewhamilton/poko/Poko.kt b/poko-annotations/src/commonMain/kotlin/dev/drewhamilton/poko/Poko.kt
@@ -22,4 +22,16 @@ package dev.drewhamilton.poko
  */
 @Retention(AnnotationRetention.SOURCE)
 @Target(AnnotationTarget.CLASS)
-public annotation class Poko
+public annotation class Poko {
+
+    /**
+     * Primary constructor properties marked with this annotation will be omitted from generated
+     * `equals`, `hashCode`, and `toString` functions, as if they were not properties.
+     *
+     * This annotation has no effect on properties declared outside the primary constructor.
+     */
+    @SkipSupport
+    @Retention(AnnotationRetention.SOURCE)
+    @Target(AnnotationTarget.PROPERTY)
+    public annotation class Skip
+}
diff --git a/.../drewhamilton/poko/ArrayContentSupport.kt → ...dev/drewhamilton/poko/optInAnnotations.kt b/.../drewhamilton/poko/ArrayContentSupport.kt → ...dev/drewhamilton/poko/optInAnnotations.kt
@@ -1,5 +1,12 @@
 package dev.drewhamilton.poko
 
+/**
+ * Denotes an experimental API that enables the ability to skip a Poko class primary constructor
+ * property when generating Poko functions.
+ */
+@RequiresOptIn
+public annotation class SkipSupport
+
 /**
  * Denotes an API that enables support for array content reading, which is experimental and may
  * change or break.

diff --git a/poko-compiler-plugin/src/main/kotlin/dev/drewhamilton/poko/fir/PokoFirCheckersExtension.kt b/poko-compiler-plugin/src/main/kotlin/dev/drewhamilton/poko/fir/PokoFirCheckersExtension.kt
@@ -1,5 +1,6 @@
 package dev.drewhamilton.poko.fir
 
+import dev.drewhamilton.poko.BuildConfig.DEFAULT_POKO_ANNOTATION
 import org.jetbrains.kotlin.KtFakeSourceElementKind
 import org.jetbrains.kotlin.descriptors.ClassKind
 import org.jetbrains.kotlin.diagnostics.AbstractSourceElementPositioningStrategy
@@ -23,6 +24,9 @@ import org.jetbrains.kotlin.fir.declarations.FirRegularClass
 import org.jetbrains.kotlin.fir.declarations.primaryConstructorIfAny
 import org.jetbrains.kotlin.fir.declarations.utils.isData
 import org.jetbrains.kotlin.fir.declarations.utils.isInner
+import org.jetbrains.kotlin.fir.expressions.FirAnnotation
+import org.jetbrains.kotlin.fir.types.classId
+import org.jetbrains.kotlin.fir.types.coneTypeOrNull
 import org.jetbrains.kotlin.lexer.KtTokens
 
 internal class PokoFirCheckersExtension(
@@ -34,7 +38,7 @@ internal class PokoFirCheckersExtension(
                 setOf(PokoFirRegularClassChecker)
         }
 
-    internal object PokoFirRegularClassChecker : FirRegularClassChecker(
+    private object PokoFirRegularClassChecker : FirRegularClassChecker(
         mppKind = MppCheckerKind.Common,
     ) {
         override fun check(
@@ -46,12 +50,12 @@ internal class PokoFirCheckersExtension(
             if (matcher.pokoAnnotation(declaration) == null) return
 
             val errorFactory = when {
-                declaration.classKind != ClassKind.CLASS -> Errors.PokoOnNonClass
-                declaration.isData -> Errors.PokoOnDataClass
-                declaration.hasModifier(KtTokens.VALUE_KEYWORD) -> Errors.PokoOnValueClass
-                declaration.isInner -> Errors.PokoOnInnerClass
+                declaration.classKind != ClassKind.CLASS -> Diagnostics.PokoOnNonClass
+                declaration.isData -> Diagnostics.PokoOnDataClass
+                declaration.hasModifier(KtTokens.VALUE_KEYWORD) -> Diagnostics.PokoOnValueClass
+                declaration.isInner -> Diagnostics.PokoOnInnerClass
                 declaration.primaryConstructorIfAny(context.session) == null ->
-                    Errors.PrimaryConstructorRequired
+                    Diagnostics.PrimaryConstructorRequired
                 else -> null
             }
             if (errorFactory != null) {
@@ -67,17 +71,37 @@ internal class PokoFirCheckersExtension(
                 .filter {
                     it.source?.kind is KtFakeSourceElementKind.PropertyFromParameter
                 }
+                .filter {
+                    val skipAnnotation = matcher.pokoSkipAnnotation(it)
+                    if (
+                        skipAnnotation != null &&
+                        !skipAnnotation.isNestedInDefaultPokoAnnotation()
+                    ) {
+                        // Pseudo-opt-in warning for custom annotation consumers:
+                        reporter.reportOn(
+                            source = it.source,
+                            factory = Diagnostics.SkippedPropertyWithCustomAnnotation,
+                            context = context,
+                        )
+                    }
+                    skipAnnotation == null
+                }
             if (constructorProperties.isEmpty()) {
                 reporter.reportOn(
                     source = declaration.source,
-                    factory = Errors.PrimaryConstructorPropertiesRequired,
+                    factory = Diagnostics.PrimaryConstructorPropertiesRequired,
                     context = context,
                 )
             }
         }
+
+        private fun FirAnnotation.isNestedInDefaultPokoAnnotation(): Boolean {
+            return annotationTypeRef.coneTypeOrNull?.classId?.outerClassId?.asFqNameString() ==
+                DEFAULT_POKO_ANNOTATION
+        }
     }
 
-    private object Errors : BaseDiagnosticRendererFactory() {
+    private object Diagnostics : BaseDiagnosticRendererFactory() {
 
         /**
          * The compiler and the IDE use a different version of this class, so use reflection to find the available
@@ -116,6 +140,10 @@ internal class PokoFirCheckersExtension(
             positioningStrategy = SourceElementPositioningStrategies.NAME_IDENTIFIER,
         )
 
+        val SkippedPropertyWithCustomAnnotation by warning0(
+            positioningStrategy = SourceElementPositioningStrategies.ANNOTATION_USE_SITE,
+        )
+
         override val MAP = KtDiagnosticFactoryToRendererMap("Poko").apply {
             put(
                 factory = PokoOnNonClass,
@@ -139,7 +167,11 @@ internal class PokoFirCheckersExtension(
             )
             put(
                 factory = PrimaryConstructorPropertiesRequired,
-                message = "Poko class primary constructor must have at least one property",
+                message = "Poko class primary constructor must have at least one not-skipped property",
+            )
+            put(
+                factory = SkippedPropertyWithCustomAnnotation,
+                message = "The @Skip annotation is experimental and its behavior may change; use with caution",
             )
         }
 
@@ -148,7 +180,8 @@ internal class PokoFirCheckersExtension(
         }
 
         /**
-         * Copy of [org.jetbrains.kotlin.diagnostics.error0] with hack for correct `PsiElement` class.
+         * Copy of [org.jetbrains.kotlin.diagnostics.error0] with hack for correct `PsiElement`
+         * class.
          */
         private fun error0(
             positioningStrategy: AbstractSourceElementPositioningStrategy = SourceElementPositioningStrategies.DEFAULT,
@@ -159,5 +192,19 @@ internal class PokoFirCheckersExtension(
                 psiType = psiElementClass,
             )
         }
+
+        /**
+         * Copy of [org.jetbrains.kotlin.diagnostics.warning0] with hack for correct `PsiElement`
+         * class.
+         */
+        private fun warning0(
+            positioningStrategy: AbstractSourceElementPositioningStrategy = SourceElementPositioningStrategies.DEFAULT,
+        ): DiagnosticFactory0DelegateProvider {
+            return DiagnosticFactory0DelegateProvider(
+                severity = Severity.WARNING,
+                positioningStrategy = positioningStrategy,
+                psiType = psiElementClass,
+            )
+        }
     }
 }
diff --git a/...iler-plugin/src/main/kotlin/dev/drewhamilton/poko/fir/PokoFirExtensionSessionComponent.kt b/...iler-plugin/src/main/kotlin/dev/drewhamilton/poko/fir/PokoFirExtensionSessionComponent.kt
@@ -1,5 +1,6 @@
 package dev.drewhamilton.poko.fir
 
+import dev.drewhamilton.poko.BuildConfig.SKIP_ANNOTATION_SHORT_NAME
 import org.jetbrains.kotlin.fir.FirSession
 import org.jetbrains.kotlin.fir.declarations.FirDeclaration
 import org.jetbrains.kotlin.fir.expressions.FirAnnotation
@@ -8,6 +9,7 @@ import org.jetbrains.kotlin.fir.extensions.FirExtensionSessionComponent.Factory
 import org.jetbrains.kotlin.fir.types.classId
 import org.jetbrains.kotlin.fir.types.coneTypeOrNull
 import org.jetbrains.kotlin.name.ClassId
+import org.jetbrains.kotlin.name.Name
 
 internal class PokoFirExtensionSessionComponent(
     session: FirSession,
@@ -19,6 +21,15 @@ internal class PokoFirExtensionSessionComponent(
         }
     }
 
+    fun pokoSkipAnnotation(declaration: FirDeclaration): FirAnnotation? {
+        val skipAnnotation = pokoAnnotation.createNestedClassId(
+            name = Name.identifier(SKIP_ANNOTATION_SHORT_NAME),
+        )
+        return declaration.annotations.firstOrNull { firAnnotation ->
+            firAnnotation.classId() == skipAnnotation
+        }
+    }
+
     private fun FirAnnotation.classId(): ClassId? {
         return annotationTypeRef.coneTypeOrNull?.classId
     }

diff --git a/poko-compiler-plugin/src/main/kotlin/dev/drewhamilton/poko/ir/PokoMembersTransformer.kt b/poko-compiler-plugin/src/main/kotlin/dev/drewhamilton/poko/ir/PokoMembersTransformer.kt
@@ -1,5 +1,6 @@
 package dev.drewhamilton.poko.ir
 
+import dev.drewhamilton.poko.BuildConfig.SKIP_ANNOTATION_SHORT_NAME
 import org.jetbrains.kotlin.KtFakeSourceElementKind
 import org.jetbrains.kotlin.backend.common.IrElementTransformerVoidWithContext
 import org.jetbrains.kotlin.backend.common.extensions.IrPluginContext
@@ -128,11 +129,18 @@ internal class PokoMembersTransformer(
                     it.symbol.descriptor.source.getPsi() is KtParameter
                 }
             }
+            .filter {
+                !it.hasAnnotation(
+                    classId = ClassId.fromString(
+                        string = "${pokoAnnotationName}.$SKIP_ANNOTATION_SHORT_NAME"
+                    ),
+                )
+            }
         if (properties.isEmpty()) {
             messageCollector.log("No primary constructor properties")
             messageCollector.reportErrorOnClass(
                 irClass = parent,
-                message = "Poko class primary constructor must have at least one property",
+                message = "Poko class primary constructor must have at least one not-skipped property",
             )
             return
         }

diff --git a/poko-compiler-plugin/src/test/kotlin/dev/drewhamilton/poko/PokoCompilerPluginTest.kt b/poko-compiler-plugin/src/test/kotlin/dev/drewhamilton/poko/PokoCompilerPluginTest.kt
@@ -119,7 +119,7 @@ class PokoCompilerPluginTest(
             }
             assertThat(result.messages).all {
                 contains(expectedLocation)
-                contains("Poko class primary constructor must have at least one property")
+                contains("Poko class primary constructor must have at least one not-skipped property")
             }
         }
     }

diff --git a/poko-tests-without-k2/src/commonMain/kotlin/poko/SkippedProperty.kt b/poko-tests-without-k2/src/commonMain/kotlin/poko/SkippedProperty.kt
@@ -0,0 +1,11 @@
+package poko
+
+import dev.drewhamilton.poko.Poko
+import dev.drewhamilton.poko.SkipSupport
+
+@OptIn(SkipSupport::class)
+@Suppress("Unused")
+@Poko class SkippedProperty(
+    val id: String,
+    @Poko.Skip val callback: () -> Unit,
+)
diff --git a/poko-tests-without-k2/src/commonTest/kotlin/SkippedPropertyTest.kt b/poko-tests-without-k2/src/commonTest/kotlin/SkippedPropertyTest.kt
@@ -0,0 +1,28 @@
+
+import assertk.assertAll
+import assertk.assertThat
+import assertk.assertions.isEqualTo
+import kotlin.test.Test
+import poko.SkippedProperty
+
+class SkippedPropertyTest {
+
+    @Test fun skipped_property_omitted_from_all_generated_functions() {
+        val a = SkippedProperty(
+            id = "id",
+            callback = { println("Callback <a> invoked") },
+        )
+        val b = SkippedProperty(
+            id = "id",
+            callback = { println("Callback <b> invoked") },
+        )
+
+        assertAll {
+            assertThat(a).isEqualTo(b)
+            assertThat(b).isEqualTo(a)
+            assertThat(a.hashCode()).isEqualTo(b.hashCode())
+            assertThat(a.toString()).isEqualTo(b.toString())
+            assertThat(a.toString()).isEqualTo("SkippedProperty(id=id)")
+        }
+    }
+}
diff --git a/poko-tests/src/commonMain/kotlin/poko/SkippedProperty.kt b/poko-tests/src/commonMain/kotlin/poko/SkippedProperty.kt
@@ -0,0 +1,11 @@
+package poko
+
+import dev.drewhamilton.poko.Poko
+import dev.drewhamilton.poko.SkipSupport
+
+@OptIn(SkipSupport::class)
+@Suppress("Unused")
+@Poko class SkippedProperty(
+    val id: String,
+    @Poko.Skip val callback: () -> Unit,
+)
diff --git a/poko-tests/src/commonTest/kotlin/SkippedPropertyTest.kt b/poko-tests/src/commonTest/kotlin/SkippedPropertyTest.kt
@@ -0,0 +1,28 @@
+
+import assertk.assertAll
+import assertk.assertThat
+import assertk.assertions.isEqualTo
+import kotlin.test.Test
+import poko.SkippedProperty
+
+class SkippedPropertyTest {
+
+    @Test fun skipped_property_omitted_from_all_generated_functions() {
+        val a = SkippedProperty(
+            id = "id",
+            callback = { println("Callback <a> invoked") },
+        )
+        val b = SkippedProperty(
+            id = "id",
+            callback = { println("Callback <b> invoked") },
+        )
+
+        assertAll {
+            assertThat(a).isEqualTo(b)
+            assertThat(b).isEqualTo(a)
+            assertThat(a.hashCode()).isEqualTo(b.hashCode())
+            assertThat(a.toString()).isEqualTo(b.toString())
+            assertThat(a.toString()).isEqualTo("SkippedProperty(id=id)")
+        }
+    }
+}
diff --git a/sample/jvm/build.gradle.kts b/sample/jvm/build.gradle.kts
@@ -4,6 +4,10 @@ plugins {
     `java-test-fixtures`
 }
 
+poko {
+    pokoAnnotation = "dev/drewhamilton/poko/sample/jvm/MyData"
+}
+
 dependencies {
     testImplementation(libs.junit)
     testImplementation(libs.assertk)

diff --git a/sample/jvm/src/main/kotlin/dev/drewhamilton/poko/sample/jvm/MyData.kt b/sample/jvm/src/main/kotlin/dev/drewhamilton/poko/sample/jvm/MyData.kt
@@ -0,0 +1,8 @@
+package dev.drewhamilton.poko.sample.jvm
+
+/**
+ * Local replacement for default Poko annotation.
+ */
+@Retention(AnnotationRetention.SOURCE)
+@Target(AnnotationTarget.CLASS)
+annotation class MyData
diff --git a/sample/jvm/src/main/kotlin/dev/drewhamilton/poko/sample/jvm/Sample.kt b/sample/jvm/src/main/kotlin/dev/drewhamilton/poko/sample/jvm/Sample.kt
@@ -1,9 +1,7 @@
 package dev.drewhamilton.poko.sample.jvm
 
-import dev.drewhamilton.poko.Poko
-
 @Suppress("unused")
-@Poko class Sample(
+@MyData class Sample(
     val int: Int,
     val requiredString: String,
     val optionalString: String?,