Merge pull request #3 from AJIEKCX/v0.2.0/release

Add SerializableDiscriminatorRule

README.md

@@ -38,11 +38,42 @@ java.lang.IllegalArgumentException: SavedStateProvider with the given key is alr
 
 Avoid using the `defaultComponentContext` function inside Composable functions.
 
+### SerializableDiscriminatorRule
+
+Decompose uses the kotlinx.serialization library to serialize component configurations.
+A common mistake is naming a property the same as the class discriminator (default: "type"). For example:
+
+```kotlin
+import kotlinx.serialization.Serializable
+
+@Serializable
+sealed class ScreenConfig {
+    @Serializable
+    data object Input : ScreenConfig()
+
+    @Serializable
+    data class Details(
+        val message: String,
+        val type: String // DON'T DO THIS
+    ) : ScreenConfig()
+}
+```
+
+If Decompose attempts to serialize this component configuration, you will encounter the following exception:
+```
+java.lang.IllegalStateException: Sealed class 'ScreenConfig.Details' cannot be serialized as base class 'ScreenConfig' because it has property name that conflicts with JSON class discriminator 'type'.
+```
+
+Do not name properties in polymorphic serializable classes as a class discriminator.
+
 ## Enabling rules
 
 By default, all rules are enabled, but you can configure them in your `detekt.yml` configuration file:
 ```yaml
 DecomposeRuleSet:
   DecomposeComponentContextRule:
     active: true
+  SerializableDiscriminatorRule:
+    active: true
+    classDiscriminator: 'type' # optional
 ```

gradle.properties

@@ -1,4 +1,4 @@
 kotlin.code.style=official
 
 GROUP=io.github.ajiekcx.detekt
-VERSION_NAME=0.1.0
+VERSION_NAME=0.2.0

src/main/kotlin/io/github/ajiekcx/detekt/DecomposeRuleSetProvider.kt

@@ -11,7 +11,8 @@ class DecomposeRuleSetProvider : RuleSetProvider {
         return RuleSet(
             ruleSetId,
             listOf(
-                DecomposeComponentContextRule(config)
+                DecomposeComponentContextRule(config),
+                SerializableDiscriminatorRule(config)
             ),
         )
     }

src/main/kotlin/io/github/ajiekcx/detekt/SerializableDiscriminatorRule.kt

@@ -0,0 +1,156 @@
+package io.github.ajiekcx.detekt
+
+import io.gitlab.arturbosch.detekt.api.CodeSmell
+import io.gitlab.arturbosch.detekt.api.Config
+import io.gitlab.arturbosch.detekt.api.Debt
+import io.gitlab.arturbosch.detekt.api.Entity
+import io.gitlab.arturbosch.detekt.api.Issue
+import io.gitlab.arturbosch.detekt.api.Rule
+import io.gitlab.arturbosch.detekt.api.Severity
+import org.jetbrains.kotlin.psi.KtClass
+import org.jetbrains.kotlin.psi.KtFile
+import org.jetbrains.kotlin.psi.KtNamedDeclaration
+import org.jetbrains.kotlin.psi.psiUtil.findPropertyByName
+
+/**
+ * A Detekt rule that checks for properties named as class discriminators
+ * in polymorphic serializable classes. It prevents naming properties
+ * using a reserved discriminator (e.g., "type") to avoid serialization issues.
+ *
+ * @see https://github.com/Kotlin/kotlinx.serialization/blob/master/docs/polymorphism.md
+ */
+class SerializableDiscriminatorRule(config: Config) : Rule(config) {
+    // Retrieves the forbidden property name from the configuration (default is "type")
+    private val classDiscriminator = config
+        .subConfig("SerializableDiscriminatorRule")
+        .valueOrDefault("classDiscriminator", "type")
+
+    override val issue = Issue(
+        javaClass.simpleName,
+        Severity.CodeSmell,
+        "Do not name properties in polymorphic serializable classes as a class discriminator: $classDiscriminator",
+        Debt.FIVE_MINS
+    )
+
+    /**
+     * Visits the entire Kotlin file and checks for serializable sealed class hierarchies.
+     * It ensures that subclasses of sealed serializable classes do not contain a property
+     * named as the forbidden class discriminator.
+     */
+    override fun visitKtFile(file: KtFile) {
+        super.visitKtFile(file)
+
+        // Skip processing if the file does not import kotlinx.serialization.Serializable
+        if (!file.hasSerializableImport()) return
+
+        // Iterate through all declarations in the file, filtering only sealed serializable classes
+        file.declarations
+            .filterIsInstance<KtClass>()
+            .filter { it.isSealed() && it.isSerializable() }
+            .forEach { sealedClass ->
+                // Find all serializable subclasses of the sealed class and check for violations
+                file.declarations
+                    .filterIsInstance<KtClass>()
+                    .filter { it.isSubclassOf(sealedClass) && it.isSerializable() }
+                    .forEach { it.findAndReportClassDiscriminatorProperty() }
+            }
+    }
+
+    /**
+     * Visits each class declaration and checks for forbidden properties if the class
+     * is a sealed and contains nested serializable classes.
+     */
+    override fun visitClass(klass: KtClass) {
+        super.visitClass(klass)
+
+        // Skip processing if the containing file does not import kotlinx.serialization.Serializable
+        if (!klass.containingKtFile.hasSerializableImport()) return
+
+        // If the class is a sealed class and is marked as @Serializable
+        if (klass.isSealed() && klass.isSerializable()) {
+            // Check nested classes inside the sealed class
+            klass.declarations
+                .filterIsInstance<KtClass>()
+                .filter { it.isSerializable() }
+                .forEach { nestedClass ->
+                    nestedClass.findAndReportClassDiscriminatorProperty()
+                }
+        }
+    }
+
+    /**
+     * Checks whether a Kotlin file has the required import for kotlinx.serialization.Serializable.
+     */
+    private fun KtFile.hasSerializableImport(): Boolean {
+        return hasImport("import kotlinx.serialization.Serializable")
+    }
+
+    /**
+     * Finds and reports properties that match the forbidden class discriminator name.
+     */
+    private fun KtClass.findAndReportClassDiscriminatorProperty() {
+        // If a property is explicitly annotated with @SerialName(classDiscriminator), report an issue
+        if (findPropertyWithSerialName(classDiscriminator)) {
+            reportIssue()
+
+            return
+        }
+
+        // Otherwise, check if the property exists by name
+        val property = findPropertyByName(classDiscriminator) ?: return
+
+        // If the property has a different @SerialName annotation, do not report it
+        if (property.hasSerialNameWithoutClassDiscriminator()) return
+
+        // Report an issue if the property is named as the class discriminator
+        reportIssue()
+    }
+
+    /**
+     * Checks if a class has a primary constructor parameter with @SerialName matching the forbidden name.
+     */
+    private fun KtClass.findPropertyWithSerialName(name: String): Boolean {
+        // Ensure the necessary import for @SerialName exists
+        val hasImport = containingKtFile.hasSerialNameImport()
+
+        return hasImport && primaryConstructorParameters.any { param ->
+            param.hasValOrVar() && param.annotationEntries.any { it.text == "@SerialName(\"$name\")" }
+        }
+    }
+
+    /**
+     * Checks if a named property has an @SerialName annotation that does not match the forbidden discriminator.
+     */
+    private fun KtNamedDeclaration.hasSerialNameWithoutClassDiscriminator(): Boolean {
+        val hasImport = containingKtFile.hasSerialNameImport()
+        if (!hasImport) return false
+
+        val serialNameAnnotation = annotationEntries.find { it.text.startsWith("@SerialName") }
+
+        return serialNameAnnotation != null
+                && serialNameAnnotation.text != "@SerialName(\"$classDiscriminator\")"
+    }
+
+    /**
+     * Checks whether the Kotlin file has an import statement for kotlinx.serialization.SerialName.
+     */
+    private fun KtFile.hasSerialNameImport(): Boolean {
+        return hasImport("import kotlinx.serialization.SerialName")
+    }
+
+    /**
+     * Reports a code smell issue when a forbidden class discriminator property is found.
+     */
+    private fun KtClass.reportIssue() {
+        report(
+            CodeSmell(
+                issue,
+                Entity.from(this),
+                """
+                    Do not name properties in polymorphic serializable classes as a class discriminator: $classDiscriminator"
+                    See https://github.com/Kotlin/kotlinx.serialization/blob/master/docs/polymorphism.md
+                """.trimIndent()
+            )
+        )
+    }
+}

src/main/kotlin/io/github/ajiekcx/detekt/Utils.kt

@@ -55,6 +55,22 @@ internal fun KtClass.isFragment(): Boolean {
     return hasSuperTypeEndingWith("Fragment")
 }
 
+/**
+ * Determines if a class has the @Serializable annotation.
+ */
+internal fun KtClass.isSerializable(): Boolean {
+    return annotationEntries.any { it.text.startsWith("@Serializable") }
+}
+
+/**
+ * Checks if a class is a direct subclass of a given class.
+ */
+internal fun KtClass.isSubclassOf(klass: KtClass): Boolean {
+    return superTypeListEntries
+        .mapNotNull { it.typeReference?.text }
+        .any { it == klass.name }
+}
+
 /**
  * Checks if the class has a superclass that ends with a given suffix.
  *

src/main/resources/config/config.yml

@@ -1,3 +1,6 @@
 DecomposeRuleSet:
   DecomposeComponentContextRule:
     active: true
+  SerializableDiscriminatorRule:
+    active: true
+    classDiscriminator: 'type'