Rework enum for forward-compatibility

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"

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<EnumTrait>()) {
                 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 they can only contain ints and strings
             RuntimeType.std.member("cmp::Eq"),
-            // enums can be Ord because they can only contain strings
+            // enums can be Ord because they can only contain ints and strings
             RuntimeType.std.member("cmp::PartialOrd"),
             RuntimeType.std.member("cmp::Ord"),
         )

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
@@ -99,6 +100,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"
     }
@@ -153,6 +157,10 @@ open class EnumGenerator(
     }
 
     private fun renderEnum() {
+        if (target.renderUnknownVariant()) {
+            writer.renderForwardCompatibilityNote(enumName, sortedMembers, UnknownVariant, UnknownVariantValue)
+        }
+
         val renamedWarning =
             sortedMembers.mapNotNull { it.name() }.filter { it.renamedFrom != null }.joinToString("\n") {
                 val previousName = it.renamedFrom!!
@@ -167,9 +175,9 @@ open class EnumGenerator(
         meta.render(writer)
         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)")
+            if (target.renderUnknownVariant()) {
+                docs("`$UnknownVariant` contains new variants that have been added since this code was generated.")
+                rust("$UnknownVariant(#T)", unknownVariantValue())
             }
         }
     }
@@ -182,8 +190,8 @@ open class EnumGenerator(
                     sortedMembers.forEach { member ->
                         rust("""$enumName::${member.derivedName()} => ${member.value.dq()},""")
                     }
-                    if (target == CodegenTarget.CLIENT) {
-                        rust("$enumName::$UnknownVariant(s) => s.as_ref()")
+                    if (target.renderUnknownVariant()) {
+                        rust("$enumName::$UnknownVariant(value) => value.as_str()")
                     }
                 }
             }
@@ -198,14 +206,28 @@ open class EnumGenerator(
         }
     }
 
+    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")
+                }
+            }
+        }
+    }
+
     protected open fun renderFromForStr() {
         writer.rustBlock("impl #T<&str> for $enumName", RuntimeType.From) {
             rustBlock("fn from(s: &str) -> Self") {
                 rustBlock("match s") {
                     sortedMembers.forEach { member ->
                         rust("""${member.value.dq()} => $enumName::${member.derivedName()},""")
                     }
-                    rust("other => $enumName::$UnknownVariant(other.to_owned())")
+                    rust("other => $enumName::$UnknownVariant(#T(other.to_owned()))", unknownVariantValue())
                 }
             }
         }
@@ -225,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<EnumMemberModel>,
+    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("```text")
+    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(
+        """
+        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(),
+    )
+}