Skip to content

Commit 3bc78f7

Browse files
authored
fix(interpreter): set -e respects AND-OR lists in functions and loops (#824)
## Summary - Fix `set -e` incorrectly exiting on `[[ cond ]] && cmd` inside functions, brace groups, and loops - Skip errexit in `execute_command_sequence_impl` for `Command::List` with `&&`/`||` operators - Remove redundant errexit checks in for/while/until loop bodies ## Test plan - [x] `set_e_and_list_in_function` — `[[ false ]] && return 0` in function - [x] `set_e_and_list_in_brace_group` — `[[ false ]] && echo` in `{ } > file` - [x] `set_e_and_list_in_for_loop` — `[[ false ]] && return 0` in for loop - [x] `set_e_and_list_top_level` — top-level AND-OR still works - [x] `set_e_exits_on_plain_failure` — `false` still exits under set -e - [x] Full test suite passes Closes #807
1 parent 03888db commit 3bc78f7

File tree

2 files changed

+119
-32
lines changed

2 files changed

+119
-32
lines changed

crates/bashkit/src/interpreter/mod.rs

Lines changed: 12 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1532,16 +1532,7 @@ impl Interpreter {
15321532
});
15331533
}
15341534
ControlFlow::None => {
1535-
// Check if errexit caused early return from body
1536-
if self.is_errexit_enabled() && exit_code != 0 {
1537-
return Ok(ExecResult {
1538-
stdout,
1539-
stderr,
1540-
exit_code,
1541-
control_flow: ControlFlow::None,
1542-
..Default::default()
1543-
});
1544-
}
1535+
// errexit is already handled by execute_command_sequence_impl
15451536
}
15461537
}
15471538
}
@@ -1801,16 +1792,7 @@ impl Interpreter {
18011792
});
18021793
}
18031794
ControlFlow::None => {
1804-
// Check if errexit caused early return from body
1805-
if self.is_errexit_enabled() && exit_code != 0 {
1806-
return Ok(ExecResult {
1807-
stdout,
1808-
stderr,
1809-
exit_code,
1810-
control_flow: ControlFlow::None,
1811-
..Default::default()
1812-
});
1813-
}
1795+
// errexit is already handled by execute_command_sequence_impl
18141796
}
18151797
}
18161798

@@ -2250,16 +2232,7 @@ impl Interpreter {
22502232
});
22512233
}
22522234
ControlFlow::None => {
2253-
// Check if errexit caused early return from body
2254-
if self.is_errexit_enabled() && exit_code != 0 {
2255-
return Ok(ExecResult {
2256-
stdout,
2257-
stderr,
2258-
exit_code,
2259-
control_flow: ControlFlow::None,
2260-
..Default::default()
2261-
});
2262-
}
2235+
// errexit is already handled by execute_command_sequence_impl
22632236
}
22642237
}
22652238
}
@@ -2788,8 +2761,15 @@ impl Interpreter {
27882761
});
27892762
}
27902763

2791-
// Check for errexit (set -e) if enabled
2792-
if check_errexit && self.is_errexit_enabled() && exit_code != 0 {
2764+
// Check for errexit (set -e) if enabled.
2765+
// Skip errexit for commands that are AND-OR lists — per POSIX, set -e
2766+
// does not exit on failures that are part of && or || chains.
2767+
// The list executor already handles errexit internally.
2768+
let is_and_or_list = matches!(
2769+
command,
2770+
Command::List(list) if list.rest.iter().any(|(op, _)| matches!(op, ListOperator::And | ListOperator::Or))
2771+
);
2772+
if check_errexit && self.is_errexit_enabled() && exit_code != 0 && !is_and_or_list {
27932773
return Ok(ExecResult {
27942774
stdout,
27952775
stderr,
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
//! Tests for `set -e` (errexit) with AND-OR lists.
2+
//!
3+
//! Per POSIX, `set -e` should NOT cause an exit when a command fails
4+
//! as part of an AND-OR chain (`cmd1 && cmd2`, `cmd1 || cmd2`).
5+
6+
use bashkit::Bash;
7+
8+
/// set -e: [[ false ]] && cmd in function should not exit
9+
#[tokio::test]
10+
async fn set_e_and_list_in_function() {
11+
let mut bash = Bash::new();
12+
let result = bash
13+
.exec(
14+
r#"
15+
set -e
16+
f() {
17+
[[ "a" == "b" ]] && return 0
18+
echo "should reach here"
19+
}
20+
f
21+
echo "after f"
22+
"#,
23+
)
24+
.await
25+
.unwrap();
26+
assert!(result.stdout.contains("should reach here"));
27+
assert!(result.stdout.contains("after f"));
28+
}
29+
30+
/// set -e: [[ false ]] && cmd inside brace group with redirect
31+
#[tokio::test]
32+
async fn set_e_and_list_in_brace_group() {
33+
let mut bash = Bash::new();
34+
let result = bash
35+
.exec(
36+
r#"
37+
set -e
38+
x=""
39+
{
40+
echo "line1"
41+
[[ -n "$x" ]] && echo "has value"
42+
echo "line2"
43+
} > /tmp/out.txt
44+
cat /tmp/out.txt
45+
"#,
46+
)
47+
.await
48+
.unwrap();
49+
assert!(result.stdout.contains("line1"));
50+
assert!(result.stdout.contains("line2"));
51+
}
52+
53+
/// set -e: [[ false ]] && cmd inside for loop
54+
#[tokio::test]
55+
async fn set_e_and_list_in_for_loop() {
56+
let mut bash = Bash::new();
57+
let result = bash
58+
.exec(
59+
r#"
60+
set -e
61+
f() {
62+
for x in a b c; do
63+
[[ "$x" == "b" ]] && return 0
64+
done
65+
}
66+
f
67+
echo "after f"
68+
"#,
69+
)
70+
.await
71+
.unwrap();
72+
assert!(result.stdout.contains("after f"));
73+
}
74+
75+
/// set -e: top level [[ false ]] && cmd should still work
76+
#[tokio::test]
77+
async fn set_e_and_list_top_level() {
78+
let mut bash = Bash::new();
79+
let result = bash
80+
.exec(
81+
r#"
82+
set -e
83+
[[ "a" == "b" ]] && echo "match"
84+
echo "reached"
85+
"#,
86+
)
87+
.await
88+
.unwrap();
89+
assert!(result.stdout.contains("reached"));
90+
}
91+
92+
/// set -e should still exit on non-AND-OR failures
93+
#[tokio::test]
94+
async fn set_e_exits_on_plain_failure() {
95+
let mut bash = Bash::new();
96+
let result = bash
97+
.exec(
98+
r#"
99+
set -e
100+
false
101+
echo "SHOULD NOT APPEAR"
102+
"#,
103+
)
104+
.await
105+
.unwrap();
106+
assert!(!result.stdout.contains("SHOULD NOT APPEAR"));
107+
}

0 commit comments

Comments
 (0)