diff --git a/crates/plotnik-lib/src/analyze/mod.rs b/crates/plotnik-lib/src/analyze/mod.rs index 5d460da3..d09294e9 100644 --- a/crates/plotnik-lib/src/analyze/mod.rs +++ b/crates/plotnik-lib/src/analyze/mod.rs @@ -32,5 +32,5 @@ pub use link::LinkOutput; pub use recursion::validate_recursion; pub use symbol_table::{SymbolTable, UNNAMED_DEF}; pub use type_check::{TypeContext, infer_types, primary_def_name}; -pub use validation::{validate_alt_kinds, validate_anchors}; +pub use validation::{validate_alt_kinds, validate_anchors, validate_empty_constructs}; pub use visitor::{Visitor, walk_expr}; diff --git a/crates/plotnik-lib/src/analyze/validation/empty_constructs.rs b/crates/plotnik-lib/src/analyze/validation/empty_constructs.rs new file mode 100644 index 00000000..c1666e5e --- /dev/null +++ b/crates/plotnik-lib/src/analyze/validation/empty_constructs.rs @@ -0,0 +1,57 @@ +//! Semantic validation for empty constructs. +//! +//! Bans empty trees `()`, empty sequences `{}`, and empty alternations `[]`. + +use crate::SourceId; +use crate::analyze::visitor::{Visitor, walk_alt_expr, walk_named_node, walk_seq_expr}; +use crate::diagnostics::{DiagnosticKind, Diagnostics}; +use crate::parser::{AltExpr, NamedNode, Root, SeqExpr}; + +pub fn validate_empty_constructs(source_id: SourceId, ast: &Root, diag: &mut Diagnostics) { + let mut visitor = EmptyConstructsValidator { diag, source_id }; + visitor.visit(ast); +} + +struct EmptyConstructsValidator<'a> { + diag: &'a mut Diagnostics, + source_id: SourceId, +} + +impl Visitor for EmptyConstructsValidator<'_> { + fn visit_named_node(&mut self, node: &NamedNode) { + // Check for truly empty tree: no child nodes at all in CST (only tokens like parens) + // This excludes invalid content like predicates which create Error nodes + if node.as_cst().children().next().is_none() && node.node_type().is_none() { + self.diag + .report(self.source_id, DiagnosticKind::EmptyTree, node.text_range()) + .emit(); + } + walk_named_node(self, node); + } + + fn visit_seq_expr(&mut self, seq: &SeqExpr) { + if seq.children().next().is_none() { + self.diag + .report( + self.source_id, + DiagnosticKind::EmptySequence, + seq.text_range(), + ) + .emit(); + } + walk_seq_expr(self, seq); + } + + fn visit_alt_expr(&mut self, alt: &AltExpr) { + if alt.branches().next().is_none() { + self.diag + .report( + self.source_id, + DiagnosticKind::EmptyAlternation, + alt.text_range(), + ) + .emit(); + } + walk_alt_expr(self, alt); + } +} diff --git a/crates/plotnik-lib/src/analyze/validation/empty_constructs_tests.rs b/crates/plotnik-lib/src/analyze/validation/empty_constructs_tests.rs new file mode 100644 index 00000000..3e919d0c --- /dev/null +++ b/crates/plotnik-lib/src/analyze/validation/empty_constructs_tests.rs @@ -0,0 +1,209 @@ +use crate::Query; + +#[test] +fn empty_tree() { + let input = "Q = ()"; + + let res = Query::expect_invalid(input); + + insta::assert_snapshot!(res, @r" + error: empty `()` is not allowed + | + 1 | Q = () + | ^^ + | + help: use `(_)` to match any named node, or `_` for any node + "); +} + +#[test] +fn empty_tree_with_whitespace() { + let input = "Q = ( )"; + + let res = Query::expect_invalid(input); + + insta::assert_snapshot!(res, @r" + error: empty `()` is not allowed + | + 1 | Q = ( ) + | ^^^^^ + | + help: use `(_)` to match any named node, or `_` for any node + "); +} + +#[test] +fn empty_tree_with_comment() { + let input = "Q = ( /* comment */ )"; + + let res = Query::expect_invalid(input); + + insta::assert_snapshot!(res, @r" + error: empty `()` is not allowed + | + 1 | Q = ( /* comment */ ) + | ^^^^^^^^^^^^^^^^^ + | + help: use `(_)` to match any named node, or `_` for any node + "); +} + +#[test] +fn empty_sequence() { + let input = "Q = {}"; + + let res = Query::expect_invalid(input); + + insta::assert_snapshot!(res, @r" + error: empty `{}` is not allowed + | + 1 | Q = {} + | ^^ + | + help: sequences must contain at least one expression + "); +} + +#[test] +fn empty_sequence_with_whitespace() { + let input = "Q = { }"; + + let res = Query::expect_invalid(input); + + insta::assert_snapshot!(res, @r" + error: empty `{}` is not allowed + | + 1 | Q = { } + | ^^^^^ + | + help: sequences must contain at least one expression + "); +} + +#[test] +fn empty_sequence_with_comment() { + let input = "Q = { /* comment */ }"; + + let res = Query::expect_invalid(input); + + insta::assert_snapshot!(res, @r" + error: empty `{}` is not allowed + | + 1 | Q = { /* comment */ } + | ^^^^^^^^^^^^^^^^^ + | + help: sequences must contain at least one expression + "); +} + +#[test] +fn empty_alternation() { + let input = "Q = []"; + + let res = Query::expect_invalid(input); + + insta::assert_snapshot!(res, @r" + error: empty `[]` is not allowed + | + 1 | Q = [] + | ^^ + | + help: alternations must contain at least one branch + "); +} + +#[test] +fn empty_alternation_with_whitespace() { + let input = "Q = [ ]"; + + let res = Query::expect_invalid(input); + + insta::assert_snapshot!(res, @r" + error: empty `[]` is not allowed + | + 1 | Q = [ ] + | ^^^^^ + | + help: alternations must contain at least one branch + "); +} + +#[test] +fn empty_alternation_with_comment() { + let input = "Q = [ /* comment */ ]"; + + let res = Query::expect_invalid(input); + + insta::assert_snapshot!(res, @r" + error: empty `[]` is not allowed + | + 1 | Q = [ /* comment */ ] + | ^^^^^^^^^^^^^^^^^ + | + help: alternations must contain at least one branch + "); +} + +#[test] +fn nested_empty_sequence() { + let input = "Q = (foo {})"; + + let res = Query::expect_invalid(input); + + insta::assert_snapshot!(res, @r" + error: empty `{}` is not allowed + | + 1 | Q = (foo {}) + | ^^ + | + help: sequences must contain at least one expression + "); +} + +#[test] +fn nested_empty_alternation() { + let input = "Q = (foo [])"; + + let res = Query::expect_invalid(input); + + insta::assert_snapshot!(res, @r" + error: empty `[]` is not allowed + | + 1 | Q = (foo []) + | ^^ + | + help: alternations must contain at least one branch + "); +} + +#[test] +fn non_empty_sequence_valid() { + let input = "Q = {(a) (b)}"; + + let res = Query::expect_valid_ast(input); + + insta::assert_snapshot!(res, @r" + Root + Def Q + Seq + NamedNode a + NamedNode b + "); +} + +#[test] +fn non_empty_alternation_valid() { + let input = "Q = [(a) (b)]"; + + let res = Query::expect_valid_ast(input); + + insta::assert_snapshot!(res, @r" + Root + Def Q + Alt + Branch + NamedNode a + Branch + NamedNode b + "); +} diff --git a/crates/plotnik-lib/src/analyze/validation/mod.rs b/crates/plotnik-lib/src/analyze/validation/mod.rs index 1c1354f2..6202db5e 100644 --- a/crates/plotnik-lib/src/analyze/validation/mod.rs +++ b/crates/plotnik-lib/src/analyze/validation/mod.rs @@ -3,14 +3,19 @@ //! Validates semantic constraints that aren't captured by parsing or type checking: //! - Alternation kind consistency (alt_kinds) //! - Anchor placement rules (anchors) +//! - Empty constructs (empty_constructs) pub mod alt_kinds; pub mod anchors; +pub mod empty_constructs; #[cfg(test)] mod alt_kinds_tests; #[cfg(test)] mod anchors_tests; +#[cfg(test)] +mod empty_constructs_tests; pub use alt_kinds::validate_alt_kinds; pub use anchors::validate_anchors; +pub use empty_constructs::validate_empty_constructs; diff --git a/crates/plotnik-lib/src/diagnostics/message.rs b/crates/plotnik-lib/src/diagnostics/message.rs index 9e2b187c..2b1ff690 100644 --- a/crates/plotnik-lib/src/diagnostics/message.rs +++ b/crates/plotnik-lib/src/diagnostics/message.rs @@ -31,6 +31,8 @@ pub enum DiagnosticKind { // User wrote something that doesn't belong EmptyTree, EmptyAnonymousNode, + EmptySequence, + EmptyAlternation, BareIdentifier, InvalidSeparator, AnchorInAlternation, @@ -146,6 +148,8 @@ impl DiagnosticKind { Self::ExpectedFieldName => Some("e.g., `-value`"), Self::EmptyTree => Some("use `(_)` to match any named node, or `_` for any node"), Self::EmptyAnonymousNode => Some("use a valid anonymous node or remove it"), + Self::EmptySequence => Some("sequences must contain at least one expression"), + Self::EmptyAlternation => Some("alternations must contain at least one branch"), Self::TreeSitterSequenceSyntax => Some("use `{...}` for sequences"), Self::NegationSyntaxDeprecated => Some("use `-field` instead of `!field`"), Self::MixedAltBranches => { @@ -185,6 +189,8 @@ impl DiagnosticKind { // Invalid syntax Self::EmptyTree => "empty `()` is not allowed", Self::EmptyAnonymousNode => "empty anonymous node", + Self::EmptySequence => "empty `{}` is not allowed", + Self::EmptyAlternation => "empty `[]` is not allowed", Self::BareIdentifier => "bare identifier is not valid", Self::InvalidSeparator => "unexpected separator", Self::AnchorInAlternation => "anchors cannot appear directly in alternations", diff --git a/crates/plotnik-lib/src/engine/engine_tests.rs b/crates/plotnik-lib/src/engine/engine_tests.rs index 1fbfede7..23e901fc 100644 --- a/crates/plotnik-lib/src/engine/engine_tests.rs +++ b/crates/plotnik-lib/src/engine/engine_tests.rs @@ -529,23 +529,3 @@ fn wildcard_named_skips_anonymous() { fn wildcard_bare_matches_anonymous() { snap!("Q = (program (return_statement _ @x))", "return 42"); } - -/// Empty captured sequence should produce empty struct, not null or panic. -/// Regression test for: type inference treating `{ }` as Node instead of empty struct. -#[test] -fn regression_empty_captured_sequence() { - snap!( - "Q = (program (expression_statement (identifier) @id { } @empty))", - "x" - ); -} - -/// Optional empty captured sequence should produce empty struct when matched. -/// Regression test for: `{ }? @maybe` producing null instead of `{}`. -#[test] -fn regression_optional_empty_captured_sequence() { - snap!( - "Q = (program (expression_statement (identifier) @id { }? @maybe))", - "x" - ); -} diff --git a/crates/plotnik-lib/src/parser/grammar/structures.rs b/crates/plotnik-lib/src/parser/grammar/structures.rs index bc5ad505..e7407e52 100644 --- a/crates/plotnik-lib/src/parser/grammar/structures.rs +++ b/crates/plotnik-lib/src/parser/grammar/structures.rs @@ -24,11 +24,8 @@ impl Parser<'_, '_> { match self.current() { SyntaxKind::ParenClose => { + // Empty tree `()` - validation phase will report EmptyTree error self.start_node_at(checkpoint, SyntaxKind::Tree); - self.diagnostics - .report(self.source_id, DiagnosticKind::EmptyTree, open_paren_span) - .emit(); - // Fall through to close } SyntaxKind::Underscore => { self.start_node_at(checkpoint, SyntaxKind::Tree); diff --git a/crates/plotnik-lib/src/parser/tests/grammar/sequences_tests.rs b/crates/plotnik-lib/src/parser/tests/grammar/sequences_tests.rs index 28f00ad3..329bbb57 100644 --- a/crates/plotnik-lib/src/parser/tests/grammar/sequences_tests.rs +++ b/crates/plotnik-lib/src/parser/tests/grammar/sequences_tests.rs @@ -107,17 +107,16 @@ fn empty_sequence() { Q = {} "#}; - let res = Query::expect_valid_cst(input); - - insta::assert_snapshot!(res, @r#" - Root - Def - Id "Q" - Equals "=" - Seq - BraceOpen "{" - BraceClose "}" - "#); + let res = Query::expect_invalid(input); + + insta::assert_snapshot!(res, @r" + error: empty `{}` is not allowed + | + 1 | Q = {} + | ^^ + | + help: sequences must contain at least one expression + "); } #[test] diff --git a/crates/plotnik-lib/src/parser/tests/recovery/unclosed_tests.rs b/crates/plotnik-lib/src/parser/tests/recovery/unclosed_tests.rs index 8537b281..df5a0240 100644 --- a/crates/plotnik-lib/src/parser/tests/recovery/unclosed_tests.rs +++ b/crates/plotnik-lib/src/parser/tests/recovery/unclosed_tests.rs @@ -121,7 +121,7 @@ fn empty_parens() { error: empty `()` is not allowed | 1 | () - | ^ + | ^^ | help: use `(_)` to match any named node, or `_` for any node "); diff --git a/crates/plotnik-lib/src/parser/tests/recovery/unexpected_tests.rs b/crates/plotnik-lib/src/parser/tests/recovery/unexpected_tests.rs index 91f85f41..28d2edd2 100644 --- a/crates/plotnik-lib/src/parser/tests/recovery/unexpected_tests.rs +++ b/crates/plotnik-lib/src/parser/tests/recovery/unexpected_tests.rs @@ -351,6 +351,13 @@ fn alternation_recovery_to_capture() { let res = Query::expect_invalid(input); insta::assert_snapshot!(res, @r" + error: empty `[]` is not allowed + | + 1 | [^^^ @name] + | ^^^^^^^^^^^ + | + help: alternations must contain at least one branch + error: unexpected token | 1 | [^^^ @name] diff --git a/crates/plotnik-lib/src/query/stages.rs b/crates/plotnik-lib/src/query/stages.rs index a3debdc1..7aaeb27d 100644 --- a/crates/plotnik-lib/src/query/stages.rs +++ b/crates/plotnik-lib/src/query/stages.rs @@ -10,7 +10,7 @@ use crate::Diagnostics; use crate::analyze::link; use crate::analyze::symbol_table::{SymbolTable, resolve_names}; use crate::analyze::type_check::{self, Arity, TypeContext}; -use crate::analyze::validation::{validate_alt_kinds, validate_anchors}; +use crate::analyze::validation::{validate_alt_kinds, validate_anchors, validate_empty_constructs}; use crate::analyze::{dependencies, validate_recursion}; use crate::parser::{Parser, Root, SyntaxNode, lex}; @@ -74,6 +74,7 @@ impl QueryBuilder { validate_alt_kinds(source.id, &res.ast, &mut diag); validate_anchors(source.id, &res.ast, &mut diag); + validate_empty_constructs(source.id, &res.ast, &mut diag); total_fuel_consumed = total_fuel_consumed.saturating_add(res.fuel_consumed); ast.insert(source.id, res.ast); }