Revert 43 .expect("TODO: handle error") sites across 4 files (batch 2/N)

Continues the per-site reversal of the .unwrap() → .expect("TODO: handle
error") anti-pattern flagged by CRG-AUDIT-2026-04-18.

Per-site policy:

  * Test code → revert to .unwrap() (test convention; preceding asserts
    or short-circuits already prove the Some/Ok case). Files this batch:
      - process_sub.rs (11 sites in tests::test_fifo_*)
      - audit_log.rs   (13 sites in tests::test_audit_*)
      - job.rs         ( 7 sites in tests::test_get_job_*, test_remove_job)
      - functions.rs   (11 sites in tests::test_parse_function_*,
                       test_function_table_*, test_pop_frame_*)

  * Production code, functions.rs:320 — restructure
      `if name.is_empty() { return false; }
       let mut chars = name.chars();
       let first = chars.next().expect("TODO: handle error");`
    to
      `let mut chars = name.chars();
       let Some(first) = chars.next() else { return false; };`
    chars.next() returns None on an empty &str, so the let-else covers
    the empty-name case directly. One construct, no panic site, identical
    behaviour. Verified by the existing is_valid_function_name tests.

Verified locally:
  cargo build  -> EXIT 0
  cargo test   -> 703 passed / 0 failed / 14 ignored (unchanged from
                  batch 1; restructure preserves behaviour exactly)

Cumulative progress: 57 / ~530 sites cleared (~11%). Remaining: 17 files,
biggest are parser.rs (116), commands.rs (66), arith.rs (55), state.rs
(35). Plus the impl/rust-ffi/ ↔ ffi/rust/ duplicate tree (~50 more,
separate finding).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: hyperpolymath

impl/rust-cli/src/audit_log.rs

@@ -284,55 +284,55 @@ mod tests {
         let op = Operation::new(OperationType::Mkdir, "test_dir".to_string(), None);
         let entry = AuditEntry::from_operation(&op, "success", None);
 
-        let json_line = entry.to_json_line().expect("TODO: handle error");
+        let json_line = entry.to_json_line().unwrap();
         assert!(json_line.ends_with('\n'));
 
-        let parsed = AuditEntry::from_json_line(&json_line).expect("TODO: handle error");
+        let parsed = AuditEntry::from_json_line(&json_line).unwrap();
         assert_eq!(parsed.operation_id, entry.operation_id);
         assert_eq!(parsed.path, "test_dir");
         assert_eq!(parsed.outcome, "success");
     }
 
     #[test]
     fn test_audit_log_append_and_read() {
-        let temp_file = NamedTempFile::new().expect("TODO: handle error");
+        let temp_file = NamedTempFile::new().unwrap();
         let log_path = temp_file.path().to_path_buf();
 
-        let log = AuditLog::new(log_path, None).expect("TODO: handle error");
+        let log = AuditLog::new(log_path, None).unwrap();
 
         // Append some entries
         let op1 = Operation::new(OperationType::Mkdir, "dir1".to_string(), None);
         let entry1 = AuditEntry::from_operation(&op1, "success", None);
-        log.append(&entry1).expect("TODO: handle error");
+        log.append(&entry1).unwrap();
 
         let op2 = Operation::new(OperationType::CreateFile, "file1".to_string(), None);
         let entry2 = AuditEntry::from_operation(&op2, "success", None);
-        log.append(&entry2).expect("TODO: handle error");
+        log.append(&entry2).unwrap();
 
         // Read back
-        let entries = log.read_all().expect("TODO: handle error");
+        let entries = log.read_all().unwrap();
         assert_eq!(entries.len(), 2);
         assert_eq!(entries[0].path, "dir1");
         assert_eq!(entries[1].path, "file1");
     }
 
     #[test]
     fn test_audit_log_filter_by_type() {
-        let temp_file = NamedTempFile::new().expect("TODO: handle error");
+        let temp_file = NamedTempFile::new().unwrap();
         let log_path = temp_file.path().to_path_buf();
 
-        let log = AuditLog::new(log_path, None).expect("TODO: handle error");
+        let log = AuditLog::new(log_path, None).unwrap();
 
         let op1 = Operation::new(OperationType::Mkdir, "dir1".to_string(), None);
-        log.append(&AuditEntry::from_operation(&op1, "success", None)).expect("TODO: handle error");
+        log.append(&AuditEntry::from_operation(&op1, "success", None)).unwrap();
 
         let op2 = Operation::new(OperationType::CreateFile, "file1".to_string(), None);
-        log.append(&AuditEntry::from_operation(&op2, "success", None)).expect("TODO: handle error");
+        log.append(&AuditEntry::from_operation(&op2, "success", None)).unwrap();
 
         let op3 = Operation::new(OperationType::Mkdir, "dir2".to_string(), None);
-        log.append(&AuditEntry::from_operation(&op3, "success", None)).expect("TODO: handle error");
+        log.append(&AuditEntry::from_operation(&op3, "success", None)).unwrap();
 
-        let mkdirs = log.read_by_type("Mkdir").expect("TODO: handle error");
+        let mkdirs = log.read_by_type("Mkdir").unwrap();
         assert_eq!(mkdirs.len(), 2);
         assert_eq!(mkdirs[0].path, "dir1");
         assert_eq!(mkdirs[1].path, "dir2");

impl/rust-cli/src/functions.rs

@@ -313,11 +313,13 @@ fn parse_function_body(body_str: &str) -> Vec<String> {
 /// Same rules as variable names: starts with letter or underscore,
 /// then alphanumeric or underscore.
 pub fn is_valid_function_name(name: &str) -> bool {
-    if name.is_empty() {
-        return false;
-    }
+    // `let-else` covers the empty-name case directly: chars.next() returns
+    // None on an empty &str, which is the same outcome as the previous
+    // explicit `if name.is_empty()` guard. One construct, no panic site.
     let mut chars = name.chars();
-    let first = chars.next().expect("TODO: handle error");
+    let Some(first) = chars.next() else {
+        return false;
+    };
     if !first.is_alphabetic() && first != '_' {
         return false;
     }
@@ -336,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.expect("TODO: handle error");
+        let (name, body, raw_body) = result.unwrap();
         assert_eq!(name, "greet");
         assert_eq!(body, vec!["echo hello"]);
         // raw_body preserves the trailing `;` — that's harmless.
@@ -347,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.expect("TODO: handle error");
+        let (name, body, raw_body) = result.unwrap();
         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;");
@@ -357,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.expect("TODO: handle error");
+        let (name, body, raw_body) = result.unwrap();
         assert_eq!(name, "greet");
         assert_eq!(body, vec!["echo hello"]);
         assert_eq!(raw_body, "echo hello;");
@@ -367,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.expect("TODO: handle error");
+        let (name, body, raw_body) = result.unwrap();
         assert_eq!(name, "greet");
         assert_eq!(body, vec!["echo hello"]);
         assert_eq!(raw_body, "echo hello;");
@@ -379,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.expect("TODO: handle error");
+        let (name, _body, raw_body) = result.unwrap();
         assert_eq!(name, "ifunc");
         assert_eq!(raw_body, "if true; then mkdir d; fi;");
     }
@@ -389,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.expect("TODO: handle error");
+        let (_name, _body, raw_body) = result.unwrap();
         assert_eq!(raw_body, "echo '}';");
     }
 
@@ -476,7 +478,7 @@ mod tests {
                 line: 5,
             },
         });
-        let def = table.get("greet").expect("TODO: handle error");
+        let def = table.get("greet").unwrap();
         assert_eq!(def.body, vec!["echo goodbye"]);
     }
 
@@ -495,11 +497,11 @@ mod tests {
         table.push_frame(vec!["nested_arg".to_string()]);
         assert_eq!(table.call_depth(), 2);
 
-        let frame = table.pop_frame().expect("TODO: handle error");
+        let frame = table.pop_frame().unwrap();
         assert_eq!(frame.saved_params, vec!["nested_arg"]);
         assert_eq!(table.call_depth(), 1);
 
-        let frame = table.pop_frame().expect("TODO: handle error");
+        let frame = table.pop_frame().unwrap();
         assert_eq!(frame.saved_params, vec!["arg1", "arg2"]);
         assert_eq!(table.call_depth(), 0);
 
@@ -514,7 +516,7 @@ mod tests {
         table.declare_local("x", Some("old_value".to_string()));
         table.declare_local("y", None);
 
-        let frame = table.pop_frame().expect("TODO: handle error");
+        let frame = table.pop_frame().unwrap();
         assert_eq!(frame.local_vars, vec!["x", "y"]);
         assert_eq!(
             frame.saved_vars.get("x"),
@@ -533,7 +535,7 @@ mod tests {
         // Second declaration in same frame should NOT overwrite the saved value
         table.declare_local("x", Some("modified".to_string()));
 
-        let frame = table.pop_frame().expect("TODO: handle error");
+        let frame = table.pop_frame().unwrap();
         // The saved value should be "original", not "modified"
         assert_eq!(
             frame.saved_vars.get("x"),

impl/rust-cli/src/job.rs

@@ -214,7 +214,7 @@ mod tests {
 
         let job = table.get_job("%1");
         assert!(job.is_some());
-        assert_eq!(job.expect("TODO: handle error").pgid, 1234);
+        assert_eq!(job.unwrap().pgid, 1234);
     }
 
     #[test]
@@ -226,12 +226,12 @@ mod tests {
         // %+ should be most recent (id2)
         let job = table.get_job("%+");
         assert!(job.is_some());
-        assert_eq!(job.expect("TODO: handle error").id, id2);
+        assert_eq!(job.unwrap().id, id2);
 
         // %- should be previous (id1)
         let job = table.get_job("%-");
         assert!(job.is_some());
-        assert_eq!(job.expect("TODO: handle error").id, id1);
+        assert_eq!(job.unwrap().id, id1);
     }
 
     #[test]
@@ -242,11 +242,11 @@ mod tests {
 
         let job = table.get_job("%sleep");
         assert!(job.is_some());
-        assert_eq!(job.expect("TODO: handle error").command, "sleep 10");
+        assert_eq!(job.unwrap().command, "sleep 10");
 
         let job = table.get_job("%find");
         assert!(job.is_some());
-        assert_eq!(job.expect("TODO: handle error").command, "find / -name test");
+        assert_eq!(job.unwrap().command, "find / -name test");
     }
 
     #[test]
@@ -256,7 +256,7 @@ mod tests {
 
         let job = table.get_job("%?name");
         assert!(job.is_some());
-        assert_eq!(job.expect("TODO: handle error").command, "find / -name test");
+        assert_eq!(job.unwrap().command, "find / -name test");
     }
 
     #[test]
@@ -275,7 +275,7 @@ mod tests {
         table.add_job(1234, "sleep 10".to_string(), vec![1234]);
 
         table.update_job_state(1234, JobState::Stopped);
-        let job = table.get_job("%1").expect("TODO: handle error");
+        let job = table.get_job("%1").unwrap();
         assert_eq!(job.state, JobState::Stopped);
     }
 

impl/rust-cli/src/process_sub.rs

@@ -241,54 +241,54 @@ mod tests {
 
     #[test]
     fn test_fifo_creation() {
-        let temp_dir = TempDir::new().expect("TODO: handle error");
-        let mut state = ShellState::new(temp_dir.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
+        let temp_dir = TempDir::new().unwrap();
+        let mut state = ShellState::new(temp_dir.path().to_str().unwrap()).unwrap();
 
         let proc_sub = ProcessSubstitution::create(
             ProcessSubType::Input,
             "echo test".to_string(),
             &mut state,
-        ).expect("TODO: handle error");
+        ).unwrap();
 
         // FIFO should exist
         assert!(proc_sub.fifo_path.exists());
 
         // Should be a FIFO
         #[cfg(unix)]
         {
-            let metadata = std::fs::metadata(&proc_sub.fifo_path).expect("TODO: handle error");
+            let metadata = std::fs::metadata(&proc_sub.fifo_path).unwrap();
             use std::os::unix::fs::FileTypeExt;
             assert!(metadata.file_type().is_fifo());
         }
 
         // Cleanup should remove FIFO
         let fifo_path = proc_sub.fifo_path.clone();
-        proc_sub.cleanup().expect("TODO: handle error");
+        proc_sub.cleanup().unwrap();
         assert!(!fifo_path.exists());
     }
 
     #[test]
     fn test_fifo_path_unique() {
-        let temp_dir = TempDir::new().expect("TODO: handle error");
-        let mut state = ShellState::new(temp_dir.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
+        let temp_dir = TempDir::new().unwrap();
+        let mut state = ShellState::new(temp_dir.path().to_str().unwrap()).unwrap();
 
         let proc_sub1 = ProcessSubstitution::create(
             ProcessSubType::Input,
             "echo a".to_string(),
             &mut state,
-        ).expect("TODO: handle error");
+        ).unwrap();
 
         let proc_sub2 = ProcessSubstitution::create(
             ProcessSubType::Input,
             "echo b".to_string(),
             &mut state,
-        ).expect("TODO: handle error");
+        ).unwrap();
 
         // FIFOs should have different paths
         assert_ne!(proc_sub1.fifo_path, proc_sub2.fifo_path);
 
         // Cleanup
-        proc_sub1.cleanup().expect("TODO: handle error");
-        proc_sub2.cleanup().expect("TODO: handle error");
+        proc_sub1.cleanup().unwrap();
+        proc_sub2.cleanup().unwrap();
     }
 }