Make type annotation optional to allow serializing SimpleValue

8 files changed, 202 insertions, 157 deletions

diff --git a/serde_dhall/src/options/de.rs b/serde_dhall/src/options/de.rs
index 8ff794d..e4f3456 100644
--- a/serde_dhall/src/options/de.rs
+++ b/serde_dhall/src/options/de.rs
@@ -2,8 +2,9 @@ use std::path::{Path, PathBuf};
 
 use dhall::Parsed;
 
+use crate::options::{HasAnnot, ManualAnnot, NoAnnot, StaticAnnot, TypeAnnot};
 use crate::SimpleType;
-use crate::{Error, ErrorKind, FromDhall, Result, StaticType, Value};
+use crate::{Error, ErrorKind, FromDhall, Result, Value};
 
 #[derive(Debug, Clone)]
 enum Source<'a> {
@@ -12,32 +13,6 @@ enum Source<'a> {
     // Url(&'a str),
 }
 
-#[derive(Debug, Clone, Copy)]
-pub struct NoAnnot;
-#[derive(Debug, Clone, Copy)]
-pub struct ManualAnnot<'ty>(&'ty SimpleType);
-#[derive(Debug, Clone, Copy)]
-pub struct StaticAnnot;
-
-pub trait OptionalAnnot<A> {
-    fn get_annot(a: &A) -> Option<SimpleType>;
-}
-impl<T> OptionalAnnot<NoAnnot> for T {
-    fn get_annot(_: &NoAnnot) -> Option<SimpleType> {
-        None
-    }
-}
-impl<'ty, T> OptionalAnnot<ManualAnnot<'ty>> for T {
-    fn get_annot(a: &ManualAnnot<'ty>) -> Option<SimpleType> {
-        Some(a.0.clone())
-    }
-}
-impl<T: StaticType> OptionalAnnot<StaticAnnot> for T {
-    fn get_annot(_: &StaticAnnot) -> Option<SimpleType> {
-        Some(T::static_type())
-    }
-}
-
 /// Controls how a Dhall value is read.
 ///
 /// This builder exposes the ability to configure how a value is deserialized and what operations
@@ -252,7 +227,8 @@ impl<'a, A> Deserializer<'a, A> {
 
     fn _parse<T>(&self) -> dhall::error::Result<Value>
     where
-        T: OptionalAnnot<A>,
+        A: TypeAnnot,
+        T: HasAnnot<A>,
     {
         let parsed = match &self.source {
             Source::Str(s) => Parsed::parse_str(s)?,
@@ -263,7 +239,7 @@ impl<'a, A> Deserializer<'a, A> {
         } else {
             parsed.skip_resolve()?
         };
-        let typed = match &T::get_annot(&self.annot) {
+        let typed = match &T::get_annot(self.annot) {
             None => resolved.typecheck()?,
             Some(ty) => resolved.typecheck_with(ty.to_value().as_hir())?,
         };
@@ -287,7 +263,8 @@ impl<'a, A> Deserializer<'a, A> {
     /// [`StaticType`]: trait.StaticType.html
     pub fn parse<T>(&self) -> Result<T>
     where
-        T: FromDhall + OptionalAnnot<A>,
+        A: TypeAnnot,
+        T: FromDhall + HasAnnot<A>,
     {
         let val = self
             ._parse::<T>()
diff --git a/serde_dhall/src/options/mod.rs b/serde_dhall/src/options/mod.rs
index 384f318..9241c45 100644
--- a/serde_dhall/src/options/mod.rs
+++ b/serde_dhall/src/options/mod.rs
@@ -1,2 +1,36 @@
+use crate::{SimpleType, StaticType};
+
 pub(crate) mod de;
 pub(crate) mod ser;
+
+#[derive(Debug, Clone, Copy)]
+pub struct NoAnnot;
+#[derive(Debug, Clone, Copy)]
+pub struct ManualAnnot<'ty>(&'ty SimpleType);
+#[derive(Debug, Clone, Copy)]
+pub struct StaticAnnot;
+
+pub trait TypeAnnot: Copy {}
+pub trait HasAnnot<A: TypeAnnot> {
+    fn get_annot(a: A) -> Option<SimpleType>;
+}
+
+impl TypeAnnot for NoAnnot {}
+impl TypeAnnot for ManualAnnot<'_> {}
+impl TypeAnnot for StaticAnnot {}
+
+impl<T> HasAnnot<NoAnnot> for T {
+    fn get_annot(_: NoAnnot) -> Option<SimpleType> {
+        None
+    }
+}
+impl<T> HasAnnot<ManualAnnot<'_>> for T {
+    fn get_annot(a: ManualAnnot<'_>) -> Option<SimpleType> {
+        Some(a.0.clone())
+    }
+}
+impl<T: StaticType> HasAnnot<StaticAnnot> for T {
+    fn get_annot(_: StaticAnnot) -> Option<SimpleType> {
+        Some(T::static_type())
+    }
+}
diff --git a/serde_dhall/src/options/ser.rs b/serde_dhall/src/options/ser.rs
index 026dd21..ea5d16a 100644
--- a/serde_dhall/src/options/ser.rs
+++ b/serde_dhall/src/options/ser.rs
@@ -1,25 +1,5 @@
-use crate::{Result, SimpleType, StaticType, ToDhall};
-
-#[derive(Debug, Clone, Copy)]
-pub struct NoAnnot;
-#[derive(Debug, Clone, Copy)]
-pub struct ManualAnnot<'ty>(&'ty SimpleType);
-#[derive(Debug, Clone, Copy)]
-pub struct StaticAnnot;
-
-pub trait RequiredAnnot<A> {
-    fn get_annot(a: &A) -> SimpleType;
-}
-impl<'ty, T> RequiredAnnot<ManualAnnot<'ty>> for T {
-    fn get_annot(a: &ManualAnnot<'ty>) -> SimpleType {
-        a.0.clone()
-    }
-}
-impl<T: StaticType> RequiredAnnot<StaticAnnot> for T {
-    fn get_annot(_: &StaticAnnot) -> SimpleType {
-        T::static_type()
-    }
-}
+use crate::options::{HasAnnot, ManualAnnot, NoAnnot, StaticAnnot, TypeAnnot};
+use crate::{Result, SimpleType, ToDhall};
 
 #[derive(Debug, Clone)]
 pub struct Serializer<'a, T, A> {
@@ -46,12 +26,15 @@ impl<'a, T> Serializer<'a, T, NoAnnot> {
     }
 }
 
-impl<'a, T, A> Serializer<'a, T, A> {
+impl<'a, T, A> Serializer<'a, T, A>
+where
+    A: TypeAnnot,
+{
     pub fn to_string(&self) -> Result<String>
     where
-        T: ToDhall + RequiredAnnot<A>,
+        T: ToDhall + HasAnnot<A>,
     {
-        let val = self.data.to_dhall(&T::get_annot(&self.annot))?;
+        let val = self.data.to_dhall(T::get_annot(self.annot).as_ref())?;
         Ok(val.to_string())
     }
 }
diff --git a/serde_dhall/src/serialize.rs b/serde_dhall/src/serialize.rs
index dd3e426..21b1931 100644
--- a/serde_dhall/src/serialize.rs
+++ b/serde_dhall/src/serialize.rs
@@ -11,7 +11,7 @@ pub trait Sealed {}
 
 pub trait ToDhall: Sealed {
     #[doc(hidden)]
-    fn to_dhall(&self, ty: &SimpleType) -> Result<Value>;
+    fn to_dhall(&self, ty: Option<&SimpleType>) -> Result<Value>;
 }
 
 impl<T> Sealed for T where T: ser::Serialize {}
@@ -20,7 +20,7 @@ impl<T> ToDhall for T
 where
     T: ser::Serialize,
 {
-    fn to_dhall(&self, ty: &SimpleType) -> Result<Value> {
+    fn to_dhall(&self, ty: Option<&SimpleType>) -> Result<Value> {
         let sval: SimpleValue = self.serialize(Serializer)?;
         sval.into_value(ty)
     }
@@ -310,3 +310,15 @@ impl ser::SerializeStruct for StructSerializer {
         Ok(Record(self.0))
     }
 }
+
+impl serde::ser::Serialize for SimpleValue {
+    fn serialize<S>(
+        &self,
+        serializer: S,
+    ) -> std::result::Result<S::Ok, S::Error>
+    where
+        S: serde::ser::Serializer,
+    {
+        todo!()
+    }
+}
diff --git a/serde_dhall/src/value.rs b/serde_dhall/src/value.rs
index ca05658..50c12bd 100644
--- a/serde_dhall/src/value.rs
+++ b/serde_dhall/src/value.rs
@@ -293,87 +293,103 @@ impl SimpleValue {
         })
     }
 
-    fn to_hir(&self, ty: &SimpleType) -> Result<Hir> {
+    // Converts this to `Hir`, using the optional type annotation. Without the type, things like
+    // empty lists and unions will fail to convert.
+    fn to_hir(&self, ty: Option<&SimpleType>) -> Result<Hir> {
         use SimpleType as T;
         use SimpleValue as V;
         let hir = |k| Hir::new(HirKind::Expr(k), Span::Artificial);
-        let type_error = |msg| Error(ErrorKind::Serialize(msg));
+        let type_error = || {
+            Error(ErrorKind::Serialize(format!(
+                "expected a value of type {}, found {:?}",
+                ty.unwrap().to_hir().to_expr(Default::default()),
+                self
+            )))
+        };
+        let type_missing = || {
+            Error(ErrorKind::Serialize(format!(
+                "cannot serialize value without a type annotation: {:?}",
+                self
+            )))
+        };
         let kind = match (self, ty) {
-            (V::Num(num @ NumKind::Bool(_)), T::Bool)
-            | (V::Num(num @ NumKind::Natural(_)), T::Natural)
-            | (V::Num(num @ NumKind::Integer(_)), T::Integer)
-            | (V::Num(num @ NumKind::Double(_)), T::Double) => {
-                ExprKind::Num(num.clone())
+            (V::Num(num @ NumKind::Bool(_)), Some(T::Bool))
+            | (V::Num(num @ NumKind::Natural(_)), Some(T::Natural))
+            | (V::Num(num @ NumKind::Integer(_)), Some(T::Integer))
+            | (V::Num(num @ NumKind::Double(_)), Some(T::Double))
+            | (V::Num(num), None) => ExprKind::Num(num.clone()),
+            (V::Text(v), Some(T::Text)) | (V::Text(v), None) => {
+                ExprKind::TextLit(v.clone().into())
             }
-            (V::Text(v), T::Text) => ExprKind::TextLit(v.clone().into()),
-            (V::Optional(None), T::Optional(t)) => ExprKind::Op(OpKind::App(
-                hir(ExprKind::Builtin(Builtin::OptionalNone)),
-                t.to_hir(),
-            )),
-            (V::Optional(Some(v)), T::Optional(t)) => {
-                ExprKind::SomeLit(v.to_hir(t)?)
+
+            (V::Optional(None), None) => return Err(type_missing()),
+            (V::Optional(None), Some(T::Optional(t))) => {
+                ExprKind::Op(OpKind::App(
+                    hir(ExprKind::Builtin(Builtin::OptionalNone)),
+                    t.to_hir(),
+                ))
+            }
+            (V::Optional(Some(v)), None) => ExprKind::SomeLit(v.to_hir(None)?),
+            (V::Optional(Some(v)), Some(T::Optional(t))) => {
+                ExprKind::SomeLit(v.to_hir(Some(t))?)
             }
-            (V::List(v), T::List(t)) if v.is_empty() => {
+
+            (V::List(v), None) if v.is_empty() => return Err(type_missing()),
+            (V::List(v), Some(T::List(t))) if v.is_empty() => {
                 ExprKind::EmptyListLit(hir(ExprKind::Op(OpKind::App(
                     hir(ExprKind::Builtin(Builtin::List)),
                     t.to_hir(),
                 ))))
             }
-            (V::List(v), T::List(t)) => ExprKind::NEListLit(
-                v.iter().map(|v| v.to_hir(t)).collect::<Result<_>>()?,
+            (V::List(v), None) => ExprKind::NEListLit(
+                v.iter().map(|v| v.to_hir(None)).collect::<Result<_>>()?,
+            ),
+            (V::List(v), Some(T::List(t))) => ExprKind::NEListLit(
+                v.iter().map(|v| v.to_hir(Some(t))).collect::<Result<_>>()?,
+            ),
+
+            (V::Record(v), None) => ExprKind::RecordLit(
+                v.iter()
+                    .map(|(k, v)| Ok((k.clone().into(), v.to_hir(None)?)))
+                    .collect::<Result<_>>()?,
             ),
-            (V::Record(v), T::Record(t)) => ExprKind::RecordLit(
+            (V::Record(v), Some(T::Record(t))) => ExprKind::RecordLit(
                 v.iter()
                     .map(|(k, v)| match t.get(k) {
-                        Some(t) => Ok((k.clone().into(), v.to_hir(t)?)),
-                        None => Err(type_error(format!(
-                            "expected a value of type {}, found {:?}",
-                            ty.to_hir().to_expr(Default::default()),
-                            self
-                        ))),
+                        Some(t) => Ok((k.clone().into(), v.to_hir(Some(t))?)),
+                        None => Err(type_error()),
                     })
                     .collect::<Result<_>>()?,
             ),
-            (V::Union(variant, Some(v)), T::Union(t)) => match t.get(variant) {
-                Some(Some(variant_t)) => ExprKind::Op(OpKind::App(
-                    hir(ExprKind::Op(OpKind::Field(
-                        ty.to_hir(),
-                        variant.clone().into(),
-                    ))),
-                    v.to_hir(variant_t)?,
-                )),
-                _ => {
-                    return Err(type_error(format!(
-                        "expected a value of type {}, found {:?}",
-                        ty.to_hir().to_expr(Default::default()),
-                        self
-                    )))
+
+            (V::Union(..), None) => return Err(type_missing()),
+            (V::Union(variant, Some(v)), Some(T::Union(t))) => {
+                match t.get(variant) {
+                    Some(Some(variant_t)) => ExprKind::Op(OpKind::App(
+                        hir(ExprKind::Op(OpKind::Field(
+                            ty.unwrap().to_hir(),
+                            variant.clone().into(),
+                        ))),
+                        v.to_hir(Some(variant_t))?,
+                    )),
+                    _ => return Err(type_error()),
                 }
-            },
-            (V::Union(variant, None), T::Union(t)) => match t.get(variant) {
-                Some(None) => ExprKind::Op(OpKind::Field(
-                    ty.to_hir(),
-                    variant.clone().into(),
-                )),
-                _ => {
-                    return Err(type_error(format!(
-                        "expected a value of type {}, found {:?}",
-                        ty.to_hir().to_expr(Default::default()),
-                        self
-                    )))
+            }
+            (V::Union(variant, None), Some(T::Union(t))) => {
+                match t.get(variant) {
+                    Some(None) => ExprKind::Op(OpKind::Field(
+                        ty.unwrap().to_hir(),
+                        variant.clone().into(),
+                    )),
+                    _ => return Err(type_error()),
                 }
-            },
-            _ => {
-                return Err(type_error(format!(
-                    "expected a value of type {}, found {:?}",
-                    ty.to_hir().to_expr(Default::default()),
-                    self
-                )))
             }
+
+            (_, Some(_)) => return Err(type_error()),
         };
         Ok(hir(kind))
     }
-    pub(crate) fn into_value(self, ty: &SimpleType) -> Result<Value> {
+    pub(crate) fn into_value(self, ty: Option<&SimpleType>) -> Result<Value> {
         Ok(Value {
             hir: self.to_hir(ty)?,
             as_simple_val: Some(self),
@@ -467,7 +483,6 @@ impl SimpleType {
 impl crate::deserialize::Sealed for Value {}
 impl crate::deserialize::Sealed for SimpleType {}
 impl crate::serialize::Sealed for Value {}
-impl crate::serialize::Sealed for SimpleValue {}
 
 impl FromDhall for Value {
     fn from_dhall(v: &Value) -> Result<Self> {
@@ -485,15 +500,10 @@ impl FromDhall for SimpleType {
     }
 }
 impl ToDhall for Value {
-    fn to_dhall(&self, _ty: &SimpleType) -> Result<Value> {
+    fn to_dhall(&self, _ty: Option<&SimpleType>) -> Result<Value> {
         Ok(self.clone())
     }
 }
-impl ToDhall for SimpleValue {
-    fn to_dhall(&self, ty: &SimpleType) -> Result<Value> {
-        self.clone().into_value(ty)
-    }
-}
 
 impl Eq for Value {}
 impl PartialEq for Value {
diff --git a/serde_dhall/tests/de.rs b/serde_dhall/tests/de.rs
index 0b43de2..f49bee4 100644
--- a/serde_dhall/tests/de.rs
+++ b/serde_dhall/tests/de.rs
@@ -1,5 +1,5 @@
 use serde::Deserialize;
-use serde_dhall::{from_str, FromDhall, NumKind, SimpleValue};
+use serde_dhall::{from_str, FromDhall};
 
 #[test]
 fn test_de_untyped() {
@@ -57,37 +57,5 @@ fn test_de_untyped() {
     assert!(from_str("List/length [True, 42]").parse::<bool>().is_err());
 }
 
-#[test]
-fn test_de_simplevalue() {
-    let bool_true = SimpleValue::Num(NumKind::Bool(true));
-    // https://github.com/Nadrieril/dhall-rust/issues/184
-    assert_eq!(
-        from_str("[ True ]").parse::<Vec<SimpleValue>>().unwrap(),
-        vec![bool_true.clone()]
-    );
-
-    assert_eq!(
-        from_str("< Foo >.Foo").parse::<SimpleValue>().unwrap(),
-        SimpleValue::Union("Foo".into(), None)
-    );
-    assert_eq!(
-        from_str("< Foo: Bool >.Foo True")
-            .parse::<SimpleValue>()
-            .unwrap(),
-        SimpleValue::Union("Foo".into(), Some(Box::new(bool_true.clone())))
-    );
-
-    #[derive(Debug, PartialEq, Eq, Deserialize)]
-    struct Foo {
-        foo: SimpleValue,
-    }
-    assert_eq!(
-        from_str("{ foo = True }").parse::<Foo>().unwrap(),
-        Foo {
-            foo: bool_true.clone()
-        },
-    );
-}
-
 // TODO: test various builder configurations
 // In particular test cloning and reusing builder
diff --git a/serde_dhall/tests/serde.rs b/serde_dhall/tests/serde.rs
index 39f2f79..ce25380 100644
--- a/serde_dhall/tests/serde.rs
+++ b/serde_dhall/tests/serde.rs
@@ -75,9 +75,13 @@ mod serde {
 
     #[test]
     fn optional() {
-        assert_serde::<Option<u64>>("None Natural", None);
-        assert_serde::<Option<String>>("None Text", None);
+        assert_serde("None Natural", None::<u64>);
+        assert_serde("None Text", None::<String>);
         assert_serde("Some 1", Some(1u64));
+        assert_eq!(
+            serialize(&None::<u64>).to_string().map_err(|e| e.to_string()),
+            Err("cannot serialize value without a type annotation: Optional(None)".to_string())
+        );
     }
 
     #[test]
diff --git a/serde_dhall/tests/simple_value.rs b/serde_dhall/tests/simple_value.rs
new file mode 100644
index 0000000..d2792d4
--- /dev/null
+++ b/serde_dhall/tests/simple_value.rs
@@ -0,0 +1,57 @@
+mod simple_value {
+    use serde::{Deserialize, Serialize};
+    use serde_dhall::{
+        from_str, serialize, FromDhall, NumKind, SimpleValue, ToDhall,
+    };
+
+    fn assert_de<T>(s: &str, x: T)
+    where
+        T: FromDhall + PartialEq + std::fmt::Debug,
+    {
+        assert_eq!(from_str(s).parse::<T>().map_err(|e| e.to_string()), Ok(x));
+    }
+    fn assert_ser<T>(s: &str, x: T)
+    where
+        T: ToDhall + PartialEq + std::fmt::Debug,
+    {
+        assert_eq!(
+            serialize(&x).to_string().map_err(|e| e.to_string()),
+            Ok(s.to_string())
+        );
+    }
+    fn assert_serde<T>(s: &str, x: T)
+    where
+        T: ToDhall + FromDhall + PartialEq + std::fmt::Debug + Clone,
+    {
+        assert_de(s, x.clone());
+        assert_ser(s, x);
+    }
+
+    #[test]
+    fn test_serde() {
+        let bool_true = SimpleValue::Num(NumKind::Bool(true));
+        // https://github.com/Nadrieril/dhall-rust/issues/184
+        // assert_serde("[ True ]", vec![bool_true.clone()]);
+        assert_de("< Foo >.Foo", SimpleValue::Union("Foo".into(), None));
+        assert_de(
+            "< Foo: Bool >.Foo True",
+            SimpleValue::Union("Foo".into(), Some(Box::new(bool_true.clone()))),
+        );
+
+        // assert_eq!(
+        //     serialize(&SimpleValue::Optional(None)).to_string().map_err(|e| e.to_string()),
+        //     Err("cannot serialize value without a type annotation: Optional(None)".to_string())
+        // );
+
+        // #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+        // struct Foo {
+        //     foo: SimpleValue,
+        // }
+        // assert_serde(
+        //     "{ foo = True }",
+        //     Foo {
+        //         foo: bool_true.clone(),
+        //     },
+        // );
+    }
+}