fix: properly check if a member can be nullable (#702)

Author: sugmanue

.changes/69672ec4-a822-465c-80ac-2656dd3ff334.json

@@ -0,0 +1,5 @@
+{
+    "id": "69672ec4-a822-465c-80ac-2656dd3ff334",
+    "type": "bugfix",
+    "description": "Properly check if a member can be nullable"
+}

gradle.properties

@@ -24,7 +24,7 @@ jsoupVersion=1.14.3
 okHttpVersion=5.0.0-alpha.10
 
 # codegen
-smithyVersion=1.23.0
+smithyVersion=1.25.0
 smithyGradleVersion=0.6.0
 
 # testing/utility

smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinSymbolProvider.kt

@@ -9,6 +9,7 @@ import software.amazon.smithy.kotlin.codegen.KotlinSettings
 import software.amazon.smithy.kotlin.codegen.lang.kotlinReservedWords
 import software.amazon.smithy.kotlin.codegen.model.*
 import software.amazon.smithy.model.Model
+import software.amazon.smithy.model.knowledge.NullableIndex
 import software.amazon.smithy.model.shapes.*
 import software.amazon.smithy.model.traits.SparseTrait
 import software.amazon.smithy.model.traits.StreamingTrait
@@ -26,6 +27,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli
     private val service = model.expectShape<ServiceShape>(settings.service)
     private val logger = Logger.getLogger(javaClass.name)
     private val escaper: ReservedWordSymbolProvider.Escaper
+    private val nullableIndex = NullableIndex(model)
 
     // model depth; some shapes use `toSymbol()` internally as they convert (e.g.) member shapes to symbols, this tracks
     // how deep in the model we have recursed
@@ -168,8 +170,11 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli
         val targetShape =
             model.getShape(shape.target).orElseThrow { CodegenException("Shape not found: ${shape.target}") }
 
-        val targetSymbol = toSymbol(targetShape)
-
+        val targetSymbol = if (nullableIndex.isMemberNullable(shape, NullableIndex.CheckMode.CLIENT_ZERO_VALUE_V1)) {
+            toSymbol(targetShape).toBuilder().boxed().build()
+        } else {
+            toSymbol(targetShape)
+        }
         // figure out if we are referencing an event stream or not.
         // NOTE: unlike blob streams we actually re-use the target (union) shape which is why we can't do this
         // when visiting a unionShape() like we can for blobShape()
@@ -249,9 +254,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli
         val builder = Symbol.builder()
             .putProperty(SymbolProperty.SHAPE_KEY, shape)
             .name(typeName)
-
-        val explicitlyBoxed = shape?.hasTrait<@Suppress("DEPRECATION") software.amazon.smithy.model.traits.BoxTrait>() ?: false
-        if (explicitlyBoxed || boxed) {
+        if (boxed) {
             builder.boxed()
         }
         return builder

smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/StructureGenerator.kt

@@ -9,10 +9,8 @@ import software.amazon.smithy.codegen.core.Symbol
 import software.amazon.smithy.kotlin.codegen.core.*
 import software.amazon.smithy.kotlin.codegen.lang.KotlinTypes
 import software.amazon.smithy.kotlin.codegen.model.*
-import software.amazon.smithy.model.shapes.BlobShape
-import software.amazon.smithy.model.shapes.MemberShape
-import software.amazon.smithy.model.shapes.ShapeType
-import software.amazon.smithy.model.shapes.StructureShape
+import software.amazon.smithy.model.knowledge.NullableIndex
+import software.amazon.smithy.model.shapes.*
 import software.amazon.smithy.model.traits.ErrorTrait
 import software.amazon.smithy.model.traits.HttpLabelTrait
 import software.amazon.smithy.model.traits.RetryableTrait
@@ -30,6 +28,7 @@ class StructureGenerator(
     private val symbolProvider = ctx.symbolProvider
     private val model = ctx.model
     private val symbol = ctx.symbolProvider.toSymbol(ctx.shape)
+    private val nullableIndex = NullableIndex(ctx.model)
 
     fun render() {
         writer.renderDocumentation(shape)
@@ -149,17 +148,15 @@ class StructureGenerator(
     // Return the appropriate hashCode fragment based on ShapeID of member target.
     private fun selectHashFunctionForShape(member: MemberShape): String {
         val targetShape = model.expectShape(member.target)
-        // also available already in the byMember map
-        val targetSymbol = symbolProvider.toSymbol(targetShape)
-
+        val isNullable = nullableIndex.isMemberNullable(member, NullableIndex.CheckMode.CLIENT_ZERO_VALUE_V1_NO_INPUT)
         return when (targetShape.type) {
             ShapeType.INTEGER ->
-                when (targetSymbol.isBoxed) {
+                when (isNullable) {
                     true -> " ?: 0"
                     else -> ""
                 }
             ShapeType.BYTE ->
-                when (targetSymbol.isBoxed) {
+                when (isNullable) {
                     true -> "?.toInt() ?: 0"
                     else -> ".toInt()"
                 }
@@ -172,7 +169,7 @@ class StructureGenerator(
                     "?.contentHashCode() ?: 0"
                 }
             else ->
-                when (targetSymbol.isBoxed) {
+                when (isNullable) {
                     true -> "?.hashCode() ?: 0"
                     else -> ".hashCode()"
                 }

smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/SymbolProviderTest.kt

@@ -19,6 +19,7 @@ import software.amazon.smithy.kotlin.codegen.test.*
 import software.amazon.smithy.model.shapes.*
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertTrue
 
 class SymbolProviderTest {
     @Test
@@ -97,6 +98,42 @@ class SymbolProviderTest {
         else -> typeName
     }
 
+    @Test
+    fun `can read box trait from member`() {
+        val model = """
+        structure MyStruct {
+           @box
+           foo: MyFoo
+        }
+        long MyFoo
+        """.prependNamespaceAndService().toSmithyModel()
+
+        val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model)
+        val member = model.expectShape<MemberShape>("com.test#MyStruct\$foo")
+        val memberSymbol = provider.toSymbol(member)
+        assertEquals("kotlin", memberSymbol.namespace)
+        assertEquals("null", memberSymbol.defaultValue())
+        assertTrue(memberSymbol.isBoxed)
+    }
+
+    @Test
+    fun `can read box trait from target`() {
+        val model = """
+        structure MyStruct {
+           foo: MyFoo
+        }
+        @box
+        long MyFoo
+        """.prependNamespaceAndService().toSmithyModel()
+
+        val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model)
+        val member = model.expectShape<MemberShape>("com.test#MyStruct\$foo")
+        val memberSymbol = provider.toSymbol(member)
+        assertEquals("kotlin", memberSymbol.namespace)
+        assertEquals("null", memberSymbol.defaultValue())
+        assertTrue(memberSymbol.isBoxed)
+    }
+
     @Test
     fun `creates blobs`() {
         val model = """