Fix generated summary docs for convience HashMap / Vec members

Previously the docs said `Vec<String>` when the method actually accepted `String`, similarly for HashMap.
smithy-lang · Aug 9, 2023 · a24a453 · a24a453
1 parent 61b675c
commit a24a453
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 8 deletions.
diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml
@@ -22,3 +22,9 @@ message = "`RuntimeComponents` and `RuntimeComponentsBuilder` are now re-exporte
 references = ["smithy-rs#2904"]
 meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client"}
 author = "jdisanti"
+
+[[smithy-rs]]
+message = "Fix incorrect summary docs for builders"
+references = ["smithy-rs#2914", "aws-sdk-rust#825"]
+meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" }
+author = "rcoh"
diff --git a/...tware/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt b/...tware/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt
@@ -715,11 +715,18 @@ private fun OperationShape.fullyQualifiedFluentBuilder(
  *
  * _NOTE: This function generates the type names that appear under **"The fluent builder is configurable:"**_
  */
-private fun MemberShape.asFluentBuilderInputDoc(symbolProvider: SymbolProvider): String {
+internal fun MemberShape.asFluentBuilderInputDoc(symbolProvider: SymbolProvider): String {
     val memberName = symbolProvider.toMemberName(this)
-    val outerType = symbolProvider.toSymbol(this).rustType()
+    val outerType = symbolProvider.toSymbol(this).rustType().stripOuter<RustType.Option>()
+    // We generate Vec/HashMap helpers
+    val renderedType = when (outerType) {
+        is RustType.Vec -> listOf(outerType.member)
+        is RustType.HashMap -> listOf(outerType.key, outerType.member)
+        else -> listOf(outerType)
+    }
+    val args = renderedType.joinToString { it.asArgumentType(fullyQualified = false) }
 
-    return "$memberName(${outerType.stripOuter<RustType.Option>().asArgumentType(fullyQualified = false)})"
+    return "$memberName($args)"
 }
 
 /**

diff --git a/...e/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGeneratorTest.kt b/...e/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGeneratorTest.kt
@@ -5,10 +5,13 @@
 
 package software.amazon.smithy.rust.codegen.client.smithy.generators.client
 
+import io.kotest.matchers.shouldBe
 import org.junit.jupiter.api.Test
+import software.amazon.smithy.model.shapes.MemberShape
 import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
 import software.amazon.smithy.rust.codegen.client.testutil.TestCodegenSettings
 import software.amazon.smithy.rust.codegen.client.testutil.clientIntegrationTest
+import software.amazon.smithy.rust.codegen.client.testutil.testSymbolProvider
 import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency
 import software.amazon.smithy.rust.codegen.core.rustlang.rust
 import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
@@ -17,6 +20,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
 import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
 import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
 import software.amazon.smithy.rust.codegen.core.testutil.integrationTest
+import software.amazon.smithy.rust.codegen.core.util.lookup
 
 class FluentClientGeneratorTest {
     val model = """
@@ -34,9 +38,39 @@ class FluentClientGeneratorTest {
         structure TestInput {
            foo: String,
            byteValue: Byte,
+           listValue: StringList,
+           mapValue: ListMap,
+           doubleListValue: DoubleList
+        }
+
+        list StringList {
+            member: String
+        }
+
+        list DoubleList {
+            member: StringList
+        }
+
+        map ListMap {
+            key: String,
+            value: StringList
         }
     """.asSmithyModel()
 
+    @Test
+    fun `generate correct input docs`() {
+        val expectations = mapOf(
+            "listValue" to "list_value(impl Into<String>)",
+            "doubleListValue" to "double_list_value(Vec<String>)",
+            "mapValue" to "map_value(impl Into<String>, Vec<String>)",
+            "byteValue" to "byte_value(i8)",
+        )
+        expectations.forEach { (name, expect) ->
+            val member = model.lookup<MemberShape>("com.example#TestInput\$$name")
+            member.asFluentBuilderInputDoc(testSymbolProvider(model)) shouldBe expect
+        }
+    }
+
     @Test
     fun `send() future implements Send`() {
         val test: (ClientCodegenContext, RustCrate) -> Unit = { codegenContext, rustCrate ->

diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt
@@ -200,7 +200,7 @@ fun RustType.qualifiedName(): String {
 
 /** Format this Rust type as an `impl Into<T>` */
 fun RustType.implInto(fullyQualified: Boolean = true): String {
-    return "impl ${RuntimeType.Into.fullyQualifiedName()}<${this.render(fullyQualified)}>"
+    return "impl ${RuntimeType.Into.fullyQualifiedName(fullyQualified)}<${this.render(fullyQualified)}>"
 }
 
 /** Format this Rust type so that it may be used as an argument type in a function definition */

diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt
@@ -115,7 +115,7 @@ data class RuntimeConfig(
 
 /**
  * `RuntimeType` captures all necessary information to render a type into a Rust file:
- * - [name]: What type is this?
+ * - [fullyQualifiedName]: What type is this?
  * - [namespace]: Where can we find this type.
  * - [dependency]: What other crates, if any, are required to use this type?
  *
@@ -126,7 +126,7 @@ data class RuntimeConfig(
  * `http::header::HeaderName`
  *  ------------  ----------
  *       |            |
- * `[namespace]`  `[name]`
+ * `[namespace]`  `[fullyQualifiedName]`
  *
  *  This type would have a [CargoDependency] pointing to the `http` crate. Writing it multiple times would still only
  *  add the dependency once.
@@ -185,8 +185,14 @@ data class RuntimeType(val path: String, val dependency: RustDependency? = null)
     /**
      * Returns the fully qualified name for this type
      */
-    fun fullyQualifiedName(): String {
-        return path
+    fun fullyQualifiedName(): String = path
+
+    fun render(fullyQualified: Boolean = true): String {
+        return if (fullyQualified) {
+            fullyQualifiedName()
+        } else {
+            name
+        }
     }
 
     /**
@@ -336,41 +342,58 @@ data class RuntimeType(val path: String, val dependency: RustDependency? = null)
 
         fun configBag(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyTypes(runtimeConfig).resolve("config_bag::ConfigBag")
+
         fun runtimeComponents(runtimeConfig: RuntimeConfig) =
             smithyRuntimeApi(runtimeConfig).resolve("client::runtime_components::RuntimeComponents")
+
         fun runtimeComponentsBuilder(runtimeConfig: RuntimeConfig) =
             smithyRuntimeApi(runtimeConfig).resolve("client::runtime_components::RuntimeComponentsBuilder")
+
         fun runtimePlugins(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("client::runtime_plugin::RuntimePlugins")
+
         fun runtimePlugin(runtimeConfig: RuntimeConfig) =
             smithyRuntimeApi(runtimeConfig).resolve("client::runtime_plugin::RuntimePlugin")
+
         fun sharedRuntimePlugin(runtimeConfig: RuntimeConfig) =
             smithyRuntimeApi(runtimeConfig).resolve("client::runtime_plugin::SharedRuntimePlugin")
+
         fun boxError(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("box_error::BoxError")
+
         fun interceptor(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::Interceptor")
+
         fun interceptorContext(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::context::InterceptorContext")
+
         fun sharedInterceptor(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::SharedInterceptor")
 
         fun afterDeserializationInterceptorContextRef(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::context::AfterDeserializationInterceptorContextRef")
+
         fun beforeSerializationInterceptorContextRef(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::context::BeforeSerializationInterceptorContextRef")
+
         fun beforeSerializationInterceptorContextMut(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::context::BeforeSerializationInterceptorContextMut")
+
         fun beforeDeserializationInterceptorContextRef(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::context::BeforeDeserializationInterceptorContextRef")
+
         fun beforeDeserializationInterceptorContextMut(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::context::BeforeDeserializationInterceptorContextMut")
+
         fun beforeTransmitInterceptorContextRef(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::context::BeforeTransmitInterceptorContextRef")
+
         fun beforeTransmitInterceptorContextMut(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::context::BeforeTransmitInterceptorContextMut")
+
         fun finalizerInterceptorContextRef(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::context::FinalizerInterceptorContextRef")
+
         fun finalizerInterceptorContextMut(runtimeConfig: RuntimeConfig): RuntimeType =
             smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::context::FinalizerInterceptorContextMut")