Repository navigation
fix(wkt): accept arbitrary_precision numbers in float fields - #6991
josh-brown-anchor wants to merge 1 commit into
Conversation
With serde_json's `arbitrary_precision` feature enabled anywhere in the dependency graph, serde_json hands non-integer numbers to `deserialize_any` as a single-entry map. The ProtoJSON float visitors rejected that map, so any fractional `float`/`double` field failed to parse. Decode the map through `serde_json::Number`.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for deserializing float values (both f32 and f64) when serde_json's arbitrary_precision feature is enabled. This is achieved by implementing visit_map in the float visitor macro to handle the single-entry map representation of numbers, using serde_json::Number and MapAccessDeserializer. Additionally, comprehensive unit and integration tests have been added to verify correct deserialization and error handling from JSON text. I have no feedback to provide.
|
/gcbrun |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6991 +/- ##
==========================================
- Coverage 97.07% 97.07% -0.01%
==========================================
Files 326 326
Lines 113032 113065 +33
==========================================
+ Hits 109730 109758 +28
- Misses 3302 3307 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
suzmue
left a comment
suzmue
left a comment
There was a problem hiding this comment.
Thanks for investigating and contributing this fix, LGTM!
I tested this locally on main with --features serde_json/arbitrary_precision and confirmed that direct JSON text deserialization of non-integer floats indeed failed with invalid type: map, expected a 32-bit float in ProtoJSON format when the feature was enabled.
Possible future improvement: we should consider additional integration tests with different feature combos for this crate.
This branch has not been deployed
3 participants
Fixes #6990
With serde_json's
arbitrary_precisionfeature enabled anywhere in the dependency graph, serde_json hands non-integer numbers todeserialize_anyas a single-entry map. The ProtoJSON float visitors rejected that map, so any fractionalfloat/doublefield failed withinvalid type: map, expected a 32-bit float in ProtoJSON format.This adds
visit_mapto the visitors invisitor_float.rs, decoding the map throughserde_json::Numberand reusing thevisit_f64range check. Without the feature,serde_json::Numberrejects the map, so behavior is unchanged.The new tests parse from JSON text (
serde_json::Deserializer::from_str/from_str), the path the feature changes; the existing tests usefrom_value, which is unaffected. They pass with and without--features serde_json/arbitrary_precision, and fail without the fix when the feature is on.