Skip to content

Change error reporting #1302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hovancik wants to merge 1 commit into
base: master
Choose a base branch
from
+198 −10
Open

Change error reporting #1302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions src/de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1399,6 +1399,8 @@ impl<'de, R: Read<'de>> de::Deserializer<'de> for &mut Deserializer<R> {
}
};

let pos = self.read.peek_position();

let value = match peek {
b'n' => {
self.eat_char();
Expand Down Expand Up @@ -1456,11 +1458,9 @@ impl<'de, R: Read<'de>> de::Deserializer<'de> for &mut Deserializer<R> {
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))),
}
}

Expand Down Expand Up @@ -1530,6 +1530,8 @@ impl<'de, R: Read<'de>> de::Deserializer<'de> for &mut Deserializer<R> {
}
};

let pos = self.read.peek_position();

let value = match peek {
b'"' => {
self.eat_char();
Expand All @@ -1544,7 +1546,7 @@ impl<'de, R: Read<'de>> de::Deserializer<'de> for &mut Deserializer<R> {

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))),
}
}

Expand Down Expand Up @@ -1639,6 +1641,8 @@ impl<'de, R: Read<'de>> de::Deserializer<'de> for &mut Deserializer<R> {
}
};

let pos = self.read.peek_position();

let value = match peek {
b'"' => {
self.eat_char();
Expand All @@ -1654,7 +1658,7 @@ impl<'de, R: Read<'de>> de::Deserializer<'de> for &mut Deserializer<R> {

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))),
}
}

Expand Down
184 changes: 184 additions & 0 deletions tests/regression/issue287.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<E: Error>(self, _: &str) -> Result<Self::Value, E> {
Err(E::custom("rejected"))
}
fn visit_bytes<E: Error>(self, _: &[u8]) -> Result<Self::Value, E> {
Err(E::custom("rejected"))
}
fn visit_bool<E: Error>(self, _: bool) -> Result<Self::Value, E> {
Err(E::custom("rejected"))
}
fn visit_unit<E: Error>(self) -> Result<Self::Value, E> {
Err(E::custom("rejected"))
}
fn visit_i64<E: Error>(self, _: i64) -> Result<Self::Value, E> {
Err(E::custom("rejected"))
}
fn visit_u64<E: Error>(self, _: u64) -> Result<Self::Value, E> {
Err(E::custom("rejected"))
}
fn visit_f64<E: Error>(self, _: f64) -> Result<Self::Value, E> {
Err(E::custom("rejected"))
}
fn visit_seq<A: serde::de::SeqAccess<'de>>(self, _: A) -> Result<Self::Value, A::Error> {
Err(Error::custom("rejected"))
}
fn visit_map<A: serde::de::MapAccess<'de>>(self, _: A) -> Result<Self::Value, A::Error> {
Err(Error::custom("rejected"))
}
}

#[derive(Debug)]
struct Str;
impl<'de> Deserialize<'de> for Str {
fn deserialize<D: Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
d.deserialize_str(Rejected).map(|_| Str)
}
}

#[derive(Debug)]
struct Any;
impl<'de> Deserialize<'de> for Any {
fn deserialize<D: Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
d.deserialize_any(Rejected).map(|_| Any)
}
}

#[derive(Debug)]
struct Bytes;
impl<'de> Deserialize<'de> for Bytes {
fn deserialize<D: Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
d.deserialize_bytes(Rejected).map(|_| Bytes)
}
}

// String tests
// 123456789012
// "hello"
// ^ ^
// start end
// (3) (9)

#[test]
fn deserialize_str() {
let err = serde_json::from_str::<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::<Any>(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::<Bytes>(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::<Any>(" 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::<Any>(" 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::<Any>(" 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::<Any>(" 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::<Any>(" -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::<Any>(" 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::<Any>(" [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::<Any>(r#" {"a":1} "#).unwrap_err();
assert_eq!(err.column(), 3, "should point to start of object, not end");
}
6 changes: 3 additions & 3 deletions tests/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ fn test_parse_char() {
test_parse_err::<char>(&[
(
"\"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",
Expand Down Expand Up @@ -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"),
Expand Down