Improve error handling

7 files changed, 114 insertions, 157 deletions

diff --git a/src/eval/walk.rs b/src/eval/walk.rs
index b28f4fde..e4f8ac7b 100644
--- a/src/eval/walk.rs
+++ b/src/eval/walk.rs
@@ -16,7 +16,7 @@ pub trait Walk {
 
 impl Walk for Markup {
     fn walk(&self, ctx: &mut EvalContext) -> TypResult<()> {
-        for node in self.iter() {
+        for node in self.nodes() {
             node.walk(ctx)?;
         }
         Ok(())
diff --git a/src/parse/mod.rs b/src/parse/mod.rs
index 773f642c..ce992834 100644
--- a/src/parse/mod.rs
+++ b/src/parse/mod.rs
@@ -138,8 +138,7 @@ fn markup_node(p: &mut Parser, at_start: &mut bool) {
         NodeKind::LeftBracket => template(p),
 
         // Comments.
-        NodeKind::LineComment | NodeKind::BlockComment => p.eat(),
-        NodeKind::Error(t, e) if t != &ErrorPosition::Full || e.contains(' ') => p.eat(),
+        NodeKind::LineComment | NodeKind::BlockComment | NodeKind::Error(_, _) => p.eat(),
 
         _ => {
             *at_start = false;
@@ -319,7 +318,7 @@ fn primary(p: &mut Parser, atomic: bool) {
         Some(NodeKind::Import) => import_expr(p),
         Some(NodeKind::Include) => include_expr(p),
 
-        Some(NodeKind::Error(t, e)) if t != &ErrorPosition::Full || e.contains(' ') => {
+        Some(NodeKind::Error(_, _)) => {
             p.eat();
         }
 
@@ -333,28 +332,26 @@ fn primary(p: &mut Parser, atomic: bool) {
 
 /// Parse a literal.
 fn literal(p: &mut Parser) -> bool {
-    let peeked = match p.peek() {
-        Some(x) => x.clone(),
-        None => return false,
-    };
-
-    match peeked {
+    match p.peek() {
         // Basic values.
-        NodeKind::None
-        | NodeKind::Auto
-        | NodeKind::Int(_)
-        | NodeKind::Float(_)
-        | NodeKind::Bool(_)
-        | NodeKind::Fraction(_)
-        | NodeKind::Length(_, _)
-        | NodeKind::Angle(_, _)
-        | NodeKind::Percentage(_)
-        | NodeKind::Str(_) => p.eat(),
+        Some(
+            NodeKind::None
+            | NodeKind::Auto
+            | NodeKind::Int(_)
+            | NodeKind::Float(_)
+            | NodeKind::Bool(_)
+            | NodeKind::Fraction(_)
+            | NodeKind::Length(_, _)
+            | NodeKind::Angle(_, _)
+            | NodeKind::Percentage(_)
+            | NodeKind::Str(_),
+        ) => {
+            p.eat();
+            true
+        }
 
-        _ => return false,
+        _ => false,
     }
-
-    true
 }
 
 /// Parse something that starts with a parenthesis, which can be either of:
@@ -395,11 +392,11 @@ fn parenthesized(p: &mut Parser) {
     // Find out which kind of collection this is.
     match kind {
         CollectionKind::Group => p.end(NodeKind::Group),
-        CollectionKind::PositionalCollection => {
+        CollectionKind::Positional => {
             p.lift();
             array(p, token_count);
         }
-        CollectionKind::NamedCollection => {
+        CollectionKind::Named => {
             p.lift();
             dict(p, token_count);
         }
@@ -413,9 +410,9 @@ enum CollectionKind {
     Group,
     /// The collection starts with a positional and has more items or a trailing
     /// comma.
-    PositionalCollection,
+    Positional,
     /// The collection starts with a named item.
-    NamedCollection,
+    Named,
 }
 
 /// Parse a collection.
@@ -424,20 +421,19 @@ enum CollectionKind {
 /// commas.
 fn collection(p: &mut Parser) -> (CollectionKind, usize) {
     let mut items = 0;
-    let mut kind = CollectionKind::PositionalCollection;
-    let mut seen_spread = false;
+    let mut kind = CollectionKind::Positional;
     let mut has_comma = false;
     let mut missing_coma = None;
 
     while !p.eof() {
         let item_kind = item(p);
         if p.success() {
-            if items == 0 && item_kind == CollectionItemKind::Named {
-                kind = CollectionKind::NamedCollection;
+            if items == 0 && item_kind == NodeKind::Named {
+                kind = CollectionKind::Named;
             }
 
-            if item_kind == CollectionItemKind::ParameterSink {
-                seen_spread = true;
+            if item_kind == NodeKind::ParameterSink {
+                has_comma = true;
             }
 
             items += 1;
@@ -458,42 +454,27 @@ fn collection(p: &mut Parser) -> (CollectionKind, usize) {
         }
     }
 
-    if !has_comma
-        && items == 1
-        && !seen_spread
-        && kind == CollectionKind::PositionalCollection
-    {
+    if !has_comma && items == 1 && kind == CollectionKind::Positional {
         kind = CollectionKind::Group;
     }
 
     (kind, items)
 }
 
-/// What kind of item is this?
-#[derive(Debug, Copy, Clone, Eq, PartialEq)]
-enum CollectionItemKind {
-    /// A named item.
-    Named,
-    /// An unnamed item.
-    Unnamed,
-    /// A parameter sink.
-    ParameterSink,
-}
-
 /// Parse an expression or a named pair. Returns if this is a named pair.
-fn item(p: &mut Parser) -> CollectionItemKind {
+fn item(p: &mut Parser) -> NodeKind {
     p.start();
     if p.eat_if(&NodeKind::Dots) {
         expr(p);
 
         p.end_or_abort(NodeKind::ParameterSink);
-        return CollectionItemKind::ParameterSink;
+        return NodeKind::ParameterSink;
     }
 
     expr(p);
 
     if p.may_lift_abort() {
-        return CollectionItemKind::Unnamed;
+        return NodeKind::None;
     }
 
     if p.eat_if(&NodeKind::Colon) {
@@ -512,10 +493,10 @@ fn item(p: &mut Parser) -> CollectionItemKind {
             p.unsuccessful();
         }
 
-        CollectionItemKind::Named
+        NodeKind::Named
     } else {
         p.lift();
-        CollectionItemKind::Unnamed
+        p.last_child().unwrap().kind().clone()
     }
 }
 
diff --git a/src/parse/tokens.rs b/src/parse/tokens.rs
index 8a480b02..7c500ce7 100644
--- a/src/parse/tokens.rs
+++ b/src/parse/tokens.rs
@@ -91,7 +91,7 @@ impl<'s> Iterator for Tokens<'s> {
             '/' if self.s.eat_if('*') => self.block_comment(),
             '/' if !self.maybe_in_url() && self.s.eat_if('/') => self.line_comment(),
             '*' if self.s.eat_if('/') => {
-                NodeKind::Error(ErrorPosition::Full, self.s.eaten_from(start).into())
+                NodeKind::Unknown(self.s.eaten_from(start).into())
             }
 
             // Other things.
@@ -173,7 +173,7 @@ impl<'s> Tokens<'s> {
             // Strings.
             '"' => self.string(),
 
-            _ => NodeKind::Error(ErrorPosition::Full, self.s.eaten_from(start).into()),
+            _ => NodeKind::Unknown(self.s.eaten_from(start).into()),
         }
     }
 
@@ -398,10 +398,10 @@ impl<'s> Tokens<'s> {
         } else {
             NodeKind::Error(
                 ErrorPosition::End,
-                if display {
+                if !display || (!escaped && dollar) {
                     "expected closing dollar sign"
                 } else {
-                    "expected display math closure sequence"
+                    "expected closing bracket and dollar sign"
                 }
                 .into(),
             )
@@ -466,11 +466,11 @@ impl<'s> Tokens<'s> {
                 "deg" => NodeKind::Angle(f, AngularUnit::Deg),
                 "rad" => NodeKind::Angle(f, AngularUnit::Rad),
                 _ => {
-                    return NodeKind::Error(ErrorPosition::Full, all.into());
+                    return NodeKind::Unknown(all.into());
                 }
             }
         } else {
-            NodeKind::Error(ErrorPosition::Full, all.into())
+            NodeKind::Unknown(all.into())
         }
     }
 
@@ -575,45 +575,31 @@ mod tests {
         text: &str,
         lang: Option<&str>,
         backticks_left: u8,
-        backticks_right: u8,
+        err_msg: Option<&str>,
         block: bool,
     ) -> NodeKind {
-        if backticks_left == backticks_right {
-            NodeKind::Raw(Rc::new(RawToken {
+        match err_msg {
+            None => NodeKind::Raw(Rc::new(RawToken {
                 text: text.into(),
                 lang: lang.map(Into::into),
                 backticks: backticks_left,
                 block,
-            }))
-        } else {
-            let remaining = backticks_left - backticks_right;
-            let noun = if remaining == 1 { "backtick" } else { "backticks" };
-
-            NodeKind::Error(
-                ErrorPosition::End,
-                if backticks_right == 0 {
-                    format!("expected {} {}", remaining, noun)
-                } else {
-                    format!("expected {} more {}", remaining, noun)
-                }
-                .into(),
-            )
+            })),
+            Some(msg) => {
+                NodeKind::Error(ErrorPosition::End, format!("expected {}", msg).into())
+            }
         }
     }
 
-    fn Math(formula: &str, display: bool, terminated: bool) -> NodeKind {
-        if terminated {
-            NodeKind::Math(Rc::new(MathToken { formula: formula.into(), display }))
-        } else {
-            NodeKind::Error(
+    fn Math(formula: &str, display: bool, err_msg: Option<&str>) -> NodeKind {
+        match err_msg {
+            None => {
+                NodeKind::Math(Rc::new(MathToken { formula: formula.into(), display }))
+            }
+            Some(msg) => NodeKind::Error(
                 ErrorPosition::End,
-                if display {
-                    "expected closing dollar sign"
-                } else {
-                    "expected display math closure sequence"
-                }
-                .into(),
-            )
+                format!("expected closing {}", msg).into(),
+            ),
         }
     }
 
@@ -634,7 +620,7 @@ mod tests {
     }
 
     fn Invalid(invalid: &str) -> NodeKind {
-        NodeKind::Error(ErrorPosition::Full, invalid.into())
+        NodeKind::Unknown(invalid.into())
     }
 
     /// Building blocks for suffix testing.
@@ -687,7 +673,7 @@ mod tests {
                 ('/', None, "//", LineComment),
                 ('/', None, "/**/", BlockComment),
                 ('/', Some(Markup), "*", Strong),
-                ('/', Some(Markup), "$ $", Math(" ", false, true)),
+                ('/', Some(Markup), "$ $", Math(" ", false, None)),
                 ('/', Some(Markup), r"\\", Text("\\")),
                 ('/', Some(Markup), "#let", Let),
                 ('/', Some(Code), "(", LeftParen),
@@ -908,42 +894,42 @@ mod tests {
     #[test]
     fn test_tokenize_raw_blocks() {
         // Test basic raw block.
-        t!(Markup: "``"     => Raw("", None, 1, 1, false));
-        t!(Markup: "`raw`"  => Raw("raw", None, 1, 1, false));
-        t!(Markup[""]: "`]" => Raw("]", None, 1, 0, false));
+        t!(Markup: "``"     => Raw("", None, 1, None, false));
+        t!(Markup: "`raw`"  => Raw("raw", None, 1, None, false));
+        t!(Markup[""]: "`]" => Raw("]", None, 1, Some("1 backtick"), false));
 
         // Test special symbols in raw block.
-        t!(Markup: "`[brackets]`" => Raw("[brackets]", None, 1, 1, false));
-        t!(Markup[""]: r"`\`` "   => Raw(r"\", None, 1, 1, false), Raw(" ", None, 1, 0, false));
+        t!(Markup: "`[brackets]`" => Raw("[brackets]", None, 1, None, false));
+        t!(Markup[""]: r"`\`` "   => Raw(r"\", None, 1, None, false), Raw(" ", None, 1, Some("1 backtick"), false));
 
         // Test separated closing backticks.
-        t!(Markup: "```not `y`e`t```" => Raw("`y`e`t", Some("not"), 3, 3, false));
+        t!(Markup: "```not `y`e`t```" => Raw("`y`e`t", Some("not"), 3, None, false));
 
         // Test more backticks.
-        t!(Markup: "``nope``"             => Raw("", None, 1, 1, false), Text("nope"), Raw("", None, 1, 1, false));
-        t!(Markup: "````🚀````"           => Raw("", Some("🚀"), 4, 4, false));
-        t!(Markup[""]: "`````👩‍🚀````noend" => Raw("````noend", Some("👩‍🚀"), 5, 0, false));
-        t!(Markup[""]: "````raw``````"    => Raw("", Some("raw"), 4, 4, false), Raw("", None, 1, 1, false));
+        t!(Markup: "``nope``"             => Raw("", None, 1, None, false), Text("nope"), Raw("", None, 1, None, false));
+        t!(Markup: "````🚀````"           => Raw("", Some("🚀"), 4, None, false));
+        t!(Markup[""]: "`````👩‍🚀````noend" => Raw("````noend", Some("👩‍🚀"), 5, Some("5 backticks"), false));
+        t!(Markup[""]: "````raw``````"    => Raw("", Some("raw"), 4, None, false), Raw("", None, 1, None, false));
     }
 
     #[test]
     fn test_tokenize_math_formulas() {
         // Test basic formula.
-        t!(Markup: "$$"        => Math("", false, true));
-        t!(Markup: "$x$"       => Math("x", false, true));
-        t!(Markup: r"$\\$"     => Math(r"\\", false, true));
-        t!(Markup: "$[x + y]$" => Math("x + y", true, true));
-        t!(Markup: r"$[\\]$"   => Math(r"\\", true, true));
+        t!(Markup: "$$"        => Math("", false, None));
+        t!(Markup: "$x$"       => Math("x", false, None));
+        t!(Markup: r"$\\$"     => Math(r"\\", false, None));
+        t!(Markup: "$[x + y]$" => Math("x + y", true, None));
+        t!(Markup: r"$[\\]$"   => Math(r"\\", true, None));
 
         // Test unterminated.
-        t!(Markup[""]: "$x"      => Math("x", false, false));
-        t!(Markup[""]: "$[x"     => Math("x", true, false));
-        t!(Markup[""]: "$[x]\n$" => Math("x]\n$", true, false));
+        t!(Markup[""]: "$x"      => Math("x", false, Some("dollar sign")));
+        t!(Markup[""]: "$[x"     => Math("x", true, Some("bracket and dollar sign")));
+        t!(Markup[""]: "$[x]\n$" => Math("x]\n$", true, Some("bracket and dollar sign")));
 
         // Test escape sequences.
-        t!(Markup: r"$\$x$"       => Math(r"\$x", false, true));
-        t!(Markup: r"$[\\\]$]$"   => Math(r"\\\]$", true, true));
-        t!(Markup[""]: r"$[ ]\\$" => Math(r" ]\\$", true, false));
+        t!(Markup: r"$\$x$"       => Math(r"\$x", false, None));
+        t!(Markup: r"$[\\\]$]$"   => Math(r"\\\]$", true, None));
+        t!(Markup[""]: r"$[ ]\\$" => Math(r" ]\\$", true, Some("bracket and dollar sign")));
     }
 
     #[test]
diff --git a/src/syntax/expr.rs b/src/syntax/expr.rs
index 8562a3a4..1439cbdb 100644
--- a/src/syntax/expr.rs
+++ b/src/syntax/expr.rs
@@ -87,32 +87,24 @@ impl Expr {
 impl TypedNode for Expr {
     fn cast_from(node: RedRef) -> Option<Self> {
         match node.kind() {
-            NodeKind::Ident(_) => Some(Self::Ident(Ident::cast_from(node).unwrap())),
-            NodeKind::Array => Some(Self::Array(ArrayExpr::cast_from(node).unwrap())),
-            NodeKind::Dict => Some(Self::Dict(DictExpr::cast_from(node).unwrap())),
-            NodeKind::Template => {
-                Some(Self::Template(TemplateExpr::cast_from(node).unwrap()))
-            }
-            NodeKind::Group => Some(Self::Group(GroupExpr::cast_from(node).unwrap())),
-            NodeKind::Block => Some(Self::Block(BlockExpr::cast_from(node).unwrap())),
-            NodeKind::Unary => Some(Self::Unary(UnaryExpr::cast_from(node).unwrap())),
-            NodeKind::Binary => Some(Self::Binary(BinaryExpr::cast_from(node).unwrap())),
-            NodeKind::Call => Some(Self::Call(CallExpr::cast_from(node).unwrap())),
-            NodeKind::Closure => {
-                Some(Self::Closure(ClosureExpr::cast_from(node).unwrap()))
-            }
-            NodeKind::WithExpr => Some(Self::With(WithExpr::cast_from(node).unwrap())),
-            NodeKind::LetExpr => Some(Self::Let(LetExpr::cast_from(node).unwrap())),
-            NodeKind::IfExpr => Some(Self::If(IfExpr::cast_from(node).unwrap())),
-            NodeKind::WhileExpr => Some(Self::While(WhileExpr::cast_from(node).unwrap())),
-            NodeKind::ForExpr => Some(Self::For(ForExpr::cast_from(node).unwrap())),
-            NodeKind::ImportExpr => {
-                Some(Self::Import(ImportExpr::cast_from(node).unwrap()))
-            }
-            NodeKind::IncludeExpr => {
-                Some(Self::Include(IncludeExpr::cast_from(node).unwrap()))
-            }
-            _ => Some(Self::Lit(Lit::cast_from(node)?)),
+            NodeKind::Ident(_) => node.cast().map(Self::Ident),
+            NodeKind::Array => node.cast().map(Self::Array),
+            NodeKind::Dict => node.cast().map(Self::Dict),
+            NodeKind::Template => node.cast().map(Self::Template),
+            NodeKind::Group => node.cast().map(Self::Group),
+            NodeKind::Block => node.cast().map(Self::Block),
+            NodeKind::Unary => node.cast().map(Self::Unary),
+            NodeKind::Binary => node.cast().map(Self::Binary),
+            NodeKind::Call => node.cast().map(Self::Call),
+            NodeKind::Closure => node.cast().map(Self::Closure),
+            NodeKind::WithExpr => node.cast().map(Self::With),
+            NodeKind::LetExpr => node.cast().map(Self::Let),
+            NodeKind::IfExpr => node.cast().map(Self::If),
+            NodeKind::WhileExpr => node.cast().map(Self::While),
+            NodeKind::ForExpr => node.cast().map(Self::For),
+            NodeKind::ImportExpr => node.cast().map(Self::Import),
+            NodeKind::IncludeExpr => node.cast().map(Self::Include),
+            _ => node.cast().map(Self::Lit),
         }
     }
 }
diff --git a/src/syntax/markup.rs b/src/syntax/markup.rs
index de547f76..49b2a519 100644
--- a/src/syntax/markup.rs
+++ b/src/syntax/markup.rs
@@ -3,17 +3,14 @@ use crate::node;
 use crate::util::EcoString;
 use std::fmt::Write;
 
-/// The syntactical root capable of representing a full parsed document.
-pub type Markup = Vec<MarkupNode>;
-
-impl TypedNode for Markup {
-    fn cast_from(node: RedRef) -> Option<Self> {
-        if node.kind() != &NodeKind::Markup {
-            return None;
-        }
+node! {
+    /// The syntactical root capable of representing a full parsed document.
+    Markup
+}
 
-        let children = node.children().filter_map(TypedNode::cast_from).collect();
-        Some(children)
+impl Markup {
+    pub fn nodes<'a>(&'a self) -> impl Iterator<Item = MarkupNode> + 'a {
+        self.0.children().filter_map(RedRef::cast)
     }
 }
 
@@ -66,14 +63,12 @@ impl TypedNode for MarkupNode {
             NodeKind::NonBreakingSpace => {
                 Some(MarkupNode::Text(EcoString::from("\u{00A0}")))
             }
-            NodeKind::Raw(_) => Some(MarkupNode::Raw(RawNode::cast_from(node).unwrap())),
-            NodeKind::Heading => {
-                Some(MarkupNode::Heading(HeadingNode::cast_from(node).unwrap()))
-            }
-            NodeKind::List => Some(MarkupNode::List(ListNode::cast_from(node).unwrap())),
-            NodeKind::Enum => Some(MarkupNode::Enum(EnumNode::cast_from(node).unwrap())),
+            NodeKind::Raw(_) => node.cast().map(MarkupNode::Raw),
+            NodeKind::Heading => node.cast().map(MarkupNode::Heading),
+            NodeKind::List => node.cast().map(MarkupNode::List),
+            NodeKind::Enum => node.cast().map(MarkupNode::Enum),
             NodeKind::Error(_, _) => None,
-            _ => Some(MarkupNode::Expr(Expr::cast_from(node)?)),
+            _ => node.cast().map(MarkupNode::Expr),
         }
     }
 }
diff --git a/src/syntax/mod.rs b/src/syntax/mod.rs
index ca5b6a1b..afa0ab86 100644
--- a/src/syntax/mod.rs
+++ b/src/syntax/mod.rs
@@ -162,6 +162,8 @@ pub enum NodeKind {
     BlockComment,
     /// Tokens that appear in the wrong place.
     Error(ErrorPosition, EcoString),
+    /// Unknown character sequences.
+    Unknown(EcoString),
     /// Template markup.
     Markup,
     /// A forced line break: `\`.
@@ -375,10 +377,11 @@ impl NodeKind {
             Self::ImportExpr => "import expression",
             Self::ImportItems => "import items",
             Self::IncludeExpr => "include expression",
-            Self::Error(_, src) => match src.as_str() {
+            Self::Unknown(src) => match src.as_str() {
                 "*/" => "end of block comment",
                 _ => "invalid token",
             },
+            Self::Error(_, _) => "parse error",
         }
     }
 }
diff --git a/src/syntax/pretty.rs b/src/syntax/pretty.rs
index db364eaa..da0bdd44 100644
--- a/src/syntax/pretty.rs
+++ b/src/syntax/pretty.rs
@@ -82,7 +82,7 @@ impl Write for Printer {
 
 impl Pretty for Markup {
     fn pretty(&self, p: &mut Printer) {
-        for node in self {
+        for node in self.nodes() {
             node.pretty(p);
         }
     }