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

src/exit_code.rs

@@ -7,30 +7,44 @@
 //!
 //! These are also described in README.md.
 
-// TODO: Maybe merge this with outcome::Status, and maybe merge with sysexit.
-
-/// Everything worked and all the mutants were caught.
-pub const SUCCESS: i32 = 0;
-
-/// The wrong arguments, etc.
-///
-/// (1 is also the value returned by Clap.)
-pub const USAGE: i32 = 1;
-
-/// Found one or mutants that were not caught by tests.
-pub const FOUND_PROBLEMS: i32 = 2;
+use std::process::{ExitCode as StdExitCode, Termination};
 
-/// One or more tests timed out: probably the mutant caused an infinite loop, or the timeout is too low.
-pub const TIMEOUT: i32 = 3;
-
-/// The tests are already failing in an unmutated tree.
-pub const BASELINE_FAILED: i32 = 4;
-
-/// The filter diff new text does not match the source tree content.
-pub const FILTER_DIFF_MISMATCH: i32 = 5;
-
-/// The filter diff could not be parsed.
-pub const FILTER_DIFF_INVALID: i32 = 6;
+// TODO: Maybe merge this with outcome::Status, and maybe merge with sysexit.
 
-/// An internal software error, from sysexit.
-pub const SOFTWARE: i32 = 70;
+/// Exit codes for cargo-mutants.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[repr(i32)]
+pub enum ExitCode {
+    /// Everything worked and all the mutants were caught.
+    Success = 0,
+    /// The wrong arguments, etc.
+    ///
+    /// (1 is also the value returned by Clap.)
+    Usage = 1,
+    /// Found one or mutants that were not caught by tests.
+    FoundProblems = 2,
+    /// One or more tests timed out: probably the mutant caused an infinite loop, or the timeout is too low.
+    Timeout = 3,
+    /// The tests are already failing in an unmutated tree.
+    BaselineFailed = 4,
+    /// The filter diff new text does not match the source tree content.
+    FilterDiffMismatch = 5,
+    /// The filter diff could not be parsed.
+    FilterDiffInvalid = 6,
+    /// An internal software error, from sysexit.
+    Software = 70,
+}
+
+impl From<ExitCode> for StdExitCode {
+    fn from(code: ExitCode) -> Self {
+        // All exit codes are known to be valid u8 values
+        #[allow(clippy::cast_possible_truncation)]
+        StdExitCode::from(code as u8)
+    }
+}
+
+impl Termination for ExitCode {
+    fn report(self) -> std::process::ExitCode {
+        self.into()
+    }
+}

src/in_diff.rs

@@ -17,9 +17,10 @@ use indoc::formatdoc;
 use itertools::Itertools;
 use tracing::{error, trace, warn};
 
+use crate::Result;
+use crate::exit_code::ExitCode;
 use crate::mutant::Mutant;
 use crate::source::SourceFile;
-use crate::{Result, exit_code};
 
 /// The result of filtering mutants based on a diff.
 #[derive(Debug, PartialEq, Eq, Clone)]
@@ -43,14 +44,14 @@ impl DiffFilterError {
     ///
     /// Some errors such as an empty diff or one that changes no Rust source files
     /// still mean we can't process any mutants, but it's not necessarily a problem.
-    pub fn exit_code(&self) -> i32 {
+    pub fn exit_code(&self) -> ExitCode {
         match self {
             DiffFilterError::EmptyDiff
             | DiffFilterError::NoSourceFiles
-            | DiffFilterError::NoMutants => exit_code::SUCCESS,
-            DiffFilterError::MismatchedDiff(_) => exit_code::FILTER_DIFF_MISMATCH,
+            | DiffFilterError::NoMutants => ExitCode::Success,
+            DiffFilterError::MismatchedDiff(_) => ExitCode::FilterDiffMismatch,
             DiffFilterError::File(_) | DiffFilterError::InvalidDiff(_) => {
-                exit_code::FILTER_DIFF_INVALID
+                ExitCode::FilterDiffInvalid
             }
         }
     }
@@ -349,7 +350,7 @@ index eb42779..a0091b7 100644
 ";
         let err = diff_filter(Vec::new(), diff);
         assert_eq!(err, Err(DiffFilterError::NoMutants));
-        assert_eq!(err.unwrap_err().exit_code(), 0);
+        assert_eq!(err.unwrap_err().exit_code(), ExitCode::Success);
     }
 
     /// <https://github.com/sourcefrog/cargo-mutants/issues/580>
@@ -415,7 +416,7 @@ index cc3ce8c..8fe9aa0 100644
 ";
         let err = diff_filter(Vec::new(), diff);
         assert_eq!(err, Err(DiffFilterError::NoSourceFiles));
-        assert_eq!(err.unwrap_err().exit_code(), 0);
+        assert_eq!(err.unwrap_err().exit_code(), ExitCode::Success);
     }
 
     fn make_diff(old: &str, new: &str) -> String {

src/main.rs

@@ -49,9 +49,8 @@ use std::env;
 use std::io;
 use std::path::Path;
 use std::path::PathBuf;
-use std::process::exit;
 
-use anyhow::{Context, Result, anyhow, ensure};
+use anyhow::{Context, Result, anyhow};
 use camino::{Utf8Path, Utf8PathBuf};
 use clap::{
     ArgAction, CommandFactory, Parser, ValueEnum,
@@ -65,6 +64,7 @@ use tracing::{debug, error, info};
 use crate::{
     build_dir::BuildDir,
     console::Console,
+    exit_code::ExitCode,
     in_diff::diff_filter_file,
     interrupt::check_interrupted,
     lab::test_mutants,
@@ -492,32 +492,30 @@ 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) => {
             e.print().expect("Failed to show clap error message");
             // Clap by default exits with code 2.
             let code = match e.exit_code() {
-                2 => exit_code::USAGE,
-                0 => 0,
-                _ => exit_code::SOFTWARE,
+                2 => ExitCode::Usage,
+                0 => ExitCode::Success,
+                _ => ExitCode::Software,
             };
-            exit(code);
+            return Ok(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,18 @@ 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() {
+            error!("Manifest path is not a file");
+            return Ok(ExitCode::Usage);
+        }
         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,19 +590,19 @@ 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 {
         mutants = match diff_filter_file(mutants, diff_path) {
             Ok(mutants) => mutants,
             Err(err) => {
-                if err.exit_code() == 0 {
+                if err.exit_code() == ExitCode::Success {
                     info!("{err}");
                 } else {
                     error!("{err}");
                 }
-                exit(err.exit_code());
+                return Ok(err.exit_code());
             }
         };
     }
@@ -609,16 +611,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(lab_outcome.exit_code())
     }
-    Ok(())
 }
 
 fn emit_schema(schema_type: SchemaType) -> Result<()> {

src/outcome.rs

@@ -16,8 +16,9 @@ use serde::ser::SerializeStruct;
 use tracing::warn;
 
 use crate::console::{format_duration, plural};
+use crate::exit_code::ExitCode;
 use crate::process::Exit;
-use crate::{Options, Result, Scenario, exit_code, output};
+use crate::{Options, Result, Scenario, output};
 
 /// What phase of running a scenario.
 ///
@@ -106,20 +107,20 @@ impl LabOutcome {
     }
 
     /// Return the overall program exit code reflecting this outcome.
-    pub fn exit_code(&self) -> i32 {
+    pub fn exit_code(&self) -> ExitCode {
         // TODO: Maybe move this into an error returned from experiment()?
         if self
             .outcomes
             .iter()
             .any(|o| !o.scenario.is_mutant() && !o.success())
         {
-            exit_code::BASELINE_FAILED
+            ExitCode::BaselineFailed
         } else if self.timeout > 0 {
-            exit_code::TIMEOUT
+            ExitCode::Timeout
         } else if self.missed > 0 {
-            exit_code::FOUND_PROBLEMS
+            ExitCode::FoundProblems
         } else {
-            exit_code::SUCCESS
+            ExitCode::Success
         }
     }
 

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")));
+}