From db3ffe258bfbe8833da96de019a69312eefc5979 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 7 Dec 2025 23:03:01 -0300 Subject: [PATCH 1/2] feat: Better diagnostics for recursion errors --- crates/plotnik-lib/src/diagnostics/message.rs | 8 +- crates/plotnik-lib/src/query/recursion.rs | 35 ++----- .../plotnik-lib/src/query/recursion_tests.rs | 96 +++++++++---------- 3 files changed, 57 insertions(+), 82 deletions(-) diff --git a/crates/plotnik-lib/src/diagnostics/message.rs b/crates/plotnik-lib/src/diagnostics/message.rs index bb0e5ff6..027c5468 100644 --- a/crates/plotnik-lib/src/diagnostics/message.rs +++ b/crates/plotnik-lib/src/diagnostics/message.rs @@ -162,8 +162,8 @@ impl DiagnosticKind { Self::DuplicateDefinition => "name already defined", 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::RecursionNoEscape => "infinite recursion: cycle has no escape path", + Self::DirectRecursion => "infinite recursion: cycle consumes no input", Self::FieldSequenceValue => "field must match exactly one node", // Link pass - grammar validation @@ -200,10 +200,6 @@ impl DiagnosticKind { Self::InvalidFieldChildType => "node type `{}` is not valid for this field".to_string(), Self::InvalidChildType => "`{}` cannot be a child of this node".to_string(), - // 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/recursion.rs b/crates/plotnik-lib/src/query/recursion.rs index 1aa8d4f0..5174f028 100644 --- a/crates/plotnik-lib/src/query/recursion.rs +++ b/crates/plotnik-lib/src/query/recursion.rs @@ -253,9 +253,9 @@ impl Query<'_> { let to = &path[(i + 1) % path.len()]; self.find_reference_location(from, to).map(|range| { let msg = if i == path.len() - 1 { - format!("`{}` references `{}` (completing cycle)", from, to) + format!("references `{}` (completing cycle)", to) } else { - format!("`{}` references `{}`", from, to) + format!("references `{}`", to) }; (range, msg) }) @@ -267,7 +267,7 @@ impl Query<'_> { if cycle.len() == 1 { return self .find_unguarded_reference_location(&cycle[0], &cycle[0]) - .map(|range| vec![(range, format!("`{}` references itself", cycle[0]))]) + .map(|range| vec![(range, "references itself".to_string())]) .unwrap_or_default(); } self.build_chain_generic(cycle, |from, to| { @@ -286,9 +286,9 @@ impl Query<'_> { 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) + format!("references `{}` (completing cycle)", to) } else { - format!("`{}` references `{}`", from, to) + format!("references `{}`", to) }; (range, msg) }) @@ -302,14 +302,6 @@ impl Query<'_> { 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) @@ -325,8 +317,7 @@ impl Query<'_> { let mut builder = self .recursion_diagnostics - .report(DiagnosticKind::RecursionNoEscape, range) - .message(format!("cycle {} has no escape path", cycle_str)); + .report(DiagnosticKind::RecursionNoEscape, range); for (rel_range, rel_msg) in related { builder = builder.related_to(rel_msg, rel_range); @@ -344,14 +335,6 @@ impl Query<'_> { 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) @@ -367,11 +350,7 @@ impl Query<'_> { let mut builder = self .recursion_diagnostics - .report(DiagnosticKind::DirectRecursion, range) - .message(format!( - "cycle {} will stuck without matching anything", - cycle_str - )); + .report(DiagnosticKind::DirectRecursion, range); for (rel_range, rel_msg) in related { builder = builder.related_to(rel_msg, rel_range); diff --git a/crates/plotnik-lib/src/query/recursion_tests.rs b/crates/plotnik-lib/src/query/recursion_tests.rs index b74eb427..cf45236f 100644 --- a/crates/plotnik-lib/src/query/recursion_tests.rs +++ b/crates/plotnik-lib/src/query/recursion_tests.rs @@ -29,7 +29,7 @@ fn invalid_recursion_with_plus() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `E` → `E` has no escape path + error: infinite recursion: cycle has no escape path | 1 | E = (call (E)+) | ^ @@ -45,12 +45,12 @@ fn invalid_unguarded_recursion_in_alternation() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: direct recursion: cycle `E` → `E` will stuck without matching anything + error: infinite recursion: cycle consumes no input | 1 | E = [(call) (E)] | ^ | | - | `E` references itself + | references itself "); } @@ -68,7 +68,7 @@ fn invalid_mandatory_recursion_in_tree_child() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `E` → `E` has no escape path + error: infinite recursion: cycle has no escape path | 1 | E = (call (E)) | ^ @@ -84,7 +84,7 @@ fn invalid_mandatory_recursion_in_field() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `E` → `E` has no escape path + error: infinite recursion: cycle has no escape path | 1 | E = (call body: (E)) | ^ @@ -100,7 +100,7 @@ fn invalid_mandatory_recursion_in_capture() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `E` → `E` has no escape path + error: infinite recursion: cycle has no escape path | 1 | E = (call (E) @inner) | ^ @@ -116,7 +116,7 @@ fn invalid_mandatory_recursion_in_sequence() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `E` → `E` has no escape path + error: infinite recursion: cycle has no escape path | 1 | E = (call {(a) (E)}) | ^ @@ -143,14 +143,14 @@ fn invalid_mutual_recursion_without_base_case() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `B` → `A` → `B` has no escape path + error: infinite recursion: cycle has no escape path | 1 | A = (foo (B)) - | - `A` references `B` (completing cycle) + | - references `B` (completing cycle) 2 | B = (bar (A)) | - ^ | | | - | | `B` references `A` + | | references `A` | `B` is defined here "); } @@ -178,16 +178,16 @@ fn invalid_three_way_mutual_recursion() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `C` → `B` → `A` → `C` has no escape path + error: infinite recursion: cycle has no escape path | 1 | A = (a (B)) - | - `A` references `B` + | - references `B` 2 | B = (b (C)) - | - `B` references `C` (completing cycle) + | - references `C` (completing cycle) 3 | C = (c (A)) | - ^ | | | - | | `C` references `A` + | | references `A` | `C` is defined here "); } @@ -217,18 +217,18 @@ fn invalid_diamond_dependency_recursion() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `C` → `D` → `B` → `A` → `C` has no escape path + error: infinite recursion: cycle has no escape path | 1 | A = (a [(B) (C)]) - | - `A` references `C` (completing cycle) + | - references `C` (completing cycle) 2 | B = (b (D)) 3 | C = (c (D)) | - ^ | | | - | | `C` references `D` + | | references `D` | `C` is defined here 4 | D = (d (A)) - | - `D` references `A` + | - references `A` "); } @@ -243,14 +243,14 @@ fn invalid_mutual_recursion_via_field() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `B` → `A` → `B` has no escape path + error: infinite recursion: cycle has no escape path | 1 | A = (foo body: (B)) - | - `A` references `B` (completing cycle) + | - references `B` (completing cycle) 2 | B = (bar (A)) | - ^ | | | - | | `B` references `A` + | | references `A` | `B` is defined here "); } @@ -266,14 +266,14 @@ fn invalid_mutual_recursion_via_capture() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `B` → `A` → `B` has no escape path + error: infinite recursion: cycle has no escape path | 1 | A = (foo (B) @cap) - | - `A` references `B` (completing cycle) + | - references `B` (completing cycle) 2 | B = (bar (A)) | - ^ | | | - | | `B` references `A` + | | references `A` | `B` is defined here "); } @@ -289,14 +289,14 @@ fn invalid_mutual_recursion_via_sequence() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `B` → `A` → `B` has no escape path + error: infinite recursion: cycle has no escape path | 1 | A = (foo {(x) (B)}) - | - `A` references `B` (completing cycle) + | - references `B` (completing cycle) 2 | B = (bar (A)) | - ^ | | | - | | `B` references `A` + | | references `A` | `B` is defined here "); } @@ -323,14 +323,14 @@ fn invalid_mutual_recursion_with_plus_quantifier() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `B` → `A` → `B` has no escape path + error: infinite recursion: cycle has no escape path | 1 | A = (foo (B)+) - | - `A` references `B` (completing cycle) + | - references `B` (completing cycle) 2 | B = (bar (A)) | - ^ | | | - | | `B` references `A` + | | references `A` | `B` is defined here "); } @@ -364,12 +364,12 @@ fn invalid_direct_left_recursion_in_alternation() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: direct recursion: cycle `E` → `E` will stuck without matching anything + error: infinite recursion: cycle consumes no input | 1 | E = [(E) (x)] | ^ | | - | `E` references itself + | references itself "); } @@ -380,12 +380,12 @@ fn invalid_direct_right_recursion_in_alternation() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: direct recursion: cycle `E` → `E` will stuck without matching anything + error: infinite recursion: cycle consumes no input | 1 | E = [(x) (E)] | ^ | | - | `E` references itself + | references itself "); } @@ -396,12 +396,12 @@ fn invalid_direct_left_recursion_in_tagged_alternation() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: direct recursion: cycle `E` → `E` will stuck without matching anything + error: infinite recursion: cycle consumes no input | 1 | E = [Left: (E) Right: (x)] | ^ | | - | `E` references itself + | references itself "); } @@ -415,12 +415,12 @@ fn invalid_unguarded_left_recursion_branch() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: direct recursion: cycle `A` → `A` will stuck without matching anything + error: infinite recursion: cycle consumes no input | 1 | A = [(A) 'escape'] | ^ | | - | `A` references itself + | references itself "); } @@ -434,12 +434,12 @@ fn invalid_unguarded_left_recursion_with_wildcard_alt() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: direct recursion: cycle `A` → `A` will stuck without matching anything + error: infinite recursion: cycle consumes no input | 1 | A = [(A) _] | ^ | | - | `A` references itself + | references itself "); } @@ -453,12 +453,12 @@ fn invalid_unguarded_left_recursion_with_tree_alt() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: direct recursion: cycle `A` → `A` will stuck without matching anything + error: infinite recursion: cycle consumes no input | 1 | A = [(A) (leaf)] | ^ | | - | `A` references itself + | references itself "); } @@ -482,7 +482,7 @@ fn invalid_mandatory_recursion_direct_child() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `A` → `A` has no escape path + error: infinite recursion: cycle has no escape path | 1 | A = (foo (A)) | ^ @@ -524,12 +524,12 @@ fn invalid_simple_unguarded_recursion() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: direct recursion: cycle `A` → `A` will stuck without matching anything + error: infinite recursion: cycle consumes no input | 3 | (A) | ^ | | - | `A` references itself + | references itself "); } @@ -544,14 +544,14 @@ fn invalid_unguarded_mutual_recursion_chain() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: direct recursion: cycle `B` → `A` → `B` will stuck without matching anything + error: infinite recursion: cycle consumes no input | 1 | A = [(B) (x)] - | - `A` references `B` (completing cycle) + | - references `B` (completing cycle) 2 | B = (A) | - ^ | | | - | | `B` references `A` + | | references `A` | `B` is defined here "); } From b2dae8103a4824f6b2eada283876de6903199a87 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 7 Dec 2025 23:05:59 -0300 Subject: [PATCH 2/2] Fix --- crates/plotnik-lib/src/query/link_tests.rs | 16 ++--- crates/plotnik-lib/src/query/recursion.rs | 14 ++-- .../plotnik-lib/src/query/recursion_tests.rs | 64 +++++++++---------- .../src/query/symbol_table_tests.rs | 8 +-- 4 files changed, 51 insertions(+), 51 deletions(-) diff --git a/crates/plotnik-lib/src/query/link_tests.rs b/crates/plotnik-lib/src/query/link_tests.rs index 9c9d340b..2c8e6c68 100644 --- a/crates/plotnik-lib/src/query/link_tests.rs +++ b/crates/plotnik-lib/src/query/link_tests.rs @@ -929,12 +929,12 @@ fn ref_followed_recursive_with_invalid_type() { | help: valid types for `name`: `identifier` - error: direct recursion: cycle `Foo` → `Foo` will stuck without matching anything + error: infinite recursion: cycle consumes no input | 1 | Foo = [(number) (Foo)] | ^^^ | | - | `Foo` references itself + | references itself "); } @@ -950,12 +950,12 @@ fn ref_followed_recursive_valid() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: direct recursion: cycle `Foo` → `Foo` will stuck without matching anything + error: infinite recursion: cycle consumes no input | 1 | Foo = [(identifier) (Foo)] | ^^^ | | - | `Foo` references itself + | references itself "); } @@ -991,15 +991,15 @@ fn ref_followed_mutual_recursion() { | help: valid types for `name`: `identifier` - error: direct recursion: cycle `Bar` → `Foo` → `Bar` will stuck without matching anything + error: infinite recursion: cycle consumes no input | 1 | Foo = [(number) (Bar)] - | --- `Foo` references `Bar` (completing cycle) + | --- references Bar (completing cycle) 2 | Bar = [(string) (Foo)] | --- ^^^ | | | - | | `Bar` references `Foo` - | `Bar` is defined here + | | 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 5174f028..ae579bac 100644 --- a/crates/plotnik-lib/src/query/recursion.rs +++ b/crates/plotnik-lib/src/query/recursion.rs @@ -204,7 +204,7 @@ impl Query<'_> { 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))]) + .map(|range| vec![(range, format!("{} references itself", name))]) .unwrap_or_default() } @@ -253,9 +253,9 @@ impl Query<'_> { let to = &path[(i + 1) % path.len()]; self.find_reference_location(from, to).map(|range| { let msg = if i == path.len() - 1 { - format!("references `{}` (completing cycle)", to) + format!("references {} (completing cycle)", to) } else { - format!("references `{}`", to) + format!("references {}", to) }; (range, msg) }) @@ -286,9 +286,9 @@ impl Query<'_> { 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)", to) + format!("references {} (completing cycle)", to) } else { - format!("references `{}`", to) + format!("references {}", to) }; (range, msg) }) @@ -324,7 +324,7 @@ impl Query<'_> { } if let Some(range) = def_range { - builder = builder.related_to(format!("`{}` is defined here", primary_name), range); + builder = builder.related_to(format!("{} is defined here", primary_name), range); } builder.emit(); @@ -357,7 +357,7 @@ impl Query<'_> { } if let Some(range) = def_range { - builder = builder.related_to(format!("`{}` is defined here", primary_name), range); + builder = builder.related_to(format!("{} is defined here", primary_name), range); } builder.emit(); diff --git a/crates/plotnik-lib/src/query/recursion_tests.rs b/crates/plotnik-lib/src/query/recursion_tests.rs index cf45236f..ad3f23bf 100644 --- a/crates/plotnik-lib/src/query/recursion_tests.rs +++ b/crates/plotnik-lib/src/query/recursion_tests.rs @@ -34,7 +34,7 @@ fn invalid_recursion_with_plus() { 1 | E = (call (E)+) | ^ | | - | `E` references itself + | E references itself "); } @@ -73,7 +73,7 @@ fn invalid_mandatory_recursion_in_tree_child() { 1 | E = (call (E)) | ^ | | - | `E` references itself + | E references itself "); } @@ -89,7 +89,7 @@ fn invalid_mandatory_recursion_in_field() { 1 | E = (call body: (E)) | ^ | | - | `E` references itself + | E references itself "); } @@ -105,7 +105,7 @@ fn invalid_mandatory_recursion_in_capture() { 1 | E = (call (E) @inner) | ^ | | - | `E` references itself + | E references itself "); } @@ -121,7 +121,7 @@ fn invalid_mandatory_recursion_in_sequence() { 1 | E = (call {(a) (E)}) | ^ | | - | `E` references itself + | E references itself "); } @@ -146,12 +146,12 @@ fn invalid_mutual_recursion_without_base_case() { error: infinite recursion: cycle has no escape path | 1 | A = (foo (B)) - | - references `B` (completing cycle) + | - references B (completing cycle) 2 | B = (bar (A)) | - ^ | | | - | | references `A` - | `B` is defined here + | | references A + | B is defined here "); } @@ -181,14 +181,14 @@ fn invalid_three_way_mutual_recursion() { error: infinite recursion: cycle has no escape path | 1 | A = (a (B)) - | - references `B` + | - references B 2 | B = (b (C)) - | - references `C` (completing cycle) + | - references C (completing cycle) 3 | C = (c (A)) | - ^ | | | - | | references `A` - | `C` is defined here + | | references A + | C is defined here "); } @@ -220,15 +220,15 @@ fn invalid_diamond_dependency_recursion() { error: infinite recursion: cycle has no escape path | 1 | A = (a [(B) (C)]) - | - references `C` (completing cycle) + | - references C (completing cycle) 2 | B = (b (D)) 3 | C = (c (D)) | - ^ | | | - | | references `D` - | `C` is defined here + | | references D + | C is defined here 4 | D = (d (A)) - | - references `A` + | - references A "); } @@ -246,12 +246,12 @@ fn invalid_mutual_recursion_via_field() { error: infinite recursion: cycle has no escape path | 1 | A = (foo body: (B)) - | - references `B` (completing cycle) + | - references B (completing cycle) 2 | B = (bar (A)) | - ^ | | | - | | references `A` - | `B` is defined here + | | references A + | B is defined here "); } @@ -269,12 +269,12 @@ fn invalid_mutual_recursion_via_capture() { error: infinite recursion: cycle has no escape path | 1 | A = (foo (B) @cap) - | - references `B` (completing cycle) + | - references B (completing cycle) 2 | B = (bar (A)) | - ^ | | | - | | references `A` - | `B` is defined here + | | references A + | B is defined here "); } @@ -292,12 +292,12 @@ fn invalid_mutual_recursion_via_sequence() { error: infinite recursion: cycle has no escape path | 1 | A = (foo {(x) (B)}) - | - references `B` (completing cycle) + | - references B (completing cycle) 2 | B = (bar (A)) | - ^ | | | - | | references `A` - | `B` is defined here + | | references A + | B is defined here "); } @@ -326,12 +326,12 @@ fn invalid_mutual_recursion_with_plus_quantifier() { error: infinite recursion: cycle has no escape path | 1 | A = (foo (B)+) - | - references `B` (completing cycle) + | - references B (completing cycle) 2 | B = (bar (A)) | - ^ | | | - | | references `A` - | `B` is defined here + | | references A + | B is defined here "); } @@ -487,7 +487,7 @@ fn invalid_mandatory_recursion_direct_child() { 1 | A = (foo (A)) | ^ | | - | `A` references itself + | A references itself "); } @@ -547,11 +547,11 @@ fn invalid_unguarded_mutual_recursion_chain() { error: infinite recursion: cycle consumes no input | 1 | A = [(B) (x)] - | - references `B` (completing cycle) + | - references B (completing cycle) 2 | B = (A) | - ^ | | | - | | references `A` - | `B` is defined here + | | 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 33e081a7..b18fc915 100644 --- a/crates/plotnik-lib/src/query/symbol_table_tests.rs +++ b/crates/plotnik-lib/src/query/symbol_table_tests.rs @@ -78,15 +78,15 @@ fn mutual_recursion() { let query = Query::try_from(input).unwrap(); assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_diagnostics(), @r" - error: infinite recursion: cycle `B` → `A` → `B` has no escape path + error: infinite recursion: cycle has no escape path | 1 | A = (foo (B)) - | - `A` references `B` (completing cycle) + | - references B (completing cycle) 2 | B = (bar (A)) | - ^ | | | - | | `B` references `A` - | `B` is defined here + | | references A + | B is defined here "); }