Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for renaming fields and containers in StaticType #173

Open
Kixiron opened this issue Jul 13, 2020 · 3 comments
Open

Support for renaming fields and containers in StaticType #173

Kixiron opened this issue Jul 13, 2020 · 3 comments
Labels
help wanted Extra attention is needed

Comments

@Kixiron
Copy link

Kixiron commented Jul 13, 2020

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 Nadrieril added the help wanted Extra attention is needed label Jul 14, 2020
@Nadrieril
Copy link
Owner

Yeah that'd be awesome!

@rushsteve1
Copy link

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
Copy link
Owner

Nadrieril commented Sep 14, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants