diff --git a/src/de.rs b/src/de.rs index 4080c54ac..d29a9bbf2 100644 --- a/src/de.rs +++ b/src/de.rs @@ -1399,6 +1399,8 @@ impl<'de, R: Read<'de>> de::Deserializer<'de> for &mut Deserializer { } }; + let pos = self.read.peek_position(); + let value = match peek { b'n' => { self.eat_char(); @@ -1456,11 +1458,9 @@ impl<'de, R: Read<'de>> de::Deserializer<'de> for &mut Deserializer { match value { Ok(value) => Ok(value), // The de::Error impl creates errors with unknown line and column. - // Fill in the position here by looking at the current index in the - // input. There is no way to tell whether this should call `error` - // or `peek_error` so pick the one that seems correct more often. - // Worst case, the position is off by one character. - Err(err) => Err(self.fix_position(err)), + // Fill in the position here by looking at the saved position from + // before the value was consumed. + Err(err) => Err(err.fix_position(|code| Error::syntax(code, pos.line, pos.column))), } } @@ -1530,6 +1530,8 @@ impl<'de, R: Read<'de>> de::Deserializer<'de> for &mut Deserializer { } }; + let pos = self.read.peek_position(); + let value = match peek { b'"' => { self.eat_char(); @@ -1544,7 +1546,7 @@ impl<'de, R: Read<'de>> de::Deserializer<'de> for &mut Deserializer { match value { Ok(value) => Ok(value), - Err(err) => Err(self.fix_position(err)), + Err(err) => Err(err.fix_position(|code| Error::syntax(code, pos.line, pos.column))), } } @@ -1639,6 +1641,8 @@ impl<'de, R: Read<'de>> de::Deserializer<'de> for &mut Deserializer { } }; + let pos = self.read.peek_position(); + let value = match peek { b'"' => { self.eat_char(); @@ -1654,7 +1658,7 @@ impl<'de, R: Read<'de>> de::Deserializer<'de> for &mut Deserializer { match value { Ok(value) => Ok(value), - Err(err) => Err(self.fix_position(err)), + Err(err) => Err(err.fix_position(|code| Error::syntax(code, pos.line, pos.column))), } } diff --git a/tests/regression/issue287.rs b/tests/regression/issue287.rs new file mode 100644 index 000000000..f114f2edf --- /dev/null +++ b/tests/regression/issue287.rs @@ -0,0 +1,184 @@ +// When a Visitor rejects a value, the error should point to the +// start of the value, not the end. + +use serde::de::{Deserialize, Deserializer, Error, Visitor}; + +#[derive(Debug)] +struct Rejected; + +impl<'de> Visitor<'de> for Rejected { + type Value = Rejected; + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("...") + } + fn visit_str(self, _: &str) -> Result { + Err(E::custom("rejected")) + } + fn visit_bytes(self, _: &[u8]) -> Result { + Err(E::custom("rejected")) + } + fn visit_bool(self, _: bool) -> Result { + Err(E::custom("rejected")) + } + fn visit_unit(self) -> Result { + Err(E::custom("rejected")) + } + fn visit_i64(self, _: i64) -> Result { + Err(E::custom("rejected")) + } + fn visit_u64(self, _: u64) -> Result { + Err(E::custom("rejected")) + } + fn visit_f64(self, _: f64) -> Result { + Err(E::custom("rejected")) + } + fn visit_seq>(self, _: A) -> Result { + Err(Error::custom("rejected")) + } + fn visit_map>(self, _: A) -> Result { + Err(Error::custom("rejected")) + } +} + +#[derive(Debug)] +struct Str; +impl<'de> Deserialize<'de> for Str { + fn deserialize>(d: D) -> Result { + d.deserialize_str(Rejected).map(|_| Str) + } +} + +#[derive(Debug)] +struct Any; +impl<'de> Deserialize<'de> for Any { + fn deserialize>(d: D) -> Result { + d.deserialize_any(Rejected).map(|_| Any) + } +} + +#[derive(Debug)] +struct Bytes; +impl<'de> Deserialize<'de> for Bytes { + fn deserialize>(d: D) -> Result { + d.deserialize_bytes(Rejected).map(|_| Bytes) + } +} + +// String tests +// 123456789012 +// "hello" +// ^ ^ +// start end +// (3) (9) + +#[test] +fn deserialize_str() { + let err = serde_json::from_str::(r#" "hello" "#).unwrap_err(); + assert_eq!(err.column(), 3, "should point to start of string, not end"); +} + +#[test] +fn deserialize_any_string() { + let err = serde_json::from_str::(r#" "hello" "#).unwrap_err(); + assert_eq!(err.column(), 3, "should point to start of string, not end"); +} + +#[test] +fn deserialize_bytes() { + let err = serde_json::from_str::(r#" "hello" "#).unwrap_err(); + assert_eq!(err.column(), 3, "should point to start of string, not end"); +} + +// Null test +// 12345678 +// null +// ^ ^ +// start end +// (3) (6) + +#[test] +fn deserialize_any_null() { + let err = serde_json::from_str::(" null ").unwrap_err(); + assert_eq!(err.column(), 3, "should point to start of null, not end"); +} + +// Bool tests +// 123456789 +// true +// ^ ^ +// (3)(6) + +#[test] +fn deserialize_any_true() { + let err = serde_json::from_str::(" true ").unwrap_err(); + assert_eq!(err.column(), 3, "should point to start of true, not end"); +} + +// 1234567890 +// false +// ^ ^ +// (3) (7) + +#[test] +fn deserialize_any_false() { + let err = serde_json::from_str::(" false ").unwrap_err(); + assert_eq!(err.column(), 3, "should point to start of false, not end"); +} + +// Number tests +// 12345678 +// 123 +// ^ ^ +// (3)(5) + +#[test] +fn deserialize_any_positive_int() { + let err = serde_json::from_str::(" 123 ").unwrap_err(); + assert_eq!(err.column(), 3, "should point to start of number, not end"); +} + +// 123456789 +// -456 +// ^ ^ +// (3)(6) + +#[test] +fn deserialize_any_negative_int() { + let err = serde_json::from_str::(" -456 ").unwrap_err(); + assert_eq!(err.column(), 3, "should point to start of number, not end"); +} + +// 1234567890 +// 3.14 +// ^ ^ +// (3)(6) + +#[test] +fn deserialize_any_float() { + let err = serde_json::from_str::(" 3.14 ").unwrap_err(); + assert_eq!(err.column(), 3, "should point to start of number, not end"); +} + +// Array test +// 123456789 +// [1,2] +// ^ ^ +// (3) (6) + +#[test] +fn deserialize_any_array() { + let err = serde_json::from_str::(" [1,2] ").unwrap_err(); + assert_eq!(err.column(), 3, "should point to start of array, not end"); +} + +// Object test +// 12345678901234 +// {"a":1} +// ^ ^ +// (3) (9) + +#[test] +fn deserialize_any_object() { + let err = serde_json::from_str::(r#" {"a":1} "#).unwrap_err(); + assert_eq!(err.column(), 3, "should point to start of object, not end"); +} diff --git a/tests/test.rs b/tests/test.rs index f608679e6..cc67ae8a7 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -727,7 +727,7 @@ fn test_parse_char() { test_parse_err::(&[ ( "\"ab\"", - "invalid value: string \"ab\", expected a character at line 1 column 4", + "invalid value: string \"ab\", expected a character at line 1 column 1", ), ( "10", @@ -1308,9 +1308,9 @@ fn test_parse_enum_errors() { ("{}", "expected value at line 1 column 2"), ("[]", "expected value at line 1 column 1"), ("\"unknown\"", - "unknown variant `unknown`, expected one of `Dog`, `Frog`, `Cat`, `AntHive` at line 1 column 9"), + "unknown variant `unknown`, expected one of `Dog`, `Frog`, `Cat`, `AntHive` at line 1 column 1"), ("{\"unknown\":null}", - "unknown variant `unknown`, expected one of `Dog`, `Frog`, `Cat`, `AntHive` at line 1 column 10"), + "unknown variant `unknown`, expected one of `Dog`, `Frog`, `Cat`, `AntHive` at line 1 column 2"), ("{\"Dog\":", "EOF while parsing a value at line 1 column 7"), ("{\"Dog\":}", "expected value at line 1 column 8"), ("{\"Dog\":{}}", "invalid type: map, expected unit at line 1 column 7"),