From 17e78b68c70e6e18266d7715955931343c7e39d4 Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Mon, 14 Aug 2023 12:17:39 -0400 Subject: [PATCH] Fix generated summary docs for convenience HashMap / Vec members (#2914) Previously the docs said `Vec` when the method actually accepted `String`, similarly for HashMap. ## Motivation and Context - https://github.com/awslabs/aws-sdk-rust/issues/825 ## Description Fix the generator to be aware of Vec/Hashmap methods ## Testing - UT - [x] audit codegen diff ## Checklist - [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- CHANGELOG.next.toml | 6 ++++ .../client/FluentClientGenerator.kt | 13 +++++-- .../client/FluentClientGeneratorTest.kt | 34 +++++++++++++++++++ .../rust/codegen/core/rustlang/RustType.kt | 2 +- .../rust/codegen/core/smithy/RuntimeType.kt | 31 ++++++++++++++--- 5 files changed, 78 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 8cf6a7fb2a..e0ea56e68b 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -23,6 +23,12 @@ 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" + [[aws-sdk-rust]] message = "Fix requests to S3 with `no_credentials` set." references = ["smithy-rs#2907", "aws-sdk-rust#864"] diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt index d7fb4560b5..e581774fbf 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt +++ b/codegen-client/src/main/kotlin/software/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() + // 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().asArgumentType(fullyQualified = false)})" + return "$memberName($args)" } /** diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGeneratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGeneratorTest.kt index 1e02493212..abda9bf633 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGeneratorTest.kt +++ b/codegen-client/src/test/kotlin/software/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)", + "doubleListValue" to "double_list_value(Vec)", + "mapValue" to "map_value(impl Into, Vec)", + "byteValue" to "byte_value(i8)", + ) + expectations.forEach { (name, expect) -> + val member = model.lookup("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 index 798218ab96..9b7f9cbc1d 100644 --- 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` */ fun RustType.implInto(fullyQualified: Boolean = true): String { - return "impl ${RuntimeType.Into.fullyQualifiedName()}<${this.render(fullyQualified)}>" + return "impl ${RuntimeType.Into.render(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 index e24edc7f86..2341c36e5c 100644 --- 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")