ruma / js_int

JavaScript-interoperable integer types for Rust
MIT License
16 stars 8 forks source link

Support deserializing from floating point without fractional component. #24

Closed FSMaxB closed 2 years ago

FSMaxB commented 2 years ago

Currently if you have a JSON value like 42.0 it fails to deserialize to Int or UInt.

Given that a JSON encoder in a language that only supports IEEE754 doubles will not necessarily omit the fractional part like .0 it would be great to support deserializing from those as well.

This feature would be different from lax_deserialize in that it only accepts doubles that only contain an integer value, values like 0.5 would be rejected.

If there is interest in this feature I can provide a pull request.

This can either be added under the serde feature, or a new feature flag, similar to lax_deserialize.

As an example of how this could look like see: https://github.com/communityvi/communityvi/blob/01a0beea2d95fd8256625774bfe3b6fca948e823/communityvi-server/src/utils/portable_unsigned_integer.rs#L50-L63

jplatte commented 2 years ago

Cargo features aren't great for this sort of thing since they are computed once for the entire dependency graph, but since this would be no worse than what we already have with lax_deserialize, I would accept a PR that implements this.

I'd suggest the name float_deserialize. Maybe we should then rename lax_deserialize to lax_float_deserialize in the next breaking release.

To keep the logic simple, I think it would make sense to replace lax_deserialize with float_deserialize in all the existing code, but add a #[cfg(not(feature = "lax_deserialize"))] check in there for .fract() != 0.

jplatte commented 2 years ago

Done in #27.