From cd61a8e9a5056ba714629d60c49d897433784ae4 Mon Sep 17 00:00:00 2001 From: Saito Date: Wed, 2 Nov 2022 17:12:01 -0500 Subject: [PATCH 01/21] Make enum forward-compatible This commit implements the suggested approach described in https://github.com/awslabs/smithy-rs/issues/627#issuecomment-995923092 The idea is that once the user writes a match expression against an enum and assumes that an execution path comes to a particular match arm, we should guarantee that when the user upgrades a version of SDK, the execution path should come to the same match arm as before. --- .../core/smithy/SymbolMetadataProvider.kt | 8 ++++-- .../core/smithy/generators/EnumGenerator.kt | 26 ++++++++++++++++--- .../smithy/generators/EnumGeneratorTest.kt | 14 ++++++---- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt index 8c5a56b577..d76cfb1353 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt @@ -57,6 +57,7 @@ abstract class SymbolMetadataProvider(private val base: RustSymbolProvider) : Wr is StringShape -> if (shape.hasTrait()) { enumMeta(shape) } else null + else -> null } return baseSymbol.toBuilder().meta(meta).build() @@ -100,11 +101,13 @@ class BaseSymbolMetadataProvider( ) } } + container.isUnionShape || container.isListShape || container.isSetShape || container.isMapShape -> RustMetadata(visibility = Visibility.PUBLIC) + else -> TODO("Unrecognized container type: $container") } } @@ -120,9 +123,10 @@ class BaseSymbolMetadataProvider( override fun enumMeta(stringShape: StringShape): RustMetadata { return containerDefault.withDerives( RuntimeType.std.member("hash::Hash"), - ).withDerives( // enums can be eq because they can only contain strings + ).withDerives( + // enums can be eq because the inner data also implements Eq RuntimeType.std.member("cmp::Eq"), - // enums can be Ord because they can only contain strings + // enums can be Ord because the inner data also implements Ord RuntimeType.std.member("cmp::PartialOrd"), RuntimeType.std.member("cmp::Ord"), ) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt index e735ec5e11..4db0b8ca7b 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt @@ -99,6 +99,9 @@ open class EnumGenerator( /** Name of the generated unknown enum member name for enums with named members. */ const val UnknownVariant = "Unknown" + /** Name of the opaque struct that is inner data for the generated [UnknownVariant]. */ + const val UnknownVariantValue = "UnknownVariantValue" + /** Name of the function on the enum impl to get a vec of value names */ const val Values = "values" } @@ -108,6 +111,10 @@ open class EnumGenerator( // pub enum Blah { V1, V2, .. } renderEnum() writer.insertTrailingNewline() + if (target == CodegenTarget.CLIENT) { + renderUnknownVariantValue() + } + writer.insertTrailingNewline() // impl From for Blah { ... } renderFromForStr() // impl FromStr for Blah { ... } @@ -168,8 +175,8 @@ open class EnumGenerator( writer.rustBlock("enum $enumName") { sortedMembers.forEach { member -> member.render(writer) } if (target == CodegenTarget.CLIENT) { - docs("$UnknownVariant contains new variants that have been added since this code was generated.") - rust("$UnknownVariant(String)") + docs("`$UnknownVariant` contains new variants that have been added since this code was generated.") + rust("$UnknownVariant($UnknownVariantValue)") } } } @@ -183,7 +190,7 @@ open class EnumGenerator( rust("""$enumName::${member.derivedName()} => ${member.value.dq()},""") } if (target == CodegenTarget.CLIENT) { - rust("$enumName::$UnknownVariant(s) => s.as_ref()") + rust("$enumName::$UnknownVariant(value) => value.as_str()") } } } @@ -198,6 +205,17 @@ open class EnumGenerator( } } + private fun renderUnknownVariantValue() { + meta.render(writer) + writer.write("struct $UnknownVariantValue(String);") + writer.rustBlock("impl $UnknownVariantValue") { + // The generated as_str is not pub as we need to prevent users from calling it on this opaque struct. + rustBlock("fn as_str(&self) -> &str") { + rust("&self.0") + } + } + } + protected open fun renderFromForStr() { writer.rustBlock("impl #T<&str> for $enumName", RuntimeType.From) { rustBlock("fn from(s: &str) -> Self") { @@ -205,7 +223,7 @@ open class EnumGenerator( sortedMembers.forEach { member -> rust("""${member.value.dq()} => $enumName::${member.derivedName()},""") } - rust("other => $enumName::$UnknownVariant(other.to_owned())") + rust("other => $enumName::$UnknownVariant($UnknownVariantValue(other.to_owned()))") } } } diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt index 90679f9773..d5858fcd52 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt @@ -117,7 +117,7 @@ class EnumGeneratorTest { let instance = InstanceType::T2Micro; assert_eq!(instance.as_str(), "t2.micro"); assert_eq!(InstanceType::from("t2.nano"), InstanceType::T2Nano); - assert_eq!(InstanceType::from("other"), InstanceType::Unknown("other".to_owned())); + assert_eq!(InstanceType::from("other"), InstanceType::Unknown(UnknownVariantValue("other".to_owned()))); // round trip unknown variants: assert_eq!(InstanceType::from("other").as_str(), "other"); """, @@ -250,7 +250,7 @@ class EnumGeneratorTest { """ assert_eq!(SomeEnum::from("Unknown"), SomeEnum::UnknownValue); assert_eq!(SomeEnum::from("UnknownValue"), SomeEnum::UnknownValue_); - assert_eq!(SomeEnum::from("SomethingNew"), SomeEnum::Unknown("SomethingNew".into())); + assert_eq!(SomeEnum::from("SomethingNew"), SomeEnum::Unknown(UnknownVariantValue("SomethingNew".to_owned()))); """, ) } @@ -271,7 +271,9 @@ class EnumGeneratorTest { val shape: StringShape = model.lookup("test#SomeEnum") val trait = shape.expectTrait() val provider = testSymbolProvider(model) - val rendered = RustWriter.forModule("model").also { EnumGenerator(model, provider, it, shape, trait).render() }.toString() + val rendered = + RustWriter.forModule("model").also { EnumGenerator(model, provider, it, shape, trait).render() } + .toString() rendered shouldContain """ @@ -297,7 +299,9 @@ class EnumGeneratorTest { val shape: StringShape = model.lookup("test#SomeEnum") val trait = shape.expectTrait() val provider = testSymbolProvider(model) - val rendered = RustWriter.forModule("model").also { EnumGenerator(model, provider, it, shape, trait).render() }.toString() + val rendered = + RustWriter.forModule("model").also { EnumGenerator(model, provider, it, shape, trait).render() } + .toString() rendered shouldContain """ @@ -326,7 +330,7 @@ class EnumGeneratorTest { writer.compileAndTest( """ assert_eq!(SomeEnum::from("other"), SomeEnum::SelfValue); - assert_eq!(SomeEnum::from("SomethingNew"), SomeEnum::Unknown("SomethingNew".into())); + assert_eq!(SomeEnum::from("SomethingNew"), SomeEnum::Unknown(UnknownVariantValue("SomethingNew".to_owned()))); """, ) } From 120be37851f6e93ce4b3de3c5bcb1ccf612e3f06 Mon Sep 17 00:00:00 2001 From: Saito Date: Wed, 2 Nov 2022 21:51:44 -0500 Subject: [PATCH 02/21] Add unit test to ensure enums are forward-compatible The test first mimics the user's interaction with the enum generated from a model where the user writes a match expression on the enum, wishing to hit the match arm corresponding to Variant3, which is not yet supported by the model. The test then simulates a scenario where the user now has access to the updated enum generated from the next version of the model that does support Variant3. The user's code should compile in both of the use cases. --- .../smithy/generators/EnumGeneratorTest.kt | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt index d5858fcd52..c320d19d74 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt @@ -9,6 +9,7 @@ import io.kotest.matchers.shouldBe import io.kotest.matchers.string.shouldContain import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test +import software.amazon.smithy.model.Model import software.amazon.smithy.model.shapes.StringShape import software.amazon.smithy.model.traits.EnumTrait import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter @@ -334,4 +335,50 @@ class EnumGeneratorTest { """, ) } + + @Test + fun `matching on enum should be forward-compatible`() { + fun expectMatchExpressionCompiles(model: Model, shapeId: String, enumToMatchOn: String) { + val shape: StringShape = model.lookup(shapeId) + val trait = shape.expectTrait() + val provider = testSymbolProvider(model) + val writer = RustWriter.forModule("model") + EnumGenerator(model, provider, writer, shape, trait).render() + + val matchExpressionUnderTest = """ + match $enumToMatchOn { + SomeEnum::Variant1 => assert!(false, "expected `Variant3` but got `Variant1`"), + SomeEnum::Variant2 => assert!(false, "expected `Variant3` but got `Variant2`"), + other @ _ if other.as_str() == "Variant3" => assert!(true), + _ => assert!(false, "expected `Variant3` but got `_`"), + } + """ + writer.compileAndTest(matchExpressionUnderTest) + } + + val modelV1 = """ + namespace test + + @enum([ + { name: "Variant1", value: "Variant1" }, + { name: "Variant2", value: "Variant2" }, + ]) + string SomeEnum + """.asSmithyModel() + val variant3AsUnknown = """SomeEnum::Unknown(UnknownVariantValue("Variant3".to_owned()))""" + expectMatchExpressionCompiles(modelV1, "test#SomeEnum", variant3AsUnknown) + + val modelV2 = """ + namespace test + + @enum([ + { name: "Variant1", value: "Variant1" }, + { name: "Variant2", value: "Variant2" }, + { name: "Variant3", value: "Variant3" }, + ]) + string SomeEnum + """.asSmithyModel() + val variant3AsVariant3 = "SomeEnum::Variant3" + expectMatchExpressionCompiles(modelV2, "test#SomeEnum", variant3AsVariant3) + } } From 57a5147221c0ad91296b68484748ce26e11ec7f1 Mon Sep 17 00:00:00 2001 From: Saito Date: Thu, 3 Nov 2022 22:25:25 -0500 Subject: [PATCH 03/21] Generate rustdoc for enum's forward-compatibility This commits generates rustdoc explaining to the users how they might write a match expression against a generated enum so their code is forward-compatible. While it is explained in https://github.com/awslabs/smithy-rs/issues/627#issuecomment-995923092, it can be helpful if the users can read it in rustdoc right below where the enum's definition is shown. --- .../core/smithy/generators/EnumGenerator.kt | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt index 4db0b8ca7b..9bc6a7d11b 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt @@ -171,6 +171,10 @@ open class EnumGenerator( ) writer.deprecatedShape(shape) + if (target == CodegenTarget.CLIENT) { + writer.renderForwardCompatibilityNote(enumName, sortedMembers, UnknownVariant, UnknownVariantValue) + } + meta.render(writer) writer.rustBlock("enum $enumName") { sortedMembers.forEach { member -> member.render(writer) } @@ -243,3 +247,61 @@ open class EnumGenerator( ) } } + +/** + * Generate the rustdoc describing how to write a match expression against a generated enum in a + * forward-compatible way. + */ +private fun RustWriter.renderForwardCompatibilityNote( + enumName: String, sortedMembers: List, + unknownVariant: String, unknownVariantValue: String, +) { + docs( + """ + When writing a match expression against `$enumName`, it is important to ensure + your code is forward-compatible. That is, if a match arm handles a case for a + feature that is supported by the service but has not been represented as an enum + variant in a current version of SDK, your code should continue to work when you + upgrade SDK to a future version in which the enum does include a variant for that + feature. + """.trimIndent(), + ) + docs("") + docs("Here is an example of how you can make a match expression forward-compatible:") + docs("") + docs("```rust,no_run") + rust("/// ## let ${enumName.lowercase()} = unimplemented!();") + rust("/// match ${enumName.lowercase()} {") + sortedMembers.mapNotNull { it.name() }.forEach { member -> + rust("/// $enumName::${member.name} => { /* ... */ },") + } + rust("""/// other @ _ if other.as_str() == "NewFeature" => { /* handles a case for `NewFeature` */ },""") + rust("/// _ => { /* ... */ },") + rust("/// }") + docs("```") + docs( + """ + The above code demonstrates that when `${enumName.lowercase()}` represents + `NewFeature`, the execution path will lead to the second last match arm, + even though the enum does not contain a variant `$enumName::NewFeature` + in the current version of SDK. The reason is that the variable `other`, + created by the `@` operator, is bound to + `$enumName::$unknownVariant($unknownVariantValue("NewFeature".to_owned()))` + and calling `as_str` on it yields `"NewFeature"`. + This match expression is forward-compatible when executed with a newer + version of SDK where the variant `$enumName::NewFeature` is defined. + Specifically, when `${enumName.lowercase()}` represents `NewFeature`, + the execution path will hit the second last match arm as before by virtue of + calling `as_str` on `$enumName::NewFeature` also yielding `"NewFeature"`. + """.trimIndent(), + ) + docs("") + docs( + """ + It is worth pointing out that explicitly matching on the `$unknownVariant` variant should + be avoided for two reasons: + - The inner data `$unknownVariantValue` is opaque, and no further information can be extracted. + - It might inadvertently shadow other intended match arms. + """.trimIndent(), + ) +} From fa86f2d102d970b53f6ada2fe48cd5beff15a805 Mon Sep 17 00:00:00 2001 From: Saito Date: Fri, 4 Nov 2022 12:27:29 -0500 Subject: [PATCH 04/21] Make snippet in rustdoc text This commit updates a rustdoc code snippet generated for an enum from `rust,no_run` to `text`. We observed that the snippet fails to compile in CI because the name of the enum was not fully qualified; code in rustdoc is treated in a similar way that code in integration tests is treated as being outside of a crate, but not part of the crate, which means the name of the crate needs to be spelled out for a `use` statement if we want to import the enum to the code in rustdoc. However, the name of the crate is usually one-off, generated on the fly for testing, making it not obvious to hard-code it or obtain it programmatically from within Kotlin code. --- .../smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt index 9bc6a7d11b..81fc99ea20 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt @@ -269,7 +269,7 @@ private fun RustWriter.renderForwardCompatibilityNote( docs("") docs("Here is an example of how you can make a match expression forward-compatible:") docs("") - docs("```rust,no_run") + docs("```text") rust("/// ## let ${enumName.lowercase()} = unimplemented!();") rust("/// match ${enumName.lowercase()} {") sortedMembers.mapNotNull { it.name() }.forEach { member -> From 6511cf7f66aea239dffe07e3d6b7caff67061eb9 Mon Sep 17 00:00:00 2001 From: Saito Date: Fri, 4 Nov 2022 12:50:38 -0500 Subject: [PATCH 05/21] Suppress missing doc lint for UnknownVariantValue This commit marks UnknownVariantValue as `#[allow(missing_docs)]` because failing to do so will cause tests in CI to fail. --- .../codegen/core/smithy/generators/EnumGenerator.kt | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt index 81fc99ea20..19f2940537 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt @@ -160,6 +160,10 @@ open class EnumGenerator( } private fun renderEnum() { + if (target == CodegenTarget.CLIENT) { + writer.renderForwardCompatibilityNote(enumName, sortedMembers, UnknownVariant, UnknownVariantValue) + } + val renamedWarning = sortedMembers.mapNotNull { it.name() }.filter { it.renamedFrom != null }.joinToString("\n") { val previousName = it.renamedFrom!! @@ -171,10 +175,6 @@ open class EnumGenerator( ) writer.deprecatedShape(shape) - if (target == CodegenTarget.CLIENT) { - writer.renderForwardCompatibilityNote(enumName, sortedMembers, UnknownVariant, UnknownVariantValue) - } - meta.render(writer) writer.rustBlock("enum $enumName") { sortedMembers.forEach { member -> member.render(writer) } @@ -210,6 +210,9 @@ open class EnumGenerator( } private fun renderUnknownVariantValue() { + // No doc or note comes with this inner opaque struct. + // We do want to mark it as #[allow(missing_docs)] to suppress the missing docs lint. + writer.docWithNote(null, null) meta.render(writer) writer.write("struct $UnknownVariantValue(String);") writer.rustBlock("impl $UnknownVariantValue") { From 975fa1d762a4439dba3b2a7e08a75577dbb8f561 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Mon, 7 Nov 2022 22:52:27 -0600 Subject: [PATCH 06/21] Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: Russell Cohen --- .../smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt index 19f2940537..04e0573c6c 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt @@ -301,7 +301,7 @@ private fun RustWriter.renderForwardCompatibilityNote( docs("") docs( """ - It is worth pointing out that explicitly matching on the `$unknownVariant` variant should + Explicitly matching on the `$unknownVariant` variant should be avoided for two reasons: - The inner data `$unknownVariantValue` is opaque, and no further information can be extracted. - It might inadvertently shadow other intended match arms. From 99a645fce04dd99036965b2a538b32d2400db504 Mon Sep 17 00:00:00 2001 From: Saito Date: Mon, 7 Nov 2022 23:25:51 -0600 Subject: [PATCH 07/21] Generate UnknownVariantValue via forInlineFun This commit attempts to create UnknownVariantValue using RuntimeType.forInlineFun. Prior to this commit, it was generated per enum but that caused multiple definitions of UnknownVariantValue in a single file as there can be multiple enums in the file. By using RuntimeType.forInlineFun, we can generate a single UnknownVariantValue per module and the enums within the module can refer to the same instance. This commit, however, fails to pass the unit tests in EnumGeneratorTest. The issue seems to be that a RustWriter used in the tests does not end up calling the finalize method, failing to render inline functions. --- .../core/smithy/generators/EnumGenerator.kt | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt index 19f2940537..5fc3b44be5 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt @@ -12,6 +12,7 @@ import software.amazon.smithy.model.traits.DocumentationTrait import software.amazon.smithy.model.traits.EnumDefinition import software.amazon.smithy.model.traits.EnumTrait import software.amazon.smithy.rust.codegen.core.rustlang.Attribute +import software.amazon.smithy.rust.codegen.core.rustlang.RustModule import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.deprecatedShape import software.amazon.smithy.rust.codegen.core.rustlang.docs @@ -111,10 +112,6 @@ open class EnumGenerator( // pub enum Blah { V1, V2, .. } renderEnum() writer.insertTrailingNewline() - if (target == CodegenTarget.CLIENT) { - renderUnknownVariantValue() - } - writer.insertTrailingNewline() // impl From for Blah { ... } renderFromForStr() // impl FromStr for Blah { ... } @@ -180,7 +177,7 @@ open class EnumGenerator( sortedMembers.forEach { member -> member.render(writer) } if (target == CodegenTarget.CLIENT) { docs("`$UnknownVariant` contains new variants that have been added since this code was generated.") - rust("$UnknownVariant($UnknownVariantValue)") + rust("$UnknownVariant(#T)", unknownVariantValue()) } } } @@ -209,16 +206,16 @@ open class EnumGenerator( } } - private fun renderUnknownVariantValue() { - // No doc or note comes with this inner opaque struct. - // We do want to mark it as #[allow(missing_docs)] to suppress the missing docs lint. - writer.docWithNote(null, null) - meta.render(writer) - writer.write("struct $UnknownVariantValue(String);") - writer.rustBlock("impl $UnknownVariantValue") { - // The generated as_str is not pub as we need to prevent users from calling it on this opaque struct. - rustBlock("fn as_str(&self) -> &str") { - rust("&self.0") + private fun unknownVariantValue(): RuntimeType { + return RuntimeType.forInlineFun(UnknownVariantValue, RustModule.Model) { + rust("##[allow(missing_docs)]") + meta.render(this) + rust("struct $UnknownVariantValue(String);") + rustBlock("impl $UnknownVariantValue") { + // The generated as_str is not pub as we need to prevent users from calling it on this opaque struct. + rustBlock("fn as_str(&self) -> &str") { + rust("&self.0") + } } } } @@ -230,7 +227,7 @@ open class EnumGenerator( sortedMembers.forEach { member -> rust("""${member.value.dq()} => $enumName::${member.derivedName()},""") } - rust("other => $enumName::$UnknownVariant($UnknownVariantValue(other.to_owned()))") + rust("other => $enumName::$UnknownVariant(#T(other.to_owned()))", unknownVariantValue()) } } } From 83a15e0768f67b795f1381246affb2058c300dbc Mon Sep 17 00:00:00 2001 From: Saito Date: Tue, 8 Nov 2022 12:38:19 -0600 Subject: [PATCH 08/21] Replace "target == CodegenTarget.CLIENT" with a helper This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1014390811 but uses an existing helper CodegenTarget.renderUnknownVariant. --- .../rust/codegen/core/smithy/generators/EnumGenerator.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt index 920d4f7424..b777d07dad 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt @@ -157,7 +157,7 @@ open class EnumGenerator( } private fun renderEnum() { - if (target == CodegenTarget.CLIENT) { + if (target.renderUnknownVariant()) { writer.renderForwardCompatibilityNote(enumName, sortedMembers, UnknownVariant, UnknownVariantValue) } @@ -175,7 +175,7 @@ open class EnumGenerator( meta.render(writer) writer.rustBlock("enum $enumName") { sortedMembers.forEach { member -> member.render(writer) } - if (target == CodegenTarget.CLIENT) { + if (target.renderUnknownVariant()) { docs("`$UnknownVariant` contains new variants that have been added since this code was generated.") rust("$UnknownVariant(#T)", unknownVariantValue()) } @@ -190,7 +190,7 @@ open class EnumGenerator( sortedMembers.forEach { member -> rust("""$enumName::${member.derivedName()} => ${member.value.dq()},""") } - if (target == CodegenTarget.CLIENT) { + if (target.renderUnknownVariant()) { rust("$enumName::$UnknownVariant(value) => value.as_str()") } } From 712c983be0172746fd127b0c66f4d7068a6a0b6f Mon Sep 17 00:00:00 2001 From: Saito Date: Tue, 8 Nov 2022 15:39:05 -0600 Subject: [PATCH 09/21] Update EnumGeneratorTests to use TestWorkspace This commit addresses failures for unit tests in EnumGeneratorTests. Now that we use RuntimeType.forInlineFcn in EnumGenerator, we need to ensure that the inline functions should be rendered to a Rust source file during testing. RustWriter, used in EnumGeneratorTests prior to this commit, was not capable of doing so. We thus update EnumGeneratorTests to use TestWorkspace by which we ensure that the inline functions are rendered during testing. --- .../smithy/generators/EnumGeneratorTest.kt | 261 ++++++++++-------- 1 file changed, 153 insertions(+), 108 deletions(-) diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt index c320d19d74..1bd31fbc45 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt @@ -12,11 +12,14 @@ import org.junit.jupiter.api.Test import software.amazon.smithy.model.Model import software.amazon.smithy.model.shapes.StringShape import software.amazon.smithy.model.traits.EnumTrait +import software.amazon.smithy.rust.codegen.core.rustlang.RustModule import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.rust +import software.amazon.smithy.rust.codegen.core.testutil.TestWorkspace import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel import software.amazon.smithy.rust.codegen.core.testutil.compileAndTest import software.amazon.smithy.rust.codegen.core.testutil.testSymbolProvider +import software.amazon.smithy.rust.codegen.core.testutil.unitTest import software.amazon.smithy.rust.codegen.core.util.expectTrait import software.amazon.smithy.rust.codegen.core.util.lookup import software.amazon.smithy.rust.codegen.core.util.orNull @@ -107,28 +110,34 @@ class EnumGeneratorTest { @deprecated(since: "1.2.3") string InstanceType """.asSmithyModel() - val provider = testSymbolProvider(model) - val writer = RustWriter.forModule("model") - writer.rust("##![allow(deprecated)]") + val shape = model.lookup("test#InstanceType") - val generator = EnumGenerator(model, provider, writer, shape, shape.expectTrait()) - generator.render() - writer.compileAndTest( - """ - let instance = InstanceType::T2Micro; - assert_eq!(instance.as_str(), "t2.micro"); - assert_eq!(InstanceType::from("t2.nano"), InstanceType::T2Nano); - assert_eq!(InstanceType::from("other"), InstanceType::Unknown(UnknownVariantValue("other".to_owned()))); - // round trip unknown variants: - assert_eq!(InstanceType::from("other").as_str(), "other"); - """, - ) - val output = writer.toString() - output shouldContain "#[non_exhaustive]" - // on enum variant `T2Micro` - output shouldContain "#[deprecated]" - // on enum itself - output shouldContain "#[deprecated(since = \"1.2.3\")]" + val trait = shape.expectTrait() + val provider = testSymbolProvider(model) + val project = TestWorkspace.testProject(provider) + project.withModule(RustModule.Model) { + rust("##![allow(deprecated)]") + val generator = EnumGenerator(model, provider, this, shape, trait) + generator.render() + unitTest( + "it_generates_named_enums", + """ + let instance = InstanceType::T2Micro; + assert_eq!(instance.as_str(), "t2.micro"); + assert_eq!(InstanceType::from("t2.nano"), InstanceType::T2Nano); + assert_eq!(InstanceType::from("other"), InstanceType::Unknown(UnknownVariantValue("other".to_owned()))); + // round trip unknown variants: + assert_eq!(InstanceType::from("other").as_str(), "other"); + """, + ) + val output = toString() + output.shouldContain("#[non_exhaustive]") + // on enum variant `T2Micro` + output.shouldContain("#[deprecated]") + // on enum itself + output.shouldContain("#[deprecated(since = \"1.2.3\")]") + } + project.compileAndTest() } @Test @@ -146,19 +155,25 @@ class EnumGeneratorTest { }]) string FooEnum """.asSmithyModel() + val shape = model.lookup("test#FooEnum") val trait = shape.expectTrait() - val writer = RustWriter.forModule("model") - val generator = EnumGenerator(model, testSymbolProvider(model), writer, shape, trait) - generator.render() - writer.compileAndTest( - """ - assert_eq!(FooEnum::Foo, FooEnum::Foo); - assert_ne!(FooEnum::Bar, FooEnum::Foo); - let mut hash_of_enums = std::collections::HashSet::new(); - hash_of_enums.insert(FooEnum::Foo); - """.trimIndent(), - ) + val provider = testSymbolProvider(model) + val project = TestWorkspace.testProject(provider) + project.withModule(RustModule.Model) { + val generator = EnumGenerator(model, provider, this, shape, trait) + generator.render() + unitTest( + "named_enums_implement_eq_and_hash", + """ + assert_eq!(FooEnum::Foo, FooEnum::Foo); + assert_ne!(FooEnum::Bar, FooEnum::Foo); + let mut hash_of_enums = std::collections::HashSet::new(); + hash_of_enums.insert(FooEnum::Foo); + """.trimIndent(), + ) + } + project.compileAndTest() } @Test @@ -175,20 +190,26 @@ class EnumGeneratorTest { @deprecated string FooEnum """.asSmithyModel() + val shape = model.lookup("test#FooEnum") val trait = shape.expectTrait() - val writer = RustWriter.forModule("model") - writer.rust("##![allow(deprecated)]") - val generator = EnumGenerator(model, testSymbolProvider(model), writer, shape, trait) - generator.render() - writer.compileAndTest( - """ - assert_eq!(FooEnum::from("Foo"), FooEnum::from("Foo")); - assert_ne!(FooEnum::from("Bar"), FooEnum::from("Foo")); - let mut hash_of_enums = std::collections::HashSet::new(); - hash_of_enums.insert(FooEnum::from("Foo")); - """, - ) + val provider = testSymbolProvider(model) + val project = TestWorkspace.testProject(provider) + project.withModule(RustModule.Model) { + rust("##![allow(deprecated)]") + val generator = EnumGenerator(model, provider, this, shape, trait) + generator.render() + unitTest( + "unnamed_enums_implement_eq_and_hash", + """ + assert_eq!(FooEnum::from("Foo"), FooEnum::from("Foo")); + assert_ne!(FooEnum::from("Bar"), FooEnum::from("Foo")); + let mut hash_of_enums = std::collections::HashSet::new(); + hash_of_enums.insert(FooEnum::from("Foo")); + """.trimIndent(), + ) + } + project.compileAndTest() } @Test @@ -214,19 +235,24 @@ class EnumGeneratorTest { ]) string FooEnum """.asSmithyModel() + val shape = model.lookup("test#FooEnum") val trait = shape.expectTrait() val provider = testSymbolProvider(model) - val writer = RustWriter.forModule("model") - writer.rust("##![allow(deprecated)]") - val generator = EnumGenerator(model, provider, writer, shape, trait) - generator.render() - writer.compileAndTest( - """ - // Values should be sorted - assert_eq!(FooEnum::${EnumGenerator.Values}(), ["0", "1", "Bar", "Baz", "Foo"]); - """, - ) + val project = TestWorkspace.testProject(provider) + project.withModule(RustModule.Model) { + rust("##![allow(deprecated)]") + val generator = EnumGenerator(model, provider, this, shape, trait) + generator.render() + unitTest( + "it_generates_unnamed_enums", + """ + // Values should be sorted + assert_eq!(FooEnum::${EnumGenerator.Values}(), ["0", "1", "Bar", "Baz", "Foo"]); + """.trimIndent(), + ) + } + project.compileAndTest() } @Test @@ -241,19 +267,23 @@ class EnumGeneratorTest { string SomeEnum """.asSmithyModel() - val shape: StringShape = model.lookup("test#SomeEnum") + val shape = model.lookup("test#SomeEnum") val trait = shape.expectTrait() val provider = testSymbolProvider(model) - val writer = RustWriter.forModule("model") - EnumGenerator(model, provider, writer, shape, trait).render() - - writer.compileAndTest( - """ - assert_eq!(SomeEnum::from("Unknown"), SomeEnum::UnknownValue); - assert_eq!(SomeEnum::from("UnknownValue"), SomeEnum::UnknownValue_); - assert_eq!(SomeEnum::from("SomethingNew"), SomeEnum::Unknown(UnknownVariantValue("SomethingNew".to_owned()))); - """, - ) + val project = TestWorkspace.testProject(provider) + project.withModule(RustModule.Model) { + val generator = EnumGenerator(model, provider, this, shape, trait) + generator.render() + unitTest( + "it_escapes_the_unknown_variant_if_the_enum_has_an_unknown_value_in_the_model", + """ + assert_eq!(SomeEnum::from("Unknown"), SomeEnum::UnknownValue); + assert_eq!(SomeEnum::from("UnknownValue"), SomeEnum::UnknownValue_); + assert_eq!(SomeEnum::from("SomethingNew"), SomeEnum::Unknown(UnknownVariantValue("SomethingNew".to_owned()))); + """.trimIndent(), + ) + } + project.compileAndTest() } @Test @@ -269,19 +299,22 @@ class EnumGeneratorTest { string SomeEnum """.asSmithyModel() - val shape: StringShape = model.lookup("test#SomeEnum") + val shape = model.lookup("test#SomeEnum") val trait = shape.expectTrait() val provider = testSymbolProvider(model) - val rendered = - RustWriter.forModule("model").also { EnumGenerator(model, provider, it, shape, trait).render() } - .toString() - - rendered shouldContain - """ - /// Some top-level documentation. - /// - /// _Note: `SomeEnum::Unknown` has been renamed to `::UnknownValue`._ - """.trimIndent() + val project = TestWorkspace.testProject(provider) + project.withModule(RustModule.Model) { + val generator = EnumGenerator(model, provider, this, shape, trait) + generator.render() + val rendered = toString() + rendered shouldContain + """ + /// Some top-level documentation. + /// + /// _Note: `SomeEnum::Unknown` has been renamed to `::UnknownValue`._ + """.trimIndent() + } + project.compileAndTest() } @Test @@ -297,17 +330,20 @@ class EnumGeneratorTest { string SomeEnum """.asSmithyModel() - val shape: StringShape = model.lookup("test#SomeEnum") + val shape = model.lookup("test#SomeEnum") val trait = shape.expectTrait() val provider = testSymbolProvider(model) - val rendered = - RustWriter.forModule("model").also { EnumGenerator(model, provider, it, shape, trait).render() } - .toString() - - rendered shouldContain - """ - /// Some top-level documentation. - """.trimIndent() + val project = TestWorkspace.testProject(provider) + project.withModule(RustModule.Model) { + val generator = EnumGenerator(model, provider, this, shape, trait) + generator.render() + val rendered = toString() + rendered shouldContain + """ + /// Some top-level documentation. + """.trimIndent() + } + project.compileAndTest() } } @@ -322,38 +358,47 @@ class EnumGeneratorTest { string SomeEnum """.asSmithyModel() - val shape: StringShape = model.lookup("test#SomeEnum") + val shape = model.lookup("test#SomeEnum") val trait = shape.expectTrait() val provider = testSymbolProvider(model) - val writer = RustWriter.forModule("model") - EnumGenerator(model, provider, writer, shape, trait).render() - - writer.compileAndTest( - """ - assert_eq!(SomeEnum::from("other"), SomeEnum::SelfValue); - assert_eq!(SomeEnum::from("SomethingNew"), SomeEnum::Unknown(UnknownVariantValue("SomethingNew".to_owned()))); - """, - ) + val project = TestWorkspace.testProject(provider) + project.withModule(RustModule.Model) { + val generator = EnumGenerator(model, provider, this, shape, trait) + generator.render() + unitTest( + "it_handles_variants_that_clash_with_rust_reserved_words", + """ + assert_eq!(SomeEnum::from("other"), SomeEnum::SelfValue); + assert_eq!(SomeEnum::from("SomethingNew"), SomeEnum::Unknown(UnknownVariantValue("SomethingNew".to_owned()))); + """.trimIndent(), + ) + } + project.compileAndTest() } @Test fun `matching on enum should be forward-compatible`() { fun expectMatchExpressionCompiles(model: Model, shapeId: String, enumToMatchOn: String) { - val shape: StringShape = model.lookup(shapeId) + val shape = model.lookup("test#SomeEnum") val trait = shape.expectTrait() val provider = testSymbolProvider(model) - val writer = RustWriter.forModule("model") - EnumGenerator(model, provider, writer, shape, trait).render() - - val matchExpressionUnderTest = """ - match $enumToMatchOn { - SomeEnum::Variant1 => assert!(false, "expected `Variant3` but got `Variant1`"), - SomeEnum::Variant2 => assert!(false, "expected `Variant3` but got `Variant2`"), - other @ _ if other.as_str() == "Variant3" => assert!(true), - _ => assert!(false, "expected `Variant3` but got `_`"), - } - """ - writer.compileAndTest(matchExpressionUnderTest) + val project = TestWorkspace.testProject(provider) + project.withModule(RustModule.Model) { + val generator = EnumGenerator(model, provider, this, shape, trait) + generator.render() + unitTest( + "matching_on_enum_should_be_forward_compatible", + """ + match $enumToMatchOn { + SomeEnum::Variant1 => assert!(false, "expected `Variant3` but got `Variant1`"), + SomeEnum::Variant2 => assert!(false, "expected `Variant3` but got `Variant2`"), + other @ _ if other.as_str() == "Variant3" => assert!(true), + _ => assert!(false, "expected `Variant3` but got `_`"), + } + """.trimIndent(), + ) + } + project.compileAndTest() } val modelV1 = """ From 73bb0deeddebf6ef58861c2a8dda90f1de6f5b2f Mon Sep 17 00:00:00 2001 From: Saito Date: Tue, 8 Nov 2022 15:59:19 -0600 Subject: [PATCH 10/21] Make sure to use the passed-in variable for shapeId This commit fixes a copy-and-paste error introduced in 712c983. The function should use the passed-in variable rather than the hard-coded literal "test#SomeEnum". --- .../rust/codegen/core/smithy/generators/EnumGeneratorTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt index 1bd31fbc45..d0c5b5e6e6 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt @@ -379,7 +379,7 @@ class EnumGeneratorTest { @Test fun `matching on enum should be forward-compatible`() { fun expectMatchExpressionCompiles(model: Model, shapeId: String, enumToMatchOn: String) { - val shape = model.lookup("test#SomeEnum") + val shape = model.lookup(shapeId) val trait = shape.expectTrait() val provider = testSymbolProvider(model) val project = TestWorkspace.testProject(provider) From 2055e04e1def4086e3da54e650efa77f46ad0b92 Mon Sep 17 00:00:00 2001 From: Saito Date: Tue, 8 Nov 2022 16:05:53 -0600 Subject: [PATCH 11/21] Address https://github.com/awslabs/smithy-rs/pull/1945\#discussion_r1014389852 --- .../smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt index d76cfb1353..e8fa8b06e0 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt @@ -124,9 +124,9 @@ class BaseSymbolMetadataProvider( return containerDefault.withDerives( RuntimeType.std.member("hash::Hash"), ).withDerives( - // enums can be eq because the inner data also implements Eq + // enums can be eq because they can only contain ints and strings RuntimeType.std.member("cmp::Eq"), - // enums can be Ord because the inner data also implements Ord + // enums can be Ord because they can only contain ints and strings RuntimeType.std.member("cmp::PartialOrd"), RuntimeType.std.member("cmp::Ord"), ) From 38d371b7ded9fab5540bd54acb3c1143a656112a Mon Sep 17 00:00:00 2001 From: Saito Date: Tue, 8 Nov 2022 16:13:50 -0600 Subject: [PATCH 12/21] Update CHANGELOG.next.toml --- CHANGELOG.next.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 6e34a9093f..351c1c6bbe 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -92,3 +92,8 @@ references = ["smithy-rs#1935"] meta = { "breaking" = true, "tada" = false, "bug" = false } author = "jdisanti" +[[smithy-rs]] +message = "Generate enums that guide the users to write match expressions in a forward-compatible way." +references = ["smithy-rs#1945"] +meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client"} +author = "ysaito1001" From 6e8cbe65a1ec3c9b4f48029eef847b49bc16e7e6 Mon Sep 17 00:00:00 2001 From: Saito Date: Wed, 9 Nov 2022 10:33:56 -0600 Subject: [PATCH 13/21] Update CHANGELOG.next.toml This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1017235315 --- CHANGELOG.next.toml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 351c1c6bbe..24d0388d93 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -97,3 +97,30 @@ message = "Generate enums that guide the users to write match expressions in a f references = ["smithy-rs#1945"] meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client"} author = "ysaito1001" + +[[aws-sdk-rust]] +message = """ +Before this change, users could write a match expression against an enum in a non-forward-compatible way: +```rust +match some_enum { + SomeEnum::Variant1 => { /* ... */ }, + SomeEnum::Variant2 => { /* ... */ }, + Unknown(value) if value == "NewVariant" => { /* ... */ }, + _ => { /* ... */ }, +} +``` +This code can handle a case for "NewVariant" with a version of SDK where the enum does not yet include `SomeEnum::NewVariant`, but breaks with another version of SDK where the enum defines `SomeEnum::NewVariant` because the execution will hit a different match arm, i.e. the last one. +After this change, users are guided to write the above match expression as follows: +```rust +match some_enum { + SomeEnum::Variant1 => { /* ... */ }, + SomeEnum::Variant2 => { /* ... */ }, + other @ _ if other.as_str() == "NewVariant" => { /* ... */ }, + _ => { /* ... */ }, +} +``` +This is forward-compatible because the execution will hit the second last match arm regardless of whether the enum defines `SomeEnum::NewVariant` or not. +""" +references = ["smithy-rs#1945"] +meta = { "breaking" = true, "tada" = false, "bug" = false } +author = "ysaito1001" From 7e14280a241caf069b41476fea870a5dfacdfd93 Mon Sep 17 00:00:00 2001 From: Saito Date: Wed, 9 Nov 2022 16:34:55 -0600 Subject: [PATCH 14/21] Avoid potential name collisions by UnknownVariantValue This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1017237745. It changes the module in which the `UnknownVariantValue` is rendered. Before the commit, it was emitted to the `model` module but that might cause name collisions in the future when a Smithy model has a shape named `UnknownVariantValue`. After this commit, we render it to the `types` module. --- .../smithy/customizations/SmithyTypesPubUseGenerator.kt | 6 ++---- .../smithy/rust/codegen/core/rustlang/RustModule.kt | 4 ++++ .../rust/codegen/core/smithy/generators/EnumGenerator.kt | 6 +++--- .../codegen/core/smithy/generators/EnumGeneratorTest.kt | 8 ++++---- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/SmithyTypesPubUseGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/SmithyTypesPubUseGenerator.kt index f605e3a712..0b17e6702e 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/SmithyTypesPubUseGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/SmithyTypesPubUseGenerator.kt @@ -8,10 +8,9 @@ package software.amazon.smithy.rust.codegen.client.smithy.customizations import software.amazon.smithy.model.Model import software.amazon.smithy.model.shapes.StructureShape import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency +import software.amazon.smithy.rust.codegen.core.rustlang.RustModule import software.amazon.smithy.rust.codegen.core.rustlang.asType -import software.amazon.smithy.rust.codegen.core.rustlang.docs import software.amazon.smithy.rust.codegen.core.rustlang.rust -import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType @@ -76,8 +75,7 @@ class SmithyTypesPubUseGenerator(private val runtimeConfig: RuntimeConfig) : Lib is LibRsSection.Body -> { val types = pubUseTypes(runtimeConfig, section.model) if (types.isNotEmpty()) { - docs("Re-exported types from supporting crates.") - rustBlock("pub mod types") { + withModule(RustModule.Types) { types.forEach { type -> rust("pub use #T;", type) } } } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustModule.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustModule.kt index 4860b627d2..1d739ecb3f 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustModule.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustModule.kt @@ -29,6 +29,10 @@ data class RustModule(val name: String, val rustMetadata: RustMetadata, val docu val Model = public("model", documentation = "Data structures used by operation inputs/outputs.") val Input = public("input", documentation = "Input structures for operations.") val Output = public("output", documentation = "Output structures for operations.") + val Types = public( + "types", + documentation = "Re-exported types from supporting crates and synthesized types from the service.", + ) /** * Helper method to generate the `operation` Rust module. diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt index b777d07dad..c0f1e0fe80 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt @@ -207,13 +207,13 @@ open class EnumGenerator( } private fun unknownVariantValue(): RuntimeType { - return RuntimeType.forInlineFun(UnknownVariantValue, RustModule.Model) { + return RuntimeType.forInlineFun(UnknownVariantValue, RustModule.Types) { rust("##[allow(missing_docs)]") meta.render(this) - rust("struct $UnknownVariantValue(String);") + rust("struct $UnknownVariantValue(pub(crate) String);") rustBlock("impl $UnknownVariantValue") { // The generated as_str is not pub as we need to prevent users from calling it on this opaque struct. - rustBlock("fn as_str(&self) -> &str") { + rustBlock("pub(crate) fn as_str(&self) -> &str") { rust("&self.0") } } diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt index d0c5b5e6e6..963a436d08 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt @@ -125,7 +125,7 @@ class EnumGeneratorTest { let instance = InstanceType::T2Micro; assert_eq!(instance.as_str(), "t2.micro"); assert_eq!(InstanceType::from("t2.nano"), InstanceType::T2Nano); - assert_eq!(InstanceType::from("other"), InstanceType::Unknown(UnknownVariantValue("other".to_owned()))); + assert_eq!(InstanceType::from("other"), InstanceType::Unknown(crate::types::UnknownVariantValue("other".to_owned()))); // round trip unknown variants: assert_eq!(InstanceType::from("other").as_str(), "other"); """, @@ -279,7 +279,7 @@ class EnumGeneratorTest { """ assert_eq!(SomeEnum::from("Unknown"), SomeEnum::UnknownValue); assert_eq!(SomeEnum::from("UnknownValue"), SomeEnum::UnknownValue_); - assert_eq!(SomeEnum::from("SomethingNew"), SomeEnum::Unknown(UnknownVariantValue("SomethingNew".to_owned()))); + assert_eq!(SomeEnum::from("SomethingNew"), SomeEnum::Unknown(crate::types::UnknownVariantValue("SomethingNew".to_owned()))); """.trimIndent(), ) } @@ -369,7 +369,7 @@ class EnumGeneratorTest { "it_handles_variants_that_clash_with_rust_reserved_words", """ assert_eq!(SomeEnum::from("other"), SomeEnum::SelfValue); - assert_eq!(SomeEnum::from("SomethingNew"), SomeEnum::Unknown(UnknownVariantValue("SomethingNew".to_owned()))); + assert_eq!(SomeEnum::from("SomethingNew"), SomeEnum::Unknown(crate::types::UnknownVariantValue("SomethingNew".to_owned()))); """.trimIndent(), ) } @@ -410,7 +410,7 @@ class EnumGeneratorTest { ]) string SomeEnum """.asSmithyModel() - val variant3AsUnknown = """SomeEnum::Unknown(UnknownVariantValue("Variant3".to_owned()))""" + val variant3AsUnknown = """SomeEnum::from("Variant3")""" expectMatchExpressionCompiles(modelV1, "test#SomeEnum", variant3AsUnknown) val modelV2 = """ From 6fd1c5760b0ea9822586e9bd844616cb4582bfe1 Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Thu, 10 Nov 2022 19:17:45 -0600 Subject: [PATCH 15/21] Move re-exports from lib.rs to types.rs This commit moves re-export statements in client crates from `lib.rs` to `types.rs`. The motivation is that now that we render the struct `UnknownVariantValue` in a separate file for the `types` module, we can no longer have a `pub mod types {...}` block in `lib.rs` as it cannot coexist with `pub mod types;`. --- .../codegen/client/smithy/CodegenVisitor.kt | 1 + .../SmithyTypesPubUseGenerator.kt | 27 ++++++------------- .../customize/RequiredCustomizations.kt | 6 +++-- .../ServerRequiredCustomizations.kt | 6 +++-- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenVisitor.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenVisitor.kt index 80026b8659..714f1b06e8 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenVisitor.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenVisitor.kt @@ -88,6 +88,7 @@ class CodegenVisitor( RustModule.Input, RustModule.Output, RustModule.Config, + RustModule.Types, RustModule.operation(Visibility.PUBLIC), ).associateBy { it.name } rustCrate = RustCrate( diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/SmithyTypesPubUseGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/SmithyTypesPubUseGenerator.kt index 0b17e6702e..37ed357376 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/SmithyTypesPubUseGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/SmithyTypesPubUseGenerator.kt @@ -11,11 +11,9 @@ import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency import software.amazon.smithy.rust.codegen.core.rustlang.RustModule import software.amazon.smithy.rust.codegen.core.rustlang.asType import software.amazon.smithy.rust.codegen.core.rustlang.rust -import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType -import software.amazon.smithy.rust.codegen.core.smithy.generators.LibRsCustomization -import software.amazon.smithy.rust.codegen.core.smithy.generators.LibRsSection +import software.amazon.smithy.rust.codegen.core.smithy.RustCrate import software.amazon.smithy.rust.codegen.core.util.hasEventStreamMember import software.amazon.smithy.rust.codegen.core.util.hasStreamingMember @@ -68,21 +66,12 @@ internal fun pubUseTypes(runtimeConfig: RuntimeConfig, model: Model): List pubUseType.shouldExport(model) }.map { it.type } } -class SmithyTypesPubUseGenerator(private val runtimeConfig: RuntimeConfig) : LibRsCustomization() { - override fun section(section: LibRsSection) = - writable { - when (section) { - is LibRsSection.Body -> { - val types = pubUseTypes(runtimeConfig, section.model) - if (types.isNotEmpty()) { - withModule(RustModule.Types) { - types.forEach { type -> rust("pub use #T;", type) } - } - } - } - - else -> { - } - } +/** Adds re-export statements in a separate file for the types module */ +fun pubUseSmithyTypes(runtimeConfig: RuntimeConfig, model: Model, rustCrate: RustCrate) { + rustCrate.withModule(RustModule.Types) { + val types = pubUseTypes(runtimeConfig, model) + if (types.isNotEmpty()) { + types.forEach { type -> rust("pub use #T;", type) } } + } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt index 3ae662bfcd..0567f3a13a 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt @@ -15,10 +15,11 @@ import software.amazon.smithy.rust.codegen.client.smithy.customizations.HttpVers import software.amazon.smithy.rust.codegen.client.smithy.customizations.IdempotencyTokenGenerator import software.amazon.smithy.rust.codegen.client.smithy.customizations.ResiliencyConfigCustomization import software.amazon.smithy.rust.codegen.client.smithy.customizations.ResiliencyReExportCustomization -import software.amazon.smithy.rust.codegen.client.smithy.customizations.SmithyTypesPubUseGenerator +import software.amazon.smithy.rust.codegen.client.smithy.customizations.pubUseSmithyTypes import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ConfigCustomization import software.amazon.smithy.rust.codegen.client.smithy.generators.protocol.ClientProtocolGenerator import software.amazon.smithy.rust.codegen.core.rustlang.Feature +import software.amazon.smithy.rust.codegen.core.rustlang.rust import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext import software.amazon.smithy.rust.codegen.core.smithy.RustCrate import software.amazon.smithy.rust.codegen.core.smithy.customize.OperationCustomization @@ -55,7 +56,6 @@ class RequiredCustomizations : RustCodegenDecorator, ): List = baseCustomizations + CrateVersionGenerator() + - SmithyTypesPubUseGenerator(codegenContext.runtimeConfig) + AllowLintsGenerator() override fun extras(codegenContext: ClientCodegenContext, rustCrate: RustCrate) { @@ -64,6 +64,8 @@ class RequiredCustomizations : RustCodegenDecorator): Boolean = diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/ServerRequiredCustomizations.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/ServerRequiredCustomizations.kt index a8063cf898..9b2e94c87d 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/ServerRequiredCustomizations.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/ServerRequiredCustomizations.kt @@ -7,7 +7,7 @@ package software.amazon.smithy.rust.codegen.server.smithy.customizations import software.amazon.smithy.rust.codegen.client.smithy.customizations.AllowLintsGenerator import software.amazon.smithy.rust.codegen.client.smithy.customizations.CrateVersionGenerator -import software.amazon.smithy.rust.codegen.client.smithy.customizations.SmithyTypesPubUseGenerator +import software.amazon.smithy.rust.codegen.client.smithy.customizations.pubUseSmithyTypes import software.amazon.smithy.rust.codegen.client.smithy.customize.RustCodegenDecorator import software.amazon.smithy.rust.codegen.core.rustlang.Feature import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext @@ -31,11 +31,13 @@ class ServerRequiredCustomizations : RustCodegenDecorator, ): List = - baseCustomizations + CrateVersionGenerator() + SmithyTypesPubUseGenerator(codegenContext.runtimeConfig) + AllowLintsGenerator() + baseCustomizations + CrateVersionGenerator() + AllowLintsGenerator() override fun extras(codegenContext: ServerCodegenContext, rustCrate: RustCrate) { // Add rt-tokio feature for `ByteStream::from_path` rustCrate.mergeFeature(Feature("rt-tokio", true, listOf("aws-smithy-http/rt-tokio"))) + + pubUseSmithyTypes(codegenContext.runtimeConfig, codegenContext.model, rustCrate) } override fun supportsCodegenContext(clazz: Class): Boolean = From 55f8de7fde8bdcf3528ea44de6ea46997532821f Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Thu, 10 Nov 2022 22:33:26 -0600 Subject: [PATCH 16/21] Add docs on UnknownVariantValue This commit adds public docs on `UnknownVariantValue`. Since it is rendered in `types.rs`, the users may not find the `Unknown` variant of an enum in the same file and may wonder what this struct is for when seeing it in isolation. --- .../codegen/core/smithy/generators/EnumGenerator.kt | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt index c0f1e0fe80..ec7e1fb9d6 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt @@ -208,7 +208,15 @@ open class EnumGenerator( private fun unknownVariantValue(): RuntimeType { return RuntimeType.forInlineFun(UnknownVariantValue, RustModule.Types) { - rust("##[allow(missing_docs)]") + docs( + """ + Opaque struct used as inner data for the `Unknown` variant defined in enums in + the crate + + While this is not used by users directly, it is marked as `pub` because it is + part of the enums that are public interface. + """.trimIndent(), + ) meta.render(this) rust("struct $UnknownVariantValue(pub(crate) String);") rustBlock("impl $UnknownVariantValue") { From 04e1a222db8486ec4b37684eb2f2077ca17c7c59 Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Fri, 11 Nov 2022 14:56:58 -0600 Subject: [PATCH 17/21] Update CHANGELOG.next.toml This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1020414459 --- CHANGELOG.next.toml | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 402ae37f22..33bf7301f5 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -105,13 +105,36 @@ meta = { "breaking" = false, "tada" = false, "bug" = false } author = "rcoh" [[smithy-rs]] -message = "Generate enums that guide the users to write match expressions in a forward-compatible way." +message = """ +Generate enums that guide the users to write match expressions in a forward-compatible way. +Before this change, users could write a match expression against an enum in a non-forward-compatible way: +```rust +match some_enum { + SomeEnum::Variant1 => { /* ... */ }, + SomeEnum::Variant2 => { /* ... */ }, + Unknown(value) if value == "NewVariant" => { /* ... */ }, + _ => { /* ... */ }, +} +``` +This code can handle a case for "NewVariant" with a version of SDK where the enum does not yet include `SomeEnum::NewVariant`, but breaks with another version of SDK where the enum defines `SomeEnum::NewVariant` because the execution will hit a different match arm, i.e. the last one. +After this change, users are guided to write the above match expression as follows: +```rust +match some_enum { + SomeEnum::Variant1 => { /* ... */ }, + SomeEnum::Variant2 => { /* ... */ }, + other @ _ if other.as_str() == "NewVariant" => { /* ... */ }, + _ => { /* ... */ }, +} +``` +This is forward-compatible because the execution will hit the second last match arm regardless of whether the enum defines `SomeEnum::NewVariant` or not. +""" references = ["smithy-rs#1945"] meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client"} author = "ysaito1001" [[aws-sdk-rust]] message = """ +Generate enums that guide the users to write match expressions in a forward-compatible way. Before this change, users could write a match expression against an enum in a non-forward-compatible way: ```rust match some_enum { From 5972c11fc243bdcccd3d6a54b4466540f50231ab Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Mon, 14 Nov 2022 11:01:47 -0600 Subject: [PATCH 18/21] Update the module documentation for types This commit updates rustdoc for the types module to reflect that fact that the module now contains not only re-exports but also an auxiliary struct `UnknownVariantValue`. --- .../amazon/smithy/rust/codegen/core/rustlang/RustModule.kt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustModule.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustModule.kt index 1d739ecb3f..aa7a0fb8d3 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustModule.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustModule.kt @@ -29,10 +29,7 @@ data class RustModule(val name: String, val rustMetadata: RustMetadata, val docu val Model = public("model", documentation = "Data structures used by operation inputs/outputs.") val Input = public("input", documentation = "Input structures for operations.") val Output = public("output", documentation = "Output structures for operations.") - val Types = public( - "types", - documentation = "Re-exported types from supporting crates and synthesized types from the service.", - ) + val Types = public("types", documentation = "Data primitives referenced by other data types.") /** * Helper method to generate the `operation` Rust module. From 7d4d43af301145c600d7464817f3eee00d42d9d5 Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Mon, 14 Nov 2022 12:18:59 -0600 Subject: [PATCH 19/21] Add extensions to run code block depending on the target This commit introduces extensions on CodegenTarget that execute the block of code depending on the target is for client or for server. These extensions are intended to replace if-else expressions throughout the codebase explicitly checking whether the target is for client or for server. We do not update such if-else expressions all at once in this commit. Rather, we are planning to make incremental changes to update them opportunistically. --- .../rust/codegen/core/smithy/CodegenTarget.kt | 18 ++++++++++++++++++ .../core/smithy/generators/EnumGenerator.kt | 8 +++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenTarget.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenTarget.kt index 27ea5dcc68..094e6a0805 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenTarget.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenTarget.kt @@ -11,3 +11,21 @@ package software.amazon.smithy.rust.codegen.core.smithy enum class CodegenTarget { CLIENT, SERVER } + +/** + * Convenience extension to execute thunk if the target is for CodegenTarget.CLIENT + */ +fun CodegenTarget.ifClient(thunk: () -> B): B? = if (this == CodegenTarget.CLIENT) { + thunk() +} else { + null +} + +/** + * Convenience extension to execute thunk if the target is for CodegenTarget.SERVER + */ +fun CodegenTarget.ifServer(thunk: () -> B): B? = if (this == CodegenTarget.SERVER) { + thunk() +} else { + null +} diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt index ec7e1fb9d6..5b4d239a81 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt @@ -26,6 +26,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.MaybeRenamed import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata +import software.amazon.smithy.rust.codegen.core.smithy.ifClient import software.amazon.smithy.rust.codegen.core.util.doubleQuote import software.amazon.smithy.rust.codegen.core.util.dq import software.amazon.smithy.rust.codegen.core.util.getTrait @@ -157,7 +158,7 @@ open class EnumGenerator( } private fun renderEnum() { - if (target.renderUnknownVariant()) { + target.ifClient { writer.renderForwardCompatibilityNote(enumName, sortedMembers, UnknownVariant, UnknownVariantValue) } @@ -175,7 +176,7 @@ open class EnumGenerator( meta.render(writer) writer.rustBlock("enum $enumName") { sortedMembers.forEach { member -> member.render(writer) } - if (target.renderUnknownVariant()) { + target.ifClient { docs("`$UnknownVariant` contains new variants that have been added since this code was generated.") rust("$UnknownVariant(#T)", unknownVariantValue()) } @@ -190,7 +191,8 @@ open class EnumGenerator( sortedMembers.forEach { member -> rust("""$enumName::${member.derivedName()} => ${member.value.dq()},""") } - if (target.renderUnknownVariant()) { + + target.ifClient { rust("$enumName::$UnknownVariant(value) => value.as_str()") } } From c56ad074ccb9c250c3b26c9e5f4f8263556518c5 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Mon, 14 Nov 2022 15:40:37 -0600 Subject: [PATCH 20/21] Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: John DiSanti --- .../smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt index 5b4d239a81..fd89ffb7f9 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt @@ -215,7 +215,7 @@ open class EnumGenerator( Opaque struct used as inner data for the `Unknown` variant defined in enums in the crate - While this is not used by users directly, it is marked as `pub` because it is + While this is not intended to be used directly, it is marked as `pub` because it is part of the enums that are public interface. """.trimIndent(), ) From 7b719e10b66146f776529da39bb519be7d0df553 Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Mon, 14 Nov 2022 16:10:11 -0600 Subject: [PATCH 21/21] Move extensions into CodegenTarget as methods This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1021952411. By moving extensions into the CodegenTarget enum, no separate import is required to use them. --- .../rust/codegen/core/smithy/CodegenTarget.kt | 34 +++++++++---------- .../core/smithy/generators/EnumGenerator.kt | 1 - 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenTarget.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenTarget.kt index 094e6a0805..b1dba3e33f 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenTarget.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenTarget.kt @@ -9,23 +9,23 @@ package software.amazon.smithy.rust.codegen.core.smithy * Code generation mode: In some situations, codegen has different behavior for client vs. server (eg. required fields) */ enum class CodegenTarget { - CLIENT, SERVER -} + CLIENT, SERVER; -/** - * Convenience extension to execute thunk if the target is for CodegenTarget.CLIENT - */ -fun CodegenTarget.ifClient(thunk: () -> B): B? = if (this == CodegenTarget.CLIENT) { - thunk() -} else { - null -} + /** + * Convenience method to execute thunk if the target is for CLIENT + */ + fun ifClient(thunk: () -> B): B? = if (this == CLIENT) { + thunk() + } else { + null + } -/** - * Convenience extension to execute thunk if the target is for CodegenTarget.SERVER - */ -fun CodegenTarget.ifServer(thunk: () -> B): B? = if (this == CodegenTarget.SERVER) { - thunk() -} else { - null + /** + * Convenience method to execute thunk if the target is for SERVER + */ + fun ifServer(thunk: () -> B): B? = if (this == SERVER) { + thunk() + } else { + null + } } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt index fd89ffb7f9..e7112915b6 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt @@ -26,7 +26,6 @@ import software.amazon.smithy.rust.codegen.core.smithy.MaybeRenamed import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata -import software.amazon.smithy.rust.codegen.core.smithy.ifClient import software.amazon.smithy.rust.codegen.core.util.doubleQuote import software.amazon.smithy.rust.codegen.core.util.dq import software.amazon.smithy.rust.codegen.core.util.getTrait