Deserializer does not respect the "hints" from the `deserialize_xxx` functions

[TroyKomodo]

According to the documentation on the functions,

/// Hint that the `Deserialize` type is expecting a string value and does
/// not benefit from taking ownership of buffered data owned by the
/// `Deserializer`.
///
/// If the `Visitor` would benefit from taking ownership of `String` data,
/// indicate this to the `Deserializer` by using `deserialize_string`
/// instead.
fn deserialize_str<V>(self, visitor: V) -> Result<V::Value, Self::Error>
where
    V: Visitor<'de>;

The deserializer should take a "hint" that the type is a string. To me this reads as, we should try convert our input into a string if its not a string, otherwise if thats not possible give the visitor whatever we have.

This is then further implied by the Visitor traits all having default impls for every method that just return an error based on the expected impl.

However serde_json::Deserializer does not follow this behavior.

json/src/de.rs

Lines 1533 to 1543 in 8a56cfa

	let value = match peek {
	b'"' => {
	self.eat_char();
	self.scratch.clear();
	match tri!(self.read.parse_str(&mut self.scratch)) {
	Reference::Borrowed(s) => visitor.visit_borrowed_str(s),
	Reference::Copied(s) => visitor.visit_str(s),
	}
	}
	_ => Err(self.peek_invalid_type(&visitor)),
	};

Instead serde_json returns a hard error with out invoking the visitor and letting the visitor handle values it didnt expect.

[dtolnay]

this reads as, we should try convert our input into a string if its not a string

Do you mean that if the input JSON contains null, you want it to call visit_string with the string "null", and if the input contains an integer, call visit_string with the string representation of the integer, etc?

That is not what the documentation means. The documentation means if the input does not contain something that would ordinarily be deserialized as a string (i.e. a string), do not even bother calling any visitor method.

[TroyKomodo]

No, I mean rather if value is a list we call visit sequence or if it contains a map we call visit map, and then the visitor will return the unexpected error if it cannot handle it. The reason being is if we cannot deserialize this and it happens to be a list we leave the deserializer in an unrecoverable state.

The case is where we expect a string but the provided value is a list.

Essentially no matter what if we are unexpected pass the call to deserialize_any

[dtolnay]

If you want that behavior you should call deserialize_any. What you have described is what deserialize_any does.

[TroyKomodo]

That doesn't work because some deserializers do conversions on the value depending on the deserialize method that's called.

This library does this for numbers if we want an i8 and we want deserializer to convert it for us then it will. If we call deserialize any we get back the i64 or whatever number serde_json uses internally to represent the json number. I find that most deserializer implementations have some format specific conversions which take advantage of knowing the underlying type making it really hard to for us to convert after calling deserialize_any.

The problem I'm trying to fix is after the deserialize fails due to a mismatched type; depending on what type was provided in the input json the deserializer is left in an invalid state.

{ "name": [], "age": 20 }

If the struct is

struct Person {
    name: String,
    age: u8,
}

In terms of events:

• deserialize is called on person
• visit map is called
• deserialize identifier returns name
• we call String::deserialize which calls deserialize_string
• serde json returns an error

At this point we bubble up to visit map which is in the person struct; our options now are either call deserialize_(key or value). Neither of them work because the serde json deserializer is now in a state which is irrecoverable.

[TroyKomodo]

serde-rs/serde#2811 heavily agree with this PR