Don't replace nullable absent properties with null

moshi-ir/moshi-compiler-plugin/src/main/kotlin/dev/zacsweers/moshix/ir/compiler/api/MoshiAdapterGenerator.kt

@@ -412,7 +412,7 @@ internal class MoshiAdapterGenerator(
               localVars[property.localName] = property.generateLocalProperty(this, pluginContext)
               localVars[property.localHasErrorName] =
                 property.generateLocalHasErrorProperty(this, pluginContext)
-              if (property.hasLocalIsPresentName) {
+              if (property.isRequired) {
                 localVars[property.localIsPresentName] =
                   property.generateLocalIsPresentProperty(this, pluginContext)
               }
@@ -493,12 +493,12 @@ internal class MoshiAdapterGenerator(
               if (!property.isTransientOrIgnored && property.isRequired) {
                 // Note we check if we've reported an error about a local var to avoid domino
                 // error reporting
-                // Otherwise an unexpected null prop could then get reported as missing too
+                // Otherwise an unexpected absent prop could then get reported as missing too
                 val compositeCondition =
                   irAnd(
                     pluginContext,
                     irNot(irGet(localVars.getValue(property.localHasErrorName))),
-                    irEqualsNull(irGet(localVars.getValue(property.localName))),
+                    irNot(irGet(localVars.getValue(property.localIsPresentName))),
                   )
                 +irIfThen(
                   condition = compositeCondition,
@@ -623,7 +623,7 @@ internal class MoshiAdapterGenerator(
                 continue // Property already handled.
               }
               val condition =
-                if (property.hasLocalIsPresentName) {
+                if (property.isRequired) {
                   irGet(localVars.getValue(property.localIsPresentName))
                 } else {
                   irNot(irEqualsNull(irGet(localVars.getValue(property.localName))))
@@ -753,7 +753,7 @@ internal class MoshiAdapterGenerator(
                       +setVarExpression
                     }
 
-                    if (property.hasLocalIsPresentName) {
+                    if (property.isRequired) {
                       // Presence tracker for a mutable property
                       +irSet(localVars.getValue(property.localIsPresentName), irBoolean(true))
                     }

moshi-ir/moshi-compiler-plugin/src/main/kotlin/dev/zacsweers/moshix/ir/compiler/api/PropertyGenerator.kt

@@ -36,27 +36,23 @@ internal class PropertyGenerator(
   val hasDefault: Boolean = target.hasDefault
 
   lateinit var localName: String
+  /**
+   * IsPresent is required if the following conditions are met:
+   * - Is not ignored
+   * - Has a default
+   *
+   * This is used to indicate that presence should be checked first before possible assigning null
+   * to an absent value
+   */
   lateinit var localIsPresentName: String
   lateinit var localHasErrorName: String
 
   val isRequired: Boolean
-    get() = !delegateKey.nullable && !hasDefault
+    get() = !hasDefault
 
   val hasConstructorParameter: Boolean
     get() = target.parameterIndex != -1
 
-  /**
-   * IsPresent is required if the following conditions are met:
-   * - Is not transient
-   * - Has a default
-   * - Is not a constructor parameter (for constructors we use a defaults mask)
-   * - Is nullable (because we differentiate absent from null)
-   *
-   * This is used to indicate that presence should be checked first before possible assigning null
-   * to an absent value
-   */
-  val hasLocalIsPresentName: Boolean =
-    !isTransientOrIgnored && hasDefault && !hasConstructorParameter && delegateKey.nullable
   val hasConstructorDefault: Boolean = hasDefault && hasConstructorParameter
 
   internal fun allocateNames(nameAllocator: NameAllocator) {

moshi-ir/moshi-kotlin-tests/src/test/kotlin/com/squareup/moshi/kotlin/DualKotlinTest.kt

@@ -799,6 +799,44 @@ class DualKotlinTest {
 
   @JsonClass(generateAdapter = true)
   data class PropertyWithDollarSign(val `$a`: String, @Json(name = "\$b") val b: String)
+
+  @Test
+  fun nullSafety() {
+    val jsonAdapter = moshi.adapter<NullSafety>()
+
+    // Present is ok
+    jsonAdapter.fromJson(
+      """
+      {
+        "a": "hello"
+      }
+      """
+        .trimIndent()
+    )
+
+    // Explicit null is ok
+    jsonAdapter.fromJson(
+      """
+      {
+        "a": null
+      }
+      """
+        .trimIndent()
+    )
+
+    // Absent is not
+    try {
+      jsonAdapter.fromJson("{}")
+      fail()
+    } catch (expected: JsonDataException) {
+      assertThat(expected).hasMessageThat().isEqualTo("Required value 'a' missing at $")
+    }
+  }
+
+  @JsonClass(generateAdapter = true)
+  data class NullSafety(
+    val a: String? // No default value means absence should fail if absent
+  )
 }
 
 typealias TypeAlias = Int

moshi-ir/moshi-kotlin-tests/src/test/kotlin/com/squareup/moshi/kotlin/codegen/GeneratedAdaptersTest.kt

@@ -265,7 +265,7 @@ class GeneratedAdaptersTest {
 
   @Test
   fun nullableTypeParams() {
-    val adapter = moshi.adapter<NullableTypeParams<Int>>()
+    val adapter = moshi.adapter<NullableTypeParams<Int>>().serializeNulls()
     val nullSerializing = adapter.serializeNulls()
 
     val nullableTypeParams =
@@ -281,7 +281,7 @@ class GeneratedAdaptersTest {
       NullableTypeParams(
         nullableTypeParams.nullableList,
         nullableTypeParams.nullableSet,
-        nullableTypeParams.nullableMap.filterValues { it != null },
+        nullableTypeParams.nullableMap,
         null,
         1,
       )
@@ -444,9 +444,12 @@ class GeneratedAdaptersTest {
     assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"b":5}""")
     assertThat(jsonAdapter.serializeNulls().toJson(encoded)).isEqualTo("""{"a":null,"b":5}""")
 
-    val decoded = jsonAdapter.fromJson("""{"b":6}""")!!
-    assertThat(decoded.a).isNull()
-    assertThat(decoded.b).isEqualTo(6)
+    try {
+      jsonAdapter.fromJson("""{"b":6}""")!!
+      fail()
+    } catch (expected: JsonDataException) {
+      assertThat(expected).hasMessageThat().isEqualTo("Required value 'a' missing at $")
+    }
   }
 
   @JsonClass(generateAdapter = true) class AbsentNull(var a: Int?, var b: Int?)

moshi-metadata-reflect/src/main/kotlin/dev/zacsweers/moshix/reflect/MetadataKotlinJsonAdapterFactory.kt

@@ -100,19 +100,16 @@ internal class KotlinJsonAdapter<T>(
     }
     reader.endObject()
 
-    // Confirm all parameters are present, optional, or nullable.
+    // Confirm all required parameters are present or have a default value
     for (i in 0 until constructorSize) {
       if (values[i] === ABSENT_VALUE) {
         val param = constructor.parameters[i]
         if (!param.declaresDefaultValue) {
-          if (!param.isNullable) {
-            throw Util.missingProperty(
-              constructor.parameters[i].name,
-              allBindings[i]?.jsonName,
-              reader,
-            )
-          }
-          values[i] = null // Replace absent with null.
+          throw Util.missingProperty(
+            constructor.parameters[i].name,
+            allBindings[i]?.jsonName,
+            reader,
+          )
         }
       }
     }

moshi-metadata-reflect/src/test/kotlin/dev/zacsweers/moshix/reflect/KotlinJsonAdapterTest.kt

@@ -201,9 +201,12 @@ class KotlinJsonAdapterTest {
     assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"b":5}""")
     assertThat(jsonAdapter.serializeNulls().toJson(encoded)).isEqualTo("""{"a":null,"b":5}""")
 
-    val decoded = jsonAdapter.fromJson("""{"b":6}""")!!
-    assertThat(decoded.a).isNull()
-    assertThat(decoded.b).isEqualTo(6)
+    try {
+      jsonAdapter.fromJson("""{"b":6}""")!!
+      fail()
+    } catch (expected: JsonDataException) {
+      assertThat(expected).hasMessageThat().isEqualTo("Required value 'a' missing at $")
+    }
   }
 
   class AbsentNull(var a: Int?, var b: Int?)