Nadrieril / dhall-rust

Maintainable configuration files, for Rust users
Other
303 stars 27 forks source link

Support for renaming fields and containers in StaticType #173

Open Kixiron opened 4 years ago

Kixiron commented 4 years ago

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

Nadrieril commented 4 years ago

Yeah that'd be awesome!

rushsteve1 commented 4 years ago

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
Nadrieril commented 4 years ago

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.