diff --git a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinSymbolProvider.kt b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinSymbolProvider.kt index 062f7bc08e..dadec5447e 100644 --- a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinSymbolProvider.kt +++ b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinSymbolProvider.kt @@ -13,9 +13,7 @@ import software.amazon.smithy.kotlin.codegen.utils.dq 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.ClientOptionalTrait import software.amazon.smithy.model.traits.DefaultTrait -import software.amazon.smithy.model.traits.InputTrait import software.amazon.smithy.model.traits.StreamingTrait import java.util.logging.Logger @@ -183,12 +181,11 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli val targetSymbol = toSymbol(targetShape) .toBuilder() .apply { - if (nullableIndex.isMemberNullable(shape, settings.api.nullabilityCheckMode)) nullable() + val isNullable = nullableIndex.isMemberNullable(shape, settings.api.nullabilityCheckMode) + if (isNullable) nullable() - // @clientOptional supersedes @default - val container = model.expectShape(shape.container) - val isClientOptional = shape.hasTrait() || container.hasTrait() - if (!isClientOptional) { + // only use @default if type is `T` (not `T?`) or marked `@required` (in which case it will be serialized anyway) + if (!isNullable || shape.isRequired) { shape.getTrait()?.let { defaultValue(it.getDefaultValue(targetShape)) } diff --git a/codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/SymbolProviderTest.kt b/codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/SymbolProviderTest.kt index c0f57f90a6..8bd1a1c555 100644 --- a/codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/SymbolProviderTest.kt +++ b/codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/SymbolProviderTest.kt @@ -369,6 +369,26 @@ class SymbolProviderTest { @Test fun `@clientOptional overrides @default`() { + val model = """ + structure MyStruct { + @clientOptional + @default("Foo") + quux: QuuxType + } + + string QuuxType + """.prependNamespaceAndService().toSmithyModel() + + val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model) + val member = model.expectShape("com.test#MyStruct\$quux") + val memberSymbol = provider.toSymbol(member) + assertEquals("kotlin", memberSymbol.namespace) + assertTrue(memberSymbol.isNullable) + assertEquals("null", memberSymbol.defaultValue()) + } + + @Test + fun `required @clientOptional with @default`() { val model = """ structure MyStruct { @required @@ -380,6 +400,27 @@ class SymbolProviderTest { string QuuxType """.prependNamespaceAndService().toSmithyModel() + val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model) + val member = model.expectShape("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 = """ + @input + structure MyStruct { + @default(2) + quux: QuuxType + } + + long QuuxType + """.prependNamespaceAndService().toSmithyModel() + val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model) val member = model.expectShape("com.test#MyStruct\$quux") val memberSymbol = provider.toSymbol(member) @@ -389,7 +430,7 @@ class SymbolProviderTest { } @Test - fun `@input`() { + fun `@input with required`() { val model = """ @input structure MyStruct { @@ -405,8 +446,10 @@ class SymbolProviderTest { val member = model.expectShape("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("null", memberSymbol.defaultValue()) + assertEquals("2L", memberSymbol.defaultValue()) } @Test diff --git a/tests/codegen/nullability-tests/model/nullability.smithy b/tests/codegen/nullability-tests/model/nullability.smithy index 63b19b4b0e..c27d7fba23 100644 --- a/tests/codegen/nullability-tests/model/nullability.smithy +++ b/tests/codegen/nullability-tests/model/nullability.smithy @@ -17,9 +17,12 @@ operation SayHello { structure TestInput { nested: TestStruct, - @required @default("hammer") tay: String + + @required + @default("ball") + lep: String } structure TestStruct { @@ -61,6 +64,11 @@ structure TestStruct { @required @clientOptional clientOptionalValue: Integer + + @required + @default("model default") + @clientOptional + clientOptionalWithDefault: String } enum Enum { diff --git a/tests/codegen/nullability-tests/src/test/kotlin/smithy/kotlin/nullability/DefaultValueTest.kt b/tests/codegen/nullability-tests/src/test/kotlin/smithy/kotlin/nullability/DefaultValueTest.kt index d6ad5c97f4..05e785d557 100644 --- a/tests/codegen/nullability-tests/src/test/kotlin/smithy/kotlin/nullability/DefaultValueTest.kt +++ b/tests/codegen/nullability-tests/src/test/kotlin/smithy/kotlin/nullability/DefaultValueTest.kt @@ -5,6 +5,7 @@ package smithy.kotlin.nullability import kotlin.test.Test +import kotlin.test.assertEquals import kotlin.test.assertNull class DefaultValueTest { @@ -14,7 +15,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` assertNull(actual.tay) + assertEquals("ball", actual.lep) } @Test @@ -23,6 +26,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` assertNull(actual.tay) + assertEquals("ball", actual.lep) } } diff --git a/tests/codegen/nullability-tests/src/test/kotlin/smithy/kotlin/nullability/ErrorCorrectionTest.kt b/tests/codegen/nullability-tests/src/test/kotlin/smithy/kotlin/nullability/ErrorCorrectionTest.kt index 24a8f77aed..812badf47e 100644 --- a/tests/codegen/nullability-tests/src/test/kotlin/smithy/kotlin/nullability/ErrorCorrectionTest.kt +++ b/tests/codegen/nullability-tests/src/test/kotlin/smithy/kotlin/nullability/ErrorCorrectionTest.kt @@ -37,6 +37,9 @@ class ErrorCorrectionTest { assertEquals(emptyList(), actual.nestedListValue) assertEquals(emptyMap(), actual.mapValue) + // should still be nullable due to `@clientOptional` but due to `@required` we actually use the `@default` + assertEquals("model default", actual.clientOptionalWithDefault) + // error correction should apply recursively assertEquals("", actual.nested.a) @@ -71,6 +74,9 @@ class ErrorCorrectionTest { assertEquals(emptyList(), actual.nestedListValue) assertEquals(emptyMap(), actual.mapValue) + // should still be nullable due to `@clientOptional` but due to `@required` we actually use the `@default` + assertEquals("model default", actual.clientOptionalWithDefault) + // nested struct and union values should be null for client careful mode assertNull(actual.nested) assertNull(actual.union)