From 9be3c436c0a6e4ef09089d62341d7749797c7d4d Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Fri, 9 Oct 2026 00:31:50 +0000 Subject: [PATCH] fix(parser): expand aliases per line and close final Oils spec gaps Aliases expand at parse time and the rest of a script is re-read from the next line when alias/extglob options change, as bash reads line by line. Also: !( only under extglob, extglob no-match and [C\-D] globs, globskipdots, failglob abandons its line, for { } bodies and newline before in, (( )) redirects, a[ fragments, array subscript side effects, a[i]=x cmd, quoted splats in declare literals, tilde in ${u-~:~} and =~, errexit under !, per-call function redirects, ambiguous > "$@", set -v echo, top-level LINENO for (( ))/[[ ]], and compgen -A builtin lists only bash builtins. Oils spec 95.8% to 97.3% (2578 to 2618 of 2692). --- crates/bashkit/src/builtins/compgen.rs | 10 +- crates/bashkit/src/interpreter/completion.rs | 18 +- crates/bashkit/src/interpreter/declare.rs | 29 +- crates/bashkit/src/interpreter/expansion.rs | 114 +- crates/bashkit/src/interpreter/glob.rs | 12 + crates/bashkit/src/interpreter/history.rs | 7 + crates/bashkit/src/interpreter/mod.rs | 773 +++--- crates/bashkit/src/interpreter/redirection.rs | 11 +- crates/bashkit/src/lib.rs | 12 +- crates/bashkit/src/parser/ast.rs | 13 + crates/bashkit/src/parser/budget.rs | 2 + crates/bashkit/src/parser/lexer.rs | 237 +- crates/bashkit/src/parser/mod.rs | 260 +- .../tests/integration/compgen_tests.rs | 28 +- .../spec_cases/bash/oils-final-gaps.test.sh | 207 ++ knowledge/foundations/parser.md | 27 + knowledge/log.md | 4 + knowledge/operations/limitations.md | 24 +- knowledge/operations/oils-spec.md | 15 +- ...ls-spec-linux-x86_64-20261009T010244Z.json | 2330 +++++++++++++++++ ...oils-spec-linux-x86_64-20261009T010244Z.md | 167 ++ 21 files changed, 3760 insertions(+), 540 deletions(-) create mode 100644 crates/bashkit/tests/spec_cases/bash/oils-final-gaps.test.sh create mode 100644 scripts/oils-spec/results/oils-spec-linux-x86_64-20261009T010244Z.json create mode 100644 scripts/oils-spec/results/oils-spec-linux-x86_64-20261009T010244Z.md diff --git a/crates/bashkit/src/builtins/compgen.rs b/crates/bashkit/src/builtins/compgen.rs index d6e3b28a9..0aa4db5ed 100644 --- a/crates/bashkit/src/builtins/compgen.rs +++ b/crates/bashkit/src/builtins/compgen.rs @@ -140,13 +140,21 @@ impl Builtin for Compgen { // -b / -A builtin (and -c): names from the live registry — the same // source as `Bash::builtin_names()` — never a hardcoded list, which // drifted (109 names vs 156 registered) before this was wired up. + // -b / -A builtin keep the ones bash itself has built in. // `ctx.shell` is None only for custom/external builtin contexts, // which have no interpreter introspection by design. if (gen_builtins || gen_commands) && let Some(ref shell) = ctx.shell { for name in shell.builtin_names() { - if name.starts_with(pfx) { + // `-A builtin` is bash's builtins only; the other registered + // commands stand in for programs, which only `-c` lists. + // A host-registered command is the embedder's builtin. + let bash_builtin = super::BASH_BUILTIN_NAMES.contains(&name.as_str()) + || shell + .host_builtins + .is_some_and(|r| r.lookup(&name).is_some()); + if name.starts_with(pfx) && (gen_commands || bash_builtin) { completions.push(name); } } diff --git a/crates/bashkit/src/interpreter/completion.rs b/crates/bashkit/src/interpreter/completion.rs index 81225ae5a..fb2dd8c01 100644 --- a/crates/bashkit/src/interpreter/completion.rs +++ b/crates/bashkit/src/interpreter/completion.rs @@ -966,9 +966,21 @@ impl Interpreter { .iter() .map(|s| s.to_string()) .collect(), - // Live registry, not bash's list: bashkit's `awk`/`grep` are - // builtins here (compgen_tests.rs pins this contract). - "builtin" | "enabled" => self.builtin_names(), + // The live registry minus the commands that stand in for + // programs (`awk`, `grep`): bash's builtins, plus builtins the + // embedder registered. + "builtin" | "enabled" => self + .builtin_names() + .into_iter() + .filter(|n| { + crate::builtins::BASH_BUILTIN_NAMES.contains(&n.as_str()) + || self.custom_builtin_names.contains(n) + || self + .host_builtins + .as_ref() + .is_some_and(|r| r.lookup(n).is_some()) + }) + .collect(), "helptopic" => crate::builtins::BASH_BUILTIN_NAMES .iter() .map(|s| s.to_string()) diff --git a/crates/bashkit/src/interpreter/declare.rs b/crates/bashkit/src/interpreter/declare.rs index 75879d90b..30f140a4d 100644 --- a/crates/bashkit/src/interpreter/declare.rs +++ b/crates/bashkit/src/interpreter/declare.rs @@ -1565,7 +1565,7 @@ impl Interpreter { for field in self.ifs_split_limited(&expanded, limit)? { match self.expand_glob_item(&field, false).await { Ok(items) => fields.extend(items), - Err(_) => fields.push(field), + Err(pat) => return Err(self.failglob_abort(&pat)), } if fields.len() >= limit { break; @@ -1574,8 +1574,12 @@ impl Interpreter { fields.truncate(limit); return Ok(fields); } - if is_splat { - return self.expand_word_to_fields(word).await; + // `"${a[@]:1}"`, `"x${a[@]}y"`: a quoted word with an `@` splat + // keeps one field per element. + if is_splat || word.quoted && !word.has_unquoted_glob && word_has_at_splat(word) { + let mut fields = self.expand_word_to_fields(word).await?; + fields.truncate(limit); + return Ok(fields); } let value = self.expand_word(word).await?; let literal_only = word.parts.iter().all(|p| matches!(p, WordPart::Literal(_))); @@ -1592,11 +1596,18 @@ impl Interpreter { .await { Ok(items) => items, - Err(_) => vec![value], + Err(pat) => return Err(self.failglob_abort(&pat)), }; fields.truncate(limit); Ok(fields) } + + /// `shopt -s failglob` with no match in an array literal: bash reports + /// `no match` and abandons the rest of the line, status 1. + fn failglob_abort(&mut self, pattern: &str) -> crate::error::Error { + self.last_exit_code = 1; + crate::error::Error::LineAbort(self.diag(format!("no match: {pattern}\n"))) + } } /// `word` with its unquoted literal `~`s taken literally. @@ -1621,6 +1632,16 @@ fn no_tilde_word(word: &Word) -> std::borrow::Cow<'_, Word> { std::borrow::Cow::Owned(out) } +/// A part that expands to one field per element inside double quotes. +fn word_has_at_splat(word: &Word) -> bool { + word.parts.iter().any(|p| match p { + WordPart::ArrayAccess { index, .. } => index == "@", + WordPart::Variable(n) => n == "@", + WordPart::Substring { name, .. } => name == "@" || name.ends_with("[@]"), + _ => false, + }) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/bashkit/src/interpreter/expansion.rs b/crates/bashkit/src/interpreter/expansion.rs index 68dd6dd36..0b7146176 100644 --- a/crates/bashkit/src/interpreter/expansion.rs +++ b/crates/bashkit/src/interpreter/expansion.rs @@ -260,7 +260,20 @@ impl Interpreter { fn needs_glob_escape(ch: char) -> bool { matches!( ch, - '\\' | '*' | '?' | '[' | ']' | '{' | '}' | ',' | '@' | '!' | '+' | '(' | ')' | '|' + '\\' | '*' + | '?' + | '[' + | ']' + | '{' + | '}' + | ',' + | '@' + | '!' + | '+' + | '(' + | ')' + | '|' + | '-' ) } @@ -519,6 +532,14 @@ impl Interpreter { // values read into arithmetic). let value = if index.contains("$(") && !self.is_assoc_array(name) { self.array_access_after_subst(name, index).await? + } else if arith_has_side_effect(index) + && !self.is_assoc_array(name) + && !self.scoped.namerefs.contains_key(name) + { + // `${a[b=2]}`: the subscript is evaluated once, with + // its assignments (bash). + let idx = self.evaluate_arithmetic_with_assign(index); + self.expand_array_access_part(name, &idx.to_string()) } else { self.expand_array_access_part(name, index) }; @@ -868,13 +889,57 @@ impl Interpreter { /// its tildes resolved: a leading `~` and the `~` after each `:` of an /// unquoted literal. Literals holding a `~` become quoted, so later /// expansion leaves them alone. + /// A `${v-word}` operand with each unquoted `~` after a `:` replaced + /// by its directory. + fn tilde_operand_after_colons(&self, operand: &str) -> Option { + let b = operand.as_bytes(); + let mut out = String::new(); + let mut copied = 0; + let mut quote: Option = None; + let mut changed = false; + let mut i = 0; + while i < b.len() { + match (quote, b[i]) { + (None, b'\\') => i += 1, + (None, q @ (b'\'' | b'"')) => quote = Some(q), + (Some(q), c) if c == q => quote = None, + (None, b':') if b.get(i + 1) == Some(&b'~') => { + let start = i + 2; + let end = operand[start..] + .find(['/', ':', '}', '$', '\'', '"']) + .map_or(operand.len(), |k| start + k); + // The operand is re-read as a word: a directory that + // would read differently there keeps its `~`. + if matches!(b.get(end), None | Some(b'/' | b':')) + && let Some(dir) = self.tilde_dir(&operand[start..end]) + && !dir.contains(['$', '`', '\\', '\'', '"', '*', '?', '[', '}']) + { + out.push_str(&operand[copied..=i]); + out.push_str(&dir); + copied = end; + changed = true; + i = end; + continue; + } + } + _ => {} + } + i += 1; + } + changed.then(|| { + out.push_str(&operand[copied..]); + out + }) + } + pub(super) fn tilde_assignment_value<'w>(&self, word: &'w Word) -> std::borrow::Cow<'w, Word> { use std::borrow::Cow; let quoted_at = |i: usize| Self::part_is_quoted(word, i); - let has_tilde = - word.parts.iter().enumerate().any( - |(i, p)| matches!(p, WordPart::Literal(s) if !quoted_at(i) && s.contains('~')), - ); + let has_tilde = word.parts.iter().enumerate().any(|(i, p)| match p { + WordPart::Literal(s) => !quoted_at(i) && s.contains('~'), + WordPart::ParameterExpansion { operand, .. } => !quoted_at(i) && operand.contains(":~"), + _ => false, + }); if !has_tilde { return Cow::Borrowed(word); } @@ -882,6 +947,28 @@ impl Interpreter { out.part_quoted = (0..word.parts.len()).map(quoted_at).collect(); let n = word.parts.len(); for (i, part) in word.parts.iter().enumerate() { + if let WordPart::ParameterExpansion { + name, + operator: + op @ (ParameterOp::UseDefault + | ParameterOp::AssignDefault + | ParameterOp::UseReplacement), + operand, + colon_variant, + } = part + && !quoted_at(i) + && let Some(expanded) = self.tilde_operand_after_colons(operand) + { + // `x=${undef-~:~}`: the operand of an assignment value + // expands `~` after each `:` too. + out.parts[i] = WordPart::ParameterExpansion { + name: name.clone(), + operator: op.clone(), + operand: expanded, + colon_variant: *colon_variant, + }; + continue; + } let WordPart::Literal(s) = part else { continue; }; @@ -3516,6 +3603,23 @@ impl Interpreter { } } +/// Whether arithmetic text assigns: `b=2`, `i+=1`, `i++`, `--i`. +fn arith_has_side_effect(expr: &str) -> bool { + if expr.contains("++") || expr.contains("--") { + return true; + } + let b = expr.as_bytes(); + b.iter().enumerate().any(|(i, &c)| { + c == b'=' + && b.get(i + 1) != Some(&b'=') + && !matches!( + i.checked_sub(1).map(|j| b[j]), + Some(b'=' | b'!' | b'<' | b'>') + ) + || c == b'=' && i >= 2 && matches!(&b[i - 2..i], b"<<" | b">>") + }) +} + #[cfg(test)] mod expansion_charge_tests { use super::*; diff --git a/crates/bashkit/src/interpreter/glob.rs b/crates/bashkit/src/interpreter/glob.rs index fb26c63a2..49ece9e01 100644 --- a/crates/bashkit/src/interpreter/glob.rs +++ b/crates/bashkit/src/interpreter/glob.rs @@ -929,6 +929,7 @@ impl Interpreter { | '(' | ')' | '|' + | '-' ) { result.push(next); @@ -1040,6 +1041,10 @@ impl Interpreter { // Dotfiles are hidden per component unless dotglob is set or this // component explicitly starts with '.'. let component_starts_with_dot = component.starts_with('.'); + // `shopt -u globskipdots`: a pattern that starts with `.` also + // matches `.` and `..`, which the VFS `read_dir` never lists. + let dot_entries = component_starts_with_dot + && !crate::builtins::shopt_on(&self.scoped.variables, "globskipdots"); for (dir, out) in &candidates { let entries = match self.fs.read_dir(dir).await { @@ -1048,6 +1053,13 @@ impl Interpreter { }; let mut matched: Vec = Vec::new(); + if dot_entries { + for dot in [".", ".."] { + if self.glob_match_impl(dot, component, opts, 0) { + matched.push(dot.to_string()); + } + } + } for entry in entries { if entry.name.starts_with('.') && !dotglob && !component_starts_with_dot { continue; diff --git a/crates/bashkit/src/interpreter/history.rs b/crates/bashkit/src/interpreter/history.rs index b3137b7a9..0cf2583ed 100644 --- a/crates/bashkit/src/interpreter/history.rs +++ b/crates/bashkit/src/interpreter/history.rs @@ -73,6 +73,13 @@ impl LineReader { self.script_addr == script as *const Script as usize } + /// The top level re-read its rest into `to` (see `reread_script_rest`). + pub(super) fn retarget(&mut self, from: &Script, to: &Script) { + if self.reads(from) { + self.script_addr = to as *const Script as usize; + } + } + /// Source lines `first..=last` (1-based), without the final newline. fn lines(&mut self, first: usize, last: usize) -> String { if self.line_starts.is_empty() { diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index e8df0d6a0..fa8ebff93 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -931,6 +931,21 @@ fn is_dev_null(path: &Path) -> bool { /// Decision: these paths are resolved at the interpreter level like /// `/dev/null`, never through the VFS. Writing them as regular files silently /// swallowed `echo err > /dev/stderr`, a pattern agents use constantly. +/// The script a body runs (see `Interpreter::reread_script_rest`). +enum ScriptView<'a> { + Parsed(&'a Script), + Reread(Box