diff --git a/crates/ltk_ritobin/src/cst.rs b/crates/ltk_ritobin/src/cst.rs index 849892cd..d60c2987 100644 --- a/crates/ltk_ritobin/src/cst.rs +++ b/crates/ltk_ritobin/src/cst.rs @@ -15,7 +15,7 @@ mod ids; pub use ids::*; pub mod visitor; -pub use visitor::Visitor; +pub use visitor::{Visitor, WalkOutcome}; pub mod builder; pub use builder::Builder as CstBuilder; diff --git a/crates/ltk_ritobin/src/cst/visitor.rs b/crates/ltk_ritobin/src/cst/visitor.rs index 355a47e5..f1e8bf47 100644 --- a/crates/ltk_ritobin/src/cst/visitor.rs +++ b/crates/ltk_ritobin/src/cst/visitor.rs @@ -1,12 +1,31 @@ //! Visitor pattern for walking CSTs +use std::ops::ControlFlow::{self, Break, Continue}; + use super::{tree::Child, Cst}; use crate::cst::{Node, NodeId, TokenId}; +#[cfg(test)] +mod tests; + #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum Visit { - /// Stop walking immediately + /// Aborts the walk immediately, with no stack unwinding. + /// + /// [`Visitor::exit_tree`] will not be called for open nodes in the walk stack. + Abort, + /// Stop the walk + /// + /// The walk will unwind, calling [`Visitor::exit_tree`] for every node that was in the walk stack, + /// bottom-up, until the walk is fully unwound. The walk will not resume after a [`Visit::Stop`]. + /// Use [`Visit::Abort`] to bail without the exit calls. Stop, - /// Skips all remaining tokens in the current tree + /// Skip ahead, locally + /// + /// - From [`Visitor::enter_tree`]: the node's children are skipped; its + /// [`Visitor::exit_tree`] still runs. + /// - From [`Visitor::visit_token`]: the rest of the current node's children are skipped. + /// - From [`Visitor::exit_tree`]: the parent's remaining children are pruned - the walk + /// jumps straight to the parent's [`Visitor::exit_tree`] and continues from there. Skip, /// Continue walking Continue, @@ -30,7 +49,14 @@ pub trait Visitor { Visit::Continue } - /// Called after all children of a [`Node`] have finished walking. + /// Called when a [`Node`] finished its walk, got skipped, or the walk stack is unwinding. + /// + /// Runs symmetrically to [`Visitor::enter_tree`], so every node that was entered will be exited, + /// even if the walk is unwinding after a [`Visit::Stop`] - unless the walk is aborted by a + /// [`Visit::Abort`], which skips all remaining callbacks. + /// + /// Returning [`Visit::Skip`] from here prunes the parent's remaining children: the walk + /// jumps straight to the parent's `exit_tree` and continues from there. #[must_use] fn exit_tree(&mut self, ctx: &VisitCtx<'_>, tree: NodeId) -> Visit { Visit::Continue @@ -52,47 +78,100 @@ pub trait VisitorExt: Sized + Visitor { impl VisitorExt for T {} +/// How a [`Cst::walk`] ended. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WalkOutcome { + /// The walk reached the end of the tree. + Completed, + /// A visitor returned [`Visit::Stop`] and the walk unwound early. + Stopped, + /// A visitor returned [`Visit::Abort`] and the walk ended without unwinding. + Aborted, +} + +/// Walk teardown marker, propagated up the walk stack. +enum Interrupt { + /// A [`Visit::Stop`]: [`Visitor::exit_tree`] still runs for every open node, bottom-up. + Unwind, + /// A [`Visit::Abort`]: no further callbacks run. + Abort, +} + +/// Where the walk resumes after a child subtree finished. +enum Resume { + /// With the parent's remaining children. + Siblings, + /// At the parent's [`Visitor::exit_tree`]: the child's exit returned + /// [`Visit::Skip`], pruning the remaining siblings. + Parent, +} + +/// Subtree walk state +/// +/// - `Continue(resume)` continues the walk; `?` on it yields where the walk +/// resumes - with the next sibling, or at the parent. +/// - `Break(interrupt)` tears the walk down; `?` on it propagates the teardown. +type Walk = ControlFlow; + impl Cst { /// Walk a [`Visitor`] implementor along this tree. - pub fn walk(&self, visitor: &mut V) { + pub fn walk(&self, visitor: &mut V) -> WalkOutcome { if self.nodes.is_empty() { - return; + return WalkOutcome::Completed; + } + + match self.walk_inner(visitor, NodeId(0)) { + Continue(_) => WalkOutcome::Completed, + Break(Interrupt::Unwind) => WalkOutcome::Stopped, + Break(Interrupt::Abort) => WalkOutcome::Aborted, } - self.walk_inner(visitor, NodeId(0)); } - fn walk_inner(&self, visitor: &mut V, node_idx: NodeId) -> Visit { + fn walk_inner(&self, visitor: &mut V, node_idx: NodeId) -> Walk { let ctx = VisitCtx { cst: self }; - let node = self.node(node_idx).unwrap(); - if let Some(ret) = match visitor.enter_tree(&ctx, node_idx) { - Visit::Stop => Some(Visit::Stop), - Visit::Skip => Some(Visit::Continue), - _ => None, - } { - if visitor.exit_tree(&ctx, node_idx) == Visit::Stop { - return Visit::Stop; - } - return ret; + let walked = match visitor.enter_tree(&ctx, node_idx) { + Visit::Abort => Break(Interrupt::Abort), + Visit::Stop => Break(Interrupt::Unwind), + Visit::Skip => Continue(()), + Visit::Continue => self.walk_children(visitor, &ctx, node_idx), + }; + + // an abort skips the remaining exits entirely + if let Break(Interrupt::Abort) = walked { + return Break(Interrupt::Abort); } - for child in node.children.get(self) { + // exit_tree runs exactly once for every entered node, even while unwinding + match (walked, visitor.exit_tree(&ctx, node_idx)) { + (_, Visit::Abort) => Break(Interrupt::Abort), + (Break(Interrupt::Unwind), _) | (_, Visit::Stop) => Break(Interrupt::Unwind), + (_, Visit::Skip) => Continue(Resume::Parent), + (_, Visit::Continue) => Continue(Resume::Siblings), + } + } + + fn walk_children( + &self, + visitor: &mut V, + ctx: &VisitCtx<'_>, + node_idx: NodeId, + ) -> ControlFlow { + for child in self.node(node_idx).unwrap().children.get(self) { match child { - Child::Token(token) => match visitor.visit_token(&ctx, *token, node_idx) { + Child::Token(token) => match visitor.visit_token(ctx, *token, node_idx) { Visit::Continue => {} Visit::Skip => break, - Visit::Stop => return Visit::Stop, + Visit::Stop => return Break(Interrupt::Unwind), + Visit::Abort => return Break(Interrupt::Abort), }, - Child::Tree(child) => match self.walk_inner(visitor, *child) { - Visit::Continue => {} - Visit::Skip => { - break; - } - Visit::Stop => return Visit::Stop, + Child::Tree(child) => match self.walk_inner(visitor, *child)? { + Resume::Siblings => {} + Resume::Parent => break, }, } } - visitor.exit_tree(&ctx, node_idx) + Continue(()) } } diff --git a/crates/ltk_ritobin/src/cst/visitor/tests.rs b/crates/ltk_ritobin/src/cst/visitor/tests.rs new file mode 100644 index 00000000..08ecc6b3 --- /dev/null +++ b/crates/ltk_ritobin/src/cst/visitor/tests.rs @@ -0,0 +1,295 @@ +use super::*; +use crate::cst::Kind; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Event { + Enter(Kind), + Token, + Exit(Kind), +} + +/// Records every callback; optionally returns Abort/Stop/Skip when a node of +/// the configured kind is hit. +#[derive(Default)] +struct Recorder { + events: Vec, + abort_on_enter: Option, + stop_on_enter: Option, + skip_on_enter: Option, + abort_on_exit: Option, + stop_on_exit: Option, + skip_on_exit: Option, + abort_on_token: bool, + stop_on_token: bool, + skip_on_token_in: Option, +} + +impl Recorder { + fn walk(mut self, text: &str) -> Vec { + let cst = Cst::parse(text); + assert!(cst.errors.is_empty(), "parse errors: {:#?}", cst.errors); + cst.walk(&mut self); + self.events + } +} + +impl Visitor for Recorder { + fn enter_tree(&mut self, ctx: &VisitCtx<'_>, tree: NodeId) -> Visit { + let kind = ctx.node(tree).unwrap().kind; + self.events.push(Event::Enter(kind)); + if self.abort_on_enter == Some(kind) { + return Visit::Abort; + } + if self.stop_on_enter == Some(kind) { + return Visit::Stop; + } + if self.skip_on_enter == Some(kind) { + return Visit::Skip; + } + Visit::Continue + } + fn exit_tree(&mut self, ctx: &VisitCtx<'_>, tree: NodeId) -> Visit { + let kind = ctx.node(tree).unwrap().kind; + self.events.push(Event::Exit(kind)); + if self.abort_on_exit == Some(kind) { + return Visit::Abort; + } + if self.stop_on_exit == Some(kind) { + return Visit::Stop; + } + if self.skip_on_exit == Some(kind) { + return Visit::Skip; + } + Visit::Continue + } + fn visit_token(&mut self, ctx: &VisitCtx<'_>, _token: TokenId, parent: NodeId) -> Visit { + self.events.push(Event::Token); + if self.abort_on_token { + return Visit::Abort; + } + if self.stop_on_token { + return Visit::Stop; + } + if self.skip_on_token_in == Some(ctx.node(parent).unwrap().kind) { + return Visit::Skip; + } + Visit::Continue + } +} + +const TEXT: &str = "a: list[u32] = { 1 2 }\nb: u32 = 3"; + +/// Every `Enter` has a matching, properly nested `Exit`, and nothing is +/// left open at the end. +fn assert_balanced(events: &[Event]) { + let mut stack = Vec::new(); + for event in events { + match event { + Event::Enter(kind) => stack.push(*kind), + Event::Exit(kind) => { + assert_eq!(stack.pop(), Some(*kind), "mismatched exit in {events:#?}") + } + Event::Token => {} + } + } + assert!(stack.is_empty(), "nodes never exited: {stack:?}"); +} + +fn count(events: &[Event], event: Event) -> usize { + events.iter().filter(|e| **e == event).count() +} + +#[test] +fn full_walk_is_balanced() { + let events = Recorder::default().walk(TEXT); + assert_balanced(&events); + assert_eq!(events.first(), Some(&Event::Enter(Kind::File))); + assert_eq!(events.last(), Some(&Event::Exit(Kind::File))); + assert_eq!(count(&events, Event::Enter(Kind::Entry)), 2); +} + +#[test] +fn stop_from_enter_exits_open_ancestors() { + let events = Recorder { + stop_on_enter: Some(Kind::EntryValue), + ..Default::default() + } + .walk(TEXT); + assert_balanced(&events); + // nothing new is entered after the stop fired + let stop_at = events + .iter() + .position(|e| *e == Event::Enter(Kind::EntryValue)) + .unwrap(); + assert!( + events[stop_at + 1..] + .iter() + .all(|e| matches!(e, Event::Exit(_))), + "walk continued after Stop: {events:#?}" + ); +} + +#[test] +fn stop_from_token_exits_open_ancestors() { + let events = Recorder { + stop_on_token: true, + ..Default::default() + } + .walk(TEXT); + assert_balanced(&events); + assert_eq!(count(&events, Event::Token), 1); +} + +#[test] +fn stop_from_exit_exits_open_ancestors() { + let events = Recorder { + stop_on_exit: Some(Kind::EntryKey), + ..Default::default() + } + .walk(TEXT); + assert_balanced(&events); + let stop_at = events + .iter() + .position(|e| *e == Event::Exit(Kind::EntryKey)) + .unwrap(); + assert!( + events[stop_at + 1..] + .iter() + .all(|e| matches!(e, Event::Exit(_))), + "walk continued after Stop: {events:#?}" + ); +} + +#[test] +fn skip_from_enter_skips_children_but_still_exits() { + let events = Recorder { + skip_on_enter: Some(Kind::Entry), + ..Default::default() + } + .walk(TEXT); + assert_balanced(&events); + // children of both entries were skipped, the entries still exited, + // and the walk went on to the sibling entry + assert_eq!(count(&events, Event::Enter(Kind::EntryKey)), 0); + assert_eq!(count(&events, Event::Enter(Kind::Entry)), 2); + assert_eq!(count(&events, Event::Exit(Kind::Entry)), 2); +} + +#[test] +fn skip_from_token_skips_the_nodes_remaining_children() { + // skip fires on the block's `{`, so its list items are never entered + let events = Recorder { + skip_on_token_in: Some(Kind::Block), + ..Default::default() + } + .walk(TEXT); + assert_balanced(&events); + assert_eq!(count(&events, Event::Enter(Kind::ListItem)), 0); + assert_eq!(count(&events, Event::Exit(Kind::Block)), 1); +} + +#[test] +fn skip_from_exit_prunes_later_siblings() { + let events = Recorder { + skip_on_exit: Some(Kind::Entry), + ..Default::default() + } + .walk(TEXT); + assert_balanced(&events); + // the first entry's exit returned Skip, so the second entry is pruned, + // but the parent still exits normally + assert_eq!(count(&events, Event::Enter(Kind::Entry)), 1); + assert_eq!(events.last(), Some(&Event::Exit(Kind::File))); +} + +#[test] +fn pruning_is_local_and_the_walk_continues_elsewhere() { + // the first list item's exit prunes the block's remaining children, + // but everything outside the block is still walked + let events = Recorder { + skip_on_exit: Some(Kind::ListItem), + ..Default::default() + } + .walk(TEXT); + assert_balanced(&events); + assert_eq!(count(&events, Event::Enter(Kind::ListItem)), 1); + assert_eq!(count(&events, Event::Enter(Kind::Entry)), 2); +} + +#[test] +fn abort_from_enter_runs_no_more_callbacks() { + let events = Recorder { + abort_on_enter: Some(Kind::EntryValue), + ..Default::default() + } + .walk(TEXT); + // the aborting node and its open ancestors never exit + assert_eq!(events.last(), Some(&Event::Enter(Kind::EntryValue))); + assert_eq!(count(&events, Event::Exit(Kind::Entry)), 0); + assert_eq!(count(&events, Event::Exit(Kind::File)), 0); +} + +#[test] +fn abort_from_token_runs_no_more_callbacks() { + let events = Recorder { + abort_on_token: true, + ..Default::default() + } + .walk(TEXT); + assert_eq!(count(&events, Event::Token), 1); + assert_eq!(events.last(), Some(&Event::Token)); +} + +#[test] +fn abort_from_exit_skips_the_remaining_exits() { + let events = Recorder { + abort_on_exit: Some(Kind::EntryKey), + ..Default::default() + } + .walk(TEXT); + assert_eq!(events.last(), Some(&Event::Exit(Kind::EntryKey))); + assert_eq!(count(&events, Event::Exit(Kind::Entry)), 0); + assert_eq!(count(&events, Event::Exit(Kind::File)), 0); +} + +#[test] +fn abort_from_exit_while_unwinding_skips_the_remaining_exits() { + // a Stop deep in the tree starts unwinding; an ancestor's exit aborts, + // so the exits above it never run + let events = Recorder { + stop_on_enter: Some(Kind::EntryValue), + abort_on_exit: Some(Kind::Entry), + ..Default::default() + } + .walk(TEXT); + assert_eq!(events.last(), Some(&Event::Exit(Kind::Entry))); + assert_eq!(count(&events, Event::Exit(Kind::File)), 0); +} + +#[test] +fn walk_reports_how_it_ended() { + let cst = Cst::parse(TEXT); + assert_eq!(cst.walk(&mut Recorder::default()), WalkOutcome::Completed); + assert_eq!( + cst.walk(&mut Recorder { + stop_on_token: true, + ..Default::default() + }), + WalkOutcome::Stopped + ); + assert_eq!( + cst.walk(&mut Recorder { + abort_on_token: true, + ..Default::default() + }), + WalkOutcome::Aborted + ); + // pruning is not a stop + assert_eq!( + cst.walk(&mut Recorder { + skip_on_exit: Some(Kind::Entry), + ..Default::default() + }), + WalkOutcome::Completed + ); +} diff --git a/crates/ltk_ritobin/src/parse.rs b/crates/ltk_ritobin/src/parse.rs index d202a7e9..3aecd53e 100644 --- a/crates/ltk_ritobin/src/parse.rs +++ b/crates/ltk_ritobin/src/parse.rs @@ -74,6 +74,28 @@ mod test { assert!(!cst.errors.is_empty(), "Parsed successfully",); } + #[test] + fn error_spans_stay_within_the_source() { + // all of these error at the end of input, where the "point just past + // the token" error span used to run past the source + for text in ["entries: map[hash,embed] = {", "a: u32 =", "a:", ""] { + let cst = Cst::parse(text); + for err in &cst.errors { + assert!( + err.span.start <= err.span.end, + "inverted error span {:?} for {text:?}", + err.span + ); + assert!( + err.span.end as usize <= text.len(), + "error span {:?} leaves the source (len {}) for {text:?}", + err.span, + text.len() + ); + } + } + } + #[test] fn comments() { assert_success( diff --git a/crates/ltk_ritobin/src/parse/parser.rs b/crates/ltk_ritobin/src/parse/parser.rs index 564ac54a..9845220d 100644 --- a/crates/ltk_ritobin/src/parse/parser.rs +++ b/crates/ltk_ritobin/src/parse/parser.rs @@ -76,6 +76,7 @@ impl<'a> Parser<'a> { } pub fn build_tree(self, error_propagation: ErrorPropagation) -> Cst { + let source_len = self.text.len() as u32; let last_token = self.tokens.last().copied(); let mut tokens = self.tokens.into_iter().peekable(); @@ -176,11 +177,10 @@ impl<'a> Parser<'a> { ErrorKind::Expected { .. } | ErrorKind::ExpectedAny { .. } | ErrorKind::Unexpected { .. } => { - let mut span = tokens.peek().map(|t| t.span).unwrap_or(last_span); - // so we point at the character just after our token - span.end += 1; - span.start = span.end - 1; - span + let base = tokens.peek().map(|t| t.span).unwrap_or(last_span); + // point at the next token, clamped to the source end + let start = base.end.min(source_len); + Span::new(start, (start + 1).min(source_len)) } // whole tree is the problem ErrorKind::UnexpectedTree => cur_node.span, diff --git a/crates/ltk_ritobin/src/parse/span.rs b/crates/ltk_ritobin/src/parse/span.rs index cc5ed20f..ccf9fd4c 100644 --- a/crates/ltk_ritobin/src/parse/span.rs +++ b/crates/ltk_ritobin/src/parse/span.rs @@ -1,4 +1,5 @@ -/// A span in the source text (offset and length). +/// A span of text in the source file - `[start, end)` in bytes. +/// `end` marks the offset after the last byte of the span #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] pub struct Span { @@ -13,24 +14,28 @@ impl Span { Self { start, end } } + /// Whether this span contains `offset` #[must_use] #[inline] pub fn contains(&self, offset: u32) -> bool { - self.start <= offset && offset <= self.end + self.start <= offset && offset < self.end } + /// Whether two span ranges intersect #[must_use] #[inline] pub fn intersects(&self, other: &Span) -> bool { self.start < other.end && other.start < self.end } + /// The length of the span in bytes #[must_use] #[inline] pub fn len(&self) -> u32 { self.end - self.start } + /// Whether the span is empty #[must_use] #[inline] pub fn is_empty(&self) -> bool { @@ -69,3 +74,47 @@ impl std::ops::Index<&Span> for String { &self[*index] } } + +#[cfg(test)] +mod test { + use super::Span; + + #[test] + fn contains_is_half_open() { + let span = Span::new(2, 5); + assert!(!span.contains(1)); + assert!(span.contains(2)); + assert!(span.contains(4)); + assert!(!span.contains(5)); + assert!(!span.contains(6)); + } + + #[test] + fn empty_span_contains_nothing() { + let span = Span::new(3, 3); + assert!(!span.contains(2)); + assert!(!span.contains(3)); + assert!(!span.contains(4)); + } + + #[test] + fn boundary_offset_belongs_to_exactly_one_neighbor() { + let left = Span::new(0, 3); + let right = Span::new(3, 6); + assert!(!left.contains(3)); + assert!(right.contains(3)); + assert!(!left.intersects(&right)); + } + + #[test] + fn contains_matches_intersects() { + let span = Span::new(2, 5); + for offset in 0..8 { + assert_eq!( + span.contains(offset), + span.intersects(&Span::new(offset, offset + 1)), + "offset {offset}" + ); + } + } +} diff --git a/crates/ltk_ritobin/src/print/visitor.rs b/crates/ltk_ritobin/src/print/visitor.rs index be2d6146..0734e46b 100644 --- a/crates/ltk_ritobin/src/print/visitor.rs +++ b/crates/ltk_ritobin/src/print/visitor.rs @@ -651,29 +651,38 @@ impl<'a, W: Write> CstVisitor<'a, W> { impl<'a, W: fmt::Write> Visitor for CstVisitor<'a, W> { fn enter_tree(&mut self, ctx: &VisitCtx<'_>, tree: NodeId) -> Visit { + if self.error.is_some() { + return Visit::Abort; + } match self.enter_tree_inner(ctx, tree) { Ok(_) => Visit::Continue, Err(e) => { self.error.replace(e); - Visit::Stop + Visit::Abort } } } fn exit_tree(&mut self, ctx: &VisitCtx<'_>, tree: NodeId) -> Visit { + if self.error.is_some() { + return Visit::Abort; + } match self.exit_tree_inner(ctx, tree) { Ok(_) => Visit::Continue, Err(e) => { self.error.replace(e); - Visit::Stop + Visit::Abort } } } fn visit_token(&mut self, ctx: &VisitCtx<'_>, token: TokenId, parent: NodeId) -> Visit { + if self.error.is_some() { + return Visit::Abort; + } match self.visit_token_inner(ctx, token, parent) { Ok(_) => Visit::Continue, Err(e) => { self.error.replace(e); - Visit::Stop + Visit::Abort } } }