Replace std::process::exit with ExitCode enum implementing Termination

src/exit_code.rs

@@ -7,6 +7,8 @@
 //!
 //! These are also described in README.md.
 
+use std::process::ExitCode;
+
 // TODO: Maybe merge this with outcome::Status, and maybe merge with sysexit.
 
 /// Everything worked and all the mutants were caught.
@@ -34,3 +36,16 @@ pub const FILTER_DIFF_INVALID: i32 = 6;
 
 /// An internal software error, from sysexit.
 pub const SOFTWARE: i32 = 70;
+
+/// Convert an i32 exit code to `ExitCode`.
+///
+/// All exit codes defined in this module fit in u8.
+///
+/// # Panics
+///
+/// Panics if the exit code is not in the valid range 0-255.
+pub fn code_to_exit_code(code: i32) -> ExitCode {
+    ExitCode::from(
+        u8::try_from(code).unwrap_or_else(|_| panic!("exit code out of range: {code}")),
+    )
+}

src/main.rs

@@ -49,9 +49,9 @@ use std::env;
 use std::io;
 use std::path::Path;
 use std::path::PathBuf;
-use std::process::exit;
+use std::process::ExitCode;
 
-use anyhow::{Context, Result, anyhow, ensure};
+use anyhow::{Context, Result, anyhow, bail};
 use camino::{Utf8Path, Utf8PathBuf};
 use clap::{
     ArgAction, CommandFactory, Parser, ValueEnum,
@@ -492,10 +492,7 @@ pub struct Args {
     common: Common,
 }
 
-fn main() -> Result<()> {
-    // TODO: Perhaps return an ExitCode and avoid having calls to exit(). And as
-    // part of that, perhaps we should have an error type that implements Termination,
-    // to report its exit code.
+fn main() -> Result<ExitCode> {
     let args = match Cargo::try_parse() {
         Ok(Cargo::Mutants(args)) => args,
         Err(e) => {
@@ -506,18 +503,19 @@ fn main() -> Result<()> {
                 0 => 0,
                 _ => exit_code::SOFTWARE,
             };
-            exit(code);
+            return Ok(exit_code::code_to_exit_code(code));
         }
     };
 
     if args.version {
         println!("{NAME} {VERSION}");
-        return Ok(());
+        return Ok(ExitCode::SUCCESS);
     } else if let Some(shell) = args.completions {
         generate(shell, &mut Cargo::command(), "cargo", &mut io::stdout());
-        return Ok(());
+        return Ok(ExitCode::SUCCESS);
     } else if let Some(schema_type) = args.emit_schema {
-        return emit_schema(schema_type);
+        emit_schema(schema_type)?;
+        return Ok(ExitCode::SUCCESS);
     }
 
     let console = Console::new();
@@ -533,14 +531,17 @@ fn main() -> Result<()> {
             config::Config::default()
         };
         let options = Options::new(&args, &config)?;
-        return mutate_file(path, &options);
+        mutate_file(path, &options)?;
+        return Ok(ExitCode::SUCCESS);
     }
 
     let start_dir: &Utf8Path = if let Some(manifest_path) = &args.manifest_path {
-        ensure!(manifest_path.is_file(), "Manifest path is not a file");
+        if !manifest_path.is_file() {
+            bail!("Manifest path is not a file");
+        }
         manifest_path
             .parent()
-            .ok_or(anyhow!("Manifest path has no parent"))?
+            .context("Manifest path has no parent")?
     } else if let Some(dir) = &args.dir {
         dir
     } else {
@@ -588,7 +589,7 @@ fn main() -> Result<()> {
     console.clear();
     if args.list_files {
         print!("{}", list_files(&discovered.files, &options));
-        return Ok(());
+        return Ok(ExitCode::SUCCESS);
     }
     let mut mutants = discovered.mutants;
     if let Some(diff_path) = &args.in_diff {
@@ -600,7 +601,7 @@ fn main() -> Result<()> {
                 } else {
                     error!("{err}");
                 }
-                exit(err.exit_code());
+                return Ok(exit_code::code_to_exit_code(err.exit_code()));
             }
         };
     }
@@ -609,16 +610,16 @@ fn main() -> Result<()> {
     }
     if args.list {
         print!("{}", list_mutants(&mutants, &options));
+        Ok(ExitCode::SUCCESS)
     } else {
         let output_dir = OutputDir::new(&output_parent_dir)?;
         if let Some(previously_caught) = previously_caught {
             output_dir.write_previously_caught(&previously_caught)?;
         }
         console.set_debug_log(output_dir.open_debug_log()?);
         let lab_outcome = test_mutants(mutants, &workspace, output_dir, &options, &console)?;
-        exit(lab_outcome.exit_code());
+        Ok(exit_code::code_to_exit_code(lab_outcome.exit_code()))
     }
-    Ok(())
 }
 
 fn emit_schema(schema_type: SchemaType) -> Result<()> {

tests/main.rs

@@ -3778,3 +3778,49 @@ fn mutate_single_file() {
     assert_eq!(String::from_utf8_lossy(&out.stderr), "");
     assert_eq!(String::from_utf8_lossy(&out.stdout), "");
 }
+
+#[test]
+fn in_diff_with_mismatched_content_returns_exit_code_5() {
+    // Test that when the diff doesn't match the source tree, we get exit code 5
+    let tmp = copy_of_testdata("diff1");
+
+    // Create a diff that shows the new file content as something different
+    // from what's actually in the tree. The diff parser will try to match
+    // line 1 to be 'WRONG_CONTENT' but the actual file has 'pub fn one() -> String {'
+    let diff_text = "\
+--- a/src/lib.rs
++++ b/src/lib.rs
+@@ -1,3 +1,3 @@
+-pub fn old() -> String {
++WRONG_CONTENT
+     \"one\".to_owned()
+ }
+";
+
+    let mut diff_file = NamedTempFile::new().unwrap();
+    diff_file.write_all(diff_text.as_bytes()).unwrap();
+
+    run()
+        .args(["mutants", "-d"])
+        .arg(tmp.path())
+        .arg("--in-diff")
+        .arg(diff_file.path())
+        .assert()
+        .code(5)
+        .stderr(contains("Diff content doesn't match source file"));
+}
+
+#[test]
+fn in_diff_with_nonexistent_file_returns_exit_code_6() {
+    // Test that a nonexistent diff file returns exit code 6
+    let tmp = copy_of_testdata("diff1");
+
+    run()
+        .args(["mutants", "-d"])
+        .arg(tmp.path())
+        .arg("--in-diff")
+        .arg("/nonexistent/path/to/diff.patch")
+        .assert()
+        .code(6)
+        .stderr(contains("Failed to read diff file").or(contains("Failed to open diff file")));
+}