Fix isData check with value classes (#731)

elihart · web-flow · commit dfd06ea6653c · 2025-10-02T15:32:23.000-07:00
diff --git a/mvrx-common/src/main/java/com/airbnb/mvrx/MavericksMutabilityHelper.kt b/mvrx-common/src/main/java/com/airbnb/mvrx/MavericksMutabilityHelper.kt
@@ -66,13 +66,27 @@ fun assertMavericksDataClassImmutability(
  */
 internal val Class<*>.isData: Boolean
     get() {
-        if (!declaredMethods.any { it.name == "copy\$default" && it.isSynthetic }) {
-            return false
+        // When a value class is present in the constructor, Kotlin mangles the copy method name
+        // to avoid signature clashes (e.g., "copy-KtkBMb8$default" instead of "copy$default").
+        // We check for either the exact name "copy$default" or the pattern "copy-*$default".
+        val hasCopyDefault = declaredMethods.any { method ->
+            method.isSynthetic && method.name.let { name ->
+                name == "copy\$default" || (name.startsWith("copy-") && name.endsWith("\$default"))
+            }
         }
+        if (!hasCopyDefault) return false
 
-        // if the data class property is internal then kotlin appends '$module_name_debug' to the
-        // expected function name.
-        declaredMethods.firstOrNull { it.name.startsWith("component1") } ?: return false
+        // Similarly, component1 can be mangled when it's a value class type.
+        // It can also have module names appended for internal properties.
+        // Patterns: "component1", "component1$module", "component1-<hash>"
+        val hasComponent1 = declaredMethods.any { method ->
+            method.name.let { name ->
+                name == "component1" ||
+                    name.startsWith("component1\$") ||
+                    name.startsWith("component1-")
+            }
+        }
+        if (!hasComponent1) return false
 
         declaredMethods.firstOrNull { it.name == "equals" } ?: return false
         declaredMethods.firstOrNull { it.name == "hashCode" } ?: return false
diff --git a/mvrx-common/src/test/kotlin/com/airbnb/mvrx/MavericksMutabilityHelperKtTest.kt b/mvrx-common/src/test/kotlin/com/airbnb/mvrx/MavericksMutabilityHelperKtTest.kt
@@ -12,7 +12,57 @@ class MavericksMutabilityHelperKtTest {
         assertFalse(String::class.java.isData)
     }
 
+    @Test
+    fun isDataWithValueClass() {
+        assertTrue(TestDataClassWithValueClass::class.java.isData)
+    }
+
+    @Test
+    fun isDataWithValueClassAsFirstParameter() {
+        // When a value class is the first parameter, component1 is mangled
+        assertTrue(TestDataClassWithValueClassFirst::class.java.isData)
+    }
+
+    @Test
+    fun assertImmutabilityWithValueClass() {
+        // Test that assertMavericksDataClassImmutability works with value classes
+        assertMavericksDataClassImmutability(TestDataClassWithValueClass::class)
+        assertMavericksDataClassImmutability(TestDataClassWithValueClassFirst::class)
+    }
+
+    @Test
+    fun isDataWithOnlyValueClass() {
+        assertTrue(TestDataClassWithOnlyValueClass::class.java.isData)
+    }
+
+    @Test
+    fun isDataWithMultipleValueClasses() {
+        assertTrue(TestDataClassWithMultipleValueClasses::class.java.isData)
+    }
+
+    @JvmInline
+    value class TestValueClass(val value: Int)
+
     data class TestDataClass(
         internal val foo: Int
     )
+
+    data class TestDataClassWithValueClass(
+        val foo: Int,
+        val valueClass: TestValueClass
+    )
+
+    data class TestDataClassWithValueClassFirst(
+        val valueClass: TestValueClass,
+        val foo: Int
+    )
+
+    data class TestDataClassWithOnlyValueClass(
+        val valueClass: TestValueClass
+    )
+
+    data class TestDataClassWithMultipleValueClasses(
+        val valueClass1: TestValueClass,
+        val valueClass2: TestValueClass
+    )
 }
