feat: add location information to markdown parsing errors

Sponsored-by: author

3 files changed, 153 insertions, 64 deletions

diff --git a/src/error.rs b/src/error.rs
index f7dfe52..e6514b1 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -1,4 +1,4 @@
-use crate::html::HtmlError;
+use crate::html::{HtmlError, Location};
 use crate::matches::MatchedSteps;
 use crate::md::MdError;
 
@@ -148,8 +148,8 @@ pub enum SubplotError {
     /// scenario block before the first heading in the document.
     ///
     /// To fix, add a heading or move the scenario after a heading.
-    #[error("first scenario is before first heading")]
-    ScenarioBeforeHeading,
+    #[error("{0}: first scenario is before first heading")]
+    ScenarioBeforeHeading(Location),
 
     /// Step does not have a keyword.
     #[error("step has no keyword: {0}")]
diff --git a/src/html.rs b/src/html.rs
index 7237258..b39872b 100644
--- a/src/html.rs
+++ b/src/html.rs
@@ -72,7 +72,7 @@ impl HtmlPage {
 }
 
 /// Parse Markdown text into an HTML element.
-pub fn parse(markdown: &str) -> Result<Element, HtmlError> {
+pub fn parse(filename: &Path, markdown: &str) -> Result<Element, HtmlError> {
     let mut options = Options::empty();
     options.insert(Options::ENABLE_HEADING_ATTRIBUTES);
     options.insert(Options::ENABLE_STRIKETHROUGH);
@@ -85,7 +85,7 @@ pub fn parse(markdown: &str) -> Result<Element, HtmlError> {
     for (event, loc) in p {
         trace!("event {:?}", event);
         let (line, col) = linecol.get(loc.start);
-        let loc = Location::new(line, col);
+        let loc = Location::new(filename, line, col);
         match event {
             Event::Start(tag) => match tag {
                 Tag::Paragraph => stack.push_tag(ElementTag::P, loc),
@@ -162,7 +162,7 @@ pub fn parse(markdown: &str) -> Result<Element, HtmlError> {
                     let s = as_plain_text(e.children());
                     trace!("paragraph text: {:?}", s);
                     if s.starts_with(": ") || s.contains("\n: ") {
-                        return Err(HtmlError::DefinitionList(loc.line, loc.col));
+                        return Err(HtmlError::DefinitionList(loc));
                     }
                     stack.append_child(Content::Elt(e));
                 }
@@ -254,8 +254,12 @@ impl Element {
     }
 
     /// Get location.
-    pub fn location(&self) -> &Option<Location> {
-        &self.loc
+    pub fn location(&self) -> Location {
+        if let Some(loc) = &self.loc {
+            loc.clone()
+        } else {
+            Location::unknown()
+        }
     }
 
     fn set_block_attributes(&mut self, block_attrs: Vec<BlockAttr>) {
@@ -543,15 +547,76 @@ impl Content {
 }
 
 /// Location of element in source file.
-#[derive(Debug, Clone, Copy)]
-pub struct Location {
-    line: usize,
-    col: usize,
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub enum Location {
+    /// A known location.
+    Known {
+        /// Name of file.
+        filename: PathBuf,
+        /// Line in file.
+        line: usize,
+        /// Column in line.
+        col: usize,
+    },
+    /// An unknown location.
+    Unknown,
 }
 
 impl Location {
-    fn new(line: usize, col: usize) -> Self {
-        Self { line, col }
+    fn new(filename: &Path, line: usize, col: usize) -> Self {
+        Self::Known {
+            filename: filename.into(),
+            line,
+            col,
+        }
+    }
+
+    /// Create an unknown location.
+    pub fn unknown() -> Self {
+        Self::Unknown
+    }
+
+    /// Report name of source file from where this element comes from.
+    pub fn filename(&self) -> &Path {
+        if let Self::Known {
+            filename,
+            line: _,
+            col: _,
+        } = self
+        {
+            filename
+        } else {
+            Path::new("")
+        }
+    }
+
+    /// Report row and column in source where this element comes from.
+    pub fn rowcol(&self) -> (usize, usize) {
+        if let Self::Known {
+            filename: _,
+            line,
+            col,
+        } = self
+        {
+            (*line, *col)
+        } else {
+            (0, 0)
+        }
+    }
+}
+
+impl std::fmt::Display for Location {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
+        if let Self::Known {
+            filename,
+            line,
+            col,
+        } = self
+        {
+            write!(f, "{}:{}:{}", filename.display(), line, col)
+        } else {
+            write!(f, "(unknown location)")
+        }
     }
 }
 
@@ -616,8 +681,8 @@ pub enum HtmlError {
 
     /// Input contains an attempt to use a definition list in
     /// Markdown.
-    #[error("attempt to use definition lists in Markdown: line {0}, column {1}")]
-    DefinitionList(usize, usize),
+    #[error("attempt to use definition lists in Markdown: {0}")]
+    DefinitionList(Location),
 
     /// String formatting error. This is likely a programming error.
     #[error("string formatting error: {0}")]
diff --git a/src/md.rs b/src/md.rs
index 5e56198..02a827b 100644
--- a/src/md.rs
+++ b/src/md.rs
@@ -1,7 +1,7 @@
 //! A parsed Markdown document.
 
 use crate::{
-    html::{parse, Attribute, Content, Element, ElementTag},
+    html::{parse, Attribute, Content, Element, ElementTag, Location},
     parse_scenario_snippet, Bindings, EmbeddedFile, EmbeddedFiles, Scenario, ScenarioStep, Style,
     SubplotError, Warnings,
 };
@@ -22,11 +22,11 @@ impl Markdown {
         let text = std::fs::read(filename)
             .map_err(|e| SubplotError::InputFileUnreadable(filename.into(), e))?;
         let text = std::str::from_utf8(&text).map_err(SubplotError::Utf8Error)?;
-        Self::new_from_str(text)
+        Self::new_from_str(filename, text)
     }
 
-    fn new_from_str(text: &str) -> Result<Self, SubplotError> {
-        let html = parse(text)?;
+    fn new_from_str(filename: &Path, text: &str) -> Result<Self, SubplotError> {
+        let html = parse(filename, text)?;
         Ok(Self::new(html))
     }
 
@@ -159,42 +159,45 @@ impl Markdown {
 #[derive(Debug)]
 enum StructureElement {
     // Headings consist of the text and the level of the heading.
-    Heading(String, i64),
+    Heading(String, i64, Location),
 
     // Scenario snippets consist just of the unparsed text.
-    Snippet(String),
+    Snippet(String, Location),
 }
 
 impl StructureElement {
     fn heading(e: &Element, level: i64) -> Self {
-        Self::Heading(e.content(), level)
+        Self::Heading(e.content(), level, e.location())
     }
 
     fn snippet(e: &Element) -> Self {
-        Self::Snippet(e.content())
+        Self::Snippet(e.content(), e.location())
     }
 }
 
 fn embedded_file(e: &Element) -> Result<EmbeddedFile, MdError> {
     if e.tag() != ElementTag::Pre {
-        return Err(MdError::NotCodeBlockElement(e.tag().name().to_string()));
+        return Err(MdError::NotCodeBlockElement(
+            e.tag().name().to_string(),
+            e.location(),
+        ));
     }
 
     if !e.has_attr("class", "file") {
-        return Err(MdError::NotFile);
+        return Err(MdError::NotFile(e.location()));
     }
 
     let id = e.attr("id");
     if id.is_none() {
-        return Err(MdError::NoId);
+        return Err(MdError::NoId(e.location()));
     }
     let id = id.unwrap();
     if id.value().is_none() {
-        return Err(MdError::NoIdValue);
+        return Err(MdError::NoIdValue(e.location()));
     }
     let id = id.value().unwrap();
     if id.is_empty() {
-        return Err(MdError::NoIdValue);
+        return Err(MdError::NoIdValue(e.location()));
     }
 
     // The contents we get from the pulldown_cmark parser for a code
@@ -209,7 +212,7 @@ fn embedded_file(e: &Element) -> Result<EmbeddedFile, MdError> {
     if contents.ends_with('\n') {
         contents.truncate(contents.len() - 1);
     }
-    match AddNewline::parse(e.attr("add-newline"))? {
+    match AddNewline::parse(e.attr("add-newline"), e.location())? {
         AddNewline::No => {
             // Newline already isn't there.
         }
@@ -236,14 +239,14 @@ enum AddNewline {
 }
 
 impl AddNewline {
-    fn parse(attr: Option<&Attribute>) -> Result<Self, MdError> {
+    fn parse(attr: Option<&Attribute>, loc: Location) -> Result<Self, MdError> {
         if let Some(attr) = attr {
             if let Some(value) = attr.value() {
                 let value = match value {
                     "yes" => Self::Yes,
                     "no" => Self::No,
                     "auto" => Self::Auto,
-                    _ => return Err(MdError::BadAddNewline(value.into())),
+                    _ => return Err(MdError::BadAddNewline(value.into(), loc)),
                 };
                 return Ok(value);
             }
@@ -259,13 +262,13 @@ fn extract_scenario(e: &[StructureElement]) -> Result<(Option<Scenario>, usize),
     }
 
     match &e[0] {
-        StructureElement::Snippet(_) => Err(SubplotError::ScenarioBeforeHeading),
-        StructureElement::Heading(title, level) => {
+        StructureElement::Snippet(_, loc) => Err(SubplotError::ScenarioBeforeHeading(loc.clone())),
+        StructureElement::Heading(title, level, _loc) => {
             let mut scen = Scenario::new(title);
             let mut prevkind = None;
             for (i, item) in e.iter().enumerate().skip(1) {
                 match item {
-                    StructureElement::Heading(_, level2) => {
+                    StructureElement::Heading(_, level2, _loc) => {
                         let is_subsection = *level2 > *level;
                         if is_subsection {
                             if scen.has_steps() {
@@ -278,7 +281,7 @@ fn extract_scenario(e: &[StructureElement]) -> Result<(Option<Scenario>, usize),
                             return Ok((None, i));
                         }
                     }
-                    StructureElement::Snippet(text) => {
+                    StructureElement::Snippet(text, _) => {
                         for line in parse_scenario_snippet(text) {
                             let step = ScenarioStep::new_from_str(line, prevkind)?;
                             scen.add(&step);
@@ -407,39 +410,40 @@ mod typeset {
 #[derive(Debug, thiserror::Error, Eq, PartialEq)]
 pub enum MdError {
     /// Trie to treat a non-PRE element as an embedded file.
-    #[error("tried to treat wrong element as an embedded file: {0}")]
-    NotCodeBlockElement(String),
+    #[error("{1}: tried to treat wrong element as an embedded file: {0}")]
+    NotCodeBlockElement(String, Location),
 
     /// Code block lacks the "file" attribute.
-    #[error("code block is not a file")]
-    NotFile,
+    #[error("{0}; code block is not a file")]
+    NotFile(Location),
 
     /// Code block lacks an identifile to use as th filename.
-    #[error("code block lacks a filename identifier")]
-    NoId,
+    #[error("{0}: code block lacks a filename identifier")]
+    NoId(Location),
 
     /// Identifier is empty.
-    #[error("code block has an empty filename identifier")]
-    NoIdValue,
+    #[error("{0}: code block has an empty filename identifier")]
+    NoIdValue(Location),
 
     /// Value ofv add-newline attribute ie not understood.
-    #[error("value of add-newline attirubte is not understood: {0}")]
-    BadAddNewline(String),
+    #[error("{0}: value of add-newline attirubte is not understood: {0}")]
+    BadAddNewline(String, Location),
 }
 
 #[cfg(test)]
 mod test_extract {
     use super::extract_scenario;
+    use super::Location;
     use super::StructureElement;
     use crate::Scenario;
     use crate::SubplotError;
 
     fn h(title: &str, level: i64) -> StructureElement {
-        StructureElement::Heading(title.to_string(), level)
+        StructureElement::Heading(title.to_string(), level, Location::unknown())
     }
 
     fn s(text: &str) -> StructureElement {
-        StructureElement::Snippet(text.to_string())
+        StructureElement::Snippet(text.to_string(), Location::unknown())
     }
 
     fn check_result(
@@ -530,7 +534,7 @@ mod test_extract {
         let elements = vec![s("given something")];
         let r = extract_scenario(&elements);
         match r {
-            Err(SubplotError::ScenarioBeforeHeading) => (),
+            Err(SubplotError::ScenarioBeforeHeading(_)) => (),
             _ => panic!("unexpected result {:?}", r),
         }
     }
@@ -538,24 +542,25 @@ mod test_extract {
 
 #[cfg(test)]
 mod test {
-    use super::{AddNewline, Attribute, Markdown, MdError};
-    use std::path::PathBuf;
+    use super::{AddNewline, Attribute, Location, Markdown, MdError};
+    use std::path::{Path, PathBuf};
 
     #[test]
     fn loads_empty_doc() {
-        let md = Markdown::new_from_str("").unwrap();
+        let md = Markdown::new_from_str(Path::new(""), "").unwrap();
         assert!(md.html.content().is_empty());
     }
 
     #[test]
     fn finds_no_images_in_empty_doc() {
-        let md = Markdown::new_from_str("").unwrap();
+        let md = Markdown::new_from_str(Path::new(""), "").unwrap();
         assert!(md.images().is_empty());
     }
 
     #[test]
     fn finds_images() {
         let md = Markdown::new_from_str(
+            Path::new(""),
             r#"
 ![alt text](filename.jpg)
 "#,
@@ -566,13 +571,14 @@ mod test {
 
     #[test]
     fn finds_no_blocks_in_empty_doc() {
-        let md = Markdown::new_from_str("").unwrap();
+        let md = Markdown::new_from_str(Path::new(""), "").unwrap();
         assert!(md.block_classes().is_empty());
     }
 
     #[test]
     fn finds_no_classes_when_no_blocks_have_them() {
         let md = Markdown::new_from_str(
+            Path::new(""),
             r#"
 ~~~
 ~~~
@@ -585,6 +591,7 @@ mod test {
     #[test]
     fn finds_block_classes() {
         let md = Markdown::new_from_str(
+            Path::new(""),
             r#"
 ~~~scenario
 ~~~
@@ -597,7 +604,7 @@ mod test {
 
     #[test]
     fn finds_no_scenarios_in_empty_doc() {
-        let md = Markdown::new_from_str("").unwrap();
+        let md = Markdown::new_from_str(Path::new(""), "").unwrap();
         let scenarios = md.scenarios().unwrap();
         assert!(scenarios.is_empty());
     }
@@ -605,6 +612,7 @@ mod test {
     #[test]
     fn finds_scenarios() {
         let md = Markdown::new_from_str(
+            Path::new(""),
             r#"
 # Super trooper
 
@@ -627,7 +635,7 @@ given ABBA
 
     #[test]
     fn finds_no_embedded_files_in_empty_doc() {
-        let md = Markdown::new_from_str("").unwrap();
+        let md = Markdown::new_from_str(Path::new(""), "").unwrap();
         let files = md.embedded_files();
         assert!(files.files().is_empty());
     }
@@ -635,6 +643,7 @@ given ABBA
     #[test]
     fn finds_embedded_files() {
         let md = Markdown::new_from_str(
+            Path::new(""),
             r#"
 ~~~{#fileid .file .text}
 hello, world
@@ -651,33 +660,45 @@ hello, world
 
     #[test]
     fn parses_no_auto_newline_as_auto() {
-        assert_eq!(AddNewline::parse(None).unwrap(), AddNewline::Auto);
+        assert_eq!(
+            AddNewline::parse(None, Location::unknown()).unwrap(),
+            AddNewline::Auto
+        );
     }
 
     #[test]
     fn parses_auto_as_auto() {
         let attr = Attribute::new("add-newline", "auto");
-        assert_eq!(AddNewline::parse(Some(&attr)).unwrap(), AddNewline::Auto);
+        assert_eq!(
+            AddNewline::parse(Some(&attr), Location::unknown()).unwrap(),
+            AddNewline::Auto
+        );
     }
 
     #[test]
     fn parses_yes_as_yes() {
         let attr = Attribute::new("add-newline", "yes");
-        assert_eq!(AddNewline::parse(Some(&attr)).unwrap(), AddNewline::Yes);
+        assert_eq!(
+            AddNewline::parse(Some(&attr), Location::unknown()).unwrap(),
+            AddNewline::Yes
+        );
     }
 
     #[test]
     fn parses_no_as_no() {
         let attr = Attribute::new("add-newline", "no");
-        assert_eq!(AddNewline::parse(Some(&attr)).unwrap(), AddNewline::No);
+        assert_eq!(
+            AddNewline::parse(Some(&attr), Location::unknown()).unwrap(),
+            AddNewline::No
+        );
     }
 
     #[test]
     fn parses_empty_as_error() {
         let attr = Attribute::new("add-newline", "");
         assert_eq!(
-            AddNewline::parse(Some(&attr)),
-            Err(MdError::BadAddNewline("".into()))
+            AddNewline::parse(Some(&attr), Location::unknown()),
+            Err(MdError::BadAddNewline("".into(), Location::unknown()))
         );
     }
 
@@ -685,8 +706,11 @@ hello, world
     fn parses_garbage_as_error() {
         let attr = Attribute::new("add-newline", "garbage");
         assert_eq!(
-            AddNewline::parse(Some(&attr)),
-            Err(MdError::BadAddNewline("garbage".into()))
+            AddNewline::parse(Some(&attr), Location::unknown()),
+            Err(MdError::BadAddNewline(
+                "garbage".into(),
+                Location::unknown()
+            ))
         );
     }
 }