From af353507136241eddbdc8bd89324b213fd33ea9f Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Thu, 18 May 2023 10:23:02 +0300 Subject: feat: add location information to markdown parsing errors Sponsored-by: author --- src/error.rs | 6 +-- src/html.rs | 91 +++++++++++++++++++++++++++++++++++++------- src/md.rs | 120 +++++++++++++++++++++++++++++++++++------------------------ 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 { +pub fn parse(filename: &Path, markdown: &str) -> Result { 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 { 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 { 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 { - &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) { @@ -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 { - let html = parse(text)?; + fn new_from_str(filename: &Path, text: &str) -> Result { + 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 { 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 { 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 { + fn parse(attr: Option<&Attribute>, loc: Location) -> Result { 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, 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, 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() + )) ); } } -- cgit v1.2.1