fix: codegen structure members more safely to avoid conflicts with package names (#1444)

Author: ianbotsf

.changes/69d72d7f-7266-4d90-81d2-29aac4821386.json

@@ -0,0 +1,5 @@
+{
+    "id": "69d72d7f-7266-4d90-81d2-29aac4821386",
+    "type": "bugfix",
+    "description": "fix: code-generate structure members more safely to avoid conflicts with package names"
+}

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

@@ -13,6 +13,7 @@ import software.amazon.smithy.kotlin.codegen.core.withBlock
 import software.amazon.smithy.kotlin.codegen.lang.KotlinTypes
 import software.amazon.smithy.kotlin.codegen.model.*
 import software.amazon.smithy.kotlin.codegen.rendering.serde.ClientErrorCorrection
+import software.amazon.smithy.kotlin.codegen.utils.toCamelCase
 import software.amazon.smithy.model.Model
 import software.amazon.smithy.model.shapes.*
 import software.amazon.smithy.model.traits.*
@@ -32,6 +33,7 @@ class StructureGenerator(
     private val symbol = ctx.symbolProvider.toSymbol(ctx.shape)
 
     fun render() {
+        renderDslBuilderRefs()
         writer.renderDocumentation(shape)
         writer.renderAnnotations(shape)
         if (!shape.isError) {
@@ -42,11 +44,14 @@ class StructureGenerator(
     }
 
     private val sortedMembers: List<MemberShape> = shape.allMembers.values.sortedBy { it.defaultName() }
+
     private val memberNameSymbolIndex: Map<MemberShape, Pair<String, Symbol>> =
         sortedMembers.associateWith { member ->
             Pair(symbolProvider.toMemberName(member), symbolProvider.toSymbol(member))
         }
 
+    private val structMembers = sortedMembers.filter { model.expectShape(it.target).isStructureShape }
+
     /**
      * Renders a normal (non-error) Smithy structure to a Kotlin class
      */
@@ -283,18 +288,13 @@ class StructureGenerator(
                 write("@PublishedApi")
                 write("internal fun build(): #1Q = #1T(this)", symbol)
 
-                val structMembers = sortedMembers.filter { member ->
-                    val targetShape = model.getShape(member.target).get()
-                    targetShape.isStructureShape
-                }
-
                 for (member in structMembers) {
                     writer.write("")
                     val (memberName, memberSymbol) = memberNameSymbolIndex[member]!!
                     writer.dokka("construct an [${memberSymbol.fullName}] inside the given [block]")
                     writer.renderAnnotations(member)
                     openBlock("public fun #L(block: #Q.Builder.() -> #Q) {", memberName, memberSymbol, KotlinTypes.Unit)
-                        .write("this.#L = #Q.invoke(block)", memberName, memberSymbol)
+                        .write("this.#L = #L(block)", memberName, dslBuilderRef(memberSymbol))
                         .closeBlock("}")
                 }
 
@@ -324,6 +324,21 @@ class StructureGenerator(
             }
     }
 
+    /**
+     * Derives a hopefully collision-safe name for a symbol by camel-casing the full name and "DSL Builder Ref". For
+     * instance, the symbol `aws.sdk.kotlin.services.simplefooservice.model.FooBar` turns into
+     * `awsSdkKotlinServicesSimplefooserviceModelFooBarDslBuilderRef`.
+     */
+    private fun dslBuilderRef(symbol: Symbol): String = "${symbol.fullName} DSL Builder Ref".toCamelCase()
+
+    private fun renderDslBuilderRefs() {
+        val dslBuilderSymbols = structMembers.map { memberNameSymbolIndex[it]!!.second }.toSet()
+        dslBuilderSymbols.forEach { symbol ->
+            writer.write("private val #L = #Q", dslBuilderRef(symbol), symbol)
+        }
+        writer.write("")
+    }
+
     /**
      * Renders a Smithy error type to a Kotlin exception type
      */

codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/StructureGeneratorTest.kt

@@ -221,7 +221,7 @@ class StructureGeneratorTest {
                  * construct an [com.test.model.Qux] inside the given [block]
                  */
                 public fun quux(block: com.test.model.Qux.Builder.() -> kotlin.Unit) {
-                    this.quux = com.test.model.Qux.invoke(block)
+                    this.quux = comTestModelQuxDslBuilderRef(block)
                 }
                 
                 internal fun correctErrors(): Builder {
@@ -233,6 +233,11 @@ class StructureGeneratorTest {
         commonTestContents.shouldContainOnlyOnceWithDiff(expected)
     }
 
+    @Test
+    fun `it renders DSL builder references`() {
+        commonTestContents.shouldContainOnlyOnceWithDiff("private val comTestModelQuxDslBuilderRef = com.test.model.Qux")
+    }
+
     @Test
     fun `it renders class docs`() {
         commonTestContents.shouldContainOnlyOnceWithDiff("This *is* documentation about the shape.")

tests/compile/src/test/resources/kitchen-sink-model.smithy

@@ -26,7 +26,8 @@ service Example {
         UnionAggregateInput,
         UnionOutput,
         UnionAggregateOutput,
-        WaiterTest
+        WaiterTest,
+        PackageNameConflictTest,
     ]
 }
 
@@ -536,3 +537,18 @@ structure WaiterTestFoo {
 
 @error("client")
 structure WaiterTestNotFound {}
+
+// Verifies that members with the same name as a top-level package don't cause compilation issues.
+@http(method: "POST", uri: "/input/packageNameConflict")
+operation PackageNameConflictTest {
+    input: PackageNameConflictTestRequest,
+    output: PackageNameConflictTestResponse,
+}
+
+structure PackageNameConflictTestRequest {
+    com: String,
+}
+
+structure PackageNameConflictTestResponse {
+    com: String,
+}