diff --git a/Cargo.toml b/Cargo.toml index 6daa722b..ea21d423 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ test-support = [] [dev-dependencies] rstest = "0.25" +insta = "1.47.2" ddlint = { path = ".", features = ["test-support"] } [lints.clippy] diff --git a/Makefile b/Makefile index 9460a0e5..5732681b 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ nixie tools APP ?= ddlint -CARGO ?= cargo +CARGO ?= $(or $(shell command -v cargo 2>/dev/null),$(HOME)/.cargo/bin/cargo) BUILD_JOBS ?= CLIPPY_FLAGS ?= --all-targets --all-features -- -D warnings MDLINT ?= markdownlint @@ -45,7 +45,13 @@ check-fmt: ## Verify formatting $(CARGO) fmt --all -- --check markdownlint: ## Lint Markdown files - find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 $(MDLINT) + git rev-parse --verify origin/main >/dev/null + @set -e; \ + tmp=$$(mktemp); \ + trap 'rm -f "$$tmp"' EXIT; \ + git diff --name-only -z --diff-filter=ACMRT origin/main...HEAD -- \ + '*.md' '*.markdown' '*.mdx' > "$$tmp"; \ + if [ -s "$$tmp" ]; then xargs -0 $(MDLINT) < "$$tmp"; fi nixie: ## Validate Mermaid diagrams find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 $(NIXIE) diff --git a/docs/contents.md b/docs/contents.md index 641c368f..f89c890b 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -14,6 +14,8 @@ This page lists the current documents in the `docs/` directory. roadmap for the `ddlint` linter. - [Documentation style guide](./documentation-style-guide.md): Authoring conventions for Markdown structure, spelling, formatting, and ADR content. +- [Developer guide](./developers-guide.md): Parser module boundaries and + ownership notes for the current implementation split. - [Differential Datalog parser syntax specification updated](./differential-datalog-parser-syntax-spec-updated.md): Normative reference for DDlog syntax, lexer rules, operator precedence, and parser desugarings. diff --git a/docs/developers-guide.md b/docs/developers-guide.md new file mode 100644 index 00000000..27b405d1 --- /dev/null +++ b/docs/developers-guide.md @@ -0,0 +1,56 @@ +# Developer guide + +This guide records the parser module structure introduced by issue `#223`. +It is intentionally narrow and documents ownership boundaries rather than the +full parsing pipeline. + +## Parser module structure + +### `src/parser/ast/expr/sexpr.rs` + +- Owns S-expression rendering helpers for `Expr`. +- Supports test and fixture comparisons without coupling callers to debug + formatting. +- Should remain presentation-only; parsing and semantic classification belong + elsewhere. + +### `src/parser/ast/rule/classification.rs` + +- Owns rule-body term classification for raw literals. +- Handles assignment parsing, aggregation detection, and `for`-loop lowering + within the rule-body helper path. +- Keeps rule-body classification separate from the public `Rule` wrapper so + `rule.rs` stays focused on the surface API. + +### `src/parser/expression/pratt/postfix.rs` + +- Owns postfix dispatch for the Pratt parser. +- Routes function calls, bit slices, field access, tuple indexing, method + calls, and delay postfixes to the appropriate helper. +- Coordinates the pending diff-marker state across the postfix chain. + +### `src/parser/expression/pratt/diff.rs` + +- Owns diff-marker tracking and validation. +- Wraps completed postfix expressions in `Expr::AtomDiff` when a diff marker + is pending. +- Emits the targeted diagnostics for duplicate, misplaced, or dangling diff + markers. + +### `src/parser/expression/pratt/delay.rs` + +- Owns `expr -` postfix parsing. +- Consumes the `-<` token pair, reads the delay literal, and returns + `Expr::AtomDelay` on success. +- Keeps delay-specific validation separate from the generic postfix loop. + +## Boundary rules + +- Keep formatting helpers in `sexpr.rs` rather than mixing them into the core + expression parser. +- Keep rule-body classification in `classification.rs` rather than adding + helper-stage logic to `rule.rs`. +- Keep postfix dispatch in `postfix.rs`; add new postfix behaviour there only + when it needs shared chain state. +- Keep diff-marker state and delay parsing in their dedicated submodules so + `pratt.rs` remains the central parser entry point. diff --git a/docs/parser-implementation-notes.md b/docs/parser-implementation-notes.md index 7373da6a..ab3736bf 100644 --- a/docs/parser-implementation-notes.md +++ b/docs/parser-implementation-notes.md @@ -14,6 +14,9 @@ code review. It is intentionally non-normative. architecture and public contracts. - This document is the worked implementation guide for current module mapping, migration sequencing, and non-regression invariants. +- `docs/developers-guide.md` records the parser module boundaries introduced by + issue `#223` and should be kept in sync with this document when those + boundaries move. - If behaviour differs from the syntax spec, track that mismatch in `docs/parser-conformance-register.md` rather than duplicating decision text here. @@ -31,6 +34,18 @@ The parser stack is split into three cooperating layers: - AST wrappers and expression parsing (`src/parser/ast/*`, `src/parser/expression/*`): build typed views and expression trees. +Issue `#223` sharpened the internal ownership boundaries inside the AST and +expression layers: + +- `src/parser/ast/expr/sexpr.rs` owns S-expression rendering helpers for + `Expr` and is presentation-only. +- `src/parser/ast/rule/classification.rs` owns rule-body term classification + and the aggregation-tracking path used by `Rule` helpers. +- `src/parser/expression/pratt/postfix.rs`, + `src/parser/expression/pratt/diff.rs`, and + `src/parser/expression/pratt/delay.rs` split the Pratt postfix chain into a + dispatcher plus two focused helpers for diff markers and delay postfixes. + Current pipeline guarantees are intentionally narrow: - `parse()` builds the CST-backed `Parsed` result, collects top-level `for` @@ -101,6 +116,9 @@ The following invariants must hold throughout the crate split: Primary implementation points: - `src/parser/expression/pratt.rs` +- `src/parser/expression/pratt/postfix.rs` +- `src/parser/expression/pratt/diff.rs` +- `src/parser/expression/pratt/delay.rs` - `src/parser/expression/qualified.rs` ### Struct-literal guard @@ -171,6 +189,7 @@ Implementation points: - `src/parser/span_scanners/rules.rs` - `src/parser/ast/rule.rs` +- `src/parser/ast/rule/classification.rs` Aggregation and lowering stage boundaries are tracked in the conformance register. The aggregation boundary itself is now fixed as helper-stage in the @@ -287,8 +306,8 @@ Important invariants: rejects capitalized names (e.g., `Foo`) as invalid. - The span scanner keeps non-`extern` rejection separate from the output-signature check and emits the targeted diagnostic - `transformer declarations require ':' followed by at least one output identifier` - when the colon or first output identifier is missing. + `transformer declarations require ':' followed by at least one output + identifier` when the colon or first output identifier is missing. These helpers are shared intentionally to keep declaration parsing consistent across top-level constructs. @@ -346,10 +365,13 @@ token names or their human-readable equivalents. - Tokenization and keyword policy: `src/tokenizer.rs` - Entry parse orchestration: `src/parser/mod.rs` - Pratt parser: `src/parser/expression/pratt.rs` +- Pratt postfix helpers: + `src/parser/expression/pratt/{postfix,diff,delay}.rs` - Prefix/infix helpers: `src/parser/expression/*.rs` - Centralized diagnostic messages: `src/parser/error_messages.rs` - Rule span scanning: `src/parser/span_scanners/rules.rs` - Top-level scanners: `src/parser/span_scanners/*.rs` - AST wrappers: `src/parser/ast/*.rs` +- Rule-body classification helpers: `src/parser/ast/rule/classification.rs` - Shared parse utilities: `src/parser/ast/parse_utils/*.rs` - Test assertion helpers: `src/test_util/assertions.rs` diff --git a/docs/roadmap.md b/docs/roadmap.md index 89115153..30c0091a 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -120,6 +120,15 @@ split parser library surfaces defined in `docs/adr-001-parser-crate-split.md`. - [x] 2.3.2. Parse aggregation and FlatMap constructs in rule bodies. See docs/differential-datalog-parser-syntax-spec-updated.md §5.12 and docs/differential-datalog-parser-syntax-spec-updated.md §6.1. +- [x] 2.3.3. Refactor oversized parser modules to satisfy the 400-line + maintainability guideline (issue `#223`). Split `rule.rs`, `expr.rs`, `pratt.rs`, + and the expression test module into focused submodules: + `src/parser/ast/rule/classification.rs`, + `src/parser/ast/expr/sexpr.rs`, + `src/parser/expression/pratt/delay.rs`, + `src/parser/expression/pratt/diff.rs`, + `src/parser/expression/pratt/postfix.rs`, and + `src/parser/tests/expression/fixtures/`. See PR `#259`. ### 2.4. Syntax-spec lexical and expression conformance diff --git a/src/parser/ast/expr.rs b/src/parser/ast/expr.rs index c1624419..577eeb96 100644 --- a/src/parser/ast/expr.rs +++ b/src/parser/ast/expr.rs @@ -5,6 +5,8 @@ //! can inspect without re-parsing tokens. use std::fmt; +mod sexpr; + use super::number::NumberLiteral; use super::pattern::Pattern; use super::string_literal::StringLiteral; @@ -308,148 +310,3 @@ pub enum Expr { /// for later lowering stages, not current parser behaviour. MapLit(Vec<(Expr, Expr)>), } -impl Expr { - /// Display the expression as a simple S-expression for tests. - #[must_use] - pub fn to_sexpr(&self) -> String { - match self { - Self::Literal(Literal::Number(n)) => n.to_sexpr(), - Self::Literal(Literal::String(s)) => s.to_sexpr(), - Self::Literal(Literal::Bool(b)) => b.to_string(), - Self::Variable(name) => name.clone(), - Self::Apply { callee, args } | Self::Call { callee, args } => format_nary( - "call", - std::iter::once(callee.to_sexpr()).chain(args.iter().map(Self::to_sexpr)), - ), - Self::MethodCall { recv, name, args } => format_nary( - "method", - std::iter::once(recv.to_sexpr()) - .chain(std::iter::once(name.clone())) - .chain(args.iter().map(Self::to_sexpr)), - ), - Self::FieldAccess { expr, field } => { - format_nary("field", [expr.to_sexpr(), field.clone()]) - } - Self::TupleIndex { expr, index } => { - format_nary("tuple-index", [expr.to_sexpr(), index.clone()]) - } - Self::BitSlice { expr, hi, lo } => { - format_nary("bitslice", [expr.to_sexpr(), hi.to_sexpr(), lo.to_sexpr()]) - } - Self::Struct { name, fields } => format_nary( - "struct", - std::iter::once(name.clone()) - .chain(fields.iter().map(|(n, e)| format_field(n, &e.to_sexpr()))), - ), - Self::Tuple(items) => format_nary("tuple", items.iter().map(Self::to_sexpr)), - Self::Closure { params, body } => format_nary( - "closure", - [format!("({})", params.join(" ")), body.to_sexpr()], - ), - Self::IfElse { - condition, - then_branch, - else_branch, - } => format_if_else(condition, then_branch, else_branch), - Self::Unary { op, expr } => format_nary(op.symbol(), std::iter::once(expr.to_sexpr())), - Self::AtomDiff { expr } => format_nary("diff", [expr.to_sexpr()]), - Self::AtomDelay { delay, expr } => { - format_nary("delay", [delay.to_string(), expr.to_sexpr()]) - } - Self::Binary { op, lhs, rhs } => { - format_nary(op.symbol(), [lhs.to_sexpr(), rhs.to_sexpr()]) - } - Self::Group(e) => format_nary("group", std::iter::once(e.to_sexpr())), - Self::ForLoop { - pattern, - iterable, - guard, - body, - } => format_for_loop(pattern, iterable, guard.as_deref(), body), - Self::Match { scrutinee, arms } => format_match(scrutinee, arms), - Self::Break => "(break)".to_string(), - Self::Continue => "(continue)".to_string(), - Self::Return { value } => format_nary("return", [value.to_sexpr()]), - Self::VecLit(items) => format_nary("vec", items.iter().map(Self::to_sexpr)), - Self::MapLit(entries) => { - format_nary("map", entries.iter().map(|(k, v)| format_kv(k, v))) - } - } - } -} - -#[inline] -fn format_field(name: &str, value: &str) -> String { - let mut s = String::with_capacity(name.len() + value.len() + 3); - s.push('('); - s.push_str(name); - s.push(' '); - s.push_str(value); - s.push(')'); - s -} - -#[inline] -fn format_kv(key: &Expr, value: &Expr) -> String { - let k = key.to_sexpr(); - let v = value.to_sexpr(); - let mut s = String::with_capacity(k.len() + v.len() + 10); - s.push_str("(entry "); - s.push_str(&k); - s.push(' '); - s.push_str(&v); - s.push(')'); - s -} - -fn format_nary(label: &str, parts: I) -> String -where - I: IntoIterator, -{ - let parts_vec: Vec = parts.into_iter().collect(); - let cap = 2 + label.len() + parts_vec.iter().map(|p| 1 + p.len()).sum::(); - let mut out = String::with_capacity(cap); - out.push('('); - out.push_str(label); - for part in parts_vec { - out.push(' '); - out.push_str(&part); - } - out.push(')'); - out -} - -fn format_if_else(condition: &Expr, then_branch: &Expr, else_branch: &Expr) -> String { - format_nary( - "if", - [ - condition.to_sexpr(), - then_branch.to_sexpr(), - else_branch.to_sexpr(), - ], - ) -} - -fn format_for_loop( - pattern: &Pattern, - iterable: &Expr, - guard: Option<&Expr>, - body: &Expr, -) -> String { - let mut parts = vec![pattern.to_source(), iterable.to_sexpr()]; - if let Some(cond) = guard { - parts.push(cond.to_sexpr()); - } - parts.push(body.to_sexpr()); - format_nary("for", parts) -} - -fn format_match(scrutinee: &Expr, arms: &[MatchArm]) -> String { - let arm_parts = arms - .iter() - .map(|arm| format_nary("arm", [arm.pattern.to_source(), arm.body.to_sexpr()])); - format_nary( - "match", - std::iter::once(scrutinee.to_sexpr()).chain(arm_parts), - ) -} diff --git a/src/parser/ast/expr/sexpr.rs b/src/parser/ast/expr/sexpr.rs new file mode 100644 index 00000000..5b0abb3f --- /dev/null +++ b/src/parser/ast/expr/sexpr.rs @@ -0,0 +1,201 @@ +//! S-expression formatting helpers for [`super::Expr`]. +//! +//! Tests use these compact renderings to compare parsed structure without +//! coupling to debug formatting. + +use super::{Expr, Literal, MatchArm, Pattern}; + +impl Expr { + /// Display the expression as a simple S-expression for tests. + #[must_use] + pub fn to_sexpr(&self) -> String { + match self { + Self::Literal(Literal::Number(n)) => n.to_sexpr(), + Self::Literal(Literal::String(s)) => s.to_sexpr(), + Self::Literal(Literal::Bool(b)) => b.to_string(), + Self::Variable(name) => name.clone(), + Self::Apply { callee, args } | Self::Call { callee, args } => format_nary( + "call", + std::iter::once(callee.to_sexpr()).chain(args.iter().map(Self::to_sexpr)), + ), + Self::MethodCall { recv, name, args } => format_nary( + "method", + std::iter::once(recv.to_sexpr()) + .chain(std::iter::once(name.clone())) + .chain(args.iter().map(Self::to_sexpr)), + ), + Self::FieldAccess { expr, field } => { + format_nary("field", [expr.to_sexpr(), field.clone()]) + } + Self::TupleIndex { expr, index } => { + format_nary("tuple-index", [expr.to_sexpr(), index.clone()]) + } + Self::BitSlice { expr, hi, lo } => { + format_nary("bitslice", [expr.to_sexpr(), hi.to_sexpr(), lo.to_sexpr()]) + } + Self::Struct { name, fields } => format_nary( + "struct", + std::iter::once(name.clone()).chain( + fields + .iter() + .map(|(name, expr)| format_field(name, &expr.to_sexpr())), + ), + ), + Self::Tuple(items) => format_nary("tuple", items.iter().map(Self::to_sexpr)), + Self::Closure { params, body } => format_nary( + "closure", + [format!("({})", params.join(" ")), body.to_sexpr()], + ), + Self::IfElse { + condition, + then_branch, + else_branch, + } => format_if_else(condition, then_branch, else_branch), + Self::Unary { op, expr } => format_nary(op.symbol(), std::iter::once(expr.to_sexpr())), + Self::AtomDiff { expr } => format_nary("diff", [expr.to_sexpr()]), + Self::AtomDelay { delay, expr } => { + format_nary("delay", [delay.to_string(), expr.to_sexpr()]) + } + Self::Binary { op, lhs, rhs } => { + format_nary(op.symbol(), [lhs.to_sexpr(), rhs.to_sexpr()]) + } + Self::Group(expr) => format_nary("group", std::iter::once(expr.to_sexpr())), + Self::ForLoop { + pattern, + iterable, + guard, + body, + } => format_for_loop(pattern, iterable, guard.as_deref(), body), + Self::Match { scrutinee, arms } => format_match(scrutinee, arms), + Self::Break => "(break)".to_string(), + Self::Continue => "(continue)".to_string(), + Self::Return { value } => format_nary("return", [value.to_sexpr()]), + Self::VecLit(items) => format_nary("vec", items.iter().map(Self::to_sexpr)), + Self::MapLit(entries) => format_nary( + "map", + entries.iter().map(|(key, value)| format_kv(key, value)), + ), + } + } +} + +#[inline] +fn format_field(name: &str, value: &str) -> String { + let mut rendered = String::with_capacity(name.len() + value.len() + 3); + rendered.push('('); + rendered.push_str(name); + rendered.push(' '); + rendered.push_str(value); + rendered.push(')'); + rendered +} + +#[inline] +fn format_kv(key: &Expr, value: &Expr) -> String { + let key = key.to_sexpr(); + let value = value.to_sexpr(); + let mut rendered = String::with_capacity(key.len() + value.len() + 10); + rendered.push_str("(entry "); + rendered.push_str(&key); + rendered.push(' '); + rendered.push_str(&value); + rendered.push(')'); + rendered +} + +fn format_nary(label: &str, parts: I) -> String +where + I: IntoIterator, +{ + let parts: Vec = parts.into_iter().collect(); + let capacity = 2 + label.len() + parts.iter().map(|part| 1 + part.len()).sum::(); + let mut rendered = String::with_capacity(capacity); + rendered.push('('); + rendered.push_str(label); + for part in parts { + rendered.push(' '); + rendered.push_str(&part); + } + rendered.push(')'); + rendered +} + +fn format_if_else(condition: &Expr, then_branch: &Expr, else_branch: &Expr) -> String { + format_nary( + "if", + [ + condition.to_sexpr(), + then_branch.to_sexpr(), + else_branch.to_sexpr(), + ], + ) +} + +fn format_for_loop( + pattern: &Pattern, + iterable: &Expr, + guard: Option<&Expr>, + body: &Expr, +) -> String { + let mut parts = vec![pattern.to_source(), iterable.to_sexpr()]; + if let Some(condition) = guard { + parts.push(condition.to_sexpr()); + } + parts.push(body.to_sexpr()); + format_nary("for", parts) +} + +fn format_match(scrutinee: &Expr, arms: &[MatchArm]) -> String { + let arm_parts = arms + .iter() + .map(|arm| format_nary("arm", [arm.pattern.to_source(), arm.body.to_sexpr()])); + format_nary( + "match", + std::iter::once(scrutinee.to_sexpr()).chain(arm_parts), + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::parser::ast::{BinaryOp, Literal}; + + fn var(name: &str) -> Expr { + Expr::Variable(name.into()) + } + + #[test] + fn snapshots_nested_expression_format() { + let expr = Expr::Binary { + op: BinaryOp::Add, + lhs: Box::new(Expr::MethodCall { + recv: Box::new(var("stream")), + name: "map".into(), + args: vec![var("value")], + }), + rhs: Box::new(Expr::Literal(Literal::Bool(true))), + }; + let rendered = expr.to_sexpr(); + + assert_eq!(rendered, "(+ (method stream map value) true)"); + insta::assert_snapshot!(rendered, @"(+ (method stream map value) true)"); + } + + #[test] + fn snapshots_collection_expression_format() { + let expr = Expr::MapLit(vec![ + (var("key"), Expr::VecLit(vec![var("first"), var("second")])), + (var("enabled"), Expr::Literal(Literal::Bool(false))), + ]); + let rendered = expr.to_sexpr(); + + assert_eq!( + rendered, + "(map (entry key (vec first second)) (entry enabled false))" + ); + insta::assert_snapshot!( + rendered, + @"(map (entry key (vec first second)) (entry enabled false))" + ); + } +} diff --git a/src/parser/ast/rule.rs b/src/parser/ast/rule.rs index 442d3534..fa4ca038 100644 --- a/src/parser/ast/rule.rs +++ b/src/parser/ast/rule.rs @@ -29,12 +29,12 @@ use chumsky::error::Simple; +mod classification; + use super::rule_head::{RuleHead, first_head_text, parse_rule_heads}; -use super::{AstNode, BinaryOp, Expr, Pattern}; +use super::{AstNode, Expr, Pattern}; use crate::parser::delimiter::find_top_level_eq_span; -use crate::parser::expression::parse_expression; -use crate::parser::pattern::parse_pattern; -use crate::parser::span_utils::{shift_errors, trim_byte_range}; +use crate::parser::span_utils::trim_byte_range; use crate::{DdlogLanguage, Span, SyntaxKind}; /// Typed wrapper for a rule declaration. @@ -183,31 +183,6 @@ impl Rule { } } -/// Validate that at most one aggregation appears in the rule body. -/// -/// If `term` is an aggregation, checks whether a previous aggregation was already -/// seen. Records an error and returns `true` (skip) if so, otherwise stores the -/// current span as the first and returns `false` (include). -fn validate_aggregation( - term: &RuleBodyTerm, - literal_span: &Span, - first_aggregation_span: &mut Option, - errors: &mut Vec>, -) -> bool { - if let RuleBodyTerm::Aggregation(_) = term { - match first_aggregation_span { - Some(first_span) => { - errors.push(multiple_aggregations_error(first_span, literal_span)); - return true; // skip duplicate aggregation - } - None => { - *first_aggregation_span = Some(literal_span.clone()); - } - } - } - false -} - impl_ast_node!(Rule); /// Wrapper for a single rule body expression CST node. @@ -242,30 +217,7 @@ impl RuleBodyExpression { let literal_span = self.span(); - if let Some(parts) = split_assignment(&raw) { - match parse_assignment(&parts, &literal_span) { - Ok(term) => return term, - Err(mut errs) => { - errors.append(&mut errs); - return None; - } - } - } - - match parse_expression(trimmed) { - Ok(expr) => { - let mut ctx = ClassificationContext { - literal_span: &literal_span, - first_aggregation_span, - errors, - }; - classify_expression(expr, &mut ctx) - } - Err(mut errs) => { - errors.append(&mut errs); - None - } - } + classification::parse_rule_body_term(&raw, literal_span, first_aggregation_span, errors) } } @@ -394,225 +346,6 @@ pub struct RuleAggregation { pub source: AggregationSource, } -fn parse_assignment( - parts: &AssignmentParts, - literal_span: &Span, -) -> Result, Vec>> { - if parts.pattern.trim().is_empty() { - return Err(vec![Simple::custom( - literal_span.clone(), - "expected pattern before '=' in rule literal", - )]); - } - - if parts.value.is_empty() { - return Err(vec![Simple::custom( - literal_span.clone(), - "expected expression after '=' in rule literal", - )]); - } - - let pattern_base_offset = literal_span.start.saturating_add(parts.pattern_offset); - let pattern = match parse_pattern(&parts.pattern) { - Ok(pattern) => pattern, - Err(errs) => return Err(shift_errors(errs, pattern_base_offset)), - }; - - let value_base_offset = literal_span.start.saturating_add(parts.value_offset); - match parse_expression(&parts.value) { - Ok(expr) => Ok(Some(RuleBodyTerm::Assignment(RuleAssignment { - pattern, - value: expr, - }))), - Err(errs) => Err(shift_errors(errs, value_base_offset)), - } -} - -/// Context for classifying rule body expressions with aggregation tracking. -struct ClassificationContext<'a> { - literal_span: &'a Span, - first_aggregation_span: &'a mut Option, - errors: &'a mut Vec>, -} - -fn classify_expression(expr: Expr, ctx: &mut ClassificationContext<'_>) -> Option { - match expr { - Expr::ForLoop { - pattern, - iterable, - guard, - body, - } => Some(classify_for_loop( - ForLoopComponents { - pattern, - iterable: *iterable, - guard: guard.map(|g| *g), - body: *body, - }, - ctx, - )), - Expr::Apply { callee, args } => { - if let Some(source) = invocation_aggregation_source(&callee) { - return classify_aggregation_with_tracking(args, source, ctx); - } - Some(RuleBodyTerm::Expression(Expr::Apply { callee, args })) - } - Expr::Call { callee, args } => { - if let Some(source) = invocation_aggregation_source(&callee) { - return classify_aggregation_with_tracking(args, source, ctx); - } - Some(RuleBodyTerm::Expression(Expr::Call { callee, args })) - } - other => Some(RuleBodyTerm::Expression(other)), - } -} - -fn aggregation_source_for(name: &str) -> Option { - match name { - "group_by" => Some(AggregationSource::GroupBy), - "Aggregate" => Some(AggregationSource::LegacyAggregate), - _ => None, - } -} - -fn invocation_aggregation_source(callee: &Expr) -> Option { - if let Expr::Variable(name) = callee { - aggregation_source_for(name.as_str()) - } else { - None - } -} - -/// Classify an aggregation with tracking to enforce at-most-one-per-rule-body. -fn classify_aggregation_with_tracking( - args: Vec, - source: AggregationSource, - ctx: &mut ClassificationContext<'_>, -) -> Option { - if args.len() != 2 { - ctx.errors - .extend(aggregation_arity_error(ctx.literal_span, source)); - return None; - } - - let mut iter = args.into_iter(); - let first = iter - .next() - .unwrap_or_else(|| unreachable!("len pre-checked as 2; first arg missing")); - let second = iter - .next() - .unwrap_or_else(|| unreachable!("len pre-checked as 2; second arg missing")); - let (project, key) = match source { - AggregationSource::GroupBy => (first, second), - AggregationSource::LegacyAggregate => (second, first), - }; - - let term = RuleBodyTerm::Aggregation(RuleAggregation { - project, - key, - source, - }); - - // Track and validate aggregation - if validate_aggregation( - &term, - ctx.literal_span, - ctx.first_aggregation_span, - ctx.errors, - ) { - None // skip duplicate aggregation - } else { - Some(term) - } -} - -fn aggregation_arity_error( - literal_span: &Span, - source: AggregationSource, -) -> Vec> { - vec![Simple::custom( - literal_span.clone(), - format!("{} expects exactly two arguments", source.label()), - )] -} - -/// Components of a for-loop expression extracted for classification. -struct ForLoopComponents { - pattern: Pattern, - iterable: Expr, - guard: Option, - body: Expr, -} - -/// Classify a for-loop expression into a structured `RuleForLoop` term. -/// -/// Recursively classifies the body expression to produce nested body terms. -/// Aggregation validation is delegated to `classify_for_body_with_aggregation_tracking`. -fn classify_for_loop( - components: ForLoopComponents, - ctx: &mut ClassificationContext<'_>, -) -> RuleBodyTerm { - let body_terms = classify_for_body_with_aggregation_tracking(components.body, ctx); - - RuleBodyTerm::ForLoop(RuleForLoop { - pattern: components.pattern, - iterable: components.iterable, - guard: components.guard, - body_terms, - }) -} - -/// Recursively classify a for-loop body into a sequence of body terms with aggregation tracking. -/// -/// Handles sequence expressions (block bodies with multiple statements), -/// nested for-loops, and single expressions. Aggregations are tracked to enforce the -/// "at most one aggregation per rule body" rule. -fn classify_for_body_with_aggregation_tracking( - body: Expr, - ctx: &mut ClassificationContext<'_>, -) -> Vec { - match body { - // Handle sequence expressions from block bodies - Expr::Binary { - op: BinaryOp::Seq, - lhs, - rhs, - } => { - let mut terms = classify_for_body_with_aggregation_tracking(*lhs, ctx); - terms.extend(classify_for_body_with_aggregation_tracking(*rhs, ctx)); - terms - } - // Handle nested for-loops - Expr::ForLoop { - pattern, - iterable, - guard, - body, - } => { - vec![classify_for_loop( - ForLoopComponents { - pattern, - iterable: *iterable, - guard: guard.map(|g| *g), - body: *body, - }, - ctx, - )] - } - // Unwrap groups (parentheses/braces) to handle `(a; b)` bodies - Expr::Group(inner) => classify_for_body_with_aggregation_tracking(*inner, ctx), - // Single expression - classify it directly - other => classify_expression(other, ctx).into_iter().collect(), - } -} - -fn multiple_aggregations_error(_first_span: &Span, second_span: &Span) -> Simple { - Simple::custom( - second_span.clone(), - "at most one aggregation (group_by or Aggregate) is permitted per rule body".to_string(), - ) -} - #[derive(Debug)] pub(crate) struct AssignmentParts { pub(crate) pattern: String, diff --git a/src/parser/ast/rule/classification.rs b/src/parser/ast/rule/classification.rs new file mode 100644 index 00000000..160da6f1 --- /dev/null +++ b/src/parser/ast/rule/classification.rs @@ -0,0 +1,326 @@ +//! Rule-body term classification helpers for [`super::Rule`]. +//! +//! This module keeps the raw-literal classification pipeline separate from the +//! main AST wrapper types so `rule.rs` stays focused on the public surface. + +use chumsky::error::Simple; + +use super::{ + AggregationSource, AssignmentParts, RuleAggregation, RuleAssignment, RuleBodyTerm, RuleForLoop, + split_assignment, +}; +use crate::parser::ast::{BinaryOp, Expr, Pattern}; +use crate::parser::expression::parse_expression; +use crate::parser::pattern::parse_pattern; +use crate::parser::span_utils::shift_errors; +use crate::{Span, SyntaxKind}; + +/// Classify a raw rule-body literal into the structured rule-body term used by +/// semantic consumers. +/// +/// The caller provides the literal span so diagnostics can point back to the +/// CST node that supplied the raw text. `first_aggregation_span` is shared +/// across the surrounding rule body, allowing this helper to reject multiple +/// aggregations while still parsing each literal independently. +pub(super) fn parse_rule_body_term( + raw: &str, + literal_span: Span, + first_aggregation_span: &mut Option, + errors: &mut Vec>, +) -> Option { + if let Some(parts) = split_assignment(raw) { + match parse_assignment(&parts, &literal_span) { + Ok(term) => return term, + Err(mut errs) => { + errors.append(&mut errs); + return None; + } + } + } + + match parse_expression(raw.trim()) { + Ok(expr) => { + let mut ctx = ClassificationContext { + literal_span: &literal_span, + first_aggregation_span, + errors, + }; + classify_expression(expr, &mut ctx) + } + Err(mut errs) => { + errors.append(&mut errs); + None + } + } +} + +fn parse_assignment( + parts: &AssignmentParts, + literal_span: &Span, +) -> Result, Vec>> { + if parts.pattern.trim().is_empty() { + return Err(vec![Simple::custom( + literal_span.clone(), + "expected pattern before '=' in rule literal", + )]); + } + + if parts.value.trim().is_empty() { + return Err(vec![Simple::custom( + literal_span.clone(), + "expected expression after '=' in rule literal", + )]); + } + + let pattern_base_offset = literal_span.start.saturating_add(parts.pattern_offset); + let pattern = match parse_pattern(&parts.pattern) { + Ok(pattern) => pattern, + Err(errs) => return Err(shift_errors(errs, pattern_base_offset)), + }; + + let value_base_offset = literal_span.start.saturating_add(parts.value_offset); + match parse_expression(&parts.value) { + Ok(expr) => Ok(Some(RuleBodyTerm::Assignment(RuleAssignment { + pattern, + value: expr, + }))), + Err(errs) => Err(shift_errors(errs, value_base_offset)), + } +} + +struct ClassificationContext<'a> { + literal_span: &'a Span, + first_aggregation_span: &'a mut Option, + errors: &'a mut Vec>, +} + +fn classify_expression(expr: Expr, ctx: &mut ClassificationContext<'_>) -> Option { + match expr { + Expr::ForLoop { + pattern, + iterable, + guard, + body, + } => Some(classify_for_loop( + ForLoopComponents { + pattern, + iterable: *iterable, + guard: guard.map(|guard| *guard), + body: *body, + }, + ctx, + )), + Expr::Apply { callee, args } => { + if let Some(source) = invocation_aggregation_source(&callee) { + return classify_aggregation_with_tracking(args, source, ctx); + } + Some(RuleBodyTerm::Expression(Expr::Apply { callee, args })) + } + Expr::Call { callee, args } => { + if let Some(source) = invocation_aggregation_source(&callee) { + return classify_aggregation_with_tracking(args, source, ctx); + } + Some(RuleBodyTerm::Expression(Expr::Call { callee, args })) + } + other => Some(RuleBodyTerm::Expression(other)), + } +} + +fn aggregation_source_for(name: &str) -> Option { + match name { + "group_by" => Some(AggregationSource::GroupBy), + "Aggregate" => Some(AggregationSource::LegacyAggregate), + _ => None, + } +} + +fn invocation_aggregation_source(callee: &Expr) -> Option { + if let Expr::Variable(name) = callee { + aggregation_source_for(name.as_str()) + } else { + None + } +} + +fn classify_aggregation_with_tracking( + args: Vec, + source: AggregationSource, + ctx: &mut ClassificationContext<'_>, +) -> Option { + if args.len() != 2 { + ctx.errors + .extend(aggregation_arity_error(ctx.literal_span, source)); + return None; + } + + let mut iter = args.into_iter(); + let first = iter + .next() + .unwrap_or_else(|| unreachable!("len pre-checked as 2; first arg missing")); + let second = iter + .next() + .unwrap_or_else(|| unreachable!("len pre-checked as 2; second arg missing")); + let (project, key) = match source { + AggregationSource::GroupBy => (first, second), + AggregationSource::LegacyAggregate => (second, first), + }; + + let term = RuleBodyTerm::Aggregation(RuleAggregation { + project, + key, + source, + }); + + if validate_aggregation( + &term, + ctx.literal_span, + ctx.first_aggregation_span, + ctx.errors, + ) { + None + } else { + Some(term) + } +} + +fn validate_aggregation( + term: &RuleBodyTerm, + literal_span: &Span, + first_aggregation_span: &mut Option, + errors: &mut Vec>, +) -> bool { + if let RuleBodyTerm::Aggregation(_) = term { + match first_aggregation_span { + Some(first_span) => { + errors.push(multiple_aggregations_error(first_span, literal_span)); + return true; + } + None => { + *first_aggregation_span = Some(literal_span.clone()); + } + } + } + false +} + +fn aggregation_arity_error( + literal_span: &Span, + source: AggregationSource, +) -> Vec> { + vec![Simple::custom( + literal_span.clone(), + format!("{} expects exactly two arguments", source.label()), + )] +} + +struct ForLoopComponents { + pattern: Pattern, + iterable: Expr, + guard: Option, + body: Expr, +} + +fn classify_for_loop( + components: ForLoopComponents, + ctx: &mut ClassificationContext<'_>, +) -> RuleBodyTerm { + let body_terms = classify_for_body_with_aggregation_tracking(components.body, ctx); + + RuleBodyTerm::ForLoop(RuleForLoop { + pattern: components.pattern, + iterable: components.iterable, + guard: components.guard, + body_terms, + }) +} + +fn classify_for_body_with_aggregation_tracking( + body: Expr, + ctx: &mut ClassificationContext<'_>, +) -> Vec { + match body { + Expr::Binary { + op: BinaryOp::Seq, + lhs, + rhs, + } => { + let mut terms = classify_for_body_with_aggregation_tracking(*lhs, ctx); + terms.extend(classify_for_body_with_aggregation_tracking(*rhs, ctx)); + terms + } + Expr::ForLoop { + pattern, + iterable, + guard, + body, + } => { + vec![classify_for_loop( + ForLoopComponents { + pattern, + iterable: *iterable, + guard: guard.map(|guard| *guard), + body: *body, + }, + ctx, + )] + } + Expr::Group(inner) => classify_for_body_with_aggregation_tracking(*inner, ctx), + other => classify_expression(other, ctx).into_iter().collect(), + } +} + +fn multiple_aggregations_error(_first_span: &Span, second_span: &Span) -> Simple { + Simple::custom( + second_span.clone(), + "at most one aggregation (group_by or Aggregate) is permitted per rule body".to_string(), + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use chumsky::error::SimpleReason; + + fn span_for(src: &str) -> Span { + 0..src.len() + } + + #[test] + #[expect(clippy::expect_used, reason = "test asserts exact parser output")] + fn assignment_literal_becomes_assignment_term() { + let src = "var item = FlatMap(items)"; + let mut first_aggregation_span = None; + let mut errors = Vec::new(); + + let term = + parse_rule_body_term(src, span_for(src), &mut first_aggregation_span, &mut errors) + .expect("assignment should parse"); + + let RuleBodyTerm::Assignment(assignment) = term else { + panic!("expected assignment term"); + }; + assert_eq!(assignment.pattern.to_source(), "var item"); + assert_eq!(assignment.value.to_sexpr(), "(call FlatMap items)"); + assert!(errors.is_empty()); + } + + #[test] + #[expect(clippy::expect_used, reason = "test asserts exact parser error")] + fn second_aggregation_reports_error() { + let src = "group_by(count(), key)"; + let mut first_aggregation_span = Some(0..4); + let mut errors = Vec::new(); + + let term = + parse_rule_body_term(src, span_for(src), &mut first_aggregation_span, &mut errors); + + assert!(term.is_none()); + assert_eq!(errors.len(), 1); + let error = errors.first().expect("aggregation error missing"); + assert!(matches!( + error.reason(), + SimpleReason::Custom(message) + if message == "at most one aggregation (group_by or Aggregate) is permitted per rule body" + )); + } +} diff --git a/src/parser/expression/pratt.rs b/src/parser/expression/pratt.rs index 5872b5e0..c745aadc 100644 --- a/src/parser/expression/pratt.rs +++ b/src/parser/expression/pratt.rs @@ -9,12 +9,14 @@ use std::collections::HashMap; use chumsky::error::Simple; use crate::parser::ast::{Expr, StringLiteral}; -use crate::parser::span_utils::parse_u32_decimal; use crate::{Span, SyntaxKind, tokenize_without_trivia}; -use super::qualified::is_qualified_function_callee; use super::token_stream::TokenStream; +mod delay; +mod diff; +mod postfix; + const MAX_EXPR_DEPTH: usize = 256; pub(super) struct Pratt<'a, I> @@ -133,21 +135,34 @@ where &mut self, f: impl FnOnce(&mut Self) -> R, ) -> R { - if !self.struct_literals.suspend() { - return f(self); - } - let result = f(self); - self.struct_literals.resume(); - result + self.with_struct_literal_scope(StructLiteralState::suspend, StructLiteralState::resume, f) } pub(super) fn with_struct_literal_activation( &mut self, f: impl FnOnce(&mut Self) -> R, ) -> R { - self.struct_literals.activate(); + self.with_struct_literal_scope( + |state| { + state.activate(); + true + }, + StructLiteralState::deactivate, + f, + ) + } + + fn with_struct_literal_scope( + &mut self, + setup: impl FnOnce(&mut StructLiteralState) -> bool, + teardown: impl FnOnce(&mut StructLiteralState), + f: impl FnOnce(&mut Self) -> R, + ) -> R { + if !setup(&mut self.struct_literals) { + return f(self); + } let result = f(self); - self.struct_literals.deactivate(); + teardown(&mut self.struct_literals); result } @@ -185,231 +200,6 @@ where }) } - // Parse highest-precedence postfix operators, delegating each operation to a dedicated helper. - pub(super) fn parse_postfix(&mut self, mut lhs: Expr) -> Option { - let mut pending_diff_span: Option = None; - loop { - match self.ts.peek_kind() { - Some(SyntaxKind::T_APOSTROPHE) => { - self.handle_diff_marker(&mut pending_diff_span)?; - continue; - } - Some(SyntaxKind::T_LPAREN) => { - lhs = self.parse_function_call_postfix(lhs)?; - } - Some(SyntaxKind::T_LBRACKET) => { - lhs = self.parse_bit_slice_postfix(lhs)?; - } - Some(SyntaxKind::T_DOT) => { - lhs = self.parse_dot_access_postfix(lhs)?; - } - Some(SyntaxKind::T_MINUS) if self.ts.peek_nth_kind(1) == Some(SyntaxKind::T_LT) => { - self.validate_diff_not_pending(&mut pending_diff_span); - lhs = self.parse_delay_postfix(lhs)?; - } - _ => break, - } - - lhs = Self::apply_pending_diff(lhs, &mut pending_diff_span); - } - - self.validate_no_pending_diff(&mut pending_diff_span)?; - - Some(lhs) - } - - fn handle_diff_marker(&mut self, pending: &mut Option) -> Option<()> { - let (_, sp) = self.ts.next_tok()?; - if pending.is_some() { - self.ts.push_error(sp, "duplicate diff marker"); - } else { - *pending = Some(sp); - } - - if !matches!( - self.ts.peek_kind(), - Some(SyntaxKind::T_LPAREN | SyntaxKind::T_LBRACKET) - ) { - let diff_span = pending.take().unwrap_or_else(|| self.ts.eof_span()); - self.ts - .push_error(diff_span, "diff marker must precede atom arguments"); - return None; - } - - Some(()) - } - - fn apply_pending_diff(expr: Expr, pending: &mut Option) -> Expr { - if pending.take().is_some() { - Expr::AtomDiff { - expr: Box::new(expr), - } - } else { - expr - } - } - - fn validate_diff_not_pending(&mut self, pending: &mut Option) { - if let Some(diff_span) = pending.take() { - self.ts - .push_error(diff_span, "diff marker must apply to an atom"); - } - } - - fn validate_no_pending_diff(&mut self, pending: &mut Option) -> Option<()> { - if let Some(span) = pending.take() { - self.ts - .push_error(span, "diff marker must be followed by atom arguments"); - return None; - } - Some(()) - } - - fn parse_delay_postfix(&mut self, lhs: Expr) -> Option { - let (minus_kind, _) = self.ts.next_tok()?; // '-' - debug_assert_eq!( - minus_kind, - SyntaxKind::T_MINUS, - "parse_delay_postfix must be called with a leading '-' token" - ); - if !self.ts.expect(SyntaxKind::T_LT) { - return None; - } - let (kind, number_span) = self.ts.next_tok().unwrap_or_else(|| { - let sp = self.ts.eof_span(); - (SyntaxKind::N_ERROR, sp) - }); - if kind != SyntaxKind::T_NUMBER { - self.ts.push_error(number_span, "expected delay value"); - return None; - } - let raw = self.ts.slice(&number_span); - let delay = match parse_u32_decimal(&raw) { - Ok(delay) => delay, - Err(msg) => { - self.ts.push_error(number_span, msg); - return None; - } - }; - if !self.ts.expect(SyntaxKind::T_GT) { - return None; - } - Some(Expr::AtomDelay { - delay, - expr: Box::new(lhs), - }) - } - - fn parse_function_call_postfix(&mut self, lhs: Expr) -> Option { - let args = self.parse_parenthesized_args()?; - if is_qualified_function_callee(&lhs) { - Some(Expr::Call { - callee: Box::new(lhs), - args, - }) - } else { - Some(Expr::Apply { - callee: Box::new(lhs), - args, - }) - } - } - - fn parse_bit_slice_postfix(&mut self, lhs: Expr) -> Option { - self.ts.next_tok(); // opening bracket already peeked - self.with_struct_literals_suspended(move |this| { - let hi = this.parse_expr(0)?; - if !this.ts.expect(SyntaxKind::T_COMMA) { - return None; - } - let lo = this.parse_expr(0)?; - if !this.ts.expect(SyntaxKind::T_RBRACKET) { - return None; - } - Some(Expr::BitSlice { - expr: Box::new(lhs), - hi: Box::new(hi), - lo: Box::new(lo), - }) - }) - } - - fn parse_dot_access_postfix(&mut self, lhs: Expr) -> Option { - self.ts.next_tok(); // '.' - match self.ts.next_tok() { - Some((SyntaxKind::T_IDENT, sp)) => { - let name = self.ts.slice(&sp); - if matches!(self.ts.peek_kind(), Some(SyntaxKind::T_LPAREN)) { - self.parse_method_call(lhs, name) - } else { - Some(Expr::FieldAccess { - expr: Box::new(lhs), - field: name, - }) - } - } - Some((SyntaxKind::T_NUMBER, sp)) => { - let idx = self.ts.slice(&sp); - Some(Expr::TupleIndex { - expr: Box::new(lhs), - index: idx, - }) - } - other => { - let sp = other.map_or_else(|| self.ts.eof_span(), |(_, s)| s); - self.ts - .push_error(sp, "expected identifier or tuple index after '.'"); - None - } - } - } - - fn parse_method_call(&mut self, lhs: Expr, name: String) -> Option { - let args = self.parse_parenthesized_args()?; - Some(Expr::MethodCall { - recv: Box::new(lhs), - name, - args, - }) - } - - fn parse_parenthesized_args(&mut self) -> Option> { - self.ts.next_tok(); // opening parenthesis already peeked - self.with_struct_literals_suspended(|this| { - let args = this.parse_args()?; - if !this.ts.expect(SyntaxKind::T_RPAREN) { - return None; - } - Some(args) - }) - } - - pub(super) fn parse_args(&mut self) -> Option> { - let mut args = Vec::new(); - if matches!(self.ts.peek_kind(), Some(SyntaxKind::T_RPAREN)) { - return Some(args); - } - loop { - if self.ts.peek_kind().is_none() { - self.ts.expect(SyntaxKind::T_RPAREN); - return None; - } - let expr = self.parse_expr(0)?; - args.push(expr); - if !matches!(self.ts.peek_kind(), Some(SyntaxKind::T_COMMA)) { - break; - } - self.ts.next_tok(); - if matches!(self.ts.peek_kind(), Some(SyntaxKind::T_RPAREN)) { - let sp = self.ts.peek_span().unwrap_or_else(|| self.ts.eof_span()); - self.ts - .push_error(sp, "unexpected trailing comma in argument list"); - return None; - } - } - Some(args) - } - /// Parse a type expression following `:` or `as`. /// /// The RHS of ascriptions, and casts, should not consume infix operators diff --git a/src/parser/expression/pratt/delay.rs b/src/parser/expression/pratt/delay.rs new file mode 100644 index 00000000..385be9aa --- /dev/null +++ b/src/parser/expression/pratt/delay.rs @@ -0,0 +1,140 @@ +//! Delay-postfix parsing for the Pratt expression parser. +//! +//! Handles the `expr -` construct, consuming the `-<` token pair and +//! a decimal delay value to produce [`Expr::AtomDelay`]. + +use crate::parser::ast::Expr; +use crate::parser::span_utils::parse_u32_decimal; +use crate::{Span, SyntaxKind}; + +use super::Pratt; + +impl Pratt<'_, I> +where + I: Iterator + Clone, +{ + /// Parse a `-` delay postfix, returning an [`Expr::AtomDelay`] around + /// the expression parsed before the postfix marker. + pub(super) fn parse_delay_postfix(&mut self, lhs: Expr) -> Option { + let (minus_kind, _) = self.ts.next_tok()?; + debug_assert_eq!( + minus_kind, + SyntaxKind::T_MINUS, + "parse_delay_postfix must be called with a leading '-' token" + ); + if !self.ts.expect(SyntaxKind::T_LT) { + return None; + } + let (kind, number_span) = self.ts.next_tok().unwrap_or_else(|| { + let span = self.ts.eof_span(); + (SyntaxKind::N_ERROR, span) + }); + if kind != SyntaxKind::T_NUMBER { + self.ts.push_error(number_span, "expected delay value"); + return None; + } + let raw = self.ts.slice(&number_span); + let delay = match parse_u32_decimal(&raw) { + Ok(delay) => delay, + Err(message) => { + self.ts.push_error(number_span, message); + return None; + } + }; + if !self.ts.expect(SyntaxKind::T_GT) { + return None; + } + Some(Expr::AtomDelay { + delay, + expr: Box::new(lhs), + }) + } +} + +#[cfg(test)] +mod tests { + use chumsky::error::SimpleReason; + + use crate::parser::ast::Expr; + use crate::parser::expression::parse_expression; + use crate::tokenize_without_trivia; + use crate::{Span, SyntaxKind}; + + #[test] + #[expect(clippy::expect_used, reason = "test asserts exact parser output")] + fn parses_atom_delay_postfix() { + let expr = parse_expression("signal-<3>").expect("delay expression should parse"); + + assert_eq!(expr.to_sexpr(), "(delay 3 signal)"); + } + + #[test] + #[expect(clippy::expect_used, reason = "test asserts exact parser error")] + fn rejects_non_numeric_delay_value() { + let errors = + parse_expression("signal -< delay").expect_err("non-numeric delay should fail"); + + assert!(errors.iter().any(|error| matches!( + error.reason(), + SimpleReason::Custom(message) if message == "expected delay value" + ))); + } + + #[test] + #[expect(clippy::expect_used, reason = "test asserts exact parser output")] + fn parse_delay_postfix_accepts_max_u32_delay() { + let mut parser = delay_parser("-<4294967295>"); + + let expr = parser + .parse_delay_postfix(Expr::Variable("signal".into())) + .expect("u32 max delay should parse"); + + assert_eq!(expr.to_sexpr(), "(delay 4294967295 signal)"); + } + + #[test] + fn parse_delay_postfix_rejects_delay_above_u32_max() { + let mut parser = delay_parser("-<4294967296>"); + + let expr = parser.parse_delay_postfix(Expr::Variable("signal".into())); + + assert!(expr.is_none()); + let errors = parser.ts.take_errors(); + assert!(has_custom_error(&errors, "delay must fit u32")); + } + + #[test] + fn parse_delay_postfix_rejects_empty_delay_value() { + let mut parser = delay_parser("-<>"); + + let expr = parser.parse_delay_postfix(Expr::Variable("signal".into())); + + assert!(expr.is_none()); + let errors = parser.ts.take_errors(); + assert!(has_custom_error(&errors, "expected delay value")); + } + + #[test] + fn parse_delay_postfix_rejects_negative_delay_value() { + let mut parser = delay_parser("-<-1>"); + + let expr = parser.parse_delay_postfix(Expr::Variable("signal".into())); + + assert!(expr.is_none()); + let errors = parser.ts.take_errors(); + assert!(has_custom_error(&errors, "expected delay value")); + } + + fn delay_parser(src: &str) -> super::Pratt<'_, std::vec::IntoIter<(SyntaxKind, Span)>> { + super::Pratt::new(tokenize_without_trivia(src).into_iter(), src) + } + + fn has_custom_error(errors: &[chumsky::error::Simple], expected: &str) -> bool { + errors.iter().any(|error| { + matches!( + error.reason(), + SimpleReason::Custom(message) if message == expected + ) + }) + } +} diff --git a/src/parser/expression/pratt/diff.rs b/src/parser/expression/pratt/diff.rs new file mode 100644 index 00000000..fcf25dd5 --- /dev/null +++ b/src/parser/expression/pratt/diff.rs @@ -0,0 +1,158 @@ +//! Diff-marker postfix helpers for the Pratt expression parser. +//! +//! Tracks a pending `'` diff-marker span across the postfix chain and wraps +//! completed postfix expressions in [`Expr::AtomDiff`] via `apply_pending_diff`. + +use crate::parser::ast::Expr; +use crate::{Span, SyntaxKind}; + +use super::Pratt; + +impl Pratt<'_, I> +where + I: Iterator + Clone, +{ + /// Consume a diff marker and remember that the next completed postfix atom + /// should be wrapped in [`Expr::AtomDiff`]. + pub(super) fn handle_diff_marker(&mut self, pending: &mut Option) -> Option<()> { + let (_, span) = self.ts.next_tok()?; + if pending.is_some() { + self.ts.push_error(span, "duplicate diff marker"); + } else { + *pending = Some(span); + } + Some(()) + } + + /// Wrap `expr` in [`Expr::AtomDiff`] when a preceding diff marker is + /// pending, clearing that marker as part of the operation. + pub(super) fn apply_pending_diff(expr: Expr, pending: &mut Option) -> Expr { + if pending.take().is_some() { + Expr::AtomDiff { + expr: Box::new(expr), + } + } else { + expr + } + } + + /// Emit the diagnostic used when a pending diff marker cannot legally bind + /// to the postfix form about to be parsed. + pub(super) fn emit_error_if_diff_pending(&mut self, pending: &mut Option) { + if let Some(diff_span) = pending.take() { + self.ts + .push_error(diff_span, "diff marker must apply to an atom"); + } + } + + /// Ensure the postfix chain did not end with an unbound diff marker. + pub(super) fn validate_no_pending_diff(&mut self, pending: &mut Option) -> Option<()> { + if let Some(span) = pending.take() { + self.ts + .push_error(span, "diff marker must be followed by atom arguments"); + return None; + } + Some(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use chumsky::error::SimpleReason; + + #[test] + fn apply_pending_diff_wraps_and_clears_marker() { + let mut pending = Some(0..1); + let expr = Pratt::<'_, std::vec::IntoIter<(SyntaxKind, Span)>>::apply_pending_diff( + Expr::Variable("atom".into()), + &mut pending, + ); + + assert_eq!( + expr, + Expr::AtomDiff { + expr: Box::new(Expr::Variable("atom".into())) + } + ); + assert!(pending.is_none()); + } + + #[test] + fn apply_pending_diff_leaves_expression_when_no_marker_is_pending() { + let mut pending = None; + let expr = Pratt::<'_, std::vec::IntoIter<(SyntaxKind, Span)>>::apply_pending_diff( + Expr::Variable("atom".into()), + &mut pending, + ); + + assert_eq!(expr, Expr::Variable("atom".into())); + } + + #[test] + fn handle_diff_marker_reports_duplicate_marker() { + let mut parser = Pratt::new( + vec![ + (SyntaxKind::T_APOSTROPHE, 0..1), + (SyntaxKind::T_APOSTROPHE, 1..2), + ] + .into_iter(), + "''", + ); + let mut pending = None; + + assert_eq!(parser.handle_diff_marker(&mut pending), Some(())); + assert_eq!(pending, Some(0..1)); + assert_eq!(parser.handle_diff_marker(&mut pending), Some(())); + + assert_eq!(pending, Some(0..1)); + let errors = parser.ts.take_errors(); + assert!(has_custom_error(&errors, "duplicate diff marker", 1..2)); + } + + #[test] + fn emit_error_if_diff_pending_reports_and_clears_marker() { + let mut parser = Pratt::new(Vec::new().into_iter(), ""); + let mut pending = Some(2..3); + + parser.emit_error_if_diff_pending(&mut pending); + + assert!(pending.is_none()); + let errors = parser.ts.take_errors(); + assert!(has_custom_error( + &errors, + "diff marker must apply to an atom", + 2..3 + )); + } + + #[test] + fn validate_no_pending_diff_reports_unbound_marker() { + let mut parser = Pratt::new(Vec::new().into_iter(), ""); + let mut pending = Some(4..5); + + assert_eq!(parser.validate_no_pending_diff(&mut pending), None); + + assert!(pending.is_none()); + let errors = parser.ts.take_errors(); + assert!(has_custom_error( + &errors, + "diff marker must be followed by atom arguments", + 4..5 + )); + } + + fn has_custom_error( + errors: &[chumsky::error::Simple], + expected: &str, + span: Span, + ) -> bool { + errors.iter().any(|error| { + error.span() == span + && matches!( + error.reason(), + SimpleReason::Custom(message) if message == expected + ) + }) + } +} diff --git a/src/parser/expression/pratt/postfix.rs b/src/parser/expression/pratt/postfix.rs new file mode 100644 index 00000000..d69eec4a --- /dev/null +++ b/src/parser/expression/pratt/postfix.rs @@ -0,0 +1,232 @@ +//! Postfix-operator parsing for the Pratt expression parser. +//! +//! `parse_postfix` dispatches `(`, `[`, `.`, `-<`, and `'` postfix forms to +//! dedicated helpers, coordinating diff-marker tracking across the full chain. + +use crate::parser::ast::Expr; +use crate::parser::expression::qualified::is_qualified_function_callee; +use crate::{Span, SyntaxKind}; + +use super::Pratt; + +impl Pratt<'_, I> +where + I: Iterator + Clone, +{ + /// Parse highest-precedence postfix operators. + pub(super) fn parse_postfix(&mut self, mut lhs: Expr) -> Option { + let mut pending_diff_span: Option = None; + loop { + match self.ts.peek_kind() { + Some(SyntaxKind::T_APOSTROPHE) => { + self.handle_diff_marker(&mut pending_diff_span)?; + continue; + } + Some(SyntaxKind::T_LPAREN) => { + lhs = self.parse_function_call_postfix(lhs)?; + } + Some(SyntaxKind::T_LBRACKET) => { + lhs = self.parse_bit_slice_postfix(lhs)?; + } + Some(SyntaxKind::T_DOT) => { + lhs = self.parse_dot_access_postfix(lhs)?; + } + Some(SyntaxKind::T_MINUS) if self.ts.peek_nth_kind(1) == Some(SyntaxKind::T_LT) => { + self.emit_error_if_diff_pending(&mut pending_diff_span); + lhs = self.parse_delay_postfix(lhs)?; + } + _ => break, + } + + lhs = Self::apply_pending_diff(lhs, &mut pending_diff_span); + } + + self.validate_no_pending_diff(&mut pending_diff_span)?; + Some(lhs) + } + + fn parse_function_call_postfix(&mut self, lhs: Expr) -> Option { + let args = self.parse_parenthesized_args()?; + if is_qualified_function_callee(&lhs) { + Some(Expr::Call { + callee: Box::new(lhs), + args, + }) + } else { + Some(Expr::Apply { + callee: Box::new(lhs), + args, + }) + } + } + + fn parse_bit_slice_postfix(&mut self, lhs: Expr) -> Option { + let (_, _slice_span) = self.ts.next_tok()?; + self.with_struct_literals_suspended(move |this| { + let hi = this.parse_expr(0)?; + if !this.ts.expect(SyntaxKind::T_COMMA) { + return None; + } + let lo = this.parse_expr(0)?; + if !this.ts.expect(SyntaxKind::T_RBRACKET) { + return None; + } + Some(Expr::BitSlice { + expr: Box::new(lhs), + hi: Box::new(hi), + lo: Box::new(lo), + }) + }) + } + + fn parse_dot_access_postfix(&mut self, lhs: Expr) -> Option { + // token is consumed to advance past '.' + let (_, _dot_span) = self.ts.next_tok()?; + match self.ts.next_tok() { + Some((SyntaxKind::T_IDENT, span)) => { + let name = self.ts.slice(&span); + if matches!(self.ts.peek_kind(), Some(SyntaxKind::T_LPAREN)) { + self.parse_method_call(lhs, name) + } else { + Some(Expr::FieldAccess { + expr: Box::new(lhs), + field: name, + }) + } + } + Some((SyntaxKind::T_NUMBER, span)) => { + let index = self.ts.slice(&span); + Some(Expr::TupleIndex { + expr: Box::new(lhs), + index, + }) + } + other => { + let span = other.map_or_else(|| self.ts.eof_span(), |(_, span)| span); + self.ts + .push_error(span, "expected identifier or tuple index after '.'"); + None + } + } + } + + fn parse_method_call(&mut self, lhs: Expr, name: String) -> Option { + let args = self.parse_parenthesized_args()?; + Some(Expr::MethodCall { + recv: Box::new(lhs), + name, + args, + }) + } + + fn parse_parenthesized_args(&mut self) -> Option> { + // token is consumed to advance past '(' + let (_, _open_span) = self.ts.next_tok()?; + self.with_struct_literals_suspended(|this| { + let args = this.parse_args()?; + if !this.ts.expect(SyntaxKind::T_RPAREN) { + return None; + } + Some(args) + }) + } + + /// Parse a comma-separated argument list after the opening parenthesis has + /// already been consumed. + pub(super) fn parse_args(&mut self) -> Option> { + let mut args = Vec::new(); + if matches!(self.ts.peek_kind(), Some(SyntaxKind::T_RPAREN)) { + return Some(args); + } + loop { + if self.ts.peek_kind().is_none() { + self.ts.expect(SyntaxKind::T_RPAREN); + return None; + } + let expr = self.parse_expr(0)?; + args.push(expr); + if !matches!(self.ts.peek_kind(), Some(SyntaxKind::T_COMMA)) { + break; + } + let (_, comma_span) = self.ts.next_tok()?; + if matches!(self.ts.peek_kind(), Some(SyntaxKind::T_RPAREN)) { + self.ts + .push_error(comma_span, "unexpected trailing comma in argument list"); + return None; + } + } + Some(args) + } +} + +#[cfg(test)] +mod tests { + use chumsky::error::SimpleReason; + + use crate::SyntaxKind; + use crate::parser::ast::Expr; + use crate::parser::expression::parse_expression; + + #[test] + #[expect(clippy::expect_used, reason = "test asserts exact parser output")] + fn qualified_callee_uses_call_postfix() { + let expr = + parse_expression("std::map(value)").expect("qualified function call should parse"); + + assert_eq!(expr.to_sexpr(), "(call std::map value)"); + } + + #[test] + #[expect(clippy::expect_used, reason = "test asserts exact parser error")] + fn trailing_argument_comma_is_rejected() { + let errors = parse_expression("f(1,)").expect_err("trailing comma should fail"); + + assert!(errors.iter().any(|error| matches!( + error.reason(), + SimpleReason::Custom(message) + if message == "unexpected trailing comma in argument list" + ))); + assert!(errors.iter().any(|error| error.span() == (3..4))); + } + + #[test] + #[expect(clippy::expect_used, reason = "test asserts exact parser output")] + fn parse_postfix_handles_diff_postfix() { + let expr = parse_expression("S'(x)").expect("diff postfix should parse"); + + assert_eq!(expr.to_sexpr(), "(diff (call S x))"); + let Expr::AtomDiff { expr } = expr else { + panic!("expected diff postfix to wrap the completed call"); + }; + assert_eq!(expr.to_sexpr(), "(call S x)"); + } + + #[test] + #[expect(clippy::expect_used, reason = "test asserts exact parser output")] + fn parse_postfix_handles_delay_postfix() { + let expr = parse_expression("signal-<3>").expect("delay postfix should parse"); + + assert_eq!(expr.to_sexpr(), "(delay 3 signal)"); + } + + #[test] + #[expect(clippy::expect_used, reason = "test asserts exact parser error")] + fn parse_postfix_rejects_delay_when_diff_is_pending() { + let errors = parse_expression("signal'-<3>") + .expect_err("delay postfix with pending diff should fail"); + + assert!(has_custom_error( + &errors, + "diff marker must apply to an atom" + )); + } + + fn has_custom_error(errors: &[chumsky::error::Simple], expected: &str) -> bool { + errors.iter().any(|error| { + matches!( + error.reason(), + SimpleReason::Custom(message) if message == expected + ) + }) + } +} diff --git a/src/parser/tests/expression.rs b/src/parser/tests/expression.rs index 4b04f130..c68732c0 100644 --- a/src/parser/tests/expression.rs +++ b/src/parser/tests/expression.rs @@ -1,267 +1,44 @@ //! Tests for the Pratt expression parser. -use crate::parser::ast::{BinaryOp, Expr}; -use crate::parser::expression::parse_expression; -use crate::test_util::{ - assert_parse_error, assert_unclosed_delimiter_error, bit_slice, call, call_expr, closure, - field, field_access, for_loop, if_expr, lit_bool, lit_num, lit_str, map_entry, map_lit, - match_arm, match_expr, method_call, qualified_call, struct_expr, tuple, tuple_index, var, - vec_lit, +mod fixtures; + +use fixtures::{ + error_cases, expression_cases, invalid_for_loop_sources, literal_cases, + match_pattern_error_cases, postfix_error_cases, }; -use rstest::rstest; -#[rstest] -#[case("x", var("x"))] -#[case("foo()", call("foo", vec![]))] -#[case("pkg::Foo()", call("pkg::Foo", vec![]))] -#[case("pkg::foo()", qualified_call("pkg::foo", vec![]))] -#[case("add(x, 1)", call("add", vec![var("x"), lit_num("1")]))] -#[case("(1, 2)", tuple(vec![lit_num("1"), lit_num("2")]))] -#[case("(1, 2, 3)", tuple(vec![lit_num("1"), lit_num("2"), lit_num("3")]))] -#[case("(1,)", tuple(vec![lit_num("1")]))] -#[case("()", tuple(vec![]))] -#[case("(1)", Expr::Group(Box::new(lit_num("1"))))] -#[case("[]", vec_lit(vec![]))] -#[case("[1]", vec_lit(vec![lit_num("1")]))] -#[case("[1, 2]", vec_lit(vec![lit_num("1"), lit_num("2")]))] -#[case("[1, 2, 3]", vec_lit(vec![lit_num("1"), lit_num("2"), lit_num("3")]))] -#[case("[1,]", vec_lit(vec![lit_num("1")]))] -#[case("[1, 2,]", vec_lit(vec![lit_num("1"), lit_num("2")]))] -#[case("[x, y + 1]", vec_lit(vec![var("x"), Expr::Binary { op: BinaryOp::Add, lhs: Box::new(var("y")), rhs: Box::new(lit_num("1")) }]))] -#[case("{}", map_lit(vec![]))] -#[case("{a: 1}", map_lit(vec![map_entry(var("a"), lit_num("1"))]))] -#[case("{a: 1, b: 2}", map_lit(vec![map_entry(var("a"), lit_num("1")), map_entry(var("b"), lit_num("2"))]))] -#[case("{a: 1,}", map_lit(vec![map_entry(var("a"), lit_num("1"))]))] -#[case("{x: y, z: w}", map_lit(vec![map_entry(var("x"), var("y")), map_entry(var("z"), var("w"))]))] -#[case("Point { x: 1, y: 2 }", struct_expr("Point", vec![field("x", lit_num("1")), field("y", lit_num("2"))]))] -#[case("Point {}", struct_expr("Point", vec![]))] -#[case( - "Point { x: 1, y: 2, }", - struct_expr("Point", vec![field("x", lit_num("1")), field("y", lit_num("2"))]), -)] -#[case("|x, y| x + y", closure(vec!["x", "y"], Expr::Binary { op: BinaryOp::Add, lhs: Box::new(var("x")), rhs: Box::new(var("y")) }))] -#[case("|| 1", closure(std::iter::empty::<&str>(), lit_num("1")))] -#[case("|x,| x", closure(vec!["x"], var("x")))] -#[case("Point { pair: (1, 2) }", struct_expr("Point", vec![field("pair", tuple(vec![lit_num("1"), lit_num("2")]))]))] -#[case( - "(|| 1, |x| x)", - tuple(vec![ - closure(Vec::<&str>::new(), lit_num("1")), - closure(vec!["x"], var("x")), - ]), -)] -#[case("|x| Point { x: x }", closure(vec!["x"], struct_expr("Point", vec![field("x", var("x"))])))] -#[case("(f)(x)", call_expr(Expr::Group(Box::new(var("f"))), vec![var("x")]))] -#[case("foo.bar(x)", method_call(var("foo"), "bar", vec![var("x")]))] -#[case("foo.bar(x).baz", field_access( - method_call(var("foo"), "bar", vec![var("x")]), - "baz" -))] -#[case("foo.bar", field_access(var("foo"), "bar"))] -#[case("foo.bar.baz(x)", method_call( - field_access(var("foo"), "bar"), - "baz", - vec![var("x")] -))] -#[case("foo.bar.baz().qux", field_access( - method_call(field_access(var("foo"), "bar"), "baz", vec![]), - "qux" -))] -#[case("foo.bar().baz(x)", method_call( - method_call(var("foo"), "bar", vec![]), - "baz", - vec![var("x")] -))] -#[case("foo.bar.baz", field_access(field_access(var("foo"), "bar"), "baz"))] -#[case("foo.bar().baz.qux(x)", method_call( - field_access( - method_call(var("foo"), "bar", vec![]), - "baz" - ), - "qux", - vec![var("x")] -))] -#[case("e[1,0]", bit_slice(var("e"), lit_num("1"), lit_num("0")))] -#[case("t.0", tuple_index(var("t"), "0"))] -#[case( - "if x { y } else { z }", - if_expr( - var("x"), - Expr::Group(Box::new(var("y"))), - Some(Expr::Group(Box::new(var("z")))), - ) -)] -#[case( - "if (Point { x: 1 }) { y } else { z }", - if_expr( - Expr::Group(Box::new(struct_expr( - "Point", - vec![field("x", lit_num("1"))], - ))), - Expr::Group(Box::new(var("y"))), - Some(Expr::Group(Box::new(var("z")))), - ) -)] -#[case( - "if flag { Point { x: 1 } } else { z }", - if_expr( - var("flag"), - Expr::Group(Box::new(struct_expr( - "Point", - vec![field("x", lit_num("1"))], - ))), - Some(Expr::Group(Box::new(var("z")))), - ) -)] -#[case( - "if cond { Outer { inner: Inner { a: 1, b: 2 }, flag: true } } else { fallback }", - if_expr( - var("cond"), - Expr::Group(Box::new(struct_expr( - "Outer", - vec![ - field( - "inner", - struct_expr( - "Inner", - vec![ - field("a", lit_num("1")), - field("b", lit_num("2")), - ], - ), - ), - field("flag", lit_bool(true)), - ], - ))), - Some(Expr::Group(Box::new(var("fallback")))), - ) -)] -#[case( - "if ok { S { f: T { x: 1, y: U { z: 2 } }, g: 3 } } else { alt }", - if_expr( - var("ok"), - Expr::Group(Box::new(struct_expr( - "S", - vec![ - field( - "f", - struct_expr( - "T", - vec![ - field("x", lit_num("1")), - field( - "y", - struct_expr( - "U", - vec![field("z", lit_num("2"))], - ), - ), - ], - ), - ), - field("g", lit_num("3")), - ], - ))), - Some(Expr::Group(Box::new(var("alt")))), - ) -)] -#[case( - "if a and b { x } else { y }", - if_expr( - Expr::Binary { - op: BinaryOp::And, - lhs: Box::new(var("a")), - rhs: Box::new(var("b")), - }, - Expr::Group(Box::new(var("x"))), - Some(Expr::Group(Box::new(var("y")))), - ) -)] -#[case("if flag value", if_expr(var("flag"), var("value"), None))] -#[case( - "if cond { left } else if other { mid } else { right }", - if_expr( - var("cond"), - Expr::Group(Box::new(var("left"))), - Some(if_expr( - var("other"), - Expr::Group(Box::new(var("mid"))), - Some(Expr::Group(Box::new(var("right")))), - )), - ) -)] -#[case( - "match (flag) { true -> false, false -> true }", - match_expr( - var("flag"), - vec![ - match_arm("true", lit_bool(false)), - match_arm("false", lit_bool(true)), - ], - ) -)] -#[case( - "match (value) { Point { field: (x, y) } -> x, _ -> 0, }", - match_expr( - var("value"), - vec![ - match_arm("Point { field: (x, y) }", var("x")), - match_arm("_", lit_num("0")), - ], - ) -)] -#[case( - "match (x) { _ -> x, }", - match_expr( - var("x"), - vec![ - match_arm("_", var("x")), - ], - ) -)] -#[case( - "for (item in items) item", - for_loop("item", var("items"), None, var("item")) -)] -#[case( - "for (entry in items if entry.active) process(entry)", - for_loop( - "entry", - var("items"), - Some(field_access(var("entry"), "active")), - call("process", vec![var("entry")]), - ), -)] -fn parses_expressions(#[case] src: &str, #[case] expected: Expr) { - let expr = - parse_expression(src).unwrap_or_else(|errs| panic!("source {src:?} errors: {errs:?}")); - assert_eq!(expr, expected); +use crate::parser::expression::parse_expression; +use crate::test_util::{assert_parse_error, assert_unclosed_delimiter_error}; + +#[test] +fn parses_expressions() { + for case in expression_cases() { + let expr = parse_expression(case.src) + .unwrap_or_else(|errs| panic!("source {:?} errors: {errs:?}", case.src)); + assert_eq!(expr, case.expected, "source {:?}", case.src); + } } -#[rstest] -#[case("for item in items) item")] -#[case("for (in items) item")] -#[case("for (item items) item")] -#[case("for (item in items if) item")] -fn rejects_invalid_for_loop_syntax(#[case] src: &str) { - let Err(errors) = parse_expression(src) else { - panic!("expected parse failure for {src}"); - }; - assert!( - !errors.is_empty(), - "parser returned Err but without diagnostics for {src}" - ); +#[test] +fn rejects_invalid_for_loop_syntax() { + for src in invalid_for_loop_sources() { + let Err(errors) = parse_expression(src) else { + panic!("expected parse failure for {src}"); + }; + assert!( + !errors.is_empty(), + "parser returned Err but without diagnostics for {src}" + ); + } } -#[rstest] -#[case("\"hi\"", lit_str("hi"))] -#[case("true", lit_bool(true))] -#[case("false", lit_bool(false))] -#[case("42", lit_num("42"))] -fn parses_literals(#[case] src: &str, #[case] expected: Expr) { - let expr = - parse_expression(src).unwrap_or_else(|errs| panic!("source {src:?} errors: {errs:?}")); - assert_eq!(expr, expected); +#[test] +fn parses_literals() { + for case in literal_cases() { + let expr = parse_expression(case.src) + .unwrap_or_else(|errs| panic!("source {:?} errors: {errs:?}", case.src)); + assert_eq!(expr, case.expected, "source {:?}", case.src); + } } #[test] @@ -287,80 +64,38 @@ fn reports_struct_literal_disallowed_in_if_condition() { ); } -#[rstest] -#[case::addition_missing_rhs("1 +", 1)] -#[case::unclosed_parenthesis("(1 + 2", 1)] -#[case::unexpected_question_mark("1 ? 2", 1)] -#[case::trailing_colon("x :", 1)] -#[case::incomplete_as_cast("x as", 1)] -#[case::assignment_missing_rhs("x =", 1)] -#[case::extraneous_semicolon("x ;", 1)] -#[case::unexpected_fat_arrow("x =>", 1)] -#[case::empty_input("", 1)] -#[case::bit_slice_missing_comma("e[1 0]", 1)] -#[case::bit_slice_missing_lhs("e[,0]", 1)] -#[case::bit_slice_missing_rhs("e[1,]", 1)] -#[case::bit_slice_extra_comma("e[1,,0]", 1)] -#[case::tuple_index_missing_digits("t.", 1)] -#[case::tuple_index_negative("t.-1", 1)] -#[case::tuple_index_plus("t.+1", 1)] -#[case::tuple_index_double_dot("t..0", 1)] -#[case::if_missing_then("if cond else value", 1)] -#[case::if_missing_else_expr("if cond value else", 1)] -#[case::if_missing_condition("if", 1)] -#[case::match_missing_arrow("match (x) { _ => x }", 1)] -#[case::match_missing_arm("match (x) {}", 1)] -#[case::match_missing_paren("match x { _ -> x }", 1)] -#[case::match_unmatched_paren("match (x) { ) -> x }", 1)] -#[case::match_unmatched_brace("match (x) { } -> x }", 1)] -#[case::match_unmatched_bracket("match (x) { ] -> x }", 1)] -#[case::vector_unclosed("[1, 2", 1)] -#[case::map_unclosed("{a: 1", 1)] -#[case::map_comma_without_colon("{a, b}", 1)] -fn reports_errors(#[case] src: &str, #[case] min_errs: usize) { - match parse_expression(src) { - Ok(_) => panic!("expected parse error"), - Err(errs) => assert!(errs.len() >= min_errs), +#[test] +fn reports_errors() { + for case in error_cases() { + match parse_expression(case.src) { + Ok(_) => panic!("expected parse error"), + Err(errs) => assert!( + errs.len() >= case.min_errs, + "source {:?} produced too few errors: {errs:?}", + case.src + ), + } } } -#[rstest] -#[case( - "match (x) { ) -> x }", - "unmatched closing parenthesis in match pattern", - 12, - 13 -)] -#[case( - "match (x) { ( } -> x }", - "unmatched closing brace in match pattern", - 14, - 15 -)] -#[case( - "match (x) { ] -> x }", - "unmatched closing bracket in match pattern", - 12, - 13 -)] -fn match_pattern_delimiter_errors( - #[case] src: &str, - #[case] msg: &str, - #[case] start: usize, - #[case] end: usize, -) { - let Err(errors) = parse_expression(src) else { - panic!("expected error"); - }; - let Some(first) = errors.first() else { - panic!("error missing"); - }; - let rendered = format!("{first:?}"); - assert!( - rendered.contains(msg), - "expected error to contain pattern '{msg}', got '{rendered}'", - ); - assert_eq!(first.span(), start..end); +#[test] +fn match_pattern_delimiter_errors() { + for case in match_pattern_error_cases() { + let Err(errors) = parse_expression(case.src) else { + panic!("expected error"); + }; + let Some(first) = errors.first() else { + panic!("error missing"); + }; + let rendered = format!("{first:?}"); + assert!( + rendered.contains(case.msg), + "expected error to contain pattern '{}', got '{}'", + case.msg, + rendered, + ); + assert_eq!(first.span(), case.start..case.end); + } } fn reports_exactly_one_error(src: &str) { @@ -377,27 +112,16 @@ fn rejects_chained_type_ops_with_single_diag() { } } -#[rstest] -#[case::trailing_dot("foo.", "expected identifier or tuple index after '.'", 4, 4, false)] -#[case::bit_slice_missing_comma("e[1]", "expected comma", 3, 4, false)] -#[case::bit_slice_unclosed("e[1,0", "expected right bracket", 5, 5, true)] -#[case::vector_unclosed("[1, 2", "expected right bracket", 5, 5, true)] -#[case::map_unclosed("{a: 1", "expected right brace", 5, 5, true)] -#[case::map_comma_without_colon("{a, b}", "expected ':'", 2, 3, false)] -#[case::scoped_identifier_missing_segment("pkg::()", "expected identifier after '::'", 5, 6, false)] -fn postfix_expression_errors( - #[case] src: &str, - #[case] msg: &str, - #[case] start: usize, - #[case] end: usize, - #[case] unclosed: bool, -) { - let Err(errors) = parse_expression(src) else { - panic!("expected error"); - }; - if unclosed { - assert_unclosed_delimiter_error(&errors, msg, start, end); - } else { - assert_parse_error(&errors, msg, start, end); +#[test] +fn postfix_expression_errors() { + for case in postfix_error_cases() { + let Err(errors) = parse_expression(case.src) else { + panic!("expected error"); + }; + if case.unclosed { + assert_unclosed_delimiter_error(&errors, case.msg, case.start, case.end); + } else { + assert_parse_error(&errors, case.msg, case.start, case.end); + } } } diff --git a/src/parser/tests/expression/fixtures.rs b/src/parser/tests/expression/fixtures.rs new file mode 100644 index 00000000..383bd121 --- /dev/null +++ b/src/parser/tests/expression/fixtures.rs @@ -0,0 +1,59 @@ +//! Shared fixture definitions and accessors for expression parser tests. +//! +//! This module defines the case types used by the valid and error fixture +//! groups, then exposes the grouped lists consumed by the parent expression +//! parser test modules. + +mod errors; +mod valid; + +use crate::parser::ast::Expr; + +pub(super) struct ExpressionCase { + pub(super) src: &'static str, + pub(super) expected: Expr, +} + +pub(super) struct CountedErrorCase { + pub(super) src: &'static str, + pub(super) min_errs: usize, +} + +pub(super) struct SpannedErrorCase { + pub(super) src: &'static str, + pub(super) msg: &'static str, + pub(super) start: usize, + pub(super) end: usize, +} + +pub(super) struct PostfixErrorCase { + pub(super) src: &'static str, + pub(super) msg: &'static str, + pub(super) start: usize, + pub(super) end: usize, + pub(super) unclosed: bool, +} + +pub(super) fn expression_cases() -> Vec { + valid::expression_cases() +} + +pub(super) fn invalid_for_loop_sources() -> [&'static str; 4] { + valid::invalid_for_loop_sources() +} + +pub(super) fn literal_cases() -> Vec { + valid::literal_cases() +} + +pub(super) fn error_cases() -> Vec { + errors::error_cases() +} + +pub(super) fn match_pattern_error_cases() -> Vec { + errors::match_pattern_error_cases() +} + +pub(super) fn postfix_error_cases() -> Vec { + errors::postfix_error_cases() +} diff --git a/src/parser/tests/expression/fixtures/errors.rs b/src/parser/tests/expression/fixtures/errors.rs new file mode 100644 index 00000000..32b8034f --- /dev/null +++ b/src/parser/tests/expression/fixtures/errors.rs @@ -0,0 +1,132 @@ +//! Aggregated invalid-expression fixtures for parser tests. +//! +//! These cases are grouped by diagnostic style so the parent expression suite +//! can assert error counts, spans, and postfix delimiter recovery without +//! duplicating fixture wiring. + +use crate::parser::tests::expression::fixtures::{ + CountedErrorCase, PostfixErrorCase, SpannedErrorCase, +}; + +pub(super) fn error_cases() -> Vec { + let mut cases = operator_error_cases(); + cases.extend(postfix_counted_error_cases()); + cases.extend(control_flow_error_cases()); + cases.extend(match_error_cases()); + cases.extend(collection_error_cases()); + cases +} + +pub(super) fn match_pattern_error_cases() -> Vec { + vec![ + SpannedErrorCase { + src: "match (x) { ) -> x }", + msg: "unmatched closing parenthesis in match pattern", + start: 12, + end: 13, + }, + SpannedErrorCase { + src: "match (x) { ( } -> x }", + msg: "unmatched closing brace in match pattern", + start: 14, + end: 15, + }, + SpannedErrorCase { + src: "match (x) { ] -> x }", + msg: "unmatched closing bracket in match pattern", + start: 12, + end: 13, + }, + ] +} + +pub(super) fn postfix_error_cases() -> Vec { + vec![ + PostfixErrorCase { + src: "foo.", + msg: "expected identifier or tuple index after '.'", + start: 4, + end: 4, + unclosed: false, + }, + PostfixErrorCase { + src: "e[1]", + msg: "expected comma", + start: 3, + end: 4, + unclosed: false, + }, + PostfixErrorCase { + src: "e[1,0", + msg: "expected right bracket", + start: 5, + end: 5, + unclosed: true, + }, + PostfixErrorCase { + src: "[1, 2", + msg: "expected right bracket", + start: 5, + end: 5, + unclosed: true, + }, + PostfixErrorCase { + src: "{a: 1", + msg: "expected right brace", + start: 5, + end: 5, + unclosed: true, + }, + PostfixErrorCase { + src: "{a, b}", + msg: "expected ':'", + start: 2, + end: 3, + unclosed: false, + }, + PostfixErrorCase { + src: "pkg::()", + msg: "expected identifier after '::'", + start: 5, + end: 6, + unclosed: false, + }, + ] +} + +fn counted_error_cases(srcs: &[&'static str]) -> Vec { + srcs.iter() + .map(|&src| CountedErrorCase { src, min_errs: 1 }) + .collect() +} + +fn operator_error_cases() -> Vec { + counted_error_cases(&[ + "1 +", "(1 + 2", "1 ? 2", "x :", "x as", "x =", "x ;", "x =>", "", + ]) +} + +fn postfix_counted_error_cases() -> Vec { + counted_error_cases(&[ + "e[1 0]", "e[,0]", "e[1,]", "e[1,,0]", "t.", "t.-1", "t.+1", "t..0", + ]) +} + +fn control_flow_error_cases() -> Vec { + counted_error_cases(&["if cond else value", "if cond value else", "if"]) +} + +fn match_error_cases() -> Vec { + counted_error_cases(&[ + "match (x) { _ => x }", + "match (x) {}", + "match x { _ -> x }", + "match (x) { ) -> x }", + "match (x) { } -> x }", + "match (x) { ] -> x }", + ]) +} + +fn collection_error_cases() -> Vec { + counted_error_cases(&["[1, 2", "{a: 1", "{a, b}"]) +} diff --git a/src/parser/tests/expression/fixtures/valid.rs b/src/parser/tests/expression/fixtures/valid.rs new file mode 100644 index 00000000..74263026 --- /dev/null +++ b/src/parser/tests/expression/fixtures/valid.rs @@ -0,0 +1,29 @@ +//! Aggregated valid-expression fixtures for parser tests. +//! +//! Each child module covers one syntactic area, and this module combines those +//! groups into the ordered case lists exercised by the expression parser suite. + +mod basic; +mod collections; +mod control_flow; +mod postfix; +mod structured; + +use crate::parser::tests::expression::fixtures::ExpressionCase; + +pub(super) fn expression_cases() -> Vec { + let mut cases = basic::basic_expression_cases(); + cases.extend(collections::collection_expression_cases()); + cases.extend(structured::struct_and_closure_cases()); + cases.extend(postfix::postfix_expression_cases()); + cases.extend(control_flow::control_flow_expression_cases()); + cases +} + +pub(super) fn invalid_for_loop_sources() -> [&'static str; 4] { + basic::invalid_for_loop_sources() +} + +pub(super) fn literal_cases() -> Vec { + basic::literal_cases() +} diff --git a/src/parser/tests/expression/fixtures/valid/basic.rs b/src/parser/tests/expression/fixtures/valid/basic.rs new file mode 100644 index 00000000..45fe7d59 --- /dev/null +++ b/src/parser/tests/expression/fixtures/valid/basic.rs @@ -0,0 +1,83 @@ +//! Core expression fixtures for the simplest valid forms. +//! +//! These cases cover atoms, calls, tuples, and literals that the parser must +//! accept before the specialised fixture modules add broader syntax coverage. + +use crate::parser::ast::Expr; +use crate::parser::tests::expression::fixtures::ExpressionCase; +use crate::test_util::{call, lit_bool, lit_num, lit_str, qualified_call, tuple, var}; + +pub(super) fn basic_expression_cases() -> Vec { + vec![ + ExpressionCase { + src: "x", + expected: var("x"), + }, + ExpressionCase { + src: "foo()", + expected: call("foo", vec![]), + }, + ExpressionCase { + src: "pkg::Foo()", + expected: call("pkg::Foo", vec![]), + }, + ExpressionCase { + src: "pkg::foo()", + expected: qualified_call("pkg::foo", vec![]), + }, + ExpressionCase { + src: "add(x, 1)", + expected: call("add", vec![var("x"), lit_num("1")]), + }, + ExpressionCase { + src: "(1, 2)", + expected: tuple(vec![lit_num("1"), lit_num("2")]), + }, + ExpressionCase { + src: "(1, 2, 3)", + expected: tuple(vec![lit_num("1"), lit_num("2"), lit_num("3")]), + }, + ExpressionCase { + src: "(1,)", + expected: tuple(vec![lit_num("1")]), + }, + ExpressionCase { + src: "()", + expected: tuple(vec![]), + }, + ExpressionCase { + src: "(1)", + expected: Expr::Group(Box::new(lit_num("1"))), + }, + ] +} + +pub(super) fn invalid_for_loop_sources() -> [&'static str; 4] { + [ + "for item in items) item", + "for (in items) item", + "for (item items) item", + "for (item in items if) item", + ] +} + +pub(super) fn literal_cases() -> Vec { + vec![ + ExpressionCase { + src: "\"hi\"", + expected: lit_str("hi"), + }, + ExpressionCase { + src: "true", + expected: lit_bool(true), + }, + ExpressionCase { + src: "false", + expected: lit_bool(false), + }, + ExpressionCase { + src: "42", + expected: lit_num("42"), + }, + ] +} diff --git a/src/parser/tests/expression/fixtures/valid/collections.rs b/src/parser/tests/expression/fixtures/valid/collections.rs new file mode 100644 index 00000000..33b33b13 --- /dev/null +++ b/src/parser/tests/expression/fixtures/valid/collections.rs @@ -0,0 +1,75 @@ +//! Collection literal fixtures for expression parser tests. +//! +//! These cases exercise list and map forms, including nested expressions, +//! trailing commas, and key-value pairs that feed the shared valid-expression +//! suite. + +use crate::parser::ast::{BinaryOp, Expr}; +use crate::parser::tests::expression::fixtures::ExpressionCase; +use crate::test_util::{lit_num, map_entry, map_lit, var, vec_lit}; + +pub(super) fn collection_expression_cases() -> Vec { + vec![ + ExpressionCase { + src: "[]", + expected: vec_lit(vec![]), + }, + ExpressionCase { + src: "[1]", + expected: vec_lit(vec![lit_num("1")]), + }, + ExpressionCase { + src: "[1, 2]", + expected: vec_lit(vec![lit_num("1"), lit_num("2")]), + }, + ExpressionCase { + src: "[1, 2, 3]", + expected: vec_lit(vec![lit_num("1"), lit_num("2"), lit_num("3")]), + }, + ExpressionCase { + src: "[1,]", + expected: vec_lit(vec![lit_num("1")]), + }, + ExpressionCase { + src: "[1, 2,]", + expected: vec_lit(vec![lit_num("1"), lit_num("2")]), + }, + ExpressionCase { + src: "[x, y + 1]", + expected: vec_lit(vec![ + var("x"), + Expr::Binary { + op: BinaryOp::Add, + lhs: Box::new(var("y")), + rhs: Box::new(lit_num("1")), + }, + ]), + }, + ExpressionCase { + src: "{}", + expected: map_lit(vec![]), + }, + ExpressionCase { + src: "{a: 1}", + expected: map_lit(vec![map_entry(var("a"), lit_num("1"))]), + }, + ExpressionCase { + src: "{a: 1, b: 2}", + expected: map_lit(vec![ + map_entry(var("a"), lit_num("1")), + map_entry(var("b"), lit_num("2")), + ]), + }, + ExpressionCase { + src: "{a: 1,}", + expected: map_lit(vec![map_entry(var("a"), lit_num("1"))]), + }, + ExpressionCase { + src: "{x: y, z: w}", + expected: map_lit(vec![ + map_entry(var("x"), var("y")), + map_entry(var("z"), var("w")), + ]), + }, + ] +} diff --git a/src/parser/tests/expression/fixtures/valid/control_flow.rs b/src/parser/tests/expression/fixtures/valid/control_flow.rs new file mode 100644 index 00000000..60d0c05a --- /dev/null +++ b/src/parser/tests/expression/fixtures/valid/control_flow.rs @@ -0,0 +1,176 @@ +//! Control-flow fixtures for expression parser tests. +//! +//! These grouped cases cover `if`, `match`, and `for` expressions, including +//! nested forms that build larger ASTs for the shared valid-expression suite. + +use crate::parser::ast::{BinaryOp, Expr}; +use crate::parser::tests::expression::fixtures::ExpressionCase; +use crate::test_util::{ + call, field, field_access, for_loop, if_expr, lit_bool, lit_num, match_arm, match_expr, + struct_expr, var, +}; + +pub(super) fn control_flow_expression_cases() -> Vec { + let mut cases = if_expression_cases(); + cases.extend(match_and_for_expression_cases()); + cases +} + +fn if_expression_cases() -> Vec { + let mut cases = basic_if_expression_cases(); + cases.extend(struct_if_expression_cases()); + cases +} + +fn basic_if_expression_cases() -> Vec { + vec![ + ExpressionCase { + src: "if x { y } else { z }", + expected: if_expr( + var("x"), + Expr::Group(Box::new(var("y"))), + Some(Expr::Group(Box::new(var("z")))), + ), + }, + ExpressionCase { + src: "if (Point { x: 1 }) { y } else { z }", + expected: if_expr( + Expr::Group(Box::new(struct_expr( + "Point", + vec![field("x", lit_num("1"))], + ))), + Expr::Group(Box::new(var("y"))), + Some(Expr::Group(Box::new(var("z")))), + ), + }, + ExpressionCase { + src: "if flag { Point { x: 1 } } else { z }", + expected: if_expr( + var("flag"), + Expr::Group(Box::new(struct_expr( + "Point", + vec![field("x", lit_num("1"))], + ))), + Some(Expr::Group(Box::new(var("z")))), + ), + }, + ExpressionCase { + src: "if a and b { x } else { y }", + expected: if_expr( + Expr::Binary { + op: BinaryOp::And, + lhs: Box::new(var("a")), + rhs: Box::new(var("b")), + }, + Expr::Group(Box::new(var("x"))), + Some(Expr::Group(Box::new(var("y")))), + ), + }, + ExpressionCase { + src: "if flag value", + expected: if_expr(var("flag"), var("value"), None), + }, + ExpressionCase { + src: "if cond { left } else if other { mid } else { right }", + expected: if_expr( + var("cond"), + Expr::Group(Box::new(var("left"))), + Some(if_expr( + var("other"), + Expr::Group(Box::new(var("mid"))), + Some(Expr::Group(Box::new(var("right")))), + )), + ), + }, + ] +} + +fn struct_if_expression_cases() -> Vec { + vec![ + ExpressionCase { + src: "if cond { Outer { inner: Inner { a: 1, b: 2 }, flag: true } } else { fallback }", + expected: if_expr( + var("cond"), + Expr::Group(Box::new(struct_expr( + "Outer", + vec![ + field( + "inner", + struct_expr( + "Inner", + vec![field("a", lit_num("1")), field("b", lit_num("2"))], + ), + ), + field("flag", lit_bool(true)), + ], + ))), + Some(Expr::Group(Box::new(var("fallback")))), + ), + }, + ExpressionCase { + src: "if ok { S { f: T { x: 1, y: U { z: 2 } }, g: 3 } } else { alt }", + expected: if_expr( + var("ok"), + Expr::Group(Box::new(struct_expr( + "S", + vec![ + field( + "f", + struct_expr( + "T", + vec![ + field("x", lit_num("1")), + field("y", struct_expr("U", vec![field("z", lit_num("2"))])), + ], + ), + ), + field("g", lit_num("3")), + ], + ))), + Some(Expr::Group(Box::new(var("alt")))), + ), + }, + ] +} + +fn match_and_for_expression_cases() -> Vec { + vec![ + ExpressionCase { + src: "match (flag) { true -> false, false -> true }", + expected: match_expr( + var("flag"), + vec![ + match_arm("true", lit_bool(false)), + match_arm("false", lit_bool(true)), + ], + ), + }, + ExpressionCase { + src: "match (value) { Point { field: (x, y) } -> x, _ -> 0, }", + expected: match_expr( + var("value"), + vec![ + match_arm("Point { field: (x, y) }", var("x")), + match_arm("_", lit_num("0")), + ], + ), + }, + ExpressionCase { + src: "match (x) { _ -> x, }", + expected: match_expr(var("x"), vec![match_arm("_", var("x"))]), + }, + ExpressionCase { + src: "for (item in items) item", + expected: for_loop("item", var("items"), None, var("item")), + }, + ExpressionCase { + src: "for (entry in items if entry.active) process(entry)", + expected: for_loop( + "entry", + var("items"), + Some(field_access(var("entry"), "active")), + call("process", vec![var("entry")]), + ), + }, + ] +} diff --git a/src/parser/tests/expression/fixtures/valid/postfix.rs b/src/parser/tests/expression/fixtures/valid/postfix.rs new file mode 100644 index 00000000..24808a37 --- /dev/null +++ b/src/parser/tests/expression/fixtures/valid/postfix.rs @@ -0,0 +1,123 @@ +//! Postfix-expression fixtures for parser tests. +//! +//! These cases cover field access, method calls, indexing, diff markers, and +//! delay syntax that extend an already parsed atom in the shared fixture suite. + +use crate::parser::ast::Expr; +use crate::parser::tests::expression::fixtures::ExpressionCase; +use crate::test_util::{ + atom_delay, atom_diff, bit_slice, call_expr, field_access, lit_num, method_call, tuple_index, + var, +}; + +fn field_and_method_call_cases() -> Vec { + vec![ + ExpressionCase { + src: "foo.bar(x)", + expected: method_call(var("foo"), "bar", vec![var("x")]), + }, + ExpressionCase { + src: "foo.bar(x).baz", + expected: field_access(method_call(var("foo"), "bar", vec![var("x")]), "baz"), + }, + ExpressionCase { + src: "foo.bar", + expected: field_access(var("foo"), "bar"), + }, + ExpressionCase { + src: "foo.bar.baz(x)", + expected: method_call(field_access(var("foo"), "bar"), "baz", vec![var("x")]), + }, + ExpressionCase { + src: "foo.bar.baz().qux", + expected: field_access( + method_call(field_access(var("foo"), "bar"), "baz", vec![]), + "qux", + ), + }, + ExpressionCase { + src: "foo.bar().baz(x)", + expected: method_call( + method_call(var("foo"), "bar", vec![]), + "baz", + vec![var("x")], + ), + }, + ExpressionCase { + src: "foo.bar.baz", + expected: field_access(field_access(var("foo"), "bar"), "baz"), + }, + ExpressionCase { + src: "foo.bar().baz.qux(x)", + expected: method_call( + field_access(method_call(var("foo"), "bar", vec![]), "baz"), + "qux", + vec![var("x")], + ), + }, + ] +} + +fn call_and_slice_cases() -> Vec { + vec![ + ExpressionCase { + src: "(f)(x)", + expected: call_expr(Expr::Group(Box::new(var("f"))), vec![var("x")]), + }, + ExpressionCase { + src: "e[1,0]", + expected: bit_slice(var("e"), lit_num("1"), lit_num("0")), + }, + ExpressionCase { + src: "t.0", + expected: tuple_index(var("t"), "0"), + }, + ] +} + +fn diff_marker_cases() -> Vec { + vec![ + ExpressionCase { + src: "e'(x)", + expected: atom_diff(call_expr(var("e"), vec![var("x")])), + }, + ExpressionCase { + src: "e'[1,0]", + expected: atom_diff(bit_slice(var("e"), lit_num("1"), lit_num("0"))), + }, + ExpressionCase { + src: "S'(x)", + expected: Expr::AtomDiff { + expr: Box::new(call_expr(var("S"), vec![var("x")])), + }, + }, + ] +} + +fn delay_cases() -> Vec { + vec![ + ExpressionCase { + src: "e-<5>", + expected: atom_delay(5, var("e")), + }, + ExpressionCase { + src: "e-<0>", + expected: atom_delay(0, var("e")), + }, + ExpressionCase { + src: "A() -<10>", + expected: Expr::AtomDelay { + delay: 10, + expr: Box::new(call_expr(var("A"), vec![])), + }, + }, + ] +} + +pub(super) fn postfix_expression_cases() -> Vec { + let mut cases = field_and_method_call_cases(); + cases.extend(call_and_slice_cases()); + cases.extend(diff_marker_cases()); + cases.extend(delay_cases()); + cases +} diff --git a/src/parser/tests/expression/fixtures/valid/structured.rs b/src/parser/tests/expression/fixtures/valid/structured.rs new file mode 100644 index 00000000..d6c8cc85 --- /dev/null +++ b/src/parser/tests/expression/fixtures/valid/structured.rs @@ -0,0 +1,69 @@ +//! Structured-expression fixtures for parser tests. +//! +//! These cases cover struct literals and closures, including grouped +//! expressions inside compound forms and parameter lists used by the parent +//! expression suite. + +use crate::parser::ast::{BinaryOp, Expr}; +use crate::parser::tests::expression::fixtures::ExpressionCase; +use crate::test_util::{closure, field, lit_num, struct_expr, tuple, var}; + +pub(super) fn struct_and_closure_cases() -> Vec { + vec![ + ExpressionCase { + src: "Point { x: 1, y: 2 }", + expected: struct_expr( + "Point", + vec![field("x", lit_num("1")), field("y", lit_num("2"))], + ), + }, + ExpressionCase { + src: "Point {}", + expected: struct_expr("Point", vec![]), + }, + ExpressionCase { + src: "Point { x: 1, y: 2, }", + expected: struct_expr( + "Point", + vec![field("x", lit_num("1")), field("y", lit_num("2"))], + ), + }, + ExpressionCase { + src: "|x, y| x + y", + expected: closure( + vec!["x", "y"], + Expr::Binary { + op: BinaryOp::Add, + lhs: Box::new(var("x")), + rhs: Box::new(var("y")), + }, + ), + }, + ExpressionCase { + src: "|| 1", + expected: closure(std::iter::empty::<&str>(), lit_num("1")), + }, + ExpressionCase { + src: "|x,| x", + expected: closure(vec!["x"], var("x")), + }, + ExpressionCase { + src: "Point { pair: (1, 2) }", + expected: struct_expr( + "Point", + vec![field("pair", tuple(vec![lit_num("1"), lit_num("2")]))], + ), + }, + ExpressionCase { + src: "(|| 1, |x| x)", + expected: tuple(vec![ + closure(Vec::<&str>::new(), lit_num("1")), + closure(vec!["x"], var("x")), + ]), + }, + ExpressionCase { + src: "|x| Point { x: x }", + expected: closure(vec!["x"], struct_expr("Point", vec![field("x", var("x"))])), + }, + ] +} diff --git a/src/parser/tests/rules/body_terms.rs b/src/parser/tests/rules/body_terms.rs index f5117def..2d607006 100644 --- a/src/parser/tests/rules/body_terms.rs +++ b/src/parser/tests/rules/body_terms.rs @@ -139,3 +139,26 @@ fn body_terms_shift_assignment_value_error_spans() { "expected shifted value error span {expected_span:?}, got {errors:?}", ); } + +#[test] +fn body_terms_rejects_whitespace_only_value() { + let src = "Bad() :- Source(), x = ."; + let parsed = parse_err(src); + #[expect(clippy::expect_used, reason = "tests require a single rule")] + let rule = parsed + .root() + .rules() + .first() + .cloned() + .expect("rule missing"); + let errors = match rule.body_terms() { + Ok(terms) => panic!("expected body terms error, got {terms:?}"), + Err(errs) => errs, + }; + assert!( + errors + .iter() + .any(|e| format!("{e:?}").contains("expected expression after '='")), + "expected 'expected expression after =' diagnostic, got {errors:?}", + ); +} diff --git a/src/test_util/expressions.rs b/src/test_util/expressions.rs index 2a60ae62..eec48a6f 100644 --- a/src/test_util/expressions.rs +++ b/src/test_util/expressions.rs @@ -105,6 +105,23 @@ pub fn bit_slice(expr: Expr, hi: Expr, lo: Expr) -> Expr { } } +/// Construct an [`Expr::AtomDiff`] postfix node. +#[must_use] +pub fn atom_diff(expr: Expr) -> Expr { + Expr::AtomDiff { + expr: Box::new(expr), + } +} + +/// Construct an [`Expr::AtomDelay`] postfix node with the given `delay` value. +#[must_use] +pub fn atom_delay(delay: u32, expr: Expr) -> Expr { + Expr::AtomDelay { + delay, + expr: Box::new(expr), + } +} + /// Construct a struct literal [`Expr::Struct`]. #[must_use] pub fn struct_expr(name: impl Into, fields: Vec<(String, Expr)>) -> Expr { diff --git a/src/test_util/mod.rs b/src/test_util/mod.rs index c284f21e..ab71eaf3 100644 --- a/src/test_util/mod.rs +++ b/src/test_util/mod.rs @@ -19,9 +19,9 @@ pub const MISSING_OUTPUT_SIGNATURE_ERROR: &str = pub const CAPITALIZED_TRANSFORMER_NAME_ERROR: &str = crate::parser::error_messages::CAPITALIZED_TRANSFORMER_NAME_ERROR; pub use expressions::{ - bit_slice, break_expr, call, call_expr, closure, continue_expr, field, field_access, for_loop, - if_expr, map_entry, map_lit, match_arm, match_expr, method_call, pat, qualified_call, - return_expr, struct_expr, tuple, tuple_index, var, vec_lit, + atom_delay, atom_diff, bit_slice, break_expr, call, call_expr, closure, continue_expr, field, + field_access, for_loop, if_expr, map_entry, map_lit, match_arm, match_expr, method_call, pat, + qualified_call, return_expr, struct_expr, tuple, tuple_index, var, vec_lit, }; pub use literals::{ NumericText, StringBody, lit_bool, lit_interned_raw_interpolated_str, lit_interned_raw_str, diff --git a/tests/expression_postfix.rs b/tests/expression_postfix.rs index 1bb5f88a..4576fb8d 100644 --- a/tests/expression_postfix.rs +++ b/tests/expression_postfix.rs @@ -31,7 +31,13 @@ fn parses_postfix_expressions(#[case] src: &str, #[case] expected: Expr) { #[case::trailing_dot("foo.", "expected identifier or tuple index after '.'", 4, 4, false)] #[case::bit_slice_missing_comma("e[1]", "expected comma", 3, 4, false)] #[case::bit_slice_unclosed("e[1,0", "expected right bracket", 5, 5, true)] -#[case::diff_marker_without_args("S'", "diff marker must precede atom arguments", 1, 2, false)] +#[case::diff_marker_without_args( + "S'", + "diff marker must be followed by atom arguments", + 1, + 2, + false +)] #[case::delay_out_of_range("A() -<4294967296>", "delay must fit u32", 6, 16, false)] fn postfix_expression_errors( #[case] src: &str, diff --git a/tests/expression_var_and_call.rs b/tests/expression_var_and_call.rs index 26afc303..fa9330ec 100644 --- a/tests/expression_var_and_call.rs +++ b/tests/expression_var_and_call.rs @@ -38,8 +38,8 @@ enum Kind { #[case( "foo(x,)", "unexpected trailing comma in argument list", + 5, 6, - 7, Kind::Other )] #[case("foo(1 2)", "expected right paren", 6, 7, Kind::Other)]