Correctly serialize Strings in XML (#1832)

The logic that keeps track of whether we're working with a borrowed or an owned value is flawed. It just so happened to work when requesting owned values from borrowed values because we called `autoDeref` before creating the value expression. That logic should instead be used _within_ `ValueExpression` to appease Clippy there too. Fixes #1831.
smithy-lang · Oct 13, 2022 · aa60bad · aa60bad
1 parent 7866477
commit aa60bad
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 9 deletions.
diff --git a/codegen-client-test/model/rest-xml-extras.smithy b/codegen-client-test/model/rest-xml-extras.smithy
@@ -20,6 +20,7 @@ service RestXmlExtras {
         ChecksumRequired,
         StringHeader,
         CreateFoo,
+        RequiredMember,
     ]
 }
 
@@ -242,5 +243,17 @@ structure StringHeaderOutput {
     field: String,
 
     @httpHeader("x-enum")
-    enumHeader: StringEnum
+    enumHeader: StringEnum,
+}
+
+/// This operation tests that we can serialize `required` members.
+@http(uri: "/required-member", method: "GET")
+operation RequiredMember {
+    input: RequiredMemberInputOutput
+    output: RequiredMemberInputOutput
+}
+
+structure RequiredMemberInputOutput {
+    @required
+    requiredString: String
 }
diff --git a/...in/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/ValueExpression.kt b/...in/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/ValueExpression.kt
@@ -5,14 +5,16 @@
 
 package software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize
 
+import software.amazon.smithy.rust.codegen.core.rustlang.autoDeref
+
 sealed class ValueExpression {
     abstract val name: String
 
     data class Reference(override val name: String) : ValueExpression()
     data class Value(override val name: String) : ValueExpression()
 
     fun asValue(): String = when (this) {
-        is Reference -> "*$name"
+        is Reference -> autoDeref(name)
         is Value -> name
     }
 

diff --git a/...smithy/rust/codegen/core/smithy/protocols/serialize/XmlBindingTraitSerializerGenerator.kt b/...smithy/rust/codegen/core/smithy/protocols/serialize/XmlBindingTraitSerializerGenerator.kt
@@ -19,7 +19,6 @@ import software.amazon.smithy.model.shapes.StringShape
 import software.amazon.smithy.model.shapes.StructureShape
 import software.amazon.smithy.model.shapes.TimestampShape
 import software.amazon.smithy.model.shapes.UnionShape
-import software.amazon.smithy.model.traits.EnumTrait
 import software.amazon.smithy.model.traits.TimestampFormatTrait
 import software.amazon.smithy.model.traits.XmlFlattenedTrait
 import software.amazon.smithy.model.traits.XmlNamespaceTrait
@@ -285,11 +284,9 @@ class XmlBindingTraitSerializerGenerator(
     }
 
     private fun RustWriter.serializeRawMember(member: MemberShape, input: String) {
-        when (val shape = model.expectShape(member.target)) {
-            is StringShape -> if (shape.hasTrait<EnumTrait>()) {
+        when (model.expectShape(member.target)) {
+            is StringShape -> {
                 rust("$input.as_str()")
-            } else {
-                rust("$input.as_ref()")
             }
             is BooleanShape, is NumberShape -> {
                 rust(
@@ -444,7 +441,7 @@ class XmlBindingTraitSerializerGenerator(
      * ```
      *
      * If [member] is not an optional shape, generate code like:
-     * `{ .. Block }`
+     * `{ .. BLOCK }`
      *
      * [inner] is passed a new `ctx` object to use for code generation which handles the
      * potentially new name of the input.
@@ -462,7 +459,12 @@ class XmlBindingTraitSerializerGenerator(
             }
         } else {
             with(util) {
-                ignoreZeroValues(member, ValueExpression.Value(autoDeref(ctx.input))) {
+                val valueExpression = if (ctx.input.startsWith("&")) {
+                    ValueExpression.Reference(ctx.input)
+                } else {
+                    ValueExpression.Value(ctx.input)
+                }
+                ignoreZeroValues(member, valueExpression) {
                     inner(ctx)
                 }
             }