update rules to use default values only when T not T?

smithy-lang · Oct 3, 2023 · 8b21600 · 8b21600
1 parent 3b455aa
commit 8b21600
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 52 deletions.
diff --git a/...odegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinSymbolProvider.kt b/...odegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinSymbolProvider.kt
@@ -184,8 +184,8 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli
                 val isNullable = nullableIndex.isMemberNullable(shape, settings.api.nullabilityCheckMode)
                 if (isNullable) nullable()
 
-                // only use @default if type is `T` (not `T?`) or marked `@required` (in which case it will be serialized anyway)
-                if (!isNullable || shape.isRequired) {
+                // only use @default if type is `T`
+                if (!isNullable) {
                     shape.getTrait<DefaultTrait>()?.let {
                         defaultValue(it.getDefaultValue(targetShape))
                     }

diff --git a/...-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/SymbolProviderTest.kt b/...-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/SymbolProviderTest.kt
@@ -387,28 +387,6 @@ class SymbolProviderTest {
         assertEquals("null", memberSymbol.defaultValue())
     }
 
-    @Test
-    fun `required @clientOptional with @default`() {
-        val model = """
-        structure MyStruct {
-            @required
-            @clientOptional
-            @default("Foo")
-            quux: QuuxType
-        }
-
-        string QuuxType
-        """.prependNamespaceAndService().toSmithyModel()
-
-        val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model)
-        val member = model.expectShape<MemberShape>("com.test#MyStruct\$quux")
-        val memberSymbol = provider.toSymbol(member)
-        assertEquals("kotlin", memberSymbol.namespace)
-        // still nullable due to `@clientOptional` but we use the default due to `@required`
-        assertTrue(memberSymbol.isNullable)
-        assertEquals("\"Foo\"", memberSymbol.defaultValue())
-    }
-
     @Test
     fun `@input with default`() {
         val model = """
@@ -426,32 +404,10 @@ class SymbolProviderTest {
         val memberSymbol = provider.toSymbol(member)
         assertEquals("kotlin", memberSymbol.namespace)
         assertTrue(memberSymbol.isNullable)
+        // @input makes every member implicitly @clientOptional
         assertEquals("null", memberSymbol.defaultValue())
     }
 
-    @Test
-    fun `@input with required`() {
-        val model = """
-        @input
-        structure MyStruct {
-            @required
-            @default(2)
-            quux: QuuxType
-        }
-        
-        long QuuxType
-        """.prependNamespaceAndService().toSmithyModel()
-
-        val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model)
-        val member = model.expectShape<MemberShape>("com.test#MyStruct\$quux")
-        val memberSymbol = provider.toSymbol(member)
-        assertEquals("kotlin", memberSymbol.namespace)
-
-        // still nullable due to `@input` but we can use the default due to `@required`
-        assertTrue(memberSymbol.isNullable)
-        assertEquals("2L", memberSymbol.defaultValue())
-    }
-
     @Test
     fun `creates blobs`() {
         val model = """

diff --git a/...s/codegen/nullability-tests/src/test/kotlin/smithy/kotlin/nullability/DefaultValueTest.kt b/...s/codegen/nullability-tests/src/test/kotlin/smithy/kotlin/nullability/DefaultValueTest.kt
@@ -5,7 +5,6 @@
 package smithy.kotlin.nullability
 
 import kotlin.test.Test
-import kotlin.test.assertEquals
 import kotlin.test.assertNull
 
 class DefaultValueTest {
@@ -15,9 +14,9 @@ class DefaultValueTest {
             .build()
 
         // all members of structure marked with `@input` are implicitly `@clientOptional`
-        // which means they are nullable. We ignore `@default` unless it's marked `@required`
+        // which means they are nullable.
         assertNull(actual.tay)
-        assertEquals("ball", actual.lep)
+        assertNull(actual.lep)
     }
 
     @Test
@@ -26,8 +25,8 @@ class DefaultValueTest {
             .build()
 
         // all members of structure marked with `@input` are implicitly `@clientOptional`
-        // which means they are nullable. We ignore `@default` unless it's marked `@required`
+        // which means they are nullable.
         assertNull(actual.tay)
-        assertEquals("ball", actual.lep)
+        assertNull(actual.lep)
     }
 }