Repository: ron-rs/ron (rust)

Issue:
Round-trip failure: ser -> de transforms Float(1.0) into Integer(1)
This example program probably ought to exit successfully
```rust
fn main() {
    let input = "1.0";
    let v: ron::Value = ron::from_str(input).unwrap();
    println!("{:?}", v);
    let ser = ron::to_string(&v).unwrap();
    println!("{:?}", ser);
    let roundtrip = ron::from_str(&ser).unwrap();
    println!("{:?}", roundtrip);
    assert_eq!(v, roundtrip);
}
```
But it doesn't:
```
Number(Float(Float(1.0)))
"1"
Number(Integer(1))
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `Number(Float(Float(1.0)))`,
 right: `Number(Integer(1))`', src/main.rs:9:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

This seems plausible as a ser/de behavior if RON only has a number type, but if that's the case `ron::Value::Number` should depend only on the numeric value not the discriminant.

Pull request description:
Enable `decimal_floats` by default for `Value::Number(Float)` roundtrip
* [x] I've included my change in `CHANGELOG.md`

Fixes #337 


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 255b5ce8..f95a0b96 100644
--- a/CHANGELOG.md
+++ b/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
 
diff --git a/src/de/mod.rs b/src/de/mod.rs
index f7e7cce1..6108f088 100644
--- a/src/de/mod.rs
+++ b/src/de/mod.rs
@@ -431,7 +431,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
         self.newtype_variant = false;
 
         if self.bytes.consume("[") {
-            let value = visitor.visit_seq(CommaSeparated::new(b']', &mut self))?;
+            let value = visitor.visit_seq(CommaSeparated::new(b']', self))?;
             self.bytes.comma()?;
 
             if self.bytes.consume("]") {
@@ -452,7 +452,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
             let old_newtype_variant = self.newtype_variant;
             self.newtype_variant = false;
 
-            let value = visitor.visit_seq(CommaSeparated::new(b')', &mut self))?;
+            let value = visitor.visit_seq(CommaSeparated::new(b')', self))?;
             self.bytes.comma()?;
 
             if old_newtype_variant || self.bytes.consume(")") {
@@ -488,7 +488,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
         self.newtype_variant = false;
 
         if self.bytes.consume("{") {
-            let value = visitor.visit_map(CommaSeparated::new(b'}', &mut self))?;
+            let value = visitor.visit_map(CommaSeparated::new(b'}', self))?;
             self.bytes.comma()?;
 
             if self.bytes.consume("}") {
@@ -520,7 +520,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
             let old_newtype_variant = self.newtype_variant;
             self.newtype_variant = false;
 
-            let value = visitor.visit_map(CommaSeparated::new(b')', &mut self))?;
+            let value = visitor.visit_map(CommaSeparated::new(b')', self))?;
             self.bytes.comma()?;
 
             if old_newtype_variant || self.bytes.consume(")") {
diff --git a/src/ser/mod.rs b/src/ser/mod.rs
index b15ed0c1..efa7cf95 100644
--- a/src/ser/mod.rs
+++ b/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,7 +443,7 @@ 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(())
@@ -471,7 +451,7 @@ impl<'a, W: io::Write> ser::Serializer for &'a mut Serializer<W> {
 
     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(())
diff --git a/src/value.rs b/src/value.rs
index 786815e8..0038f2aa 100644
--- a/src/value.rs
+++ b/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());
     }
 }
 


Test patch:
diff --git a/src/ser/tests.rs b/src/ser/tests.rs
index f95de18b..f1056e65 100644
--- a/src/ser/tests.rs
+++ b/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]
diff --git a/tests/337_value_float_roundtrip.rs b/tests/337_value_float_roundtrip.rs
new file mode 100644
index 00000000..66873eae
--- /dev/null
+++ b/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);
+}
diff --git a/tests/floats.rs b/tests/floats.rs
index 5a16f65b..a4280d54 100644
--- a/tests/floats.rs
+++ b/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");
 }
diff --git a/tests/to_string_pretty.rs b/tests/to_string_pretty.rs
index 9379e1db..0e1f93d1 100644
--- a/tests/to_string_pretty.rs
+++ b/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()));
 }