dhall-rust icon indicating copy to clipboard operation
dhall-rust copied to clipboard

Support for renaming fields and containers in StaticType

Open Kixiron opened this issue 3 years ago • 3 comments

Serde has the rename (container) and rename (field) attributes which allow renaming things for {de}serialization. Currently dhall doesn't have this, which leads to code being broken in a rather confusing manner, e.g.

#[derive(Deserialize, StaticType)]
#[serde(rename_all = "camelCase")]
struct Transform {
    pre_variant: Option<String>,
    post_variant: Option<String>,
    merge_variant: Option<String>,
    function: String,
}

This works in every aspect, except when you run it. At this time you'll be greeted with this error

error: annot mismatch: List { context : Text, input : Text, name : Text, output : Optional Text, transforms : List { function : Text, mergeVariant : Optional Text,
postVariant : Optional Text, preVariant : Optional Text } } != List { context : Text, input : Text, name : Text, output : Optional Text, transforms : List { function : Text, `merge_variant` : Optional Text, `post_variant` : Optional Text, `pre_variant` : Optional Text } }

This error in of itself (after you've identified the core error) isn't bad, but man it confused me for a long time as I tried to figure out why my renames didn't work. This would be a wonderful addition to serde_dhall since it'd allow seamless serde integration with StaticType

Kixiron avatar Jul 13 '20 23:07 Kixiron

Yeah that'd be awesome!

Nadrieril avatar Jul 14 '20 14:07 Nadrieril

I started looking into fixing this, but for the life of me I cannot figure out what would even break to cause this issue. Renaming is apparently handled entirely on the Serde side, with many popular crates like toml and serde_json having 0 code that handles renames as far as I can tell (searching for "rename" in those crates only yields test cases using the attribute).

I wrote the following test case to reproduce the issue, based on the snippet @Kixiron gave:

use serde::{Deserialize};
use serde_dhall::{from_str, FromDhall, StaticType};

// Test Case taken from https://github.com/Nadrieril/dhall-rust/issues/173
#[derive(Debug, Deserialize, StaticType, PartialEq, Eq)]
#[serde(rename_all = "camelCase")]
struct Transform {
    pre_variant: Option<String>,
    post_variant: Option<String>,
    merge_variant: Option<String>,
    function: String,
}

#[test]
fn test_rename_all_camel_case() {
    let tx = Transform {
        pre_variant: None,
        post_variant: Some(String::from("Test")),
        merge_variant: Some(String::from("Other")),
        function: String::from("lorem ipsum"),
    };

    let test_str = "{
      preVariant = None Text,
      postVariant = Some \"Test\",
      mergeVariant = Some \"Other\",
      function = \"lorem ipsum\"
    }";

    let parsed = from_str(test_str)
        .static_type_annotation()
        .parse::<Transform>()
        .unwrap();

    assert_eq!(tx, parsed);
}

I put this in serde_dhall/tests/rename.rs and got the following test failure output, which is very similar to the error above:

---- test_rename_all_camel_case stdout ----
thread 'test_rename_all_camel_case' panicked at 'called `Result::unwrap()` on an `Err` value: Error(Dhall(Error { kind: Typecheck(TypeError { message: Custom("error: annot mismatch: { function : Text, mergeVariant : Optional Text, postVariant : Optional Text, preVariant : Optional Text } != { function : Text, `merge_variant` : Optional Text, `post_variant` : Optional Text, `pre_variant` : Optional Text }\n --> <current file>:2:1\n  |\n... |\n6 | |     }\n  | |______^ annot mismatch: { function : Text, mergeVariant : Optional Text, postVariant : Optional Text, preVariant : Optional Text } != { function : Text, `merge_variant` : Optional Text, `post_variant` : Optional Text, `pre_variant` : Optional Text }\n  |") }) }))', serde_dhall/tests/rename.rs:33:10

rushsteve1 avatar Sep 13 '20 13:09 rushsteve1

Thanks for the reproducing test case! Renaming is done completely transparently by serde. If you don't use StaticType, then renaming should just work. However if you do derive StaticType automatically, the deriving mechanism does not yet know to look for attributes like #[serde(rename_all = "camelCase")]. That's what would need to be done on the dhall side.

Nadrieril avatar Sep 14 '20 21:09 Nadrieril