Log a warning when non data objecs are annotated (#252)

ansman · web-flow · commit 19f0dc01a70b · 2023-07-17T00:44:48.000Z
diff --git a/compiler/src/main/kotlin/se/ansman/kotshi/Errors.kt b/compiler/src/main/kotlin/se/ansman/kotshi/Errors.kt
@@ -24,6 +24,7 @@ object Errors {
     const val invalidRegisterAdapterVisibility = "Types annotated @RegisterJsonAdapter must be public or internal"
     const val abstractFactoriesAreDeprecated = "Having abstract factories is deprecated and will be removed in the future. Please migrate to use objects with delegation to the generated factory."
     const val registeredAdapterWithoutFactory = "Found classes annotated with @RegisterJsonAdapter but no factory annotated with @KotshiJsonAdapterFactory"
+    const val nonDataObject = "JsonSerializable objects must be data objects. This warning will become an error in the future."
     fun privateDataClassProperty(propertyName: String) = "Property $propertyName must be public or internal"
     fun transientDataClassPropertyWithoutDefaultValue(propertyName: String) = "Transient property $propertyName must declare a default value"
     fun ignoredDataClassPropertyWithoutDefaultValue(propertyName: String) = "Ignored property $propertyName must declare a default value"
diff --git a/compiler/src/main/kotlin/se/ansman/kotshi/kapt/AdaptersProcessingStep.kt b/compiler/src/main/kotlin/se/ansman/kotshi/kapt/AdaptersProcessingStep.kt
@@ -102,32 +102,32 @@ class AdaptersProcessingStep(
                 }
 
                 val generator = when {
-                    metadata.isData -> DataClassAdapterGenerator(
+                    metadata.isObject -> ObjectAdapterGenerator(
                         metadataAccessor = metadataAccessor,
                         types = types,
-                        elements = elements,
                         element = typeElement,
                         metadata = metadata,
+                        elements = elements,
                         globalConfig = globalConfig,
                         messager = messager,
                     )
 
-                    metadata.isEnum -> EnumAdapterGenerator(
+                    metadata.isData -> DataClassAdapterGenerator(
                         metadataAccessor = metadataAccessor,
                         types = types,
                         elements = elements,
                         element = typeElement,
                         metadata = metadata,
                         globalConfig = globalConfig,
-                        messager = messager
+                        messager = messager,
                     )
 
-                    metadata.isObject -> ObjectAdapterGenerator(
+                    metadata.isEnum -> EnumAdapterGenerator(
                         metadataAccessor = metadataAccessor,
                         types = types,
+                        elements = elements,
                         element = typeElement,
                         metadata = metadata,
-                        elements = elements,
                         globalConfig = globalConfig,
                         messager = messager
                     )
diff --git a/compiler/src/main/kotlin/se/ansman/kotshi/kapt/MetadataAccessor.kt b/compiler/src/main/kotlin/se/ansman/kotshi/kapt/MetadataAccessor.kt
@@ -29,6 +29,8 @@ class MetadataAccessor(private val classInspector: ClassInspector) {
         getMetadataOrNull(type)
             ?: throw KaptProcessingError(javaClassNotSupported, type)
 
+    fun getLanguageVersion(type: Element): KotlinVersion = getMetadata(type).languageVersion
+
     fun getKmClass(metadata: Metadata): KmClass = kmClassPerMetadata.getOrPut(metadata) { metadata.toKmClass() }
     fun getKmClass(type: Element): KmClass = getKmClass(getMetadata(type))
     fun getKmClassOrNull(type: Element): KmClass? = getMetadataOrNull(type)?.let(::getKmClass)
diff --git a/compiler/src/main/kotlin/se/ansman/kotshi/kapt/generators/ObjectAdapterGenerator.kt b/compiler/src/main/kotlin/se/ansman/kotshi/kapt/generators/ObjectAdapterGenerator.kt
@@ -2,7 +2,10 @@ package se.ansman.kotshi.kapt.generators
 
 import com.squareup.kotlinpoet.metadata.isObject
 import kotlinx.metadata.KmClass
+import kotlinx.metadata.isData
+import se.ansman.kotshi.Errors
 import se.ansman.kotshi.kapt.MetadataAccessor
+import se.ansman.kotshi.kapt.logKotshiWarning
 import se.ansman.kotshi.model.GeneratableJsonAdapter
 import se.ansman.kotshi.model.GlobalConfig
 import se.ansman.kotshi.model.ObjectJsonAdapter
@@ -22,12 +25,16 @@ class ObjectAdapterGenerator(
 ) : AdapterGenerator(metadataAccessor, types, elements, element, metadata, globalConfig, messager) {
     init {
         require(metadata.isObject)
+        if (!metadata.isData && metadataAccessor.getLanguageVersion(element) >= KotlinVersion(1, 9)) {
+            messager.logKotshiWarning(Errors.nonDataObject, element)
+        }
     }
 
-    override fun getGeneratableJsonAdapter(): GeneratableJsonAdapter =
-        ObjectJsonAdapter(
+    override fun getGeneratableJsonAdapter(): GeneratableJsonAdapter {
+        return ObjectJsonAdapter(
             targetPackageName = targetClassName.packageName,
             targetSimpleNames = targetClassName.simpleNames,
             polymorphicLabels = getPolymorphicLabels()
         )
+    }
 }
diff --git a/compiler/src/main/kotlin/se/ansman/kotshi/ksp/generators/ObjectAdapterGenerator.kt b/compiler/src/main/kotlin/se/ansman/kotshi/ksp/generators/ObjectAdapterGenerator.kt
@@ -4,6 +4,10 @@ import com.google.devtools.ksp.processing.Resolver
 import com.google.devtools.ksp.processing.SymbolProcessorEnvironment
 import com.google.devtools.ksp.symbol.ClassKind
 import com.google.devtools.ksp.symbol.KSClassDeclaration
+import com.google.devtools.ksp.symbol.Modifier
+import se.ansman.kotshi.Errors
+import se.ansman.kotshi.ksp.logKotshiError
+import se.ansman.kotshi.ksp.logKotshiWarning
 import se.ansman.kotshi.model.GeneratableJsonAdapter
 import se.ansman.kotshi.model.GlobalConfig
 import se.ansman.kotshi.model.ObjectJsonAdapter
@@ -16,6 +20,10 @@ class ObjectAdapterGenerator(
 ) : AdapterGenerator(environment, element, globalConfig, resolver) {
     init {
         require(element.classKind == ClassKind.OBJECT)
+        if (Modifier.DATA !in element.modifiers && environment.apiVersion >= KotlinVersion(1, 9)) {
+            environment.logger.logKotshiError(environment.apiVersion.toString(), element)
+            environment.logger.logKotshiWarning(Errors.nonDataObject, element)
+        }
     }
 
     override fun getGeneratableJsonAdapter(): GeneratableJsonAdapter =
diff --git a/tests/ksp/src/test/kotlin/se/ansman/kotshi/KspGeneratorTest.kt b/tests/ksp/src/test/kotlin/se/ansman/kotshi/KspGeneratorTest.kt
@@ -7,6 +7,7 @@ import com.tschuchort.compiletesting.SourceFile
 import com.tschuchort.compiletesting.kspArgs
 import com.tschuchort.compiletesting.kspIncremental
 import com.tschuchort.compiletesting.symbolProcessorProviders
+import org.junit.jupiter.api.Disabled
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.parallel.Execution
 import org.junit.jupiter.api.parallel.ExecutionMode
@@ -313,4 +314,7 @@ class KspGeneratorTest : BaseGeneratorTest() {
             )
         }
     }
+
+    @Disabled("kotlin-compile-testing doesn't work well with overriding language version yet")
+    override fun `non data object logs warnings`() {}
 }
diff --git a/tests/src/main/kotlin/se/ansman/kotshi/SealedClassOnInvalidFail.kt b/tests/src/main/kotlin/se/ansman/kotshi/SealedClassOnInvalidFail.kt
@@ -9,5 +9,5 @@ sealed class SealedClassOnInvalidFail {
 
     @JsonSerializable
     @JsonDefaultValue
-    object Default : SealedClassOnInvalidFail()
+    data object Default : SealedClassOnInvalidFail()
 }
diff --git a/tests/src/main/kotlin/se/ansman/kotshi/SealedClassOnInvalidNull.kt b/tests/src/main/kotlin/se/ansman/kotshi/SealedClassOnInvalidNull.kt
@@ -9,5 +9,5 @@ sealed class SealedClassOnInvalidNull {
 
     @JsonSerializable
     @JsonDefaultValue
-    object Default : SealedClassOnInvalidNull()
+    data object Default : SealedClassOnInvalidNull()
 }
diff --git a/tests/src/main/kotlin/se/ansman/kotshi/SealedClassOnMissingFail.kt b/tests/src/main/kotlin/se/ansman/kotshi/SealedClassOnMissingFail.kt
@@ -9,5 +9,5 @@ sealed class SealedClassOnMissingFail {
 
     @JsonSerializable
     @JsonDefaultValue
-    object Default : SealedClassOnMissingFail()
+    data object Default : SealedClassOnMissingFail()
 }
diff --git a/tests/src/main/kotlin/se/ansman/kotshi/SealedClassOnMissingNull.kt b/tests/src/main/kotlin/se/ansman/kotshi/SealedClassOnMissingNull.kt
@@ -9,5 +9,5 @@ sealed class SealedClassOnMissingNull {
 
     @JsonSerializable
     @JsonDefaultValue
-    object Default : SealedClassOnMissingNull()
+    data object Default : SealedClassOnMissingNull()
 }
diff --git a/tests/src/main/kotlin/se/ansman/kotshi/SealedClassWithDefaultWithType.kt b/tests/src/main/kotlin/se/ansman/kotshi/SealedClassWithDefaultWithType.kt
@@ -18,5 +18,5 @@ sealed class SealedClassWithDefaultWithType {
     @JsonSerializable
     @JsonDefaultValue
     @PolymorphicLabel("type4")
-    object Default : SealedClassWithDefaultWithType()
+    data object Default : SealedClassWithDefaultWithType()
 }
diff --git a/tests/src/main/kotlin/se/ansman/kotshi/SealedClassWithDefaultWithoutType.kt b/tests/src/main/kotlin/se/ansman/kotshi/SealedClassWithDefaultWithoutType.kt
@@ -18,4 +18,4 @@ data class SealedClassWithDefaultWithoutTypeSubclass3(val baz: String) : SealedC
 
 @JsonSerializable
 @JsonDefaultValue
-object SealedClassWithDefaultWithoutTypeDefault : SealedClassWithDefaultWithoutType()
+data object SealedClassWithDefaultWithoutTypeDefault : SealedClassWithDefaultWithoutType()
diff --git a/tests/src/main/kotlin/se/ansman/kotshi/SealedClassWithExistingLabel.kt b/tests/src/main/kotlin/se/ansman/kotshi/SealedClassWithExistingLabel.kt
@@ -5,7 +5,7 @@ package se.ansman.kotshi
 sealed class SealedClassWithExistingLabel {
     @PolymorphicLabel("type1")
     @JsonSerializable
-    object Type1 : SealedClassWithExistingLabel()
+    data object Type1 : SealedClassWithExistingLabel()
 
     @PolymorphicLabel("type2")
     @JsonSerializable
diff --git a/tests/src/main/kotlin/se/ansman/kotshi/TestObject.kt b/tests/src/main/kotlin/se/ansman/kotshi/TestObject.kt
@@ -1,6 +1,6 @@
 package se.ansman.kotshi
 
 @JsonSerializable
-object TestObject {
+data object TestObject {
     val someProperty = "someValue"
 }
diff --git a/tests/src/test/kotlin/se/ansman/kotshi/BaseGeneratorTest.kt b/tests/src/test/kotlin/se/ansman/kotshi/BaseGeneratorTest.kt
@@ -2,6 +2,7 @@ package se.ansman.kotshi
 
 import assertk.assertThat
 import assertk.assertions.contains
+import assertk.assertions.doesNotContain
 import assertk.assertions.hasSize
 import assertk.assertions.isEqualTo
 import com.squareup.moshi.JsonAdapter
@@ -153,7 +154,7 @@ abstract class BaseGeneratorTest {
               @se.ansman.kotshi.JsonSerializable
               @se.ansman.kotshi.PolymorphicLabel("default")
               @se.ansman.kotshi.JsonDefaultValue
-              object Default : SealedClass()
+              data object Default : SealedClass()
             }
         """))
         assertThat(result::exitCode).isEqualTo(KotlinCompilation.ExitCode.COMPILATION_ERROR)
@@ -229,7 +230,7 @@ abstract class BaseGeneratorTest {
     fun `objects must not be private`() {
         val result = compile(kotlin("source.kt", """
            @se.ansman.kotshi.JsonSerializable
-           private object Singleton
+           private data object Singleton
         """))
         assertThat(result::exitCode).isEqualTo(KotlinCompilation.ExitCode.COMPILATION_ERROR)
         assertThat(result::messages).contains(privateClass)
@@ -509,7 +510,7 @@ abstract class BaseGeneratorTest {
         """)
         val source = kotlin("source.kt", """
             @se.ansman.kotshi.JsonSerializable
-            object TestObject
+            data object TestObject
 
             @se.ansman.kotshi.KotshiJsonAdapterFactory
             object TestFactory : com.squareup.moshi.JsonAdapter.Factory by KotshiTestFactory
@@ -533,7 +534,7 @@ abstract class BaseGeneratorTest {
     fun `can add jdk 9 generated annotation`() {
         val source = kotlin("source.kt", """
             @se.ansman.kotshi.JsonSerializable
-            object TestObject
+            data object TestObject
 
             @se.ansman.kotshi.KotshiJsonAdapterFactory
             object TestFactory : com.squareup.moshi.JsonAdapter.Factory by KotshiTestFactory
@@ -556,7 +557,7 @@ abstract class BaseGeneratorTest {
     fun `cannot add invalid generated annotation`() {
         val source = kotlin("source.kt", """
             @se.ansman.kotshi.JsonSerializable
-            object TestObject
+            data object TestObject
 
             @se.ansman.kotshi.KotshiJsonAdapterFactory
             object TestFactory : com.squareup.moshi.JsonAdapter.Factory by KotshiTestFactory
@@ -567,12 +568,40 @@ abstract class BaseGeneratorTest {
         assertThat(result::messages).contains(Errors.invalidGeneratedAnnotation(annotationClass))
     }
 
-    protected fun compile(vararg sources: SourceFile, options: Map<String, String> = emptyMap()) =
+    @Test
+    open fun `non data object logs warnings`() {
+        val source = kotlin("source.kt", """
+            @se.ansman.kotshi.JsonSerializable
+            object TestObject
+        """)
+        val result = compile(source)
+        assertThat(result::exitCode).isEqualTo(KotlinCompilation.ExitCode.OK)
+        assertThat(result::messages).contains(Errors.nonDataObject)
+    }
+
+    @Test
+    fun `non data object does not logs warnings when using Kotlin 1_8`() {
+        val source = kotlin("source.kt", """
+            @se.ansman.kotshi.JsonSerializable
+            object TestObject
+        """)
+        val result = compile(source, languageVersion = "1.8")
+        assertThat(result::exitCode).isEqualTo(KotlinCompilation.ExitCode.OK)
+        assertThat(result::messages).doesNotContain(Errors.nonDataObject)
+    }
+
+    protected fun compile(
+        vararg sources: SourceFile,
+        options: Map<String, String> = emptyMap(),
+        languageVersion: String = "1.9"
+    ) =
         KotlinCompilation()
             .apply {
                 workingDir = temporaryFolder
                 this.sources = sources.asList()
                 inheritClassPath = true
+                // TODO: Remove once kotlin-compile-testing supports Kotlin 1.9
+                this.languageVersion = languageVersion
                 messageOutputStream = System.out // see diagnostics in real time
                 setUp(options)
             }

Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,7 @@ import com.tschuchort.compiletesting.SourceFile`
`7`	`7`	`import com.tschuchort.compiletesting.kspArgs`
`8`	`8`	`import com.tschuchort.compiletesting.kspIncremental`
`9`	`9`	`import com.tschuchort.compiletesting.symbolProcessorProviders`
	`10`	`+import org.junit.jupiter.api.Disabled`
`10`	`11`	`import org.junit.jupiter.api.Test`
`11`	`12`	`import org.junit.jupiter.api.parallel.Execution`
`12`	`13`	`import org.junit.jupiter.api.parallel.ExecutionMode`
`@@ -313,4 +314,7 @@ class KspGeneratorTest : BaseGeneratorTest() {`
`313`	`314`	`)`
`314`	`315`	`}`
`315`	`316`	`}`
	`317`	`+`
	`318`	`+ @Disabled("kotlin-compile-testing doesn't work well with overriding language version yet")`
	`319`	+ override fun `non data object logs warnings`() {}
`316`	`320`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,5 +9,5 @@ sealed class SealedClassOnInvalidFail {`
`9`	`9`
`10`	`10`	`@JsonSerializable`
`11`	`11`	`@JsonDefaultValue`
`12`		`- object Default : SealedClassOnInvalidFail()`
	`12`	`+ data object Default : SealedClassOnInvalidFail()`
`13`	`13`	`}`