Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix client operation name collisions with the standard library prelude #2696

Merged
merged 9 commits into from
May 12, 2023
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,9 @@ message = "Avoid extending IMDS credentials' expiry unconditionally, which may i
references = ["smithy-rs#2687", "smithy-rs#2694"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "ysaito1001"

[[smithy-rs]]
message = "Fix compiler errors in generated code when naming shapes after types in the Rust standard library prelude."
references = ["smithy-rs#2696"]
meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client"}
author = "jdisanti"
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ private fun renderCustomizableOperationSendMethod(
val combinedGenerics = operationGenerics + handleGenerics

val codegenScope = arrayOf(
*RuntimeType.preludeScope,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should just merge this scope directly in rustTemplate and friends.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely thought about it. That and deleting rust() and renaming rustTemplate() to rust(). I'd like to get rid of the old style #T formatters entirely to tidy things up. All of this is quite a bit of work, and I wanted to get this initial fix out first.

"combined_generics_decl" to combinedGenerics.declaration(),
"handle_generics_bounds" to handleGenerics.bounds(),
"SdkSuccess" to RuntimeType.sdkSuccess(runtimeConfig),
Expand All @@ -238,11 +239,11 @@ private fun renderCustomizableOperationSendMethod(
#{handle_generics_bounds:W}
{
/// Sends this operation's request
pub async fn send<T, E>(self) -> Result<T, SdkError<E>>
pub async fn send<T, E>(self) -> #{Result}<T, #{SdkError}<E>>
where
E: std::error::Error + Send + Sync + 'static,
O: #{ParseHttpResponse}<Output = Result<T, E>> + Send + Sync + Clone + 'static,
Retry: #{ClassifyRetry}<#{SdkSuccess}<T>, #{SdkError}<E>> + Send + Sync + Clone,
E: std::error::Error + #{Send} + #{Sync} + 'static,
O: #{ParseHttpResponse}<Output = #{Result}<T, E>> + #{Send} + #{Sync} + #{Clone} + 'static,
Retry: #{ClassifyRetry}<#{SdkSuccess}<T>, #{SdkError}<E>> + #{Send} + #{Sync} + #{Clone},
{
self.handle.client.call(self.operation).await
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ class AwsPresignedFluentBuilderMethod(
) : FluentClientCustomization() {
private val codegenScope = (
presigningTypes + arrayOf(
*RuntimeType.preludeScope,
"Error" to AwsRuntimeType.presigning().resolve("config::Error"),
"SdkError" to RuntimeType.sdkError(runtimeConfig),
)
Expand All @@ -264,7 +265,7 @@ class AwsPresignedFluentBuilderMethod(
pub async fn presigned(
self,
presigning_config: #{PresigningConfig},
) -> Result<#{PresignedRequest}, #{SdkError}<#{OpError}>>
) -> #{Result}<#{PresignedRequest}, #{SdkError}<#{OpError}>>
""",
*codegenScope,
"OpError" to section.operationErrorType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ class FilterEndpointTests(
class S3ProtocolOverride(codegenContext: CodegenContext) : RestXml(codegenContext) {
private val runtimeConfig = codegenContext.runtimeConfig
private val errorScope = arrayOf(
*RuntimeType.preludeScope,
"Bytes" to RuntimeType.Bytes,
"ErrorMetadata" to RuntimeType.errorMetadata(runtimeConfig),
"ErrorBuilder" to RuntimeType.errorMetadataBuilder(runtimeConfig),
Expand All @@ -143,7 +144,7 @@ class S3ProtocolOverride(codegenContext: CodegenContext) : RestXml(codegenContex
override fun parseHttpErrorMetadata(operationShape: OperationShape): RuntimeType {
return ProtocolFunctions.crossOperationFn("parse_http_error_metadata") { fnName ->
rustBlockTemplate(
"pub fn $fnName(response_status: u16, _response_headers: &#{HeaderMap}, response_body: &[u8]) -> Result<#{ErrorBuilder}, #{XmlDecodeError}>",
"pub fn $fnName(response_status: u16, _response_headers: &#{HeaderMap}, response_body: &[u8]) -> #{Result}<#{ErrorBuilder}, #{XmlDecodeError}>",
*errorScope,
) {
rustTemplate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.ModuleDocProvider
import software.amazon.smithy.rust.codegen.core.smithy.ModuleProvider
import software.amazon.smithy.rust.codegen.core.smithy.ModuleProviderContext
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.core.smithy.contextName
import software.amazon.smithy.rust.codegen.core.smithy.module
import software.amazon.smithy.rust.codegen.core.smithy.traits.SyntheticInputTrait
Expand Down Expand Up @@ -122,7 +123,7 @@ class ClientModuleDocProvider(
operation call. For example, this can be used to add an additional HTTP header:

```ignore
## async fn wrapper() -> Result<(), $moduleUseName::Error> {
## async fn wrapper() -> #{Result}<(), $moduleUseName::Error> {
## let client: $moduleUseName::Client = unimplemented!();
use #{http}::header::{HeaderName, HeaderValue};

Expand All @@ -142,6 +143,7 @@ class ClientModuleDocProvider(
## }
```
""".trimIndent(),
*RuntimeType.preludeScope,
"http" to CargoDependency.Http.toDevDependency().toType(),
)
}
Expand Down Expand Up @@ -194,6 +196,10 @@ object ClientModuleProvider : ModuleProvider {
operationModuleName,
parent = ClientRustModule.Operation,
documentationOverride = "Types for the `$contextName` operation.",
// TODO(https://github.com/tokio-rs/tokio/issues/5683): Uncomment the NoImplicitPrelude attribute once this Tokio issue is resolved
// // Disable the Rust prelude since every prelude type should be referenced with its
// // fully qualified name to avoid name collisions with the generated operation shapes.
// additionalAttributes = listOf(Attribute.NoImplicitPrelude)
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ package software.amazon.smithy.rust.codegen.client.smithy.customizations
import software.amazon.smithy.model.shapes.OperationShape
import software.amazon.smithy.model.traits.IdempotencyTokenTrait
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
import software.amazon.smithy.rust.codegen.core.rustlang.rust
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope
import software.amazon.smithy.rust.codegen.core.smithy.customize.OperationCustomization
import software.amazon.smithy.rust.codegen.core.smithy.customize.OperationSection
import software.amazon.smithy.rust.codegen.core.util.findMemberWithTrait
Expand All @@ -28,12 +29,13 @@ class IdempotencyTokenGenerator(codegenContext: CodegenContext, operationShape:
val memberName = symbolProvider.toMemberName(idempotencyTokenMember)
return when (section) {
is OperationSection.MutateInput -> writable {
rust(
rustTemplate(
"""
if ${section.input}.$memberName.is_none() {
${section.input}.$memberName = Some(${section.config}.make_token.make_idempotency_token());
${section.input}.$memberName = #{Some}(${section.config}.make_token.make_idempotency_token());
}
""",
*preludeScope,
)
}
else -> emptySection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.Writable
import software.amazon.smithy.rust.codegen.core.rustlang.rust
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
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.smithy.customize.OperationCustomization
import software.amazon.smithy.rust.codegen.core.smithy.customize.OperationSection
Expand Down Expand Up @@ -166,6 +167,7 @@ class EndpointsDecorator : ClientCodegenDecorator {

override fun section(section: OperationSection): Writable {
val codegenScope = arrayOf(
*RuntimeType.preludeScope,
"Params" to typesGenerator.paramsStruct(),
"ResolveEndpointError" to types.resolveEndpointError,
)
Expand All @@ -174,10 +176,10 @@ class EndpointsDecorator : ClientCodegenDecorator {
rustTemplate(
"""
let params_result = #{Params}::builder()#{builderFields:W}.build()
.map_err(|err|#{ResolveEndpointError}::from_source("could not construct endpoint parameters", err));
.map_err(|err| #{ResolveEndpointError}::from_source("could not construct endpoint parameters", err));
let (endpoint_result, params) = match params_result {
Ok(params) => (${section.config}.endpoint_resolver.resolve_endpoint(&params), Some(params)),
Err(e) => (Err(e), None)
#{Ok}(params) => (${section.config}.endpoint_resolver.resolve_endpoint(&params), #{Some}(params)),
#{Err}(e) => (#{Err}(e), #{None})
};
""",
"builderFields" to builderFields(typesGenerator.params, section),
Expand All @@ -188,7 +190,7 @@ class EndpointsDecorator : ClientCodegenDecorator {
is OperationSection.MutateRequest -> writable {
// insert the endpoint the bag
rustTemplate("${section.request}.properties_mut().insert(endpoint_result);")
rustTemplate("""if let Some(params) = params { ${section.request}.properties_mut().insert(params); }""")
rustTemplate("""if let #{Some}(params) = params { ${section.request}.properties_mut().insert(params); }""", *codegenScope)
}

else -> emptySection
Expand All @@ -199,8 +201,8 @@ class EndpointsDecorator : ClientCodegenDecorator {
val node = this
return writable {
when (node) {
is StringNode -> rust("Some(${node.value.dq()}.to_string())")
is BooleanNode -> rust("Some(${node.value})")
is StringNode -> rustTemplate("#{Some}(${node.value.dq()}.to_string())", *RuntimeType.preludeScope)
is BooleanNode -> rustTemplate("#{Some}(${node.value})", *RuntimeType.preludeScope)
else -> PANIC("unsupported default value: $node")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope
import software.amazon.smithy.rust.codegen.core.smithy.generators.EnumGenerator
import software.amazon.smithy.rust.codegen.core.smithy.generators.EnumGeneratorContext
import software.amazon.smithy.rust.codegen.core.smithy.generators.EnumMemberModel
Expand Down Expand Up @@ -60,16 +61,17 @@ data class InfallibleEnumType(
}

override fun implFromStr(context: EnumGeneratorContext): Writable = writable {
rust(
rustTemplate(
"""
impl std::str::FromStr for ${context.enumName} {
type Err = std::convert::Infallible;
impl ::std::str::FromStr for ${context.enumName} {
type Err = ::std::convert::Infallible;

fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
Ok(${context.enumName}::from(s))
fn from_str(s: &str) -> #{Result}<Self, <Self as ::std::str::FromStr>::Err> {
#{Ok}(${context.enumName}::from(s))
}
}
""",
*preludeScope,
)
}

Expand Down Expand Up @@ -98,7 +100,7 @@ data class InfallibleEnumType(
""".trimIndent(),
)
context.enumMeta.render(this)
rust("struct $UnknownVariantValue(pub(crate) String);")
rustTemplate("struct $UnknownVariantValue(pub(crate) #{String});", *preludeScope)
rustBlock("impl $UnknownVariantValue") {
// The generated as_str is not pub as we need to prevent users from calling it on this opaque struct.
rustBlock("pub(crate) fn as_str(&self) -> &str") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope
import software.amazon.smithy.rust.codegen.core.smithy.isOptional
import software.amazon.smithy.rust.codegen.core.smithy.makeOptional
import software.amazon.smithy.rust.codegen.core.smithy.mapRustType
Expand Down Expand Up @@ -72,17 +73,18 @@ class NestedAccessorGenerator(private val codegenContext: CodegenContext) {
""
}
if (path.isEmpty()) {
rust("Some(input)")
rustTemplate("#{Some}(input)", *preludeScope)
} else {
val head = path.first()
if (symbolProvider.toSymbol(head).isOptional()) {
rust(
rustTemplate(
"""
let input = match ${ref}input.${symbolProvider.toMemberName(head)} {
None => return None,
Some(t) => t
#{None} => return #{None},
#{Some}(t) => t
};
""",
*preludeScope,
)
} else {
rust("let input = input.${symbolProvider.toMemberName(head)};")
Expand Down
Loading