diff --git a/codegen/smithy-kotlin-codegen-testutils/src/main/kotlin/software/amazon/smithy/kotlin/codegen/test/CodegenTestUtils.kt b/codegen/smithy-kotlin-codegen-testutils/src/main/kotlin/software/amazon/smithy/kotlin/codegen/test/CodegenTestUtils.kt index 321c609284..00241155ce 100644 --- a/codegen/smithy-kotlin-codegen-testutils/src/main/kotlin/software/amazon/smithy/kotlin/codegen/test/CodegenTestUtils.kt +++ b/codegen/smithy-kotlin-codegen-testutils/src/main/kotlin/software/amazon/smithy/kotlin/codegen/test/CodegenTestUtils.kt @@ -14,6 +14,7 @@ import software.amazon.smithy.build.MockManifest import software.amazon.smithy.codegen.core.Symbol import software.amazon.smithy.codegen.core.SymbolProvider import software.amazon.smithy.kotlin.codegen.KotlinCodegenPlugin +import software.amazon.smithy.kotlin.codegen.KotlinSettings import software.amazon.smithy.kotlin.codegen.core.* import software.amazon.smithy.kotlin.codegen.model.buildSymbol import software.amazon.smithy.kotlin.codegen.model.expectShape @@ -250,9 +251,15 @@ fun generateCode(generator: (KotlinWriter) -> Unit): String { return rawCodegen.substring(rawCodegen.indexOf(packageDeclaration) + packageDeclaration.length).trim() } -fun KotlinCodegenPlugin.Companion.createSymbolProvider(model: Model, rootNamespace: String = TestModelDefault.NAMESPACE, sdkId: String = TestModelDefault.SDK_ID, serviceName: String = TestModelDefault.SERVICE_NAME): SymbolProvider { - val settings = model.defaultSettings(serviceName = serviceName, packageName = rootNamespace, sdkId = sdkId) - return createSymbolProvider(model, settings) +fun KotlinCodegenPlugin.Companion.createSymbolProvider( + model: Model, + rootNamespace: String = TestModelDefault.NAMESPACE, + sdkId: String = TestModelDefault.SDK_ID, + serviceName: String = TestModelDefault.SERVICE_NAME, + settings: KotlinSettings? = null, +): SymbolProvider { + val resolvedSettings = settings ?: model.defaultSettings(serviceName = serviceName, packageName = rootNamespace, sdkId = sdkId) + return createSymbolProvider(model, resolvedSettings) } /** 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 961d08708a..d2bfc6e11b 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 @@ -15,7 +15,6 @@ 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.SparseTrait import software.amazon.smithy.model.traits.StreamingTrait import java.util.logging.Logger @@ -93,12 +92,12 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli override fun stringShape(shape: StringShape): Symbol = if (shape.isEnum) { createEnumSymbol(shape) } else { - createSymbolBuilder(shape, "String", nullable = true, namespace = "kotlin").build() + createSymbolBuilder(shape, "String", namespace = "kotlin").build() } private fun createEnumSymbol(shape: Shape): Symbol { val namespace = "$rootNamespace.model" - return createSymbolBuilder(shape, shape.defaultName(service), namespace, nullable = true) + return createSymbolBuilder(shape, shape.defaultName(service), namespace) .definitionFile("${shape.defaultName(service)}.kt") .build() } @@ -109,7 +108,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli override fun structureShape(shape: StructureShape): Symbol { val name = shape.defaultName(service) val namespace = "$rootNamespace.model" - val builder = createSymbolBuilder(shape, name, namespace, nullable = true) + val builder = createSymbolBuilder(shape, name, namespace) .definitionFile("$name.kt") // add a reference to each member symbol @@ -147,10 +146,10 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli override fun listShape(shape: ListShape): Symbol { val reference = toSymbol(shape.member) - val valueSuffix = if (shape.hasTrait()) "?" else "" + val valueSuffix = if (reference.isNullable) "?" else "" val valueType = "${reference.name}$valueSuffix" val fullyQualifiedValueType = "${reference.fullName}$valueSuffix" - return createSymbolBuilder(shape, "List<$valueType>", nullable = true) + return createSymbolBuilder(shape, "List<$valueType>") .addReferences(reference) .putProperty(SymbolProperty.FULLY_QUALIFIED_NAME_HINT, "List<$fullyQualifiedValueType>") .putProperty(SymbolProperty.MUTABLE_COLLECTION_FUNCTION, "mutableListOf<$valueType>") @@ -160,13 +159,13 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli override fun mapShape(shape: MapShape): Symbol { val reference = toSymbol(shape.value) - val valueSuffix = if (shape.hasTrait()) "?" else "" + val valueSuffix = if (reference.isNullable) "?" else "" val valueType = "${reference.name}$valueSuffix" val fullyQualifiedValueType = "${reference.fullName}$valueSuffix" val keyType = KotlinTypes.String.name val fullyQualifiedKeyType = KotlinTypes.String.fullName - return createSymbolBuilder(shape, "Map<$keyType, $valueType>", nullable = true) + return createSymbolBuilder(shape, "Map<$keyType, $valueType>") .addReferences(reference) .putProperty(SymbolProperty.FULLY_QUALIFIED_NAME_HINT, "Map<$fullyQualifiedKeyType, $fullyQualifiedValueType>") .putProperty(SymbolProperty.MUTABLE_COLLECTION_FUNCTION, "mutableMapOf<$keyType, $valueType>") @@ -182,7 +181,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli val targetSymbol = toSymbol(targetShape) .toBuilder() .apply { - if (nullableIndex.isMemberNullable(shape, NullableIndex.CheckMode.CLIENT_ZERO_VALUE_V1_NO_INPUT)) nullable() + if (nullableIndex.isMemberNullable(shape, settings.api.nullabilityCheckMode)) nullable() if (!shape.hasTrait()) { // @ClientOptional supersedes @default shape.getTrait()?.let { @@ -238,16 +237,16 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli override fun timestampShape(shape: TimestampShape?): Symbol { val dependency = KotlinDependency.CORE - return createSymbolBuilder(shape, "Instant", nullable = true) + return createSymbolBuilder(shape, "Instant") .namespace("${dependency.namespace}.time", ".") .addDependency(dependency) .build() } override fun blobShape(shape: BlobShape): Symbol = if (shape.hasTrait()) { - RuntimeTypes.Core.Content.ByteStream.asNullable() + RuntimeTypes.Core.Content.ByteStream } else { - createSymbolBuilder(shape, "ByteArray", nullable = true, namespace = "kotlin").build() + createSymbolBuilder(shape, "ByteArray", namespace = "kotlin").build() } override fun documentShape(shape: DocumentShape?): Symbol = @@ -256,7 +255,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli override fun unionShape(shape: UnionShape): Symbol { val name = shape.defaultName(service) val namespace = "$rootNamespace.model" - val builder = createSymbolBuilder(shape, name, namespace, nullable = true) + val builder = createSymbolBuilder(shape, name, namespace) .definitionFile("$name.kt") // add a reference to each member symbol @@ -265,7 +264,10 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli return builder.build() } - override fun resourceShape(shape: ResourceShape?): Symbol = createSymbolBuilder(shape, "Resource").build() + override fun resourceShape(shape: ResourceShape?): Symbol { + // The Kotlin SDK does not produce code explicitly based on Resources + error { "unexpected codegen code path" } + } override fun operationShape(shape: OperationShape?): Symbol { // The Kotlin SDK does not produce code explicitly based on Operations diff --git a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/StructureGenerator.kt b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/StructureGenerator.kt index d702d4b789..a767e4e54e 100644 --- a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/StructureGenerator.kt +++ b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/StructureGenerator.kt @@ -90,6 +90,7 @@ class StructureGenerator( memberName, memberSymbol, ) + // FIXME - shouldn't this only target string shapes? if (memberShape.isNonBlankInStruct) { writer .indent() @@ -242,7 +243,6 @@ class StructureGenerator( .withBlock("public class Builder {", "}") { for (member in sortedMembers) { val (memberName, memberSymbol) = memberNameSymbolIndex[member]!! - // we want the type names sans nullability (?) for arguments writer.renderMemberDocumentation(model, member) writer.renderAnnotations(member) write("public var #L: #E", memberName, memberSymbol) 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 c2f5236cc4..000c39cf88 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 @@ -9,17 +9,18 @@ import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.CsvSource import org.junit.jupiter.params.provider.ValueSource import software.amazon.smithy.codegen.core.SymbolProvider +import software.amazon.smithy.kotlin.codegen.ApiSettings import software.amazon.smithy.kotlin.codegen.KotlinCodegenPlugin import software.amazon.smithy.kotlin.codegen.core.KotlinDependency.Companion.CORE -import software.amazon.smithy.kotlin.codegen.model.defaultValue -import software.amazon.smithy.kotlin.codegen.model.expectShape -import software.amazon.smithy.kotlin.codegen.model.fullNameHint -import software.amazon.smithy.kotlin.codegen.model.isNullable +import software.amazon.smithy.kotlin.codegen.model.* import software.amazon.smithy.kotlin.codegen.model.traits.SYNTHETIC_NAMESPACE import software.amazon.smithy.kotlin.codegen.test.* +import software.amazon.smithy.model.knowledge.NullableIndex.CheckMode import software.amazon.smithy.model.shapes.* +import software.amazon.smithy.model.traits.ClientOptionalTrait import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertTrue class SymbolProviderTest { @@ -775,4 +776,150 @@ class SymbolProviderTest { assertEquals("com.test.model.Events", symbol.references[0].symbol.fullName) } + + @ParameterizedTest(name = "{index} ==> ''{0}''") + @ValueSource(strings = ["CLIENT_CAREFUL", "CLIENT"]) + fun `it handles client nullability for IDL v2 check modes`(rawCheckMode: String) { + val model = """ + operation TestOp { + input: OpInput + } + + @input + structure OpInput { + @httpHeader("x-test") + xTestHeader: String, + + @required + boolean: Boolean + + list: IntList, + map: StringMap, + + @required + top: MyStruct + } + + integer MyInt + + @default("foo") + string MyString + + list IntList { + member: MyInt + } + + @sparse + list SparseIntList { + member: MyInt + } + + map StringMap { + key: String, + value: MyInt + } + + structure MyStruct { + @required + union: MyUnion, + + @required + string: String, + + @required + list: IntList, + + sparseList: SparseIntList, + + @required + nested: Nested, + + @required + @clientOptional + clientOptionalString: String, + + @required + enum: MyEnum, + + @default(1) + defaultInt: MyInt, + + @default("foo") + defaultString: MyString, + + @default(null) + defaultButNullString: MyString + } + + enum MyEnum { + Variant1, + Variant2 + } + + structure Nested { + nestedString: String + } + + union MyUnion { + blob: Blob, + boolean: Boolean, + date: Timestamp, + int: Integer, + } + """.prependNamespaceAndService(operations = listOf("TestOp")).toSmithyModel() + + val checkMode = CheckMode.valueOf(rawCheckMode) + val settings = model.defaultSettings().copy(api = ApiSettings(nullabilityCheckMode = checkMode)) + val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model, settings = settings) + + // opInput members always optional because of @input + val opInputStruct = model.expectShape("com.test#OpInput") + opInputStruct.members().forEach { + assertTrue(provider.toSymbol(it).isNullable, "expected $it to be nullable because its marked with @input trait") + } + + // struct/union members optional in client careful + val myStruct = model.expectShape("com.test#MyStruct") + val unionAndStructMembers = listOf("union", "nested").map { myStruct.getMember(it).get() } + unionAndStructMembers.forEach { + val memberSymbol = provider.toSymbol(it) + when (checkMode) { + CheckMode.CLIENT_CAREFUL -> assertTrue(memberSymbol.isNullable, "struct/union $it should be optional in $checkMode mode") + else -> assertFalse(memberSymbol.isNullable, "struct/union $it should be required in $checkMode") + } + } + + // required members not optional - except client careful + myStruct.members() + .filter(MemberShape::isRequired) + .filterNot { it in unionAndStructMembers } + .forEach { + val memberSymbol = provider.toSymbol(it) + if (it.hasTrait()) { + assertTrue(memberSymbol.isNullable, "@clientOptional member $it should be nullable regardless of @required") + } else { + assertFalse(memberSymbol.isNullable, "@required member $it should not be nullable") + } + } + + // union members are not optional + val unionShape = model.expectShape("com.test#MyUnion") + assertTrue(unionShape.members().map { provider.toSymbol(it) }.none { it.isNullable }, "union members should not be nullable") + + // default null are optional + myStruct.members() + .filter { it.hasNullDefault() } + .forEach { + val memberSymbol = provider.toSymbol(it) + assertTrue(memberSymbol.isNullable, "member $it with explicit null default should be nullable") + } + + // non-null default are not optional + myStruct.members() + .filter { it.hasNonNullDefault() } + .forEach { + val memberSymbol = provider.toSymbol(it) + assertFalse(memberSymbol.isNullable, "member $it with non-null default should not be nullable") + } + } }