Claude/advance project y32 it

impl/rust-cli/src/executable.rs

@@ -714,13 +714,29 @@ impl ExecutableCommand for Command {
                 let content = std::fs::read_to_string(&path)
                     .map_err(|e| anyhow::anyhow!("source: {}: {}", path.display(), e))?;
 
+                // Strip whole-line comments first, then feed the entire
+                // content to the statement splitter so multi-line `if/fi`,
+                // `for/done`, etc. stay together.
+                let stripped: String = content
+                    .lines()
+                    .map(|line| {
+                        let trimmed = line.trim_start();
+                        if trimmed.starts_with('#') {
+                            ""
+                        } else {
+                            line
+                        }
+                    })
+                    .collect::<Vec<_>>()
+                    .join("\n");
+
                 let mut last_result = ExecutionResult::Success;
-                for line in content.lines() {
-                    let line = line.trim();
-                    if line.is_empty() || line.starts_with('#') {
+                for segment in crate::parser::split_on_statement_separators(&stripped) {
+                    let segment = segment.trim();
+                    if segment.is_empty() || segment.starts_with('#') {
                         continue;
                     }
-                    let cmd = crate::parser::parse_command(line)?;
+                    let cmd = crate::parser::parse_command(segment)?;
                     last_result = cmd.execute(state)?;
                     // Propagate Exit (shell-wide) and Return (function-scope).
                     // A `return` inside a sourced file that was itself sourced

impl/rust-cli/src/functions.rs

@@ -338,7 +338,7 @@ mod tests {
     fn test_parse_posix_function_def() {
         let result = parse_function_def("greet() { echo hello; }");
         assert!(result.is_some());
-        let (name, body, raw_body) = result.unwrap();
+        let (name, body, raw_body) = result.expect("TODO: handle error");
         assert_eq!(name, "greet");
         assert_eq!(body, vec!["echo hello"]);
         // raw_body preserves the trailing `;` — that's harmless.
@@ -349,7 +349,7 @@ mod tests {
     fn test_parse_posix_function_multi_commands() {
         let result = parse_function_def("setup() { mkdir src; touch src/main.rs; echo done; }");
         assert!(result.is_some());
-        let (name, body, raw_body) = result.unwrap();
+        let (name, body, raw_body) = result.expect("TODO: handle error");
         assert_eq!(name, "setup");
         assert_eq!(body, vec!["mkdir src", "touch src/main.rs", "echo done"]);
         assert_eq!(raw_body, "mkdir src; touch src/main.rs; echo done;");
@@ -359,7 +359,7 @@ mod tests {
     fn test_parse_bash_function_def() {
         let result = parse_function_def("function greet { echo hello; }");
         assert!(result.is_some());
-        let (name, body, raw_body) = result.unwrap();
+        let (name, body, raw_body) = result.expect("TODO: handle error");
         assert_eq!(name, "greet");
         assert_eq!(body, vec!["echo hello"]);
         assert_eq!(raw_body, "echo hello;");
@@ -369,7 +369,7 @@ mod tests {
     fn test_parse_bash_function_with_parens() {
         let result = parse_function_def("function greet() { echo hello; }");
         assert!(result.is_some());
-        let (name, body, raw_body) = result.unwrap();
+        let (name, body, raw_body) = result.expect("TODO: handle error");
         assert_eq!(name, "greet");
         assert_eq!(body, vec!["echo hello"]);
         assert_eq!(raw_body, "echo hello;");
@@ -381,7 +381,7 @@ mod tests {
         // execution can parse `if/fi`, `for/done`, etc. as single commands.
         let result = parse_function_def("ifunc() { if true; then mkdir d; fi; }");
         assert!(result.is_some());
-        let (name, _body, raw_body) = result.unwrap();
+        let (name, _body, raw_body) = result.expect("TODO: handle error");
         assert_eq!(name, "ifunc");
         assert_eq!(raw_body, "if true; then mkdir d; fi;");
     }
@@ -391,7 +391,7 @@ mod tests {
         // A `}` inside a quoted string must NOT be treated as the closing brace.
         let result = parse_function_def("lit() { echo '}'; }");
         assert!(result.is_some());
-        let (_name, _body, raw_body) = result.unwrap();
+        let (_name, _body, raw_body) = result.expect("TODO: handle error");
         assert_eq!(raw_body, "echo '}';");
     }
 

impl/rust-cli/src/main.rs

@@ -240,39 +240,51 @@ fn main() -> Result<()> {
 }
 
 /// Execute script content (string of commands, one per line or semicolon-separated)
+///
+/// We split the *entire* content on top-level statement separators (both `;`
+/// and `\n`, respecting quotes and control-structure depth), so multi-line
+/// `if/fi`, `for/done`, `while/done`, and `case/esac` work exactly as they
+/// do in a POSIX shell.
 fn execute_script_content(content: &str, state: &mut state::ShellState) -> Result<()> {
-    for line in content.lines() {
-        let line = line.trim();
-        if line.is_empty() || line.starts_with('#') {
+    // Strip full-line comments before splitting. (A `#` inside a statement
+    // may be part of a quoted string or a parameter expansion, so we only
+    // trim whole-line comments here.)
+    let stripped: String = content
+        .lines()
+        .map(|line| {
+            let trimmed = line.trim_start();
+            if trimmed.starts_with('#') {
+                ""
+            } else {
+                line
+            }
+        })
+        .collect::<Vec<_>>()
+        .join("\n");
+
+    for segment in vsh::parser::split_on_statement_separators(&stripped) {
+        let segment = segment.trim();
+        if segment.is_empty() || segment.starts_with('#') {
             continue;
         }
 
-        // Split on semicolons
-        let segments = vsh::parser::split_on_semicolons(line);
-        for segment in segments {
-            let segment = segment.trim();
-            if segment.is_empty() || segment.starts_with('#') {
-                continue;
-            }
-
-            match vsh::parser::parse_command(segment) {
-                Ok(cmd) => {
-                    let result = cmd.execute(state)?;
-                    match result {
-                        ExecutionResult::Exit => return Ok(()),
-                        ExecutionResult::ExternalCommand { exit_code }
-                        | ExecutionResult::Return { exit_code } => {
-                            state.last_exit_code = exit_code;
-                        }
-                        ExecutionResult::Success => {
-                            state.last_exit_code = 0;
-                        }
+        match vsh::parser::parse_command(segment) {
+            Ok(cmd) => {
+                let result = cmd.execute(state)?;
+                match result {
+                    ExecutionResult::Exit => return Ok(()),
+                    ExecutionResult::ExternalCommand { exit_code }
+                    | ExecutionResult::Return { exit_code } => {
+                        state.last_exit_code = exit_code;
+                    }
+                    ExecutionResult::Success => {
+                        state.last_exit_code = 0;
                     }
                 }
-                Err(e) => {
-                    eprintln!("vsh: {}", e);
-                    state.last_exit_code = 1;
-                }
+            }
+            Err(e) => {
+                eprintln!("vsh: {}", e);
+                state.last_exit_code = 1;
             }
         }
     }

impl/rust-cli/src/parser.rs

@@ -1425,13 +1425,34 @@ fn is_block_close_keyword(word: &str) -> bool {
 }
 
 pub fn split_on_semicolons(input: &str) -> Vec<&str> {
+    split_on_top_level(input, false)
+}
+
+/// Like `split_on_semicolons`, but also treats top-level newlines as statement
+/// separators (outside quotes and outside control-structure blocks).
+///
+/// Use this when feeding a multi-line script fragment into the parser: it
+/// lets `if/then/fi`, `while/do/done`, `for/do/done`, and `case/esac` span
+/// multiple lines — exactly like a POSIX shell — while still producing one
+/// segment per top-level statement.
+pub fn split_on_statement_separators(input: &str) -> Vec<&str> {
+    split_on_top_level(input, true)
+}
+
+fn split_on_top_level(input: &str, split_on_newline: bool) -> Vec<&str> {
     let mut segments = Vec::new();
     let mut start = 0;
     let mut in_single_quote = false;
     let mut in_double_quote = false;
     let mut escaped = false;
     let mut paren_depth: i32 = 0;
     let mut block_depth: i32 = 0;
+    // Brace depth is tracked only in statement-separator mode (i.e. when
+    // we're splitting a whole script). It prevents a `;` inside a function
+    // body or brace group — `foo() { cmd; }` — from being treated as a
+    // statement boundary. Normal `${VAR}` expansions balance themselves
+    // and cancel back out to zero.
+    let mut brace_depth: i32 = 0;
 
     // Pre-scan to detect control structure keywords and track nesting
     // We need to identify word boundaries for keyword detection
@@ -1470,8 +1491,29 @@ pub fn split_on_semicolons(input: &str) -> Vec<&str> {
                 }
                 i += 1;
             }
+            '{' if split_on_newline && !in_single_quote && !in_double_quote => {
+                brace_depth += 1;
+                i += 1;
+            }
+            '}' if split_on_newline && !in_single_quote && !in_double_quote => {
+                if brace_depth > 0 {
+                    brace_depth -= 1;
+                }
+                i += 1;
+            }
             ';' if !in_single_quote && !in_double_quote && paren_depth == 0 => {
-                if block_depth == 0 {
+                if block_depth == 0 && brace_depth == 0 {
+                    segments.push(&input[start..i]);
+                    start = i + 1;
+                }
+                i += 1;
+            }
+            '\n' if split_on_newline
+                && !in_single_quote
+                && !in_double_quote
+                && paren_depth == 0 =>
+            {
+                if block_depth == 0 && brace_depth == 0 {
                     segments.push(&input[start..i]);
                     start = i + 1;
                 }
@@ -2611,7 +2653,9 @@ pub fn expand_quoted_word_with_state(word: &QuotedWord, state: &mut crate::state
 /// Parse a block of commands from a string (semicolon or newline separated)
 fn parse_command_block(block: &str) -> Result<Vec<Command>> {
     let mut commands = Vec::new();
-    for segment in split_on_semicolons(block) {
+    // Control-structure bodies may span multiple lines, so treat `;` AND
+    // top-level `\n` as statement separators.
+    for segment in split_on_statement_separators(block) {
         let segment = segment.trim();
         if segment.is_empty() || segment.starts_with('#') {
             continue;

impl/rust-cli/src/state.rs

@@ -889,9 +889,21 @@ impl ShellState {
         self.positional_params.len()
     }
 
-    /// Get next unique FIFO ID for process substitution
+    /// Get next unique FIFO ID for process substitution.
+    ///
+    /// Uses a process-wide atomic so that FIFO paths are unique across all
+    /// `ShellState` instances in the same process (e.g. parallel test threads
+    /// that share a PID). A per-state counter would collide under
+    /// `cargo test`, since each test creates a fresh state whose counter
+    /// starts at 0 while they all share the same PID in the FIFO path
+    /// template `/tmp/vsh-fifo-<pid>-<id>`.
     pub fn next_fifo_id(&self) -> usize {
-        self.fifo_counter.fetch_add(1, std::sync::atomic::Ordering::SeqCst)
+        use std::sync::atomic::{AtomicUsize, Ordering};
+        static GLOBAL_FIFO_COUNTER: AtomicUsize = AtomicUsize::new(0);
+        // Keep the per-state counter too so its semantics (monotonic per
+        // state) are preserved for any caller that reads it directly.
+        let _ = self.fifo_counter.fetch_add(1, Ordering::SeqCst);
+        GLOBAL_FIFO_COUNTER.fetch_add(1, Ordering::SeqCst)
     }
 
     /// Get special variable value ($$, $?, $HOME, etc.)