Remove decimal_floats config but always include decimals for Value::Number(Float) roundtrip (#363)

* Enable `decimal_floats` by default for `Value::Number(Float)` roundtrip

* Added Changelog entry

* Removed `decimal_floats` `PrettyConfig` option

Author: juntyr

CHANGELOG.md

@@ -19,7 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fix issue [#301](https://github.com/ron-rs/ron/issues/301) with better error messages ([#354](https://github.com/ron-rs/ron/pull/354))
 - Fix issue [#152](https://github.com/ron-rs/ron/issues/152) bitflags serde ([#352](https://github.com/ron-rs/ron/pull/352))
 - Fix issue [#359](https://github.com/ron-rs/ron/issues/359) with `DeserializeSeed` support ([#360](https://github.com/ron-rs/ron/pull/360))
-- Bump MSRV to 1.46.0
+- Bump MSRV to 1.46.0 ([#361](https://github.com/ron-rs/ron/pull/361))
+- Fix issue [#337](https://github.com/ron-rs/ron/issues/337) by removing `decimal_floats` PrettyConfig option and unconditional decimals in floats ([#363](https://github.com/ron-rs/ron/pull/363))
 
 ## [0.7.0] - 2021-10-22
 

src/ser/mod.rs

@@ -85,8 +85,6 @@ pub struct PrettyConfig {
     pub separate_tuple_members: bool,
     /// Enumerate array items in comments
     pub enumerate_arrays: bool,
-    /// Always include the decimal in floats
-    pub decimal_floats: bool,
     /// Enable extensions. Only configures 'implicit_some',
     ///  'unwrap_newtypes', and 'unwrap_variant_newtypes' for now.
     pub extensions: Extensions,
@@ -170,17 +168,6 @@ impl PrettyConfig {
         self
     }
 
-    /// Configures whether floats should always include a decimal.
-    /// When false `1.0` will serialize as `1`
-    /// When true `1.0` will serialize as `1.0`
-    ///
-    /// Default: `false`
-    pub fn decimal_floats(mut self, decimal_floats: bool) -> Self {
-        self.decimal_floats = decimal_floats;
-
-        self
-    }
-
     /// Configures whether every array should be a single line (true) or a multi line one (false)
     /// When false, `["a","b"]` (as well as any array) will serialize to
     /// `
@@ -223,7 +210,6 @@ impl Default for PrettyConfig {
             separate_tuple_members: false,
             enumerate_arrays: false,
             extensions: Extensions::empty(),
-            decimal_floats: false,
             compact_arrays: false,
         }
     }
@@ -298,12 +284,6 @@ impl<W: io::Write> Serializer<W> {
             .map_or(false, |&(ref config, _)| config.separate_tuple_members)
     }
 
-    fn decimal_floats(&self) -> bool {
-        self.pretty
-            .as_ref()
-            .map_or(false, |&(ref config, _)| config.decimal_floats)
-    }
-
     fn compact_arrays(&self) -> bool {
         self.pretty
             .as_ref()
@@ -463,15 +443,15 @@ impl<'a, W: io::Write> ser::Serializer for &'a mut Serializer<W> {
 
     fn serialize_f32(self, v: f32) -> Result<()> {
         write!(self.output, "{}", v)?;
-        if self.decimal_floats() && (v - v.floor()).abs() < f32::EPSILON {
+        if (v - v.floor()).abs() < f32::EPSILON {
             write!(self.output, ".0")?;
         }
         Ok(())
     }
 
     fn serialize_f64(self, v: f64) -> Result<()> {
         write!(self.output, "{}", v)?;
-        if self.decimal_floats() && (v - v.floor()).abs() < f64::EPSILON {
+        if (v - v.floor()).abs() < f64::EPSILON {
             write!(self.output, ".0")?;
         }
         Ok(())

src/ser/tests.rs

@@ -31,7 +31,7 @@ fn test_empty_struct() {
 fn test_struct() {
     let my_struct = MyStruct { x: 4.0, y: 7.0 };
 
-    assert_eq!(to_string(&my_struct).unwrap(), "(x:4,y:7)");
+    assert_eq!(to_string(&my_struct).unwrap(), "(x:4.0,y:7.0)");
 
     #[derive(Serialize)]
     struct NewType(i32);
@@ -41,7 +41,7 @@ fn test_struct() {
     #[derive(Serialize)]
     struct TupleStruct(f32, f32);
 
-    assert_eq!(to_string(&TupleStruct(2.0, 5.0)).unwrap(), "(2,5)");
+    assert_eq!(to_string(&TupleStruct(2.0, 5.0)).unwrap(), "(2.0,5.0)");
 }
 
 #[test]

src/value.rs

@@ -280,7 +280,7 @@ impl Eq for Float {}
 
 impl Hash for Float {
     fn hash<H: Hasher>(&self, state: &mut H) {
-        state.write_u64(self.0 as u64);
+        state.write_u64(self.0.to_bits());
     }
 }
 

tests/337_value_float_roundtrip.rs

@@ -0,0 +1,29 @@
+#[test]
+fn roundtrip_value_float_with_decimals() {
+    let v: ron::Value = ron::from_str("1.0").unwrap();
+
+    assert_eq!(v, ron::Value::Number(1.0_f64.into()));
+
+    let ser = ron::ser::to_string(&v).unwrap();
+
+    let roundtrip = ron::from_str(&ser).unwrap();
+
+    assert_eq!(v, roundtrip);
+}
+
+#[test]
+#[allow(clippy::float_cmp)]
+fn roundtrip_value_float_into() {
+    let v: ron::Value = ron::from_str("1.0").unwrap();
+    assert_eq!(v, ron::Value::Number(1.0_f64.into()));
+
+    let ser = ron::ser::to_string(&v).unwrap();
+
+    let f1: f64 = ron::from_str(&ser).unwrap();
+    assert_eq!(f1, 1.0_f64);
+
+    let roundtrip: ron::Value = ron::from_str(&ser).unwrap();
+
+    let f2: f64 = roundtrip.into_rust().unwrap();
+    assert_eq!(f2, 1.0_f64);
+}

tests/floats.rs

@@ -1,6 +1,6 @@
 use ron::{
     de::from_str,
-    ser::{to_string_pretty, PrettyConfig},
+    ser::{to_string, to_string_pretty, PrettyConfig},
 };
 
 #[test]
@@ -12,19 +12,9 @@ fn test_inf_and_nan() {
 
 #[test]
 fn decimal_floats() {
-    let pretty = PrettyConfig::new().decimal_floats(false);
-    let without_decimal = to_string_pretty(&1.0, pretty).unwrap();
-    assert_eq!(without_decimal, "1");
+    let non_pretty = to_string(&1.0).unwrap();
+    assert_eq!(non_pretty, "1.0");
 
-    let pretty = PrettyConfig::new().decimal_floats(false);
-    let without_decimal = to_string_pretty(&1.1, pretty).unwrap();
-    assert_eq!(without_decimal, "1.1");
-
-    let pretty = PrettyConfig::new().decimal_floats(true);
-    let with_decimal = to_string_pretty(&1.0, pretty).unwrap();
-    assert_eq!(with_decimal, "1.0");
-
-    let pretty = PrettyConfig::new().decimal_floats(true);
-    let with_decimal = to_string_pretty(&1.1, pretty).unwrap();
-    assert_eq!(with_decimal, "1.1");
+    let with_pretty = to_string_pretty(&1.0, PrettyConfig::new()).unwrap();
+    assert_eq!(with_pretty, "1.0");
 }

tests/to_string_pretty.rs

@@ -14,8 +14,8 @@ fn test_struct_names() {
     let struct_name = to_string_pretty(&value, PrettyConfig::default().struct_names(true));
     assert_eq!(
         struct_name,
-        Ok("Point(\n    x: 1,\n    y: 2,\n)".to_string())
+        Ok("Point(\n    x: 1.0,\n    y: 2.0,\n)".to_string())
     );
     let no_struct_name = to_string(&value);
-    assert_eq!(no_struct_name, Ok("(x:1,y:2)".to_string()));
+    assert_eq!(no_struct_name, Ok("(x:1.0,y:2.0)".to_string()));
 }