Repository: serde-rs/json (language: rust)

Issue:
`arbitrary_precision` Breaks Visitors
I'm not sure if I'm doing something wrong here, but I'm trying to write a type that can deserialize from either strings or integers. 

Initially I did this using an `untagged` enumeration variant to do the heavy lifting - see [here](https://github.com/influxdata/pbjson/blob/2662672c38c4fe18cd5225ea44ea3fdc08dc9b8b/pbjson/src/lib.rs#L30).

This worked fine until an upstream crate enabled the `arbitrary_precision` feature. I found https://github.com/serde-rs/json/issues/559 and https://github.com/serde-rs/serde/issues/1183 on the topic, and so opted to implement a custom visitor as these seem to suggest the issue was in the untagged variant generation.

I therefore tried using a custom visitor

```
struct NumberVisitor<T> {
    _phantom: PhantomData<T>,
}

impl<'de, T> serde::de::Visitor<'de> for NumberVisitor<T>
where
    T: FromStr,
    <T as FromStr>::Err: std::error::Error,
    T: FromPrimitive,
{
    type Value = NumberDeserialize<T>;

    fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        formatter.write_str("an integer or string")
    }

    fn visit_i64<E>(self, v: i64) -> Result<Self::Value, E>
    where
        E: Error,
    {
        T::from_i64(v)
            .map(NumberDeserialize)
            .ok_or_else(|| serde::de::Error::custom("cannot parse from i64"))
    }

    fn visit_u64<E>(self, v: u64) -> Result<Self::Value, E>
    where
        E: Error,
    {
        T::from_u64(v)
            .map(NumberDeserialize)
            .ok_or_else(|| serde::de::Error::custom("cannot parse from u64"))
    }

    fn visit_f64<E>(self, v: f64) -> Result<Self::Value, E>
    where
        E: Error,
    {
        T::from_f64(v)
            .map(NumberDeserialize)
            .ok_or_else(|| serde::de::Error::custom("cannot parse from f64"))
    }

    fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
    where
        E: Error,
    {
        v.parse()
            .map(NumberDeserialize)
            .map_err(serde::de::Error::custom)
    }
}

impl<'de, T> serde::Deserialize<'de> for NumberDeserialize<T>
where
    T: FromStr,
    <T as FromStr>::Err: std::error::Error,
    T: FromPrimitive,
{
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: serde::Deserializer<'de>,
    {
        deserializer.deserialize_any(NumberVisitor {
            _phantom: Default::default(),
        })
    }
}
```

However, this runs into an issue where with `arbitrary_precision`, rather than calling visit_i* or similar, it calls visit_map with a custom map visitor for numeric types?! It would appear this then has a special-cased key of `$serde_json::private::Number`. 

I'm not really sure what I'm supposed to do here, it would appear enabling `arbitrary_precision` makes it so that serializers don't decode numbers as numbers? Any help would be most appreciated.

As an aside whilst messing around with this I noticed enabling `arbitrary_precision` makes this fail

```
let input = r#"{"$serde_json::private::Number":"hello"}"#;
let value = serde_json::from_str::<serde_json::Value>(input).unwrap();
```


Discussion / hints:

Fix patch:
diff --git a/src/de.rs b/src/de.rs
index b4c5ef7dc..bf103f04b 100644
--- a/src/de.rs
+++ b/src/de.rs
@@ -864,6 +864,15 @@ impl<'de, R: Read<'de>> Deserializer<R> {
             buf.push('-');
         }
         self.scan_integer(&mut buf)?;
+        if positive {
+            if let Ok(unsigned) = buf.parse() {
+                return Ok(ParserNumber::U64(unsigned));
+            }
+        } else {
+            if let Ok(signed) = buf.parse() {
+                return Ok(ParserNumber::I64(signed));
+            }
+        }
         Ok(ParserNumber::String(buf))
     }
 
diff --git a/src/lib.rs b/src/lib.rs
index c36876da3..0a815747c 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -303,6 +303,7 @@
 #![doc(html_root_url = "https://docs.rs/serde_json/1.0.74")]
 // Ignored clippy lints
 #![allow(
+    clippy::collapsible_else_if,
     clippy::comparison_chain,
     clippy::deprecated_cfg_attr,
     clippy::doc_markdown,


Test patch:
diff --git a/tests/regression/issue845.rs b/tests/regression/issue845.rs
new file mode 100644
index 000000000..dcca55694
--- /dev/null
+++ b/tests/regression/issue845.rs
@@ -0,0 +1,72 @@
+use serde::{Deserialize, Deserializer};
+use std::convert::TryFrom;
+use std::fmt::{self, Display};
+use std::marker::PhantomData;
+use std::str::FromStr;
+
+pub struct NumberVisitor<T> {
+    marker: PhantomData<T>,
+}
+
+impl<'de, T> serde::de::Visitor<'de> for NumberVisitor<T>
+where
+    T: TryFrom<u64> + TryFrom<i64> + FromStr,
+    <T as TryFrom<u64>>::Error: Display,
+    <T as TryFrom<i64>>::Error: Display,
+    <T as FromStr>::Err: Display,
+{
+    type Value = T;
+
+    fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+        formatter.write_str("an integer or string")
+    }
+
+    fn visit_u64<E>(self, v: u64) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        T::try_from(v).map_err(serde::de::Error::custom)
+    }
+
+    fn visit_i64<E>(self, v: i64) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        T::try_from(v).map_err(serde::de::Error::custom)
+    }
+
+    fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        v.parse().map_err(serde::de::Error::custom)
+    }
+}
+
+fn deserialize_integer_or_string<'de, D, T>(deserializer: D) -> Result<T, D::Error>
+where
+    D: Deserializer<'de>,
+    T: TryFrom<u64> + TryFrom<i64> + FromStr,
+    <T as TryFrom<u64>>::Error: Display,
+    <T as TryFrom<i64>>::Error: Display,
+    <T as FromStr>::Err: Display,
+{
+    deserializer.deserialize_any(NumberVisitor {
+        marker: PhantomData,
+    })
+}
+
+#[derive(Deserialize, Debug)]
+pub struct Struct {
+    #[serde(deserialize_with = "deserialize_integer_or_string")]
+    pub i: i64,
+}
+
+#[test]
+fn test() {
+    let j = r#" {"i":100} "#;
+    println!("{:?}", serde_json::from_str::<Struct>(j).unwrap());
+
+    let j = r#" {"i":"100"} "#;
+    println!("{:?}", serde_json::from_str::<Struct>(j).unwrap());
+}
diff --git a/tests/test.rs b/tests/test.rs
index 054e96088..bfcb5290f 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -715,11 +715,7 @@ fn test_parse_char() {
         ),
         (
             "10",
-            if cfg!(feature = "arbitrary_precision") {
-                "invalid type: number, expected a character at line 1 column 2"
-            } else {
-                "invalid type: integer `10`, expected a character at line 1 column 2"
-            },
+            "invalid type: integer `10`, expected a character at line 1 column 2",
         ),
     ]);
 
@@ -1203,11 +1199,7 @@ fn test_parse_struct() {
     test_parse_err::<Outer>(&[
         (
             "5",
-            if cfg!(feature = "arbitrary_precision") {
-                "invalid type: number, expected struct Outer at line 1 column 1"
-            } else {
-                "invalid type: integer `5`, expected struct Outer at line 1 column 1"
-            },
+            "invalid type: integer `5`, expected struct Outer at line 1 column 1",
         ),
         (
             "\"hello\"",