From 14c1cafc4fdf6cf4c1c13a4f72df95c1fdb799d6 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 26 Mar 2026 13:07:25 +0000 Subject: [PATCH] fix(interpreter): set -e respects AND-OR lists in functions and loops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #807 — `set -e` no longer incorrectly exits when `[[ cond ]] && cmd` has a false condition inside functions, brace groups, or for/while/until loops. Root cause: execute_command_sequence_impl and loop bodies had redundant errexit checks that didn't account for AND-OR list context. Fix: 1. Skip errexit in sequence executor for Command::List with && or || operators 2. Remove redundant errexit checks in for/while/until loop bodies --- crates/bashkit/src/interpreter/mod.rs | 44 +++------ crates/bashkit/tests/set_e_and_or_tests.rs | 107 +++++++++++++++++++++ 2 files changed, 119 insertions(+), 32 deletions(-) create mode 100644 crates/bashkit/tests/set_e_and_or_tests.rs diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 9df4808e..50e377d9 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -1532,16 +1532,7 @@ impl Interpreter { }); } ControlFlow::None => { - // Check if errexit caused early return from body - if self.is_errexit_enabled() && exit_code != 0 { - return Ok(ExecResult { - stdout, - stderr, - exit_code, - control_flow: ControlFlow::None, - ..Default::default() - }); - } + // errexit is already handled by execute_command_sequence_impl } } } @@ -1801,16 +1792,7 @@ impl Interpreter { }); } ControlFlow::None => { - // Check if errexit caused early return from body - if self.is_errexit_enabled() && exit_code != 0 { - return Ok(ExecResult { - stdout, - stderr, - exit_code, - control_flow: ControlFlow::None, - ..Default::default() - }); - } + // errexit is already handled by execute_command_sequence_impl } } @@ -2250,16 +2232,7 @@ impl Interpreter { }); } ControlFlow::None => { - // Check if errexit caused early return from body - if self.is_errexit_enabled() && exit_code != 0 { - return Ok(ExecResult { - stdout, - stderr, - exit_code, - control_flow: ControlFlow::None, - ..Default::default() - }); - } + // errexit is already handled by execute_command_sequence_impl } } } @@ -2788,8 +2761,15 @@ impl Interpreter { }); } - // Check for errexit (set -e) if enabled - if check_errexit && self.is_errexit_enabled() && exit_code != 0 { + // Check for errexit (set -e) if enabled. + // Skip errexit for commands that are AND-OR lists — per POSIX, set -e + // does not exit on failures that are part of && or || chains. + // The list executor already handles errexit internally. + let is_and_or_list = matches!( + command, + Command::List(list) if list.rest.iter().any(|(op, _)| matches!(op, ListOperator::And | ListOperator::Or)) + ); + if check_errexit && self.is_errexit_enabled() && exit_code != 0 && !is_and_or_list { return Ok(ExecResult { stdout, stderr, diff --git a/crates/bashkit/tests/set_e_and_or_tests.rs b/crates/bashkit/tests/set_e_and_or_tests.rs new file mode 100644 index 00000000..da105c19 --- /dev/null +++ b/crates/bashkit/tests/set_e_and_or_tests.rs @@ -0,0 +1,107 @@ +//! Tests for `set -e` (errexit) with AND-OR lists. +//! +//! Per POSIX, `set -e` should NOT cause an exit when a command fails +//! as part of an AND-OR chain (`cmd1 && cmd2`, `cmd1 || cmd2`). + +use bashkit::Bash; + +/// set -e: [[ false ]] && cmd in function should not exit +#[tokio::test] +async fn set_e_and_list_in_function() { + let mut bash = Bash::new(); + let result = bash + .exec( + r#" +set -e +f() { + [[ "a" == "b" ]] && return 0 + echo "should reach here" +} +f +echo "after f" +"#, + ) + .await + .unwrap(); + assert!(result.stdout.contains("should reach here")); + assert!(result.stdout.contains("after f")); +} + +/// set -e: [[ false ]] && cmd inside brace group with redirect +#[tokio::test] +async fn set_e_and_list_in_brace_group() { + let mut bash = Bash::new(); + let result = bash + .exec( + r#" +set -e +x="" +{ + echo "line1" + [[ -n "$x" ]] && echo "has value" + echo "line2" +} > /tmp/out.txt +cat /tmp/out.txt +"#, + ) + .await + .unwrap(); + assert!(result.stdout.contains("line1")); + assert!(result.stdout.contains("line2")); +} + +/// set -e: [[ false ]] && cmd inside for loop +#[tokio::test] +async fn set_e_and_list_in_for_loop() { + let mut bash = Bash::new(); + let result = bash + .exec( + r#" +set -e +f() { + for x in a b c; do + [[ "$x" == "b" ]] && return 0 + done +} +f +echo "after f" +"#, + ) + .await + .unwrap(); + assert!(result.stdout.contains("after f")); +} + +/// set -e: top level [[ false ]] && cmd should still work +#[tokio::test] +async fn set_e_and_list_top_level() { + let mut bash = Bash::new(); + let result = bash + .exec( + r#" +set -e +[[ "a" == "b" ]] && echo "match" +echo "reached" +"#, + ) + .await + .unwrap(); + assert!(result.stdout.contains("reached")); +} + +/// set -e should still exit on non-AND-OR failures +#[tokio::test] +async fn set_e_exits_on_plain_failure() { + let mut bash = Bash::new(); + let result = bash + .exec( + r#" +set -e +false +echo "SHOULD NOT APPEAR" +"#, + ) + .await + .unwrap(); + assert!(!result.stdout.contains("SHOULD NOT APPEAR")); +}