diff --git a/crates/plotnik-lib/src/diagnostics/message.rs b/crates/plotnik-lib/src/diagnostics/message.rs index dfad769a..bb0e5ff6 100644 --- a/crates/plotnik-lib/src/diagnostics/message.rs +++ b/crates/plotnik-lib/src/diagnostics/message.rs @@ -58,6 +58,7 @@ pub enum DiagnosticKind { UndefinedReference, MixedAltBranches, RecursionNoEscape, + DirectRecursion, FieldSequenceValue, // Link pass - grammar validation @@ -162,6 +163,7 @@ impl DiagnosticKind { Self::UndefinedReference => "undefined reference", Self::MixedAltBranches => "cannot mix labeled and unlabeled branches", Self::RecursionNoEscape => "infinite recursion detected", + Self::DirectRecursion => "direct recursion: query will stuck without matching anything", Self::FieldSequenceValue => "field must match exactly one node", // Link pass - grammar validation @@ -200,6 +202,7 @@ impl DiagnosticKind { // Recursion with cycle path Self::RecursionNoEscape => "infinite recursion: {}".to_string(), + Self::DirectRecursion => "direct recursion: {}".to_string(), // Alternation mixing Self::MixedAltBranches => "cannot mix labeled and unlabeled branches: {}".to_string(), diff --git a/crates/plotnik-lib/src/query/link_tests.rs b/crates/plotnik-lib/src/query/link_tests.rs index a0d2271b..9c9d340b 100644 --- a/crates/plotnik-lib/src/query/link_tests.rs +++ b/crates/plotnik-lib/src/query/link_tests.rs @@ -928,6 +928,13 @@ fn ref_followed_recursive_with_invalid_type() { | ---- field `name` on `function_declaration` | help: valid types for `name`: `identifier` + + error: direct recursion: cycle `Foo` → `Foo` will stuck without matching anything + | + 1 | Foo = [(number) (Foo)] + | ^^^ + | | + | `Foo` references itself "); } @@ -941,7 +948,15 @@ fn ref_followed_recursive_valid() { let mut query = Query::try_from(input).unwrap(); query.link(&plotnik_langs::javascript()); - assert!(query.is_valid()); + assert!(!query.is_valid()); + insta::assert_snapshot!(query.dump_diagnostics(), @r" + error: direct recursion: cycle `Foo` → `Foo` will stuck without matching anything + | + 1 | Foo = [(identifier) (Foo)] + | ^^^ + | | + | `Foo` references itself + "); } #[test] @@ -975,6 +990,16 @@ fn ref_followed_mutual_recursion() { | ---- field `name` on `function_declaration` | help: valid types for `name`: `identifier` + + error: direct recursion: cycle `Bar` → `Foo` → `Bar` will stuck without matching anything + | + 1 | Foo = [(number) (Bar)] + | --- `Foo` references `Bar` (completing cycle) + 2 | Bar = [(string) (Foo)] + | --- ^^^ + | | | + | | `Bar` references `Foo` + | `Bar` is defined here "); } diff --git a/crates/plotnik-lib/src/query/recursion.rs b/crates/plotnik-lib/src/query/recursion.rs index 4fd49625..1aa8d4f0 100644 --- a/crates/plotnik-lib/src/query/recursion.rs +++ b/crates/plotnik-lib/src/query/recursion.rs @@ -1,7 +1,8 @@ //! Escape path analysis for recursive definitions. //! //! Detects patterns that can never match because they require -//! infinitely nested structures (recursion with no escape path). +//! infinitely nested structures (recursion with no escape path), +//! or infinite runtime loops where the cursor never advances (left recursion). use indexmap::{IndexMap, IndexSet}; use rowan::TextRange; @@ -17,6 +18,8 @@ impl Query<'_> { for scc in sccs { let scc_set: IndexSet<&str> = scc.iter().map(|s| s.as_str()).collect(); + // 1. Check for infinite tree structure (Escape Analysis) + // Existing logic: at least one definition must have a non-recursive path. let has_escape = scc.iter().any(|name| { self.symbol_table .get(name.as_str()) @@ -24,16 +27,22 @@ impl Query<'_> { .unwrap_or(true) }); - if has_escape { + if !has_escape { + let chain = if scc.len() == 1 { + self.build_self_ref_chain(&scc[0]) + } else { + self.build_cycle_chain(&scc) + }; + self.emit_recursion_error(&scc[0], &scc, chain); continue; } - let chain = if scc.len() == 1 { - self.build_self_ref_chain(&scc[0]) - } else { - self.build_cycle_chain(&scc) - }; - self.emit_recursion_error(&scc[0], &scc, chain); + // 2. Check for infinite loops (Guarded Recursion Analysis) + // Ensure every recursive cycle consumes at least one node. + if let Some(cycle) = self.find_unguarded_cycle(&scc, &scc_set) { + let chain = self.build_unguarded_chain(&cycle); + self.emit_direct_recursion_error(&cycle[0], &cycle, chain); + } } } @@ -118,6 +127,63 @@ impl Query<'_> { .collect() } + fn find_unguarded_cycle( + &self, + scc: &[String], + scc_set: &IndexSet<&str>, + ) -> Option> { + // Build dependency graph for unguarded calls within the SCC + let mut adj = IndexMap::new(); + for name in scc { + if let Some(body) = self.symbol_table.get(name.as_str()) { + let mut refs = IndexSet::new(); + collect_unguarded_refs(body, scc_set, &mut refs); + adj.insert(name.clone(), refs); + } + } + + // Detect cycle + let mut visited = IndexSet::new(); + let mut stack = IndexSet::new(); + + for start_node in scc { + if let Some(target) = Self::detect_cycle(start_node, &adj, &mut visited, &mut stack) { + let index = stack.get_index_of(&target).unwrap(); + return Some(stack.iter().skip(index).cloned().collect()); + } + } + + None + } + + fn detect_cycle( + node: &String, + adj: &IndexMap>, + visited: &mut IndexSet, + stack: &mut IndexSet, + ) -> Option { + if stack.contains(node) { + return Some(node.clone()); + } + if visited.contains(node) { + return None; + } + + visited.insert(node.clone()); + stack.insert(node.clone()); + + if let Some(neighbors) = adj.get(node) { + for neighbor in neighbors { + if let Some(target) = Self::detect_cycle(neighbor, adj, visited, stack) { + return Some(target); + } + } + } + + stack.pop(); + None + } + fn find_def_by_name(&self, name: &str) -> Option { self.ast .defs() @@ -130,6 +196,12 @@ impl Query<'_> { find_ref_in_expr(&body, to) } + fn find_unguarded_reference_location(&self, from: &str, to: &str) -> Option { + let def = self.find_def_by_name(from)?; + let body = def.body()?; + find_unguarded_ref_in_expr(&body, to) + } + fn build_self_ref_chain(&self, name: &str) -> Vec<(TextRange, String)> { self.find_reference_location(name, name) .map(|range| vec![(range, format!("`{}` references itself", name))]) @@ -137,6 +209,8 @@ impl Query<'_> { } fn build_cycle_chain(&self, scc: &[String]) -> Vec<(TextRange, String)> { + // Since Tarjan's sccs are not guaranteed to be ordered as a cycle, + // we need to find the cycle path explicitly. let scc_set: IndexSet<&str> = scc.iter().map(|s| s.as_str()).collect(); let mut visited = IndexSet::new(); let mut path = Vec::new(); @@ -189,6 +263,39 @@ impl Query<'_> { .collect() } + fn build_unguarded_chain(&self, cycle: &[String]) -> Vec<(TextRange, String)> { + if cycle.len() == 1 { + return self + .find_unguarded_reference_location(&cycle[0], &cycle[0]) + .map(|range| vec![(range, format!("`{}` references itself", cycle[0]))]) + .unwrap_or_default(); + } + self.build_chain_generic(cycle, |from, to| { + self.find_unguarded_reference_location(from, to) + }) + } + + fn build_chain_generic(&self, path_nodes: &[String], find_loc: F) -> Vec<(TextRange, String)> + where + F: Fn(&str, &str) -> Option, + { + path_nodes + .iter() + .enumerate() + .filter_map(|(i, from)| { + let to = &path_nodes[(i + 1) % path_nodes.len()]; + find_loc(from, to).map(|range| { + let msg = if i == path_nodes.len() - 1 { + format!("`{}` references `{}` (completing cycle)", from, to) + } else { + format!("`{}` references `{}`", from, to) + }; + (range, msg) + }) + }) + .collect() + } + fn emit_recursion_error( &mut self, primary_name: &str, @@ -208,6 +315,14 @@ impl Query<'_> { .map(|(r, _)| *r) .unwrap_or_else(|| TextRange::empty(0.into())); + let def_range = if scc.len() > 1 { + self.find_def_by_name(primary_name) + .and_then(|def| def.name()) + .map(|n| n.text_range()) + } else { + None + }; + let mut builder = self .recursion_diagnostics .report(DiagnosticKind::RecursionNoEscape, range) @@ -217,6 +332,55 @@ impl Query<'_> { builder = builder.related_to(rel_msg, rel_range); } + if let Some(range) = def_range { + builder = builder.related_to(format!("`{}` is defined here", primary_name), range); + } + + builder.emit(); + } + fn emit_direct_recursion_error( + &mut self, + primary_name: &str, + scc: &[String], + related: Vec<(TextRange, String)>, + ) { + let cycle_str = if scc.len() == 1 { + format!("`{}` → `{}`", primary_name, primary_name) + } else { + let mut cycle: Vec<_> = scc.iter().map(|s| format!("`{}`", s)).collect(); + cycle.push(format!("`{}`", scc[0])); + cycle.join(" → ") + }; + + let range = related + .first() + .map(|(r, _)| *r) + .unwrap_or_else(|| TextRange::empty(0.into())); + + let def_range = if scc.len() > 1 { + self.find_def_by_name(primary_name) + .and_then(|def| def.name()) + .map(|n| n.text_range()) + } else { + None + }; + + let mut builder = self + .recursion_diagnostics + .report(DiagnosticKind::DirectRecursion, range) + .message(format!( + "cycle {} will stuck without matching anything", + cycle_str + )); + + for (rel_range, rel_msg) in related { + builder = builder.related_to(rel_msg, rel_range); + } + + if let Some(range) = def_range { + builder = builder.related_to(format!("`{}` is defined here", primary_name), range); + } + builder.emit(); } } @@ -250,6 +414,24 @@ fn expr_has_escape(expr: &Expr, scc: &IndexSet<&str>) -> bool { } } +fn expr_guarantees_consumption(expr: &Expr) -> bool { + match expr { + Expr::NamedNode(_) | Expr::AnonymousNode(_) => true, + Expr::Ref(_) => false, + Expr::AltExpr(_) => expr.children().iter().all(expr_guarantees_consumption), + Expr::SeqExpr(_) => expr.children().iter().any(expr_guarantees_consumption), + Expr::QuantifiedExpr(q) => { + !q.is_optional() + && q.inner() + .map(|i| expr_guarantees_consumption(&i)) + .unwrap_or(false) + } + Expr::CapturedExpr(_) | Expr::FieldExpr(_) => { + expr.children().iter().all(expr_guarantees_consumption) + } + } +} + fn collect_refs(expr: &Expr) -> IndexSet { let mut refs = IndexSet::new(); collect_refs_into(expr, &mut refs); @@ -268,6 +450,42 @@ fn collect_refs_into(expr: &Expr, refs: &mut IndexSet) { } } +fn collect_unguarded_refs(expr: &Expr, scc: &IndexSet<&str>, refs: &mut IndexSet) { + match expr { + Expr::Ref(r) => { + if let Some(name) = r.name().filter(|n| scc.contains(n.text())) { + refs.insert(name.text().to_string()); + } + } + Expr::NamedNode(_) | Expr::AnonymousNode(_) => { + // Consumes input, so guards recursion. Do not collect refs inside. + } + Expr::AltExpr(_) => { + for c in expr.children() { + collect_unguarded_refs(&c, scc, refs); + } + } + Expr::SeqExpr(_) => { + for c in expr.children() { + collect_unguarded_refs(&c, scc, refs); + if expr_guarantees_consumption(&c) { + break; + } + } + } + Expr::QuantifiedExpr(q) => { + if let Some(inner) = q.inner() { + collect_unguarded_refs(&inner, scc, refs); + } + } + Expr::CapturedExpr(_) | Expr::FieldExpr(_) => { + for c in expr.children() { + collect_unguarded_refs(&c, scc, refs); + } + } + } +} + fn find_ref_in_expr(expr: &Expr, target: &str) -> Option { if let Expr::Ref(r) = expr { let name_token = r.name()?; @@ -280,3 +498,35 @@ fn find_ref_in_expr(expr: &Expr, target: &str) -> Option { .iter() .find_map(|child| find_ref_in_expr(child, target)) } + +fn find_unguarded_ref_in_expr(expr: &Expr, target: &str) -> Option { + match expr { + Expr::Ref(r) => r + .name() + .filter(|n| n.text() == target) + .map(|n| n.text_range()), + Expr::NamedNode(_) | Expr::AnonymousNode(_) => None, + Expr::AltExpr(_) => expr + .children() + .iter() + .find_map(|c| find_unguarded_ref_in_expr(c, target)), + Expr::SeqExpr(_) => { + for c in expr.children() { + if let Some(range) = find_unguarded_ref_in_expr(&c, target) { + return Some(range); + } + if expr_guarantees_consumption(&c) { + return None; + } + } + None + } + Expr::QuantifiedExpr(q) => q + .inner() + .and_then(|i| find_unguarded_ref_in_expr(&i, target)), + Expr::CapturedExpr(_) | Expr::FieldExpr(_) => expr + .children() + .iter() + .find_map(|c| find_unguarded_ref_in_expr(c, target)), + } +} diff --git a/crates/plotnik-lib/src/query/recursion_tests.rs b/crates/plotnik-lib/src/query/recursion_tests.rs index 81fadca3..b74eb427 100644 --- a/crates/plotnik-lib/src/query/recursion_tests.rs +++ b/crates/plotnik-lib/src/query/recursion_tests.rs @@ -2,28 +2,28 @@ use crate::Query; use indoc::indoc; #[test] -fn escape_via_alternation() { +fn valid_recursion_with_alternation_base_case() { let query = Query::try_from("E = [(x) (call (E))]").unwrap(); assert!(query.is_valid()); } #[test] -fn escape_via_optional() { +fn valid_recursion_with_optional() { let query = Query::try_from("E = (call (E)?)").unwrap(); assert!(query.is_valid()); } #[test] -fn escape_via_star() { +fn valid_recursion_with_star() { let query = Query::try_from("E = (call (E)*)").unwrap(); assert!(query.is_valid()); } #[test] -fn no_escape_via_plus() { +fn invalid_recursion_with_plus() { let query = Query::try_from("E = (call (E)+)").unwrap(); assert!(!query.is_valid()); @@ -39,21 +39,30 @@ fn no_escape_via_plus() { } #[test] -fn escape_via_empty_tree() { +fn invalid_unguarded_recursion_in_alternation() { let query = Query::try_from("E = [(call) (E)]").unwrap(); - assert!(query.is_valid()); + assert!(!query.is_valid()); + + insta::assert_snapshot!(query.dump_diagnostics(), @r" + error: direct recursion: cycle `E` → `E` will stuck without matching anything + | + 1 | E = [(call) (E)] + | ^ + | | + | `E` references itself + "); } #[test] -fn lazy_quantifiers_same_as_greedy() { +fn validity_of_lazy_quantifiers_matches_greedy() { assert!(Query::try_from("E = (call (E)??)").unwrap().is_valid()); assert!(Query::try_from("E = (call (E)*?)").unwrap().is_valid()); assert!(!Query::try_from("E = (call (E)+?)").unwrap().is_valid()); } #[test] -fn recursion_in_tree_child() { +fn invalid_mandatory_recursion_in_tree_child() { let query = Query::try_from("E = (call (E))").unwrap(); assert!(!query.is_valid()); @@ -69,7 +78,7 @@ fn recursion_in_tree_child() { } #[test] -fn recursion_in_field() { +fn invalid_mandatory_recursion_in_field() { let query = Query::try_from("E = (call body: (E))").unwrap(); assert!(!query.is_valid()); @@ -85,7 +94,7 @@ fn recursion_in_field() { } #[test] -fn recursion_in_capture() { +fn invalid_mandatory_recursion_in_capture() { let query = Query::try_from("E = (call (E) @inner)").unwrap(); assert!(!query.is_valid()); @@ -101,7 +110,7 @@ fn recursion_in_capture() { } #[test] -fn recursion_in_sequence() { +fn invalid_mandatory_recursion_in_sequence() { let query = Query::try_from("E = (call {(a) (E)})").unwrap(); assert!(!query.is_valid()); @@ -117,14 +126,14 @@ fn recursion_in_sequence() { } #[test] -fn recursion_through_multiple_children() { +fn valid_recursion_with_base_case_and_descent() { let query = Query::try_from("E = [(x) (call (a) (E))]").unwrap(); assert!(query.is_valid()); } #[test] -fn mutual_recursion_no_escape() { +fn invalid_mutual_recursion_without_base_case() { let input = indoc! {r#" A = (foo (B)) B = (bar (A)) @@ -139,14 +148,15 @@ fn mutual_recursion_no_escape() { 1 | A = (foo (B)) | - `A` references `B` (completing cycle) 2 | B = (bar (A)) - | ^ - | | - | `B` references `A` + | - ^ + | | | + | | `B` references `A` + | `B` is defined here "); } #[test] -fn mutual_recursion_one_has_escape() { +fn valid_mutual_recursion_with_base_case() { let input = indoc! {r#" A = [(x) (foo (B))] B = (bar (A)) @@ -157,7 +167,7 @@ fn mutual_recursion_one_has_escape() { } #[test] -fn three_way_cycle_no_escape() { +fn invalid_three_way_mutual_recursion() { let input = indoc! {r#" A = (a (B)) B = (b (C)) @@ -175,14 +185,15 @@ fn three_way_cycle_no_escape() { 2 | B = (b (C)) | - `B` references `C` (completing cycle) 3 | C = (c (A)) - | ^ - | | - | `C` references `A` + | - ^ + | | | + | | `C` references `A` + | `C` is defined here "); } #[test] -fn three_way_cycle_one_has_escape() { +fn valid_three_way_mutual_recursion_with_base_case() { let input = indoc! {r#" A = [(x) (a (B))] B = (b (C)) @@ -194,7 +205,7 @@ fn three_way_cycle_one_has_escape() { } #[test] -fn diamond_dependency() { +fn invalid_diamond_dependency_recursion() { let input = indoc! {r#" A = (a [(B) (C)]) B = (b (D)) @@ -212,16 +223,17 @@ fn diamond_dependency() { | - `A` references `C` (completing cycle) 2 | B = (b (D)) 3 | C = (c (D)) - | ^ - | | - | `C` references `D` + | - ^ + | | | + | | `C` references `D` + | `C` is defined here 4 | D = (d (A)) | - `D` references `A` "); } #[test] -fn cycle_ref_in_field() { +fn invalid_mutual_recursion_via_field() { let input = indoc! {r#" A = (foo body: (B)) B = (bar (A)) @@ -236,14 +248,15 @@ fn cycle_ref_in_field() { 1 | A = (foo body: (B)) | - `A` references `B` (completing cycle) 2 | B = (bar (A)) - | ^ - | | - | `B` references `A` + | - ^ + | | | + | | `B` references `A` + | `B` is defined here "); } #[test] -fn cycle_ref_in_capture() { +fn invalid_mutual_recursion_via_capture() { let input = indoc! {r#" A = (foo (B) @cap) B = (bar (A)) @@ -258,14 +271,15 @@ fn cycle_ref_in_capture() { 1 | A = (foo (B) @cap) | - `A` references `B` (completing cycle) 2 | B = (bar (A)) - | ^ - | | - | `B` references `A` + | - ^ + | | | + | | `B` references `A` + | `B` is defined here "); } #[test] -fn cycle_ref_in_sequence() { +fn invalid_mutual_recursion_via_sequence() { let input = indoc! {r#" A = (foo {(x) (B)}) B = (bar (A)) @@ -280,14 +294,15 @@ fn cycle_ref_in_sequence() { 1 | A = (foo {(x) (B)}) | - `A` references `B` (completing cycle) 2 | B = (bar (A)) - | ^ - | | - | `B` references `A` + | - ^ + | | | + | | `B` references `A` + | `B` is defined here "); } #[test] -fn cycle_with_quantifier_escape() { +fn valid_mutual_recursion_with_optional_quantifier() { let input = indoc! {r#" A = (foo (B)?) B = (bar (A)) @@ -298,7 +313,7 @@ fn cycle_with_quantifier_escape() { } #[test] -fn cycle_with_plus_no_escape() { +fn invalid_mutual_recursion_with_plus_quantifier() { let input = indoc! {r#" A = (foo (B)+) B = (bar (A)) @@ -313,14 +328,15 @@ fn cycle_with_plus_no_escape() { 1 | A = (foo (B)+) | - `A` references `B` (completing cycle) 2 | B = (bar (A)) - | ^ - | | - | `B` references `A` + | - ^ + | | | + | | `B` references `A` + | `B` is defined here "); } #[test] -fn non_recursive_reference() { +fn valid_non_recursive_reference() { let input = indoc! {r#" Leaf = (identifier) Tree = (call (Leaf)) @@ -331,7 +347,7 @@ fn non_recursive_reference() { } #[test] -fn entry_point_uses_recursive_def() { +fn valid_entry_point_using_recursive_def() { let input = indoc! {r#" E = [(x) (call (E))] (program (E)) @@ -342,44 +358,112 @@ fn entry_point_uses_recursive_def() { } #[test] -fn direct_self_ref_in_alternation() { +fn invalid_direct_left_recursion_in_alternation() { let query = Query::try_from("E = [(E) (x)]").unwrap(); - assert!(query.is_valid()); + assert!(!query.is_valid()); + + insta::assert_snapshot!(query.dump_diagnostics(), @r" + error: direct recursion: cycle `E` → `E` will stuck without matching anything + | + 1 | E = [(E) (x)] + | ^ + | | + | `E` references itself + "); } #[test] -fn escape_via_literal_string() { +fn invalid_direct_right_recursion_in_alternation() { + let query = Query::try_from("E = [(x) (E)]").unwrap(); + + assert!(!query.is_valid()); + + insta::assert_snapshot!(query.dump_diagnostics(), @r" + error: direct recursion: cycle `E` → `E` will stuck without matching anything + | + 1 | E = [(x) (E)] + | ^ + | | + | `E` references itself + "); +} + +#[test] +fn invalid_direct_left_recursion_in_tagged_alternation() { + let query = Query::try_from("E = [Left: (E) Right: (x)]").unwrap(); + + assert!(!query.is_valid()); + + insta::assert_snapshot!(query.dump_diagnostics(), @r" + error: direct recursion: cycle `E` → `E` will stuck without matching anything + | + 1 | E = [Left: (E) Right: (x)] + | ^ + | | + | `E` references itself + "); +} + +#[test] +fn invalid_unguarded_left_recursion_branch() { let input = indoc! {r#" - A = [(A) "escape"] + A = [(A) 'escape'] "#}; let query = Query::try_from(input).unwrap(); - assert!(query.is_valid()); + assert!(!query.is_valid()); + + insta::assert_snapshot!(query.dump_diagnostics(), @r" + error: direct recursion: cycle `A` → `A` will stuck without matching anything + | + 1 | A = [(A) 'escape'] + | ^ + | | + | `A` references itself + "); } #[test] -fn escape_via_wildcard() { +fn invalid_unguarded_left_recursion_with_wildcard_alt() { let input = indoc! {r#" A = [(A) _] - "#}; + "#}; let query = Query::try_from(input).unwrap(); - assert!(query.is_valid()); + assert!(!query.is_valid()); + + insta::assert_snapshot!(query.dump_diagnostics(), @r" + error: direct recursion: cycle `A` → `A` will stuck without matching anything + | + 1 | A = [(A) _] + | ^ + | | + | `A` references itself + "); } #[test] -fn escape_via_childless_tree() { +fn invalid_unguarded_left_recursion_with_tree_alt() { let input = indoc! {r#" A = [(A) (leaf)] "#}; let query = Query::try_from(input).unwrap(); - assert!(query.is_valid()); + assert!(!query.is_valid()); + + insta::assert_snapshot!(query.dump_diagnostics(), @r" + error: direct recursion: cycle `A` → `A` will stuck without matching anything + | + 1 | A = [(A) (leaf)] + | ^ + | | + | `A` references itself + "); } #[test] -fn escape_via_anchor() { +fn valid_recursion_guarded_by_anchor() { let input = indoc! {r#" A = (foo . [(A) (x)]) "#}; @@ -389,7 +473,7 @@ fn escape_via_anchor() { } #[test] -fn no_escape_tree_all_recursive() { +fn invalid_mandatory_recursion_direct_child() { let input = indoc! {r#" A = (foo (A)) "#}; @@ -408,7 +492,7 @@ fn no_escape_tree_all_recursive() { } #[test] -fn escape_in_capture_inner() { +fn valid_recursion_with_capture_base_case() { let input = indoc! {r#" A = [(x)@cap (foo (A))] "#}; @@ -418,7 +502,7 @@ fn escape_in_capture_inner() { } #[test] -fn ref_in_quantifier_plus_no_escape() { +fn invalid_mandatory_recursion_nested_plus() { let input = indoc! {r#" A = (foo (A)+) "#}; @@ -426,3 +510,48 @@ fn ref_in_quantifier_plus_no_escape() { assert!(!query.is_valid()); } + +#[test] +fn invalid_simple_unguarded_recursion() { + let input = indoc! {r#" + A = [ + (foo) + (A) + ] + "#}; + let query = Query::try_from(input).unwrap(); + + assert!(!query.is_valid()); + + insta::assert_snapshot!(query.dump_diagnostics(), @r" + error: direct recursion: cycle `A` → `A` will stuck without matching anything + | + 3 | (A) + | ^ + | | + | `A` references itself + "); +} + +#[test] +fn invalid_unguarded_mutual_recursion_chain() { + let input = indoc! {r#" + A = [(B) (x)] + B = (A) + "#}; + let query = Query::try_from(input).unwrap(); + + assert!(!query.is_valid()); + + insta::assert_snapshot!(query.dump_diagnostics(), @r" + error: direct recursion: cycle `B` → `A` → `B` will stuck without matching anything + | + 1 | A = [(B) (x)] + | - `A` references `B` (completing cycle) + 2 | B = (A) + | - ^ + | | | + | | `B` references `A` + | `B` is defined here + "); +} diff --git a/crates/plotnik-lib/src/query/symbol_table_tests.rs b/crates/plotnik-lib/src/query/symbol_table_tests.rs index d127c3a0..33e081a7 100644 --- a/crates/plotnik-lib/src/query/symbol_table_tests.rs +++ b/crates/plotnik-lib/src/query/symbol_table_tests.rs @@ -83,9 +83,10 @@ fn mutual_recursion() { 1 | A = (foo (B)) | - `A` references `B` (completing cycle) 2 | B = (bar (A)) - | ^ - | | - | `B` references `A` + | - ^ + | | | + | | `B` references `A` + | `B` is defined here "); }