Skip to content

Commit 1036832

Browse files
hyperpolymathclaude
andcommitted
Revert 66 .expect("TODO: handle error") sites in commands.rs (batch 6/N)
Second of the big-3. 64 test sites + 2 production sites (in fg/bg builtins). Per-site policy: * Test code (64 sites in `tests::test_chmod_*`, `test_chown_*`, `test_checkpoint_*`, `test_restore_*`, `test_diff_*`, `test_replay_*`, `test_explain_*`): mass replace_all to .unwrap(). * Production code, fg() at L1683 + bg() at L1737: both functions re-fetched the job from `state.jobs` after a mutating `update_job_state` call, purely to read `job.command` for printing. The re-fetch was provably Some (initial `.ok_or_else(...)?` guard plus exclusive `&mut state` borrow held across the function), but cleaner to clone the command up-front and skip the re-lookup. Both panic sites eliminated; one duplicated lookup removed in each. Verified locally: cargo build -> EXIT 0 cargo test -> 703 passed / 0 failed / 14 ignored (unchanged across batches 1-6) Cumulative progress: 292 / ~530 sites cleared (~55%) across 16 files. Remaining big-1 in `impl/rust-cli/src/`: parser.rs (116) — biggest single file, warrants its own dedicated session. Plus `impl/rust-ffi/` ↔ `ffi/rust/` duplicate tree (~50 more, separate finding). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c6b59b8 commit 1036832

1 file changed

Lines changed: 77 additions & 68 deletions

File tree

impl/rust-cli/src/commands.rs

Lines changed: 77 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,8 +1641,13 @@ pub fn fg(state: &mut ShellState, job_spec: Option<&str>) -> Result<()> {
16411641
let pgid = job.pgid;
16421642
let job_id = job.id;
16431643
let job_state = job.state;
1644+
// Clone the command up-front so subsequent use doesn't require
1645+
// re-borrowing state.jobs after the mutating update_job_state call
1646+
// below. Removes a previous panic site that re-fetched the job
1647+
// purely to read this field.
1648+
let job_cmd = job.command.clone();
16441649

1645-
println!("{}", job.command.trim_end_matches(" &"));
1650+
println!("{}", job_cmd.trim_end_matches(" &"));
16461651

16471652
#[cfg(unix)]
16481653
{
@@ -1680,7 +1685,7 @@ pub fn fg(state: &mut ShellState, job_spec: Option<&str>) -> Result<()> {
16801685
// Update job state based on wait result
16811686
if unsafe { libc::WIFSTOPPED(status) } {
16821687
state.jobs.update_job_state(pgid, JobState::Stopped);
1683-
println!("\n[{}]+ Stopped {}", job_id, state.jobs.get_job(spec).expect("TODO: handle error").command.trim_end_matches(" &"));
1688+
println!("\n[{}]+ Stopped {}", job_id, job_cmd.trim_end_matches(" &"));
16841689
} else {
16851690
// Job completed - remove from table
16861691
state.jobs.remove_job(job_id);
@@ -1716,6 +1721,11 @@ pub fn bg(state: &mut ShellState, job_spec: Option<&str>) -> Result<()> {
17161721
let pgid = job.pgid;
17171722
let job_id = job.id;
17181723
let job_state = job.state;
1724+
// Clone the command up-front so subsequent use doesn't require
1725+
// re-borrowing state.jobs after the mutating update_job_state call
1726+
// below. Removes a previous panic site that re-fetched the job
1727+
// purely to read this field.
1728+
let job_cmd = job.command.clone();
17191729

17201730
#[cfg(unix)]
17211731
{
@@ -1734,8 +1744,7 @@ pub fn bg(state: &mut ShellState, job_spec: Option<&str>) -> Result<()> {
17341744
state.jobs.update_job_state(pgid, JobState::Running);
17351745

17361746
// Print job info (bash-style)
1737-
let job = state.jobs.get_job(spec).expect("TODO: handle error");
1738-
println!("[{}]+ {}", job_id, job.command);
1747+
println!("[{}]+ {}", job_id, job_cmd);
17391748
}
17401749

17411750
#[cfg(not(unix))]
@@ -2452,49 +2461,49 @@ mod tests {
24522461

24532462
#[test]
24542463
fn test_parse_chmod_octal_755() {
2455-
let result = parse_chmod_mode("755", 0o100644).expect("TODO: handle error");
2464+
let result = parse_chmod_mode("755", 0o100644).unwrap();
24562465
assert_eq!(result & 0o7777, 0o755);
24572466
}
24582467

24592468
#[test]
24602469
fn test_parse_chmod_octal_644() {
2461-
let result = parse_chmod_mode("644", 0o100755).expect("TODO: handle error");
2470+
let result = parse_chmod_mode("644", 0o100755).unwrap();
24622471
assert_eq!(result & 0o7777, 0o644);
24632472
}
24642473

24652474
#[test]
24662475
fn test_parse_chmod_symbolic_u_plus_x() {
2467-
let result = parse_chmod_mode("u+x", 0o100644).expect("TODO: handle error");
2476+
let result = parse_chmod_mode("u+x", 0o100644).unwrap();
24682477
assert_eq!(result & 0o7777, 0o744);
24692478
}
24702479

24712480
#[test]
24722481
fn test_parse_chmod_symbolic_go_minus_w() {
2473-
let result = parse_chmod_mode("go-w", 0o100666).expect("TODO: handle error");
2482+
let result = parse_chmod_mode("go-w", 0o100666).unwrap();
24742483
assert_eq!(result & 0o7777, 0o644);
24752484
}
24762485

24772486
#[test]
24782487
fn test_parse_chmod_symbolic_a_equals_r() {
2479-
let result = parse_chmod_mode("a=r", 0o100777).expect("TODO: handle error");
2488+
let result = parse_chmod_mode("a=r", 0o100777).unwrap();
24802489
assert_eq!(result & 0o7777, 0o444);
24812490
}
24822491

24832492
#[test]
24842493
fn test_parse_chmod_symbolic_plus_x() {
2485-
let result = parse_chmod_mode("+x", 0o100644).expect("TODO: handle error");
2494+
let result = parse_chmod_mode("+x", 0o100644).unwrap();
24862495
assert_eq!(result & 0o7777, 0o755);
24872496
}
24882497

24892498
#[test]
24902499
fn test_parse_chmod_symbolic_comma_separated() {
2491-
let result = parse_chmod_mode("u+x,go-w", 0o100666).expect("TODO: handle error");
2500+
let result = parse_chmod_mode("u+x,go-w", 0o100666).unwrap();
24922501
assert_eq!(result & 0o7777, 0o744);
24932502
}
24942503

24952504
#[test]
24962505
fn test_parse_chmod_preserves_file_type_bits() {
2497-
let result = parse_chmod_mode("755", 0o100644).expect("TODO: handle error");
2506+
let result = parse_chmod_mode("755", 0o100644).unwrap();
24982507
assert_eq!(result & 0o170000, 0o100000);
24992508
}
25002509

@@ -2507,30 +2516,30 @@ mod tests {
25072516
fn test_chmod_undo_data_roundtrip() {
25082517
let mode: u32 = 0o100755;
25092518
let bytes = mode.to_le_bytes().to_vec();
2510-
let restored = u32::from_le_bytes(bytes[..4].try_into().expect("TODO: handle error"));
2519+
let restored = u32::from_le_bytes(bytes[..4].try_into().unwrap());
25112520
assert_eq!(mode, restored);
25122521
}
25132522

25142523
#[cfg(unix)]
25152524
#[test]
25162525
fn test_parse_chown_user_only() {
2517-
let (uid, gid) = parse_chown_spec("1000", 500, 500).expect("TODO: handle error");
2526+
let (uid, gid) = parse_chown_spec("1000", 500, 500).unwrap();
25182527
assert_eq!(uid, 1000);
25192528
assert_eq!(gid, 500);
25202529
}
25212530

25222531
#[cfg(unix)]
25232532
#[test]
25242533
fn test_parse_chown_user_colon_group() {
2525-
let (uid, gid) = parse_chown_spec("1000:1001", 500, 500).expect("TODO: handle error");
2534+
let (uid, gid) = parse_chown_spec("1000:1001", 500, 500).unwrap();
25262535
assert_eq!(uid, 1000);
25272536
assert_eq!(gid, 1001);
25282537
}
25292538

25302539
#[cfg(unix)]
25312540
#[test]
25322541
fn test_parse_chown_colon_group_only() {
2533-
let (uid, gid) = parse_chown_spec(":1001", 500, 500).expect("TODO: handle error");
2542+
let (uid, gid) = parse_chown_spec(":1001", 500, 500).unwrap();
25342543
assert_eq!(uid, 500);
25352544
assert_eq!(gid, 1001);
25362545
}
@@ -2542,20 +2551,20 @@ mod tests {
25422551
let gid: u32 = 1001;
25432552
let data = format!("{}:{}", uid, gid);
25442553
let parts: Vec<&str> = data.splitn(2, ':').collect();
2545-
assert_eq!(parts[0].parse::<u32>().expect("TODO: handle error"), 1000);
2546-
assert_eq!(parts[1].parse::<u32>().expect("TODO: handle error"), 1001);
2554+
assert_eq!(parts[0].parse::<u32>().unwrap(), 1000);
2555+
assert_eq!(parts[1].parse::<u32>().unwrap(), 1001);
25472556
}
25482557

25492558
// ── P8: Wow-factor feature tests ──────────────────────────────────
25502559

25512560
#[test]
25522561
fn test_checkpoint_save_and_list() {
2553-
let tmp = tempfile::tempdir().expect("TODO: handle error");
2554-
let mut state = ShellState::new(tmp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
2562+
let tmp = tempfile::tempdir().unwrap();
2563+
let mut state = ShellState::new(tmp.path().to_str().unwrap()).unwrap();
25552564

25562565
assert!(state.checkpoints.is_empty());
25572566

2558-
checkpoint(&mut state, "clean").expect("TODO: handle error");
2567+
checkpoint(&mut state, "clean").unwrap();
25592568
assert_eq!(state.checkpoints.len(), 1);
25602569
assert!(state.checkpoints.contains_key("clean"));
25612570

@@ -2565,28 +2574,28 @@ mod tests {
25652574

25662575
#[test]
25672576
fn test_checkpoint_after_operations() {
2568-
let tmp = tempfile::tempdir().expect("TODO: handle error");
2569-
let mut state = ShellState::new(tmp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
2577+
let tmp = tempfile::tempdir().unwrap();
2578+
let mut state = ShellState::new(tmp.path().to_str().unwrap()).unwrap();
25702579

2571-
mkdir(&mut state, "testdir", false).expect("TODO: handle error");
2580+
mkdir(&mut state, "testdir", false).unwrap();
25722581
assert_eq!(state.history.len(), 1);
25732582

2574-
checkpoint(&mut state, "after-mkdir").expect("TODO: handle error");
2583+
checkpoint(&mut state, "after-mkdir").unwrap();
25752584
let (idx, _) = state.checkpoints["after-mkdir"];
25762585
assert_eq!(idx, 1);
25772586
}
25782587

25792588
#[test]
25802589
fn test_checkpoint_overwrite() {
2581-
let tmp = tempfile::tempdir().expect("TODO: handle error");
2582-
let mut state = ShellState::new(tmp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
2590+
let tmp = tempfile::tempdir().unwrap();
2591+
let mut state = ShellState::new(tmp.path().to_str().unwrap()).unwrap();
25832592

2584-
checkpoint(&mut state, "snap").expect("TODO: handle error");
2593+
checkpoint(&mut state, "snap").unwrap();
25852594
let (idx1, _) = state.checkpoints["snap"];
25862595

2587-
mkdir(&mut state, "dir1", false).expect("TODO: handle error");
2596+
mkdir(&mut state, "dir1", false).unwrap();
25882597

2589-
checkpoint(&mut state, "snap").expect("TODO: handle error");
2598+
checkpoint(&mut state, "snap").unwrap();
25902599
let (idx2, _) = state.checkpoints["snap"];
25912600

25922601
assert!(idx2 > idx1);
@@ -2595,106 +2604,106 @@ mod tests {
25952604

25962605
#[test]
25972606
fn test_restore_to_checkpoint() {
2598-
let tmp = tempfile::tempdir().expect("TODO: handle error");
2599-
let mut state = ShellState::new(tmp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
2607+
let tmp = tempfile::tempdir().unwrap();
2608+
let mut state = ShellState::new(tmp.path().to_str().unwrap()).unwrap();
26002609

2601-
checkpoint(&mut state, "start").expect("TODO: handle error");
2610+
checkpoint(&mut state, "start").unwrap();
26022611

2603-
mkdir(&mut state, "dir1", false).expect("TODO: handle error");
2604-
mkdir(&mut state, "dir2", false).expect("TODO: handle error");
2612+
mkdir(&mut state, "dir1", false).unwrap();
2613+
mkdir(&mut state, "dir2", false).unwrap();
26052614
assert_eq!(state.history.len(), 2);
26062615
assert!(tmp.path().join("dir1").exists());
26072616
assert!(tmp.path().join("dir2").exists());
26082617

2609-
restore(&mut state, "start").expect("TODO: handle error");
2618+
restore(&mut state, "start").unwrap();
26102619
assert!(!tmp.path().join("dir1").exists());
26112620
assert!(!tmp.path().join("dir2").exists());
26122621
}
26132622

26142623
#[test]
26152624
fn test_restore_nonexistent_checkpoint() {
2616-
let tmp = tempfile::tempdir().expect("TODO: handle error");
2617-
let mut state = ShellState::new(tmp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
2625+
let tmp = tempfile::tempdir().unwrap();
2626+
let mut state = ShellState::new(tmp.path().to_str().unwrap()).unwrap();
26182627
let result = restore(&mut state, "nope");
26192628
assert!(result.is_err());
26202629
}
26212630

26222631
#[test]
26232632
fn test_restore_already_at_checkpoint() {
2624-
let tmp = tempfile::tempdir().expect("TODO: handle error");
2625-
let mut state = ShellState::new(tmp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
2626-
checkpoint(&mut state, "here").expect("TODO: handle error");
2627-
restore(&mut state, "here").expect("TODO: handle error");
2633+
let tmp = tempfile::tempdir().unwrap();
2634+
let mut state = ShellState::new(tmp.path().to_str().unwrap()).unwrap();
2635+
checkpoint(&mut state, "here").unwrap();
2636+
restore(&mut state, "here").unwrap();
26282637
}
26292638

26302639
#[test]
26312640
fn test_diff_state_empty() {
2632-
let tmp = tempfile::tempdir().expect("TODO: handle error");
2633-
let state = ShellState::new(tmp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
2634-
diff_state(&state, 0).expect("TODO: handle error");
2641+
let tmp = tempfile::tempdir().unwrap();
2642+
let state = ShellState::new(tmp.path().to_str().unwrap()).unwrap();
2643+
diff_state(&state, 0).unwrap();
26352644
}
26362645

26372646
#[test]
26382647
fn test_diff_state_with_ops() {
2639-
let tmp = tempfile::tempdir().expect("TODO: handle error");
2640-
let mut state = ShellState::new(tmp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
2648+
let tmp = tempfile::tempdir().unwrap();
2649+
let mut state = ShellState::new(tmp.path().to_str().unwrap()).unwrap();
26412650

2642-
mkdir(&mut state, "d1", false).expect("TODO: handle error");
2651+
mkdir(&mut state, "d1", false).unwrap();
26432652
assert_eq!(state.history.len(), 1);
26442653

2645-
diff_state(&state, 0).expect("TODO: handle error");
2654+
diff_state(&state, 0).unwrap();
26462655
}
26472656

26482657
#[test]
26492658
fn test_replay_empty_range() {
2650-
let tmp = tempfile::tempdir().expect("TODO: handle error");
2651-
let state = ShellState::new(tmp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
2652-
replay(&state, 5, 5).expect("TODO: handle error");
2653-
replay(&state, 10, 5).expect("TODO: handle error");
2659+
let tmp = tempfile::tempdir().unwrap();
2660+
let state = ShellState::new(tmp.path().to_str().unwrap()).unwrap();
2661+
replay(&state, 5, 5).unwrap();
2662+
replay(&state, 10, 5).unwrap();
26542663
}
26552664

26562665
#[test]
26572666
fn test_replay_with_operations() {
2658-
let tmp = tempfile::tempdir().expect("TODO: handle error");
2659-
let mut state = ShellState::new(tmp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
2667+
let tmp = tempfile::tempdir().unwrap();
2668+
let mut state = ShellState::new(tmp.path().to_str().unwrap()).unwrap();
26602669

2661-
mkdir(&mut state, "r1", false).expect("TODO: handle error");
2662-
mkdir(&mut state, "r2", false).expect("TODO: handle error");
2670+
mkdir(&mut state, "r1", false).unwrap();
2671+
mkdir(&mut state, "r2", false).unwrap();
26632672

2664-
replay(&state, 0, 2).expect("TODO: handle error");
2673+
replay(&state, 0, 2).unwrap();
26652674
}
26662675

26672676
#[test]
26682677
fn test_replay_clamps_end() {
2669-
let tmp = tempfile::tempdir().expect("TODO: handle error");
2670-
let mut state = ShellState::new(tmp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
2678+
let tmp = tempfile::tempdir().unwrap();
2679+
let mut state = ShellState::new(tmp.path().to_str().unwrap()).unwrap();
26712680

2672-
mkdir(&mut state, "clamp", false).expect("TODO: handle error");
2681+
mkdir(&mut state, "clamp", false).unwrap();
26732682

2674-
replay(&state, 0, 999).expect("TODO: handle error");
2683+
replay(&state, 0, 999).unwrap();
26752684
}
26762685

26772686
#[test]
26782687
fn test_explain_mkdir() {
2679-
let tmp = tempfile::tempdir().expect("TODO: handle error");
2680-
let state = ShellState::new(tmp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
2688+
let tmp = tempfile::tempdir().unwrap();
2689+
let state = ShellState::new(tmp.path().to_str().unwrap()).unwrap();
26812690
let cmd = crate::parser::Command::Mkdir {
26822691
path: "newdir".to_string(),
26832692
redirects: vec![],
26842693
};
2685-
explain_command(&cmd, &state).expect("TODO: handle error");
2694+
explain_command(&cmd, &state).unwrap();
26862695
}
26872696

26882697
#[test]
26892698
fn test_explain_external_command() {
2690-
let tmp = tempfile::tempdir().expect("TODO: handle error");
2691-
let state = ShellState::new(tmp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
2699+
let tmp = tempfile::tempdir().unwrap();
2700+
let state = ShellState::new(tmp.path().to_str().unwrap()).unwrap();
26922701
let cmd = crate::parser::Command::External {
26932702
program: "git".to_string(),
26942703
args: vec!["status".to_string()],
26952704
redirects: vec![],
26962705
background: false,
26972706
};
2698-
explain_command(&cmd, &state).expect("TODO: handle error");
2707+
explain_command(&cmd, &state).unwrap();
26992708
}
27002709
}

0 commit comments

Comments
 (0)