From d63f336d188eb15a4bd8c870e7ee37617923270a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Sat, 23 Apr 2022 01:11:38 -0700 Subject: [PATCH] feat(errors): overhauled error reporting a ton --- src/error.rs | 66 +++++++++++++++++++++++++++++++++++------- src/lib.rs | 28 ++++++++++++++++++ src/parser.rs | 80 +++++++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 155 insertions(+), 19 deletions(-) diff --git a/src/error.rs b/src/error.rs index b21afec..b99f13f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,6 +1,6 @@ use std::num::{ParseFloatError, ParseIntError}; -use miette::Diagnostic; +use miette::{Diagnostic, SourceSpan}; use nom::error::{ContextError, ErrorKind, FromExternalError, ParseError}; use thiserror::Error; @@ -11,16 +11,46 @@ use { }; /// An error that occurs when parsing a KDL document. +/// +/// This error implements [`miette::Diagnostic`] and can be used to display +/// detailed, pretty-printed diagnostic messages when using [`miette::Result`] +/// and the `"pretty"` feature flag for `miette`: +/// +/// ```no_run +/// fn main() -> miette::Result<()> { +/// "foo 1.".parse::()?; +/// Ok(()) +/// } +/// ``` +/// +/// This will display a message like: +/// ```text +/// Error: +/// × Expected valid value. +/// ╭──── +/// 1 │ foo 1. +/// · ─┬ +/// · ╰── invalid float +/// ╰──── +/// help: Floating point numbers must be base 10, and have numbers after the decimal point. +/// ``` #[derive(Debug, Diagnostic, Clone, Eq, PartialEq, Error)] #[error("{kind}")] pub struct KdlError { - #[source_code] /// Source string for the KDL document that failed to parse. + #[source_code] pub input: String, /// Offset in chars of the error. - #[label = "here"] - pub offset: usize, + #[label("{}", label.unwrap_or("here"))] + pub span: SourceSpan, + + /// Label text for this span. Defaults to `"here"`. + pub label: Option<&'static str>, + + /// Suggestion for fixing the parser error. + #[help] + pub help: Option<&'static str>, /// Specific error kind for this parser error. pub kind: KdlErrorKind, @@ -29,26 +59,26 @@ pub struct KdlError { /// A type reprenting additional information specific to the type of error being returned. #[derive(Debug, Diagnostic, Clone, Eq, PartialEq, Error)] pub enum KdlErrorKind { + /// An error occurred while parsing an integer. #[error(transparent)] #[diagnostic(code(kdl::parse_int))] - /// An error occurred while parsing an integer. ParseIntError(ParseIntError), + /// An error occurred while parsing a floating point number. #[error(transparent)] #[diagnostic(code(kdl::parse_float))] - /// An error occurred while parsing a floating point number. ParseFloatError(ParseFloatError), - #[error("Expected {0}.")] - #[diagnostic(code(kdl::parse_component))] /// Generic parsing error. The given context string denotes the component /// that failed to parse. + #[error("Expected {0}.")] + #[diagnostic(code(kdl::parse_component))] Context(&'static str), - #[error("An unspecified error occurred.")] - #[diagnostic(code(kdl::other))] /// Generic unspecified error. If this is returned, the call site should /// be annotated with context, if possible. + #[error("An unspecified error occurred.")] + #[diagnostic(code(kdl::other))] Other, } @@ -64,15 +94,23 @@ pub struct TryFromKdlNodeValueError { pub(crate) struct KdlParseError { pub(crate) input: I, pub(crate) context: Option<&'static str>, + pub(crate) len: usize, + pub(crate) label: Option<&'static str>, + pub(crate) help: Option<&'static str>, pub(crate) kind: Option, + pub(crate) touched: bool, } impl ParseError for KdlParseError { fn from_error_kind(input: I, _kind: nom::error::ErrorKind) -> Self { Self { input, + len: 0, + label: None, + help: None, context: None, kind: None, + touched: false, } } @@ -92,8 +130,12 @@ impl<'a> FromExternalError<&'a str, ParseIntError> for KdlParseError<&'a str> { fn from_external_error(input: &'a str, _kind: ErrorKind, e: ParseIntError) -> Self { KdlParseError { input, + len: 0, + label: None, + help: None, context: None, kind: Some(KdlErrorKind::ParseIntError(e)), + touched: false, } } } @@ -102,8 +144,12 @@ impl<'a> FromExternalError<&'a str, ParseFloatError> for KdlParseError<&'a str> fn from_external_error(input: &'a str, _kind: ErrorKind, e: ParseFloatError) -> Self { KdlParseError { input, + len: 0, + label: None, + help: None, context: None, kind: Some(KdlErrorKind::ParseFloatError(e)), + touched: false, } } } diff --git a/src/lib.rs b/src/lib.rs index 1d42907..09907bb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,6 +6,11 @@ //! You can think of this crate as //! [`toml_edit`](https://crates.io/crates/toml_edit), but for KDL. //! +//! If you don't care about formatting or programmatic manipulation, you +//! should check out [`knuffel`](https://crates.io/crates/knuffel) or +//! [`kaydle`](https://crates.io/crates/kaydle) instead for serde (or +//! serde-like) parsing. +//! //! ## Example //! //! ```rust @@ -57,6 +62,29 @@ //! [`KdlDocument`], [`KdlNode`], [`KdlEntry`], and [`KdlIdentifier`] can all //! be parsed and managed this way. //! +//! +//! This error implements [`miette::Diagnostic`] and can be used to display +//! detailed, pretty-printed diagnostic messages when using [`miette::Result`] +//! and the `"pretty"` feature flag for `miette`: +//! +//! ```no_run +//! fn main() -> miette::Result<()> { +//! "foo 1.".parse::()?; +//! Ok(()) +//! } +//! ``` +//! +//! This will display a message like: +//! ```text +//! Error: +//! × Expected valid value. +//! ╭──── +//! 1 │ foo 1. +//! · ─┬ +//! · ╰── invalid float +//! ╰──── +//! help: Floating point numbers must be base 10, and have numbers after the decimal point. +//! ``` //! ## License //! //! The code in this repository is covered by [the Apache-2.0 diff --git a/src/parser.rs b/src/parser.rs index 9a3b741..e91bbf2 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -24,7 +24,9 @@ where let prefix = &input[..(input.len() - e.input.len())]; KdlError { input: input.into(), - offset: prefix.chars().count(), + span: (prefix.chars().count(), e.len).into(), + help: e.help, + label: e.label, kind: if let Some(kind) = e.kind { kind } else if let Some(ctx) = e.context { @@ -36,6 +38,27 @@ where }) } +fn set_details<'a>( + mut err: nom::Err>, + start: &'a str, + label: Option<&'static str>, + help: Option<&'static str>, +) -> nom::Err> { + match &mut err { + nom::Err::Error(e) | nom::Err::Failure(e) => { + if !e.touched { + e.len = start.offset(e.input); + e.input = start; + e.label = label; + e.help = help; + e.touched = true; + } + } + _ => {} + } + err +} + pub(crate) fn document(input: &str) -> IResult<&str, KdlDocument, KdlParseError<&str>> { let (input, nodes) = many0(node)(input)?; let (input, trailing) = all_whitespace(input)?; @@ -48,12 +71,13 @@ pub(crate) fn document(input: &str) -> IResult<&str, KdlDocument, KdlParseError< pub(crate) fn node(input: &str) -> IResult<&str, KdlNode, KdlParseError<&str>> { let (input, leading) = all_whitespace(input)?; + let start = input; let (input, ty) = opt(context("valid node type annotation", annotation))(input)?; let (input, name) = context("valid node name", identifier)(input)?; let (input, entries) = many0(context("valid node entry", entry))(input)?; let (input, children) = opt(context("valid node children block", children))(input)?; let (input, trailing) = context( - "trailing whitespace after node", + "valid node terminator", cut(recognize(preceded( many0(node_space), alt(( @@ -61,7 +85,15 @@ pub(crate) fn node(input: &str) -> IResult<&str, KdlNode, KdlParseError<&str>> { alt((newline, single_line_comment, eof)), )), ))), - )(input)?; + )(input) + .map_err(|e| { + set_details( + e, + start, + Some("parsed node"), + Some("Nodes can only be terminated by `;` or a valid line ending."), + ) + })?; let mut node = KdlNode::new(name); node.set_leading(leading); node.set_trailing(trailing); @@ -81,10 +113,11 @@ pub(crate) fn identifier(input: &str) -> IResult<&str, KdlIdentifier, KdlParseEr } fn plain_identifier(input: &str) -> IResult<&str, KdlIdentifier, KdlParseError<&str>> { + let start = input; let (input, name) = recognize(preceded( take_while_m_n(1, 1, KdlIdentifier::is_initial_char), cut(take_while(KdlIdentifier::is_identifier_char)), - ))(input)?; + ))(input).map_err(|e| set_details(e, start, Some("invalid identifier character"), Some("See https://github.com/kdl-org/kdl/blob/main/SPEC.md#identifier for an explanation of valid KDL identifiers.")))?; let mut ident = KdlIdentifier::from(name); ident.set_repr(name); Ok((input, ident)) @@ -116,7 +149,7 @@ fn property(input: &str) -> IResult<&str, KdlEntry, KdlParseError<&str>> { let (input, ty) = opt(annotation)(input)?; let (input, name) = identifier(input)?; let (input, _) = context("'=' after property name", tag("="))(input)?; - let (input, (raw, value)) = context("property value", cut(value))(input)?; + let (input, (raw, value)) = context("property value", cut(value))(input).map_err(|e| set_details(e, input, Some("invalid value"), Some("Please refer to https://github.com/kdl-org/kdl/blob/main/SPEC.md#value for valid KDL value syntaxes.")))?; let mut entry = KdlEntry::new_prop(name, value); entry.ty = ty; entry.set_leading(leading); @@ -157,16 +190,20 @@ fn value(input: &str) -> IResult<&str, (String, KdlValue), KdlParseError<&str>> fn children(input: &str) -> IResult<&str, (&str, KdlDocument), KdlParseError<&str>> { let (input, before) = recognize(many0(node_space))(input)?; + let start = input; let (input, _) = tag("{")(input)?; let (input, children) = document(input)?; - let (input, _) = cut(context("closing '}' in node children block", tag("}")))(input)?; + let (input, _) = cut(context("closing '}' in node children block", tag("}")))(input) + .map_err(|e| set_details(e, start, Some("children block body"), None))?; Ok((input, (before, children))) } fn annotation(input: &str) -> IResult<&str, KdlIdentifier, KdlParseError<&str>> { + let start = input; let (input, _) = tag("(")(input)?; let (input, ty) = cut(identifier)(input)?; - let (input, _) = context("closing ')' for type annotation", cut(tag(")")))(input)?; + let (input, _) = context("closing ')' for type annotation", cut(tag(")")))(input) + .map_err(|e| set_details(e, start, Some("annotation"), Some("annotations can only be KDL identifiers (including string identifiers), and can't have any space inside the parentheses.")))?; Ok((input, ty)) } @@ -203,7 +240,7 @@ fn escline(input: &str) -> IResult<&str, &str, KdlParseError<&str>> { alt((single_line_comment, newline)), )), ), - ))(input) + ))(input).map_err(|e| set_details(e, input, Some("line escape starts here"), Some("line escapes can only be followed by whitespace plus a newline (or single-line comment)."))) } fn unicode_space(input: &str) -> IResult<&str, &str, KdlParseError<&str>> { @@ -256,6 +293,7 @@ fn single_line_comment(input: &str) -> IResult<&str, &str, KdlParseError<&str>> context("newline or eof after //", alt((newline, eof))), )), ))(input) + .map_err(|e| set_details(e, input, Some("comment"), None)) } /// `multi-line-comment := '/*' commented-block @@ -264,6 +302,7 @@ fn multi_line_comment(input: &str) -> IResult<&str, &str, KdlParseError<&str>> { tag("/*"), context("comment block body", cut(commented_block)), ))(input) + .map_err(|e| set_details(e, input, Some("comment"), None)) } /// `commented-block := '*/' | (multi-line-comment | '*' | '/' | [^*/]+) commented-block` @@ -285,10 +324,12 @@ fn node_slashdash(input: &str) -> IResult<&str, &str, KdlParseError<&str>> { cut(alt((recognize(entry), recognize(children)))), ), ))(input) + .map_err(|e| set_details(e, input, Some("slashdash"), None)) } fn slashdash_comment(input: &str) -> IResult<&str, &str, KdlParseError<&str>> { recognize(preceded(tag("/-"), cut(node)))(input) + .map_err(|e| set_details(e, input, Some("slashdash"), None)) } fn boolean(input: &str) -> IResult<&str, (String, KdlValue), KdlParseError<&str>> { @@ -313,7 +354,8 @@ fn string(input: &str) -> IResult<&str, (String, KdlValue), KdlParseError<&str>> original.push_str(raw); value.push(processed); } - let (input, _) = cut(tag("\""))(input)?; + let (input, _) = + cut(tag("\""))(input).map_err(|e| set_details(e, input, Some("string"), None))?; original.push('"'); Ok((input, (original, KdlValue::String(value)))) } @@ -407,6 +449,16 @@ fn float(input: &str) -> IResult<&str, (String, KdlValue), KdlParseError<&str>> .map(|x| (raw.into(), KdlValue::Base10Float(x))) }, )(input) + .map_err(|e| { + set_details( + e, + input, + Some("invalid float"), + Some( + "Floating point numbers must be base 10, and have numbers after the decimal point.", + ), + ) + }) } /// ```text @@ -454,6 +506,7 @@ fn hexadecimal(input: &str) -> IResult<&str, (String, KdlValue), KdlParseError<& .map(|x| (raw.clone(), KdlValue::Base16(x))) }, )(input) + .map_err(|e| set_details(e, input, Some("invalid hexadecimal"), Some("Hexadecimal values can only include the characters 0-9 and a-f (case-insensitive), with optional `_` separators."))) } /// `octal := sign? '0o' [0-7] [0-7_]*` @@ -479,6 +532,14 @@ fn octal(input: &str) -> IResult<&str, (String, KdlValue), KdlParseError<&str>> .map(|x| (raw.clone(), KdlValue::Base8(x))) }, )(input) + .map_err(|e| { + set_details( + e, + input, + Some("invalid octal"), + Some("octal values can only include the characters 0-7, with optional `_` separators."), + ) + }) } /// `binary := sign? '0b' ('0' | '1') ('0' | '1' | '_')*` @@ -501,6 +562,7 @@ fn binary(input: &str) -> IResult<&str, (String, KdlValue), KdlParseError<&str>> .map(|x| (raw.clone(), KdlValue::Base2(x))) }, )(input) + .map_err(|e| set_details(e, input, Some("invalid binary"), Some("Hexadecimal values can only include the characters 0 and 1, with optional `_` separators."))) } fn sign(input: &str) -> IResult<&str, i64, KdlParseError<&str>> {