From 26abc5efa96e903fbd4af6ce22441d8bd56d7f2c Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 16 Jul 2026 12:57:29 -0400 Subject: [PATCH 1/2] =?UTF-8?q?fix(lexer):=20bare=20${X:-${Y}}=20=E2=80=94?= =?UTF-8?q?=20extend=20VarRef=20to=20the=20balanced=20closing=20brace=20(G?= =?UTF-8?q?H=20#173)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found during the #95 census: the VarRef regex \$\{[^}]+\} stops at the first }, so a nested braced reference in a bare default word lexed as VarRef("${X:-${Y}") + RBrace and died as a confusing parse error, while the quoted form worked (the in-string interpolation parser hand-rolls brace matching) and ${X:-$Y} worked (no inner braces). logos regexes can't count nesting, but a callback can extend a match: the pattern now matches only the `${` opener and lex_varref walks the remainder to the balanced `}` via lex.bump(). The parser side needed nothing — find_default_separator was already ${}-nesting-aware and default words already route through parse_interpolated_string, which is why the quoted form always worked. ${#X} still lexes as VarLength (its full regex out-matches the two-character opener in logos rule selection). Depth counting stays quote-blind, matching both the old regex and the scanner's ${...} region tracking; `${}` stays an error (the old regex required one inner character); `${X:-${Y}` unbalanced is now a proper UnterminatedVarRef instead of a generic unexpected-character. Tests: token-shape + error inline tests, kernel-routed evaluation tests (single/double nesting, set-wins-over-default), and a nested example in LANGUAGE.md's `:-` section. Full gates green (4,609 tests, clippy clean, insta clean, no-default-features). Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 8 +++ crates/kaish-kernel/src/lexer.rs | 67 +++++++++++++++++-- .../tests/lexer_pipeline_tests.rs | 32 +++++++++ docs/LANGUAGE.md | 1 + 4 files changed, 103 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce12ea6..0aec658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,14 @@ breaking entries are marked **BREAKING**. ## [Unreleased] +### Fixed +- **Bare `${X:-${Y}}` works** (GH #173) — a nested braced reference in a + default word outside quotes was a parse error (the `VarRef` token stopped at + the first `}`); the reference now extends to the balanced closing brace, + matching the quoted form and bash. Defaults nest to any depth + (`${A:-${B:-${C}}}`); an unbalanced reference is a loud + `unterminated variable reference` error. + ### Added - **GitHub Actions CI** (`.github/workflows/ci.yml`): every PR and push to `main` runs the workspace test suite, clippy with warnings denied (test targets diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index 679219e..3088b4f 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -509,8 +509,14 @@ pub enum Token { #[regex(r"'[^']*'", lex_single_string)] SingleString(String), - /// Braced variable reference: `${VAR}` or `${VAR.field}` - value is the raw inner content - #[regex(r"\$\{[^}]+\}", lex_varref)] + /// Braced variable reference: `${VAR}`, `${VAR.field}`, or a default + /// form with a NESTED reference like `${X:-${Y}}` — value is the raw + /// `${...}` text. The regex matches only the `${` opener; the callback + /// extends the token to the BALANCED closing brace (GH #173 — a plain + /// `[^}]+` regex stopped at the first `}` and split nested references). + /// `${#VAR}` still lexes as `VarLength`: its full regex out-matches this + /// two-character opener, so logos selects it first. + #[regex(r"\$\{", lex_varref)] VarRef(String), /// Simple variable reference: `$NAME` - just the identifier @@ -816,9 +822,37 @@ fn lex_single_string(lex: &mut logos::Lexer) -> String { } /// Lex a braced variable reference, extracting the inner content. -fn lex_varref(lex: &mut logos::Lexer) -> String { - // Keep the full ${...} for later parsing of path segments - lex.slice().to_string() +/// Extend a `${` match across the remainder to the balanced closing `}` +/// and return the full `${...}` text for later parsing of path segments +/// and default words. Brace depth counts raw `{`/`}` characters, matching +/// the scanner's `${...}` region tracking (quote-blind, like the old +/// first-`}` regex — a quoted `}` inside a default word still closes; see +/// GH #173). An empty `${}` stays an error (as it was when the regex +/// required at least one inner character); a reference that never closes +/// is a loud `UnterminatedVarRef`. +fn lex_varref(lex: &mut logos::Lexer) -> Result { + let mut depth = 1usize; + let mut extra = 0usize; + for c in lex.remainder().chars() { + extra += c.len_utf8(); + match c { + '{' => depth += 1, + '}' => { + depth -= 1; + if depth == 0 { + if extra == 1 { + // `${}` — empty reference, same error class the + // old non-matching regex produced. + return Err(LexerError::UnexpectedCharacter); + } + lex.bump(extra); + return Ok(lex.slice().to_string()); + } + } + _ => {} + } + } + Err(LexerError::UnterminatedVarRef) } /// Lex a simple variable reference: `$NAME` → `NAME` @@ -3078,6 +3112,29 @@ mod tests { assert_eq!(lex("${VAR[0]}"), vec![Token::VarRef("${VAR[0]}".to_string())]); } + #[test] + fn var_ref_nested_default_is_one_token() { + // GH #173: the balanced-brace callback keeps a nested reference in + // a default word as ONE VarRef token (the old first-`}` regex split + // it into VarRef + RBrace). + assert_eq!( + lex("${X:-${Y}}"), + vec![Token::VarRef("${X:-${Y}}".to_string())] + ); + assert_eq!( + lex("${A:-${B:-${C}}}"), + vec![Token::VarRef("${A:-${B:-${C}}}".to_string())] + ); + // VarLength still out-matches the two-character `${` opener. + assert_eq!(lex("${#X}"), vec![Token::VarLength("X".to_string())]); + } + + #[test] + fn var_ref_unterminated_and_empty_are_errors() { + assert!(tokenize("${X:-${Y}").is_err(), "unbalanced nesting is loud"); + assert!(tokenize("${}").is_err(), "empty reference is loud"); + } + // ═══════════════════════════════════════════════════════════════════ // Identifier tests // ═══════════════════════════════════════════════════════════════════ diff --git a/crates/kaish-kernel/tests/lexer_pipeline_tests.rs b/crates/kaish-kernel/tests/lexer_pipeline_tests.rs index 05eb969..fb554f5 100644 --- a/crates/kaish-kernel/tests/lexer_pipeline_tests.rs +++ b/crates/kaish-kernel/tests/lexer_pipeline_tests.rs @@ -10,6 +10,7 @@ use kaish_kernel::lexer::{tokenize, LexerError, Token}; use kaish_kernel::parser::parse; +use kaish_kernel::{Kernel, KernelConfig}; fn toks(src: &str) -> Vec { tokenize(src) @@ -411,6 +412,37 @@ fn arg_count_does_not_open_a_comment() { ); } +// ═══════════════════════════════════════════════════════════════════ +// Nested braced references in bare default words (GH #173) +// ═══════════════════════════════════════════════════════════════════ + +#[tokio::test] +async fn bare_nested_varref_default_evaluates() { + // `${X:-${Y}}` outside quotes used to be a parse error (the VarRef + // regex stopped at the first `}`); the quoted form always worked. + // Both now evaluate, matching bash. + let kernel = Kernel::new(KernelConfig::isolated().with_skip_validation(true)) + .expect("kernel") + .into_arc(); + let result = kernel + .execute("Y=fallback; echo ${X:-${Y}}") + .await + .expect("bare nested default evaluates"); + assert_eq!(result.text_out().trim(), "fallback"); + + let result = kernel + .execute("Y=inner; echo ${A:-${B:-${Y}}}") + .await + .expect("doubly nested default evaluates"); + assert_eq!(result.text_out().trim(), "inner"); + + let result = kernel + .execute("X=set; Y=fallback; echo ${X:-${Y}}") + .await + .expect("set variable wins over default"); + assert_eq!(result.text_out().trim(), "set"); +} + // ═══════════════════════════════════════════════════════════════════ // Line continuation on a heredoc introducer line (bash semantics: the // continuation joins the introducer line; the body starts after the diff --git a/docs/LANGUAGE.md b/docs/LANGUAGE.md index 7951194..2cf3599 100644 --- a/docs/LANGUAGE.md +++ b/docs/LANGUAGE.md @@ -160,6 +160,7 @@ cfg=$(fromjson '{"port":9000}') echo ${cfg[port]:-8080} # 9000 (present) echo ${cfg[host]:-localhost} # localhost (missing key → default) echo ${cfg[flag]:-on} # if flag is false/0/[]/{} you get that value, not "on" +echo ${X:-${Y:-fallback}} # defaults nest — first set value wins ``` A **shape** error (integer index on a record, string key on a list, subscripting From ce200091ffd0f7777894bb1bc21dc0409879d3e2 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 16 Jul 2026 13:06:02 -0400 Subject: [PATCH 2/2] test(lexer): pin the early-close and extra-open-brace contracts (kaibo review) Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/lexer.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index 3088b4f..fe9969f 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -3132,9 +3132,24 @@ mod tests { #[test] fn var_ref_unterminated_and_empty_are_errors() { assert!(tokenize("${X:-${Y}").is_err(), "unbalanced nesting is loud"); + assert!(tokenize("${a{b}").is_err(), "extra open brace is loud"); assert!(tokenize("${}").is_err(), "empty reference is loud"); } + #[test] + fn var_ref_closes_at_first_balanced_brace() { + // Trailing `b}` after the balanced close is separate tokens — the + // early-close contract (kaibo review, GH #173). + assert_eq!( + lex("${a}b}"), + vec![ + Token::VarRef("${a}".to_string()), + Token::Ident("b".to_string()), + Token::RBrace, + ] + ); + } + // ═══════════════════════════════════════════════════════════════════ // Identifier tests // ═══════════════════════════════════════════════════════════════════