From 8933b1750da6b177137dbabd18a615ef072a27f8 Mon Sep 17 00:00:00 2001
From: Aaron J Todd <todaaron@amazon.com>
Date: Thu, 28 Sep 2023 15:10:44 -0400
Subject: [PATCH] improve when explicit null checking is required

---
 .../smithy/kotlin/codegen/KotlinSettings.kt   |  2 +-
 .../smithy/kotlin/codegen/model/SymbolExt.kt  |  7 +++
 .../codegen/rendering/StructureGenerator.kt   | 43 ++++++++++---------
 3 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/KotlinSettings.kt b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/KotlinSettings.kt
index 7c1df76d30..0449c32982 100644
--- a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/KotlinSettings.kt
+++ b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/KotlinSettings.kt
@@ -77,7 +77,7 @@ data class KotlinSettings(
          * @return Returns the extracted settings
          */
         fun from(model: Model, config: ObjectNode): KotlinSettings {
-            config.warnIfAdditionalProperties(listOf(SERVICE, PACKAGE_SETTINGS, BUILD_SETTINGS, SDK_ID))
+            config.warnIfAdditionalProperties(listOf(SERVICE, PACKAGE_SETTINGS, BUILD_SETTINGS, SDK_ID, API_SETTINGS))
 
             val serviceId = config.getStringMember(SERVICE)
                 .map(StringNode::expectShapeId)
diff --git a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/model/SymbolExt.kt b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/model/SymbolExt.kt
index 943df9aa12..b13e21953d 100644
--- a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/model/SymbolExt.kt
+++ b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/model/SymbolExt.kt
@@ -54,6 +54,13 @@ object SymbolProperty {
     const val FULLY_QUALIFIED_NAME_HINT: String = "fullyQualifiedNameHint"
 }
 
+/**
+ * Test if a symbol is required (not-nullable) with no default value set. This means there is no builder
+ * set default so the constructor will have to generate a runtime check that a value is set.
+ */
+val Symbol.isRequiredWithNoDefault: Boolean
+    get() = !isNullable && defaultValue() == null
+
 /**
  * Test if a symbol is nullable
  */
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 d0e5796a46..27f1f55619 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
@@ -85,26 +85,25 @@ class StructureGenerator(
             "public"
         }
 
-        when {
-            memberShape.isRequiredInStruct && !memberSymbol.isNullable -> {
-                writer.write(
-                    """#1L val #2L: #3F = requireNotNull(builder.#2L) { "A non-null value must be provided for #2L" }""",
-                    prefix,
+        if (memberShape.isRequiredInStruct && memberSymbol.isRequiredWithNoDefault) {
+            writer.write(
+                """#1L val #2L: #3F = requireNotNull(builder.#2L) { "A non-null value must be provided for #2L" }""",
+                prefix,
+                memberName,
+                memberSymbol,
+            )
+        } else {
+            writer.write("#1L val #2L: #3F = builder.#2L", prefix, memberName, memberSymbol)
+        }
+
+        if (memberShape.isNonBlankInStruct) {
+            writer
+                .indent()
+                .write(
+                    """.apply { require(isNotBlank()) { "A non-blank value must be provided for #L" } }""",
                     memberName,
-                    memberSymbol,
                 )
-                if (memberShape.isNonBlankInStruct) {
-                    writer
-                        .indent()
-                        .write(
-                            """.apply { require(isNotBlank()) { "A non-blank value must be provided for #L" } }""",
-                            memberName,
-                        )
-                        .dedent()
-                }
-            }
-
-            else -> writer.write("#1L val #2L: #3F = builder.#2L", prefix, memberName, memberSymbol)
+                .dedent()
         }
     }
 
@@ -248,7 +247,7 @@ class StructureGenerator(
                     val (memberName, memberSymbol) = memberNameSymbolIndex[member]!!
                     writer.renderMemberDocumentation(model, member)
                     writer.renderAnnotations(member)
-                    val builderMemberSymbol = if (!memberSymbol.isNullable && memberSymbol.defaultValue() == null) {
+                    val builderMemberSymbol = if (memberSymbol.isRequiredWithNoDefault) {
                         // nullabilty is w.r.t to the immmutable property type, builders have to account for the user
                         // providing required values though and thus must be nullable. They are then checked
                         // at runtime in the ctor to ensure a value was provided
@@ -299,7 +298,11 @@ class StructureGenerator(
                     "}",
                 ) {
                     sortedMembers
-                        .filter(MemberShape::isRequired)
+                        .filter {
+                            val (_, memberSymbol) = memberNameSymbolIndex[it]!!
+                            // required members with no default
+                            memberSymbol.isRequiredWithNoDefault
+                        }
                         .filterNot {
                             val target = ctx.model.expectShape(it.target)
                             target.isStreaming || it.hasTrait<ClientOptionalTrait>()