only subtype checking for reference type (#395)

* only subtype checking for reference type; For opt type, add error type handling

* use unsafe for opt

rust/candid/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "candid"
-version = "0.8.4"
+version = "0.9.0-beta"
 edition = "2018"
 authors = ["DFINITY Team"]
 description = "Candid is an interface description language (IDL) for interacting with canisters running on the Internet Computer."

rust/candid/src/error.rs

@@ -19,6 +19,9 @@ pub enum Error {
     #[error("binary parser error: {}", .0.get(0).map(|f| format!("{} at byte offset {}", f.message, f.range.start/2)).unwrap_or_else(|| "io error".to_string()))]
     Binread(Vec<Label<()>>),
 
+    #[error("Subtyping error: {0}")]
+    Subtype(String),
+
     #[error(transparent)]
     Custom(#[from] anyhow::Error),
 }
@@ -27,6 +30,9 @@ impl Error {
     pub fn msg<T: ToString>(msg: T) -> Self {
         Error::Custom(anyhow::anyhow!(msg.to_string()))
     }
+    pub fn subtype<T: ToString>(msg: T) -> Self {
+        Error::Subtype(msg.to_string())
+    }
     pub fn report(&self) -> Diagnostic<()> {
         match self {
             Error::Parse(e) => {
@@ -57,6 +63,7 @@ impl Error {
                 let diag = Diagnostic::error().with_message("decoding error");
                 diag.with_labels(labels.to_vec())
             }
+            Error::Subtype(e) => Diagnostic::error().with_message(e),
             Error::Custom(e) => Diagnostic::error().with_message(e.to_string()),
         }
     }
@@ -135,6 +142,9 @@ impl de::Error for Error {
     fn custom<T: std::fmt::Display>(msg: T) -> Self {
         Error::msg(format!("Deserialize error: {}", msg))
     }
+    fn invalid_type(_: de::Unexpected<'_>, exp: &dyn de::Expected) -> Self {
+        Error::Subtype(format!("{}", exp))
+    }
 }
 
 impl From<io::Error> for Error {

rust/candid/tests/serde.rs

@@ -61,10 +61,7 @@ fn test_integer() {
         Int::parse(b"-60000000000000000").unwrap(),
         "4449444c00017c8080e88b96cab5957f",
     );
-    check_error(
-        || test_decode(&hex("4449444c00017c2a"), &42i64),
-        "int is not a subtype of int64",
-    );
+    check_error(|| test_decode(&hex("4449444c00017c2a"), &42i64), "Int64");
 }
 
 #[test]
@@ -136,7 +133,10 @@ fn test_reserved() {
 
 #[test]
 fn test_reference() {
-    use candid::{Func, Principal, Service};
+    use candid::{
+        types::{Function, Type},
+        Func, Principal, Service,
+    };
     let principal = Principal::from_text("w7x7r-cok77-xa").unwrap();
     all_check(principal, "4449444c0001680103caffee");
     all_check(Service { principal }, "4449444c01690001000103caffee");
@@ -147,6 +147,25 @@ fn test_reference() {
         },
         "4449444c016a0000000100010103caffee066d6574686f64",
     );
+    #[derive(Deserialize, PartialEq, Debug)]
+    struct CustomFunc(Func);
+    impl CandidType for CustomFunc {
+        fn _ty() -> Type {
+            Type::Func(Function {
+                args: vec![],
+                rets: vec![Type::Nat],
+                modes: vec![],
+            })
+        }
+        fn idl_serialize<S: candid::types::Serializer>(
+            &self,
+            _serializer: S,
+        ) -> Result<(), S::Error> {
+            unimplemented!()
+        }
+    }
+    let bytes = hex("4449444c016a00017f000100010100016d");
+    test_decode(&bytes, &None::<CustomFunc>);
 }
 
 #[test]
@@ -236,6 +255,61 @@ fn test_struct() {
     all_check(list, "4449444c026e016c02a0d2aca8047c90eddae70400010000");
 }
 
+#[test]
+fn optional_fields() {
+    #[derive(PartialEq, Debug, Deserialize, CandidType)]
+    struct OldStruct {
+        bar: bool,
+        baz: Option<Old>,
+    }
+    #[derive(PartialEq, Debug, Deserialize, CandidType)]
+    enum Old {
+        Foo,
+        Bar(bool),
+    }
+    #[derive(PartialEq, Debug, Deserialize, CandidType)]
+    enum New {
+        Foo,
+        Bar(bool),
+        Baz(bool),
+    }
+    #[derive(PartialEq, Debug, Deserialize, CandidType)]
+    struct NewStruct {
+        foo: Option<u8>,
+        bar: bool,
+        baz: Option<New>,
+    }
+    let bytes = encode(&OldStruct {
+        bar: true,
+        baz: Some(Old::Foo),
+    });
+    test_decode(
+        &bytes,
+        &NewStruct {
+            foo: None,
+            bar: true,
+            baz: Some(New::Foo),
+        },
+    );
+    let bytes = encode(&NewStruct {
+        foo: Some(42),
+        bar: false,
+        baz: Some(New::Baz(true)),
+    });
+    test_decode(
+        &bytes,
+        &OldStruct {
+            bar: false,
+            baz: None,
+        },
+    );
+    let bytes = encode(&New::Baz(false));
+    check_error(
+        || test_decode(&bytes, &Old::Bar(false)),
+        "Unknown variant field",
+    );
+}
+
 #[test]
 fn test_equivalent_types() {
     #[derive(PartialEq, Debug, Deserialize, CandidType)]
@@ -320,10 +394,7 @@ fn test_extra_fields() {
 
     let bytes = encode(&E2::Foo);
     test_decode(&bytes, &Some(E2::Foo));
-    check_error(
-        || test_decode(&bytes, &E1::Foo),
-        "Variant field 3_303_867 not found",
-    );
+    test_decode(&bytes, &E1::Foo);
 }
 
 #[test]
@@ -490,7 +561,7 @@ fn test_tuple() {
                 &(Int::from(42), "💩"),
             )
         },
-        "field 1: text is only in the expected type",
+        "is not a tuple type",
     );
 }
 
@@ -509,7 +580,7 @@ fn test_variant() {
 
     check_error(
         || test_decode(&hex("4449444c016b02b4d3c9017fe6fdd5017f010000"), &Unit::Bar),
-        "Variant field 3_303_860 not found",
+        "Unknown variant field 3_303_860",
     );
 
     let res: Result<String, String> = Ok("good".to_string());
@@ -527,6 +598,17 @@ fn test_variant() {
         v,
         "4449444c036b03b3d3c90101bbd3c90102e6fdd5017f6c02007e017c6c02617c627d010000012a",
     );
+
+    let bytes = encode(&Some(E::Foo));
+    test_decode(&bytes, &Some(Unit::Foo));
+    let bytes = encode(&E::Baz {
+        a: 42.into(),
+        b: 42.into(),
+    });
+    test_decode(&bytes, &None::<Unit>);
+    let bytes = encode(&E::Bar(true, 42.into()));
+    test_decode(&bytes, &None::<Unit>);
+    check_error(|| test_decode(&bytes, &Unit::Bar), "Subtyping error");
 }
 
 #[test]
@@ -609,12 +691,12 @@ fn test_multiargs() {
         Vec<(Int, &str)>,
         (Int, String),
         Option<i32>,
-        (),
+        candid::Reserved,
         candid::Reserved
     )
     .unwrap();
     assert_eq!(tuple.2, None);
-    assert_eq!(tuple.3, ());
+    assert_eq!(tuple.3, candid::Reserved);
     assert_eq!(tuple.4, candid::Reserved);
 }
 

test/construct.test.did

@@ -54,7 +54,7 @@ assert blob "DIDL\00\00" == "(null)"                                 : (Opt) "op
 
 // vector
 assert blob "DIDL\01\6d\7c\01\00\00" == "(vec {})"                          : (vec int) "vec: empty";
-assert blob "DIDL\01\6d\7c\01\00\00"                                       !: (vec int8) "vec: non subtype empty";
+assert blob "DIDL\01\6d\7c\01\00\00"                                        : (vec int8) "vec: non subtype empty";
 assert blob "DIDL\01\6d\7c\01\00\02\01\02" == "(vec { 1; 2 })"              : (vec int) "vec";
 assert blob "DIDL\01\6d\7b\01\00\02\01\02" == "(blob \"\\01\\02\")"         : (vec nat8) "vec: blob";
 assert blob "DIDL\01\6d\00\01\00\00" == "(vec {})"                          : (Vec) "vec: recursive vector";
@@ -139,10 +139,10 @@ assert "(variant {})"                                                          !
 assert blob "DIDL\01\6b\00\01\00"                                              !: (variant {}) "variant: no empty value";
 assert blob "DIDL\01\6b\01\00\7f\01\00\00" == "(variant {0})"                   : (variant {0}) "variant: numbered field";
 assert blob "DIDL\01\6b\01\00\7f\01\00\00\2a"                                  !: (variant {0:int}) "variant: type mismatch";
-assert blob "DIDL\01\6b\02\00\7f\01\7c\01\00\01\2a"                            !: (variant {0:int; 1:int}) "variant: type mismatch in unused tag";
+assert blob "DIDL\01\6b\02\00\7f\01\7c\01\00\01\2a"                             : (variant {0:int; 1:int}) "variant: type mismatch in unused tag";
 assert blob "DIDL\01\6b\01\00\7f\01\00\00" == "(variant {0})"                   : (variant {0;1}) "variant: ignore field";
-assert blob "DIDL\01\6b\02\00\7f\01\7f\01\00\00"                               !: (variant {0}) "variant {0;1} !<: variant {0}";
-assert blob "DIDL\01\6b\02\00\7f\01\7f\01\00\00" == "(null)"                    : (opt variant {0}) "variant {0;1} <: opt variant {0}";
+assert blob "DIDL\01\6b\02\00\7f\01\7f\01\00\00"                                : (variant {0}) "variant {0;1} <: variant {0}";
+assert blob "DIDL\01\6b\02\00\7f\01\7f\01\00\00" == "(opt variant {0})"         : (opt variant {0}) "variant {0;1} <: opt variant {0}";
 assert blob "DIDL\01\6b\02\00\7f\01\7f\01\00\01" == "(variant {1})"             : (variant {0;1;2}) "variant: change index";
 assert blob "DIDL\01\6b\01\00\7f\01\00\00"                                     !: (variant {1}) "variant: missing field";
 assert blob "DIDL\01\6b\01\00\7f\01\00\01"                                     !: (variant {0}) "variant: index out of range";
@@ -188,7 +188,7 @@ assert blob "DIDL\02\6b\02\d1\a7\cf\02\7f\f1\f3\92\8e\04\01\6c\02\a0\d2\ac\a8\04
     == "(variant { cons = record { head = 1; tail = variant { cons = record { head = 2; tail = variant { nil } } } } })" : (VariantList) "variant: list";
 
 assert blob "DIDL\02\6b\02\d1\a7\cf\02\7f\f1\f3\92\8e\04\01\6c\02\a0\d2\ac\a8\04\7c\90\ed\da\e7\04\00\01\00\00"
-    == "(variant {nil}, null, null, null, null)" : (VariantList, opt VariantList, null, reserved, opt int) "variant: extra args";
+    == "(variant {nil}, null, null, null, null)" : (VariantList, opt VariantList, opt empty, reserved, opt int) "variant: extra args";
 
 assert blob "DIDL\02\6b\02\d1\a7\cf\02\7f\f1\f3\92\8e\04\01\6c\02\a0\d2\ac\a8\04\7c\90\ed\da\e7\04\00\01\00\00" !: (VariantList, opt int, vec int) "non-null extra args";
 
@@ -203,3 +203,8 @@ assert blob "DIDL\07\6c\07\c3\e3\aa\02\01\d3\e3\aa\02\7e\d5\e3\aa\02\02\db\e3\aa
 
 assert blob "DIDL\07\6c\07\c3\e3\aa\02\01\d3\e3\aa\02\7e\d5\e3\aa\02\02\db\e3\aa\02\01\a2\e5\aa\02\04\bb\f1\aa\02\06\86\8e\b7\02\75\6c\02\d3\e3\aa\02\7e\86\8e\b7\02\75\6b\02\d1\a7\cf\02\7f\f1\f3\92\8e\04\03\6c\02\a0\d2\ac\a8\04\7c\90\ed\da\e7\04\02\6e\05\6c\02\a0\d2\ac\a8\04\7c\90\ed\da\e7\04\04\6b\02\d3\e3\aa\02\7f\86\8e\b7\02\7f\01\00\01\0b\00\00\00\01\00\00\0a\00\00\00\01\14\00\00\2a\00\00\00" !: (record { foo: int; new_field: bool }) "new record field";
 
+// Future types
+// This uses 0x67 for the “future type”; bump (well, decrement) once that
+// becomes a concrete future type
+//assert blob "DIDL\01\67\00\02\00\7e\00\00\01" == "(null, true)" : (opt empty,bool) "skipping minimal future type";
+//assert blob "DIDL\01\67\03ABC\02\00\7e\05\00hello\01" == "(null,true)" : (opt empty,bool) "skipping future type with data";

test/prim.test.did

@@ -1,7 +1,7 @@
 /*
 Encoding tests for primitive types
 
-Corresponding to spec version version 0.1.3
+Corresponding to spec version version 0.1.4
 */
 
 // fundamentally wrong
@@ -19,11 +19,19 @@ assert blob "DIDL\00\01\7f"  : () "Additional parameters are ignored";
 assert blob "DIDL\00\01\6e" !: () "Not a primitive type";
 assert blob "DIDL\00\01\5e" !: () "Out of range type";
 
+// Missing arguments
+assert blob "DIDL\00\00"                        !: (nat)       "missing argument: nat   fails";
+assert blob "DIDL\00\00"                        !: (empty)     "missing argument: empty fails";
+assert blob "DIDL\00\00"                        !: (null)      "missing argument: null  fails";
+assert blob "DIDL\00\00" == "(null)"             : (opt empty) "missing argument: opt empty";
+assert blob "DIDL\00\00" == "(null)"             : (opt null)  "missing argument: opt null";
+assert blob "DIDL\00\00" == "(null)"             : (opt nat)   "missing argument: opt nat";
+assert blob "DIDL\00\00" == blob "DIDL\00\01\70" : (reserved)  "missing argument: reserved";
+
 // primitive types
 assert blob "DIDL\00\01\7f"     : (null);
 assert blob "DIDL\00\01\7e"    !: (null) "wrong type";
 assert blob "DIDL\00\01\7f\00" !: (null) "null: too long";
-assert blob "DIDL\00\00"        : (null) "null: extra null values";
 
 assert blob "DIDL\00\01\7e\00" == "(false)" : (bool) "bool: false";
 assert blob "DIDL\00\01\7e\01" == "(true)"  : (bool) "bool: true";
@@ -181,7 +189,6 @@ assert blob "DIDL\00\01\70" == blob "DIDL\00\01\7f"          : (reserved) "reser
 assert blob "DIDL\00\01\70" == blob "DIDL\00\01\7e\01"       : (reserved) "reserved from bool";
 assert blob "DIDL\00\01\70" == blob "DIDL\00\01\7d\80\01"    : (reserved) "reserved from nat";
 assert blob "DIDL\00\01\70" == blob "DIDL\00\01\71\06Motoko" : (reserved) "reserved from text";
-assert blob "DIDL\00\00"                                     : (reserved) "reserved extra value";
 assert blob "DIDL\00\01\71\05Motoko"    !: (reserved) "reserved from too short text";
 assert blob "DIDL\00\01\71\03\e2\28\a1" !: (reserved) "reserved from invalid utf8 text";