Repository: martinohmann/hcl-rs
Pull request #71 (2022-09-10 11:24:26)

fix: correctly handle `Expression` variants in `ExpressionSerializer`


Patch:
diff --git a/src/structure/ser/expression.rs b/src/structure/ser/expression.rs
index 3cd7326..56733e4 100644
--- a/src/structure/ser/expression.rs
+++ b/src/structure/ser/expression.rs
@@ -1,7 +1,11 @@
 use super::{
-    conditional::ConditionalSerializer, element_access::SerializeElementAccessStruct,
-    for_expr::ForExprSerializer, func::SerializeFuncCallStruct, operation::OperationSerializer,
-    template::TemplateExprSerializer, StringSerializer,
+    conditional::{ConditionalSerializer, SerializeConditionalStruct},
+    element_access::SerializeElementAccessStruct,
+    for_expr::{ForExprSerializer, SerializeForExprStruct},
+    func::SerializeFuncCallStruct,
+    operation::{OperationSerializer, SerializeOperationStruct},
+    template::{SerializeTemplateExprStruct, TemplateExprSerializer},
+    StringSerializer,
 };
 use crate::{Error, Expression, Identifier, Object, ObjectKey, RawExpression, Result};
 use serde::ser::{self, Impossible, SerializeMap};
@@ -133,27 +137,28 @@ impl ser::Serializer for ExpressionSerializer {
     where
         T: ?Sized + ser::Serialize,
     {
-        if name == "$hcl::expression" {
-            match variant {
-                "Conditional" => Ok(Expression::Conditional(Box::new(
-                    value.serialize(ConditionalSerializer)?,
-                ))),
-                "Operation" => Ok(Expression::Operation(Box::new(
-                    value.serialize(OperationSerializer)?,
-                ))),
-                "ForExpr" => Ok(Expression::ForExpr(Box::new(
-                    value.serialize(ForExprSerializer)?,
-                ))),
-                "SubExpr" => Ok(Expression::SubExpr(Box::new(value.serialize(self)?))),
-                "TemplateExpr" => Ok(Expression::TemplateExpr(Box::new(
-                    value.serialize(TemplateExprSerializer)?,
-                ))),
-                _ => value.serialize(self),
+        match (name, variant) {
+            ("$hcl::expression", "Conditional") => {
+                Ok(Expression::from(value.serialize(ConditionalSerializer)?))
+            }
+            ("$hcl::expression", "Operation") | ("$hcl::operation", _) => {
+                Ok(Expression::from(value.serialize(OperationSerializer)?))
+            }
+            ("$hcl::expression", "ForExpr") | ("$hcl::for_expr", _) => {
+                Ok(Expression::from(value.serialize(ForExprSerializer)?))
+            }
+            ("$hcl::expression", "SubExpr") => {
+                Ok(Expression::SubExpr(Box::new(value.serialize(self)?)))
+            }
+            ("$hcl::expression", "TemplateExpr") | ("$hcl::template_expr", _) => {
+                Ok(Expression::from(value.serialize(TemplateExprSerializer)?))
+            }
+            ("$hcl::expression", _) => value.serialize(self),
+            (_, _) => {
+                let mut object = Object::new();
+                object.insert(ObjectKey::identifier(variant), value.serialize(self)?);
+                Ok(Expression::Object(object))
             }
-        } else {
-            let mut object = Object::new();
-            object.insert(ObjectKey::identifier(variant), value.serialize(self)?);
-            Ok(Expression::Object(object))
         }
     }
 
@@ -314,8 +319,12 @@ impl ser::SerializeMap for SerializeExpressionMap {
 }
 
 pub enum SerializeExpressionStruct {
+    Conditional(SerializeConditionalStruct),
     ElementAccess(SerializeElementAccessStruct),
+    ForExpr(SerializeForExprStruct),
     FuncCall(SerializeFuncCallStruct),
+    Operation(SerializeOperationStruct),
+    TemplateExpr(SerializeTemplateExprStruct),
     Other(SerializeExpressionMap),
 }
 
@@ -323,12 +332,24 @@ impl SerializeExpressionStruct {
     fn new(name: &'static str, len: usize) -> Self {
         // Specialization for the `ElementAccess`.
         match name {
+            "$hcl::conditional" => {
+                SerializeExpressionStruct::Conditional(SerializeConditionalStruct::new())
+            }
             "$hcl::element_access" => {
                 SerializeExpressionStruct::ElementAccess(SerializeElementAccessStruct::new())
             }
+            "$hcl::for_list_expr" | "$hcl::for_object_expr" => {
+                SerializeExpressionStruct::ForExpr(SerializeForExprStruct::new(name))
+            }
             "$hcl::func_call" => {
                 SerializeExpressionStruct::FuncCall(SerializeFuncCallStruct::new())
             }
+            "$hcl::unary_op" | "$hcl::binary_op" => {
+                SerializeExpressionStruct::Operation(SerializeOperationStruct::new(name))
+            }
+            "$hcl::heredoc" => {
+                SerializeExpressionStruct::TemplateExpr(SerializeTemplateExprStruct::new())
+            }
             _ => SerializeExpressionStruct::Other(SerializeExpressionMap::new(Some(len))),
         }
     }
@@ -343,16 +364,24 @@ impl ser::SerializeStruct for SerializeExpressionStruct {
         T: ?Sized + ser::Serialize,
     {
         match self {
+            SerializeExpressionStruct::Conditional(ser) => ser.serialize_field(key, value),
             SerializeExpressionStruct::ElementAccess(ser) => ser.serialize_field(key, value),
+            SerializeExpressionStruct::ForExpr(ser) => ser.serialize_field(key, value),
             SerializeExpressionStruct::FuncCall(ser) => ser.serialize_field(key, value),
+            SerializeExpressionStruct::Operation(ser) => ser.serialize_field(key, value),
+            SerializeExpressionStruct::TemplateExpr(ser) => ser.serialize_field(key, value),
             SerializeExpressionStruct::Other(ser) => ser.serialize_entry(key, value),
         }
     }
 
     fn end(self) -> Result<Self::Ok> {
         match self {
+            SerializeExpressionStruct::Conditional(ser) => ser.end().map(Into::into),
             SerializeExpressionStruct::ElementAccess(ser) => ser.end().map(Into::into),
+            SerializeExpressionStruct::ForExpr(ser) => ser.end().map(Into::into),
             SerializeExpressionStruct::FuncCall(ser) => ser.end().map(Into::into),
+            SerializeExpressionStruct::Operation(ser) => ser.end().map(Into::into),
+            SerializeExpressionStruct::TemplateExpr(ser) => ser.end().map(Into::into),
             SerializeExpressionStruct::Other(ser) => ser.end(),
         }
     }
diff --git a/src/structure/ser/for_expr.rs b/src/structure/ser/for_expr.rs
index d99564d..167b7b3 100644
--- a/src/structure/ser/for_expr.rs
+++ b/src/structure/ser/for_expr.rs
@@ -53,7 +53,7 @@ pub enum SerializeForExprStruct {
 }
 
 impl SerializeForExprStruct {
-    fn new(name: &'static str) -> Self {
+    pub fn new(name: &'static str) -> Self {
         // Specialization for the `ForListExpr` and `ForObjectExpr` types.
         match name {
             "$hcl::for_object_expr" => {
diff --git a/src/structure/ser/operation.rs b/src/structure/ser/operation.rs
index 72a890c..012a64c 100644
--- a/src/structure/ser/operation.rs
+++ b/src/structure/ser/operation.rs
@@ -108,7 +108,7 @@ pub enum SerializeOperationStruct {
 }
 
 impl SerializeOperationStruct {
-    fn new(name: &'static str) -> Self {
+    pub fn new(name: &'static str) -> Self {
         // Specialization for the `UnaryOp` and `BinaryOp` types.
         match name {
             "$hcl::binary_op" => SerializeOperationStruct::Binary(SerializeBinaryOpStruct::new()),
diff --git a/src/structure/ser/template.rs b/src/structure/ser/template.rs
index 33ccf9c..4f42212 100644
--- a/src/structure/ser/template.rs
+++ b/src/structure/ser/template.rs
@@ -15,13 +15,13 @@ impl ser::Serializer for TemplateExprSerializer {
     type SerializeTupleStruct = Impossible<TemplateExpr, Error>;
     type SerializeTupleVariant = Impossible<TemplateExpr, Error>;
     type SerializeMap = Impossible<TemplateExpr, Error>;
-    type SerializeStruct = Impossible<TemplateExpr, Error>;
+    type SerializeStruct = SerializeTemplateExprStruct;
     type SerializeStructVariant = Impossible<TemplateExpr, Error>;
 
     serialize_unsupported! {
         i8 i16 i32 i64 u8 u16 u32 u64
         bool f32 f64 bytes unit unit_struct none
-        seq tuple tuple_struct tuple_variant map struct struct_variant
+        seq tuple tuple_struct tuple_variant map struct_variant
     }
     serialize_self! { some newtype_struct }
 
@@ -64,6 +64,10 @@ impl ser::Serializer for TemplateExprSerializer {
         }
     }
 
+    fn serialize_struct(self, _name: &'static str, _len: usize) -> Result<Self::SerializeStruct> {
+        Ok(SerializeTemplateExprStruct::new())
+    }
+
     fn collect_str<T>(self, value: &T) -> Result<Self::Ok>
     where
         T: ?Sized + Display,
@@ -72,6 +76,34 @@ impl ser::Serializer for TemplateExprSerializer {
     }
 }
 
+pub struct SerializeTemplateExprStruct {
+    inner: SerializeHeredocStruct,
+}
+
+impl SerializeTemplateExprStruct {
+    pub fn new() -> Self {
+        SerializeTemplateExprStruct {
+            inner: SerializeHeredocStruct::new(),
+        }
+    }
+}
+
+impl ser::SerializeStruct for SerializeTemplateExprStruct {
+    type Ok = TemplateExpr;
+    type Error = Error;
+
+    fn serialize_field<T>(&mut self, key: &'static str, value: &T) -> Result<()>
+    where
+        T: ?Sized + ser::Serialize,
+    {
+        self.inner.serialize_field(key, value)
+    }
+
+    fn end(self) -> Result<Self::Ok> {
+        self.inner.end().map(Into::into)
+    }
+}
+
 pub struct HeredocSerializer;
 
 impl ser::Serializer for HeredocSerializer {


Test patch:
diff --git a/src/structure/ser/tests.rs b/src/structure/ser/tests.rs
index 5f54a86..7a5776d 100644
--- a/src/structure/ser/tests.rs
+++ b/src/structure/ser/tests.rs
@@ -214,6 +214,81 @@ fn custom() {
         Expression::Number(1u8.into()),
     );
 
+    test_serialize(
+        ExpressionSerializer,
+        Conditional::new(Identifier::new("some_cond_var"), "yes", "no"),
+        Expression::from(Conditional::new(
+            Identifier::new("some_cond_var"),
+            "yes",
+            "no",
+        )),
+    );
+
+    test_serialize(
+        ExpressionSerializer,
+        Operation::Unary(UnaryOp::new(UnaryOperator::Neg, 1)),
+        Expression::from(Operation::Unary(UnaryOp::new(UnaryOperator::Neg, 1))),
+    );
+
+    test_serialize(
+        ExpressionSerializer,
+        TemplateExpr::Heredoc(Heredoc::new(Identifier::new("EOS"), "${foo}")),
+        Expression::from(TemplateExpr::Heredoc(Heredoc::new(
+            Identifier::new("EOS"),
+            "${foo}",
+        ))),
+    );
+
+    test_serialize(
+        ExpressionSerializer,
+        ForExpr::List(
+            ForListExpr::new(
+                ForIntro::new(
+                    Identifier::new("value"),
+                    vec![Expression::String(String::from("foo"))],
+                )
+                .with_key(Identifier::new("index")),
+                Identifier::new("other_value"),
+            )
+            .with_cond(Expression::Bool(true)),
+        ),
+        Expression::from(ForExpr::List(
+            ForListExpr::new(
+                ForIntro::new(
+                    Identifier::new("value"),
+                    vec![Expression::String(String::from("foo"))],
+                )
+                .with_key(Identifier::new("index")),
+                Identifier::new("other_value"),
+            )
+            .with_cond(Expression::Bool(true)),
+        )),
+    );
+
+    test_serialize(
+        ExpressionSerializer,
+        ForListExpr::new(
+            ForIntro::new(
+                Identifier::new("value"),
+                vec![Expression::String(String::from("foo"))],
+            )
+            .with_key(Identifier::new("index")),
+            Identifier::new("other_value"),
+        )
+        .with_cond(Expression::Bool(true)),
+        Expression::from(ForExpr::List(
+            ForListExpr::new(
+                ForIntro::new(
+                    Identifier::new("value"),
+                    vec![Expression::String(String::from("foo"))],
+                )
+                .with_key(Identifier::new("index")),
+                Identifier::new("other_value"),
+            )
+            .with_cond(Expression::Bool(true)),
+        )),
+    );
+
     test_serialize(
         ConditionalSerializer,
         (