Skip to content

Commit

Permalink
Fix generated summary docs for convenience HashMap / Vec members (#2914)
Browse files Browse the repository at this point in the history
Previously the docs said `Vec<String>` when the method actually accepted
`String`, similarly for HashMap.

## Motivation and Context
- awslabs/aws-sdk-rust#825

## Description
Fix the generator to be aware of Vec/Hashmap methods

## Testing
- UT
- [x] audit codegen diff

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [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._
  • Loading branch information
rcoh authored Aug 14, 2023
1 parent 0286b9f commit 17e78b6
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 8 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 = """
Expand All @@ -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 ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.render(fullyQualified)}<${this.render(fullyQualified)}>"
}

/** Format this Rust type so that it may be used as an argument type in a function definition */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?
*
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
}

/**
Expand Down Expand Up @@ -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")

Expand Down

0 comments on commit 17e78b6

Please sign in to comment.