From b308867c196a4f3cb2da7eb0195a60699bcf1b03 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 14 Nov 2023 13:46:55 -0800 Subject: [PATCH] Incorporate feedback --- .../smithy/generators/ClientEnumGenerator.kt | 3 +- .../error/OperationErrorGenerator.kt | 23 ++++++++++---- .../generators/error/ServiceErrorGenerator.kt | 31 ++++++++++++++----- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt index 2472735b8a..7d3d795175 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt @@ -83,8 +83,7 @@ data class InfallibleEnumType( override fun additionalEnumMembers(context: EnumGeneratorContext): Writable = writable { docs("`$UnknownVariant` contains new variants that have been added since this code was generated.") rust( - """##[deprecated(note = "Don't directly match on `$UnknownVariant`. Instead, match on `_` and use \ - the `${context.enumName}::as_str()` method to retrieve information about the unknown enum value.")]""", + """##[deprecated(note = "Don't directly match on `$UnknownVariant`. See the docs on this enum for the correct way to handle unknown variants.")]""", ) rust("$UnknownVariant(#T)", unknownVariantValue(context)) } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt index dc20c9523c..6358d267e3 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt @@ -82,14 +82,14 @@ class OperationErrorGenerator( val errorVariantSymbol = symbolProvider.toSymbol(errorVariant) write("${errorVariantSymbol.name}(#T),", errorVariantSymbol) } - rust( + rustTemplate( """ /// An unexpected error occurred (e.g., invalid JSON returned by the service or an unknown error code). - ##[deprecated(note = "Don't directly match on `Unhandled`. Instead, match on `_` and use \ - the `ProvideErrorMetadata` trait to retrieve information about the unhandled error.")] - Unhandled(#T), + #{deprecation} + Unhandled(#{Unhandled}), """, - unhandledError(runtimeConfig), + "deprecation" to writable { renderUnhandledErrorDeprecation() }, + "Unhandled" to unhandledError(runtimeConfig), ) } @@ -128,7 +128,18 @@ class OperationErrorGenerator( rustBlock("fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result") { delegateToVariants(errors) { variantMatch -> when (variantMatch) { - is VariantMatch.Unhandled -> writable { rust("f.write_str(\"unhandled error\")") } + is VariantMatch.Unhandled -> writable { + rustTemplate( + """ + if let Some(code) = #{ProvideErrorMetadata}::code(self) { + write!(f, "unhandled error ({code})") + } else { + f.write_str("unhandled error") + } + """, + "ProvideErrorMetadata" to RuntimeType.provideErrorMetadataTrait(runtimeConfig), + ) + } is VariantMatch.Modeled -> writable { rust("_inner.fmt(f)") } } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt index 4b6501fa8b..239363c2ce 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt @@ -108,7 +108,16 @@ class ServiceErrorGenerator( allErrors.forEach { rust("Error::${symbolProvider.toSymbol(it).name}(inner) => inner.fmt(f),") } - rust("Error::Unhandled(_inner) => f.write_str(\"unhandled error\")") + rustTemplate( + """ + Error::Unhandled(_) => if let Some(code) = #{ProvideErrorMetadata}::code(self) { + write!(f, "unhandled error ({code})") + } else { + f.write_str("unhandled error") + } + """, + "ProvideErrorMetadata" to RuntimeType.provideErrorMetadataTrait(codegenContext.runtimeConfig), + ) } } } @@ -222,10 +231,7 @@ class ServiceErrorGenerator( rust("${sym.name}(#T),", sym) } docs("An unexpected error occurred (e.g., invalid JSON returned by the service or an unknown error code).") - rust( - "##[deprecated(note = \"Don't directly match on `Unhandled`. Instead, match on `_` and use " + - "the `ProvideErrorMetadata` trait to retrieve information about the unhandled error.\")]", - ) + renderUnhandledErrorDeprecation() rust("Unhandled(#T)", unhandledError(codegenContext.runtimeConfig)) } } @@ -246,8 +252,7 @@ fun unhandledError(rc: RuntimeConfig): RuntimeType = RuntimeType.forInlineFun( /// on the error type. /// /// This struct intentionally doesn't yield any useful information itself. - ##[deprecated(note = "Don't directly match on `Unhandled`. Instead, match on `_` and use \ - the `ProvideErrorMetadata` trait to retrieve information about the unhandled error.")] + #{deprecation} ##[derive(Debug)] pub struct Unhandled { pub(crate) source: #{BoxError}, @@ -255,7 +260,19 @@ fun unhandledError(rc: RuntimeConfig): RuntimeType = RuntimeType.forInlineFun( } """, "BoxError" to RuntimeType.smithyRuntimeApi(rc).resolve("box_error::BoxError"), + "deprecation" to writable { renderUnhandledErrorDeprecation() }, "ErrorMetadata" to RuntimeType.smithyTypes(rc).resolve("error::metadata::ErrorMetadata"), "ProvideErrorMetadata" to RuntimeType.smithyTypes(rc).resolve("error::metadata::ProvideErrorMetadata"), ) } + +fun RustWriter.renderUnhandledErrorDeprecation() { + val message = """ + Don't directly match on `Unhandled`. Instead, match using the catch all matcher (e.g., `_`), + give it a name like `err`, and use the `ProvideErrorMetadata` trait to retrieve the error + code and message. For example: `err => println!("code: {}, message: {}", err.code(), err.message())` + """.trimIndent() + // `.dq()` doesn't quite do what we want here since we actually want a Rust multi-line string + val messageEscaped = message.replace("\"", "\\\"").replace("\n", " \\\n") + rust("""##[deprecated(note = "$messageEscaped")]""") +}