Implement TryInto for Value

[phayes]

Fixes #902

This PR implements TryInto for converting Value into common types such as String, str, u64, f64, i64, (), Vec and Map. I've implemented these both for owned Value and borrowed Value.

Justification and Use Case

I know there's been some skepticism from the maintainer about the need for TryInto impls when one can use the full deserialization machinery to retrieve a concrete typed value from a Value, but I'd like to justify this PR by pointing to both the ergonomics of having TryInto and the efficiency of TryInto over full deserialization (especially when borrowing).

For my use-case, I have a struct that delays fully deserializing all values until much later in the program.

struct Job {
    pub name: String,
    pub inputs: HashMap<String, Value>
}

Much deeper in the code, inputs get wrapped into a container like so:

pub struct Inputs {
    inner: HashMap<String, Value>
}

impl Inputs {

    pub fn get_required<'a, T: ?Sized>(&'a self, key: &str) -> Result<&'a T, Error>
        where &'a Value: TryInto<&'a T>
    {
        match self.inner.get(key) {
            Some(value) => {
                value.try_into()?
            },
            None => Err(Error)
        }
    }

    pub fn get_with_default<'a, 'default: 'a, T: ?Sized>(&'a self, key: &str, default: &'default T) -> Result<&'a T, Error> 
        where &'a Value: TryInto<&'a T> 
    {
        match self.inner.get(key) {
            Some(value) => {
                value.try_into()?
            },
            None => {
                Ok(default)
            }
        }
    }
}

All of this machinery provides a really nice interface to calling code, that can simply call into Inputs like so:

let foo: &str = inputs.get_required("foo")?;
let bar: &str = inputs.get_with_default("bar", "baz")?;

And importantly, because inputs can be quite large, everything is done via borrows, avoiding unnecessary cloning and allocations.

This is my specific use-case, but I'm sure there are other use-cases where having reasonable and straightforward TryInto impls for Value would be ergonomic and timesaving.

Thank you.

[kangalio]

TryInto shouldn't be implemented explicitly, you should implement TryFrom instead. See https://doc.rust-lang.org/std/convert/trait.TryInto.html#implementing-tryinto

Also, TryInto/TryFrom in general is the wrong choice here. Similarly to From/Into, TryFrom/TryInto should be used to convert between types that are semantically equivalent, but differ only in programmatical representation. For example, std implements TryFrom to convert integer<->integer and list<->list.

Instead, there should be into_xxx() methods on Value, like the existing as_xxx() methods.

[SabrinaJewson]

I don’t see why you can’t just use T: Deserialize in the code example you posted. &Value is a Deserializer so it should just work.

Edit: Nevermind, I misread. Even so, it does feel like the kind of thing that should be done strongly-typed to begin with.

[phayes]

Edit: Nevermind, I misread. Even so, it does feel like the kind of thing that should be done strongly-typed to begin with.

Strong typing is definitely preferred all-else-equal, but in this particular case it really does make more sense to keep it loosely typed until later in the program. For my use-case, we don't have enough information up-front to know what the shape of the data should be. Once we know what the shape should be, then we use more well-defined types.

[forrestli74]

Also, TryInto/TryFrom in general is the wrong choice here. Similarly to From/Into, TryFrom/TryInto should be used to convert between types that are semantically equivalent, but differ only in programmatical representation. For example, std implements TryFrom to convert integer<->integer and list<->list.

FWIW, using u64 as an example, we already have Value implementing TryFrom, which breaks this principle. I think it's only fair to stay consistent. (Which is to either add TryInto or remove TryFrom).