Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

Hint to deserialize number as a string instead, inside untagged enum #165

Open
pwoolcoc opened this issue Jun 11, 2020 · 8 comments
Open

Comments

@pwoolcoc
Copy link

I have a data structure like this:

struct Foo {
    // ...
    foos: BTreeMap<String, usize>,
    // ...
}

And sometimes, the keys for that map can be numbers, but I'd like them deserialized as strings like the other keys.

For example:

foo:
    foos:
        bar: 42
        baz: 123
        200: 321

I'd like to end up with

BTreeMap(
    "bar" => 42,
    "baz" => 123,
    "200" => 321,
)

but instead I, understandably, get a deserialization erorr since 200 is a number.

Is there any way to hint to serde_yaml that the 200 should be deserialized as a string, or do I need to make a new type that performs these conversions in it's Deserialize impl?

@dtolnay
Copy link
Owner

dtolnay commented Jun 11, 2020

I can't reproduce this. Next time please provide a minimal compilable code example when opening an issue.

use serde::Deserialize;
use std::collections::BTreeMap;

#[derive(Deserialize, Debug)]
struct Foo {
    foos: BTreeMap<String, usize>,
}

const Y: &str = "
foos:
    bar: 42
    baz: 123
    200: 321";

fn main() {
    println!("{:#?}", serde_yaml::from_str::<Foo>(Y).unwrap());
}
Foo {
    foos: {
        "200": 321,
        "bar": 42,
        "baz": 123,
    },
}

@pwoolcoc
Copy link
Author

pwoolcoc commented Jun 15, 2020

@dtolnay sorry for the long wait, it took me a bit of debugging to get my complex example down to a minimal reproducible case. It looks like this problem only happens when an untagged enum is involved. Here's the code (playgound link):

use std::collections::BTreeMap;
use serde_yaml;
use serde::Deserialize;

fn main() {
    const Y: &str = "
foos:
    bar: 42
    baz: 123
    200: 321";
    
    const Z: &str = "
foos:
    bar: 42
    baz: 123
    '200': 321";

    // This successfully deserializes
    let foo: Foo = serde_yaml::from_str(&Y).expect("Couldn't deserialize");

    // However, when wrapped in an untagged enum, it does not:
    // 
    // swap these two lines to see that the "Z" document successfully deserializes,
    // while the "Y" document does not
    let foo: Wrapper = serde_yaml::from_str(&Y).expect("Couldn't deserialize");
    //let foo: Wrapper = serde_yaml::from_str(&Z).expect("Couldn't deserialize");
}

#[derive(Debug, Deserialize, PartialEq)]
#[serde(untagged)]
enum Wrapper {
  Foo(Foo),
}

#[derive(Debug, Deserialize, PartialEq)]
struct Foo {
  foos: BTreeMap<String, usize>,
}

@pwoolcoc
Copy link
Author

@dtolnay any chance you could take a look at this?

@dtolnay
Copy link
Owner

dtolnay commented Jul 5, 2020

I have not gotten a chance, but I would accept a PR if there is a fix in serde_yaml.

@jlgerber
Copy link

jlgerber commented Aug 2, 2020

i have a similar problem whereby I am storing a semantic version or version range in a yaml field. Examples:

version: 1
version: 1.2
verison: 1.2.33

I ended up creating an enum with something like:

Version {
   Major(u32),
   Minor(f32),
   Patch(String)
}

so that it would deserialize. I then implemented various convenience traits to make it simpler to work with. However, I would love to be able to decorate the field with a type hint that makes it deserialize to a String.

@jrray
Copy link

jrray commented Aug 13, 2021

i have a similar problem whereby I am storing a semantic version or version range in a yaml field. Examples:

version: 1
version: 1.2
verison: 1.2.33

Change your example to:

version: 1
version: 1.20
verison: 1.20.33

and you'll see the problem we're facing: you'll end up with Version::Minor(1.2) but we want to parse as a string to preserve "1.20".

@dtolnay dtolnay changed the title hint to deserialize number as a string instead? Hint to deserialize number as a string instead, inside untagged enum Jul 28, 2022
@mxndtaylor
Copy link

mxndtaylor commented Aug 30, 2022

Here is a minimal example of a similar issue, but instead caused by the use of #[serde(flatten)] on structs:
https://gist.github.com/mxndtaylor/488db8ca053d8d69e72fa0fc4f5d0a23 (playground)

Is it possible this has the same root cause? The functionality of flatten and untagged seem sort of similar from a outside perspective.

@arlyon
Copy link

arlyon commented Oct 26, 2023

Another option to resolving this is perhaps to implement a RawValue API for serde_yaml similar to that of serde_json which would surface the raw string for parsing. Some yaml users (looking at you yarn) do not know how to properly quote their strings and RawValue while more verbose would give a general-purpose escape hatch to deal with this.

chris-olszewski added a commit to vercel/turborepo that referenced this issue Oct 30, 2023
### Description

Fixes #6232

This PR allows us to properly deserialize semver versions from YAML in
`yarn.lock`. Previously we would fail at parsing ranges with trailing
zeros e.g. `(0.10f32).to_string() == "0.1"`.

The approach taken in this PR is due to some outstanding quirks in
`serde_yaml`:
-
dtolnay/serde-yaml#165 (comment)
- dtolnay/serde-yaml#388
Our usage of `#[serde(flatten)]` in `LockfileData` caused attempting to
parse `2` or `0.10` as a `String` to fail. To avoid this we first parse
the document as a map with a union of the metadata/package entries and
then convert this to the expected structure. This provides us the type
safety of the old implementation, but at the cost of rebuilding the map.

As a minor thing, I removed all unused `Serialize`/`Deserialize`
implementations to make it clear which codepaths actually get used.

### Testing Instructions

Existing unit tests pass.

I changed out the old unit tests for `SemverString` to be captured by
the new `berry_semver.lock` test fixture which covers the same cases. We
do this because even if parsing versions works when invoked directly,
adding `#[serde(flatten)]`/`#[serde(untagged)]` to any containing
structure changes the behavior.


Closes TURBO-1540

---------

Co-authored-by: Chris Olszewski <Chris Olszewski>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants