feat: implement into serde_json Value for Decimal

[tqwewe]

This PR implements From<Decimal> for serde_json::Value, given serde_json feature flag is enabled (and serde-arbitrary-precision is disabled).

The motivation for this is that my validation with garde has compile errors when used with schemars.

#[derive(JsonSchema, Validate)]
struct Input {
    #[garde(range(min = Decimal::ZERO))]
                     // ^^^^^^^^^^^^^ the trait `From<rust_decimal::Decimal>` is not implemented for `serde_json::Value`
    price: Decimal,
}

[Tony-Samuels]

Using as_f64 doesn't need the trait, and doesn't return an Option, so we don't need the unwrap.

[tqwewe]

Okay. this was copied from the Serialize implementation. But I can try to remove it if necessary... but if it didn't return an Option, then it wouldn't compile? But it compiles with the unwrap.

My intuition told me that these implementations should be essentially the exact same as the Serialize implementations.

[Tony-Samuels]

Is there deliberately no implementation for serde-float with serde-arbitrary-precision?

[tqwewe]

Yes because from what I saw it looked like that combination can potentially error? Which means TryFrom would be needed instead of From?

[Tony-Samuels]

Then this should probably implement TryFrom, instead of From, at which point we can just call Serialize directly and not need to worry about parallel implementations.

[tqwewe]

Just for the serde-float and serde-arbitrary-precision combination? The others will remain the same? Or use serde_json::to_value for all, and unwrap the other two?

[Tony-Samuels]

Ah, I missed that the trait needs From in particular. That's annoying. But yeah, we can't guarantee that we can implement From cleanly, we shouldn't be implementing it with unwrap, and we shouldn't have a From implementation that disappears with additive features.

I'm not sure this really fits if it has to be a From implementation. Maybe you would be better off using a wrapper type for your specific case.

I've not dug into garde though, maybe there's a way to do it with fallible conversions, or without conversions?

[tqwewe]

But yeah, we can't guarantee that we can implement From cleanly, we shouldn't be implementing it with unwrap, and we shouldn't have a From implementation that disappears with additive features.

This one makes sense. Having different feature flags change what trait is implemented in std library is a bit awkward. One solution could be to fallback to Value::String if the arbitrary precision fails.

Something like this:

impl From<Decimal> for Value {
    fn from(d: Decimal) -> Self {
        #[cfg(all(feature = "serde-float", feature = "serde-arbitrary-precision"))]
        {
            if let Ok(n) = serde_json::Number::from_str(&d.to_string()) {
                return Value::Number(n);
            }
            return Value::String(d.to_string());
        }

        #[cfg(all(feature = "serde-float", not(feature = "serde-arbitrary-precision")))]
        {
            return Value::from(d.as_f64());
        }

        #[cfg(not(feature = "serde-float"))]
        {
            return Value::String(d.to_string());
        }
    }
}

[Tony-Samuels]

With arbitrary precision enabled, serde_json::Number is just String, so I think we should be able to set it infallibly?

[tqwewe]

@Tony-Samuels I've pushed a new commit, which is essentially my code above but without trying to parse to serde_json::Number first.

[Tony-Samuels]

Sorry for the delay, I went to look into other options and forgot to respond...

The new implementation goes to Value::String isn't consistent with Serialize, so unfortunately you were probably right in that we need to do from_str. I guess the best we can do is an expect explaining that serde_json with arbitrary-percision shouldn't be able to fail the check, as well as some thorough tests written to validate that the behaviour doesn't change in the future.

[robjtede]

Sounds like proptest/quickcheck territory. I see one of those is already a workspace dep.

[Tony-Samuels]

serde-with-arbitrary-precision already includes serde-json, including with the arbitrary_precision force enabled.

[paupino]

I fixed up the PR so it would compile + added tests (while keeping it consistent with existing patterns - I think a separate PR can sweep up the cfg-if patterns).

Note, this is currently targeting master which is the unstable unreleased version of rust decimal. Is it instead intending to target v1?