Fix exit code to return 0 when all findings are auto-fixable (#1242)

Co-authored-by: William Woodruff <william@yossarian.net>

crates/zizmor/src/main.rs

@@ -769,12 +769,24 @@ fn run(app: &mut App) -> Result<ExitCode, Error> {
         }
     };
 
-    if let Some(fix_mode) = app.fix {
-        output::fix::apply_fixes(fix_mode, &results, &registry).map_err(Error::Fix)?;
-    }
+    let all_fixed = if let Some(fix_mode) = app.fix {
+        let fix_result =
+            output::fix::apply_fixes(fix_mode, &results, &registry).map_err(Error::Fix)?;
+
+        // If all findings have applicable fixes and all were successfully applied,
+        // we should exit with success.
+        results.all_findings_have_applicable_fixes(fix_mode)
+            && fix_result.failed_count == 0
+            && fix_result.applied_count > 0
+    } else {
+        false
+    };
 
     if app.no_exit_codes || matches!(app.format, OutputFormat::Sarif) {
         Ok(ExitCode::SUCCESS)
+    } else if all_fixed {
+        // All findings were auto-fixed, no manual intervention needed
+        Ok(ExitCode::SUCCESS)
     } else {
         Ok(results.exit_code())
     }

crates/zizmor/src/output/fix.rs

@@ -13,12 +13,21 @@ use crate::{
     registry::{FindingRegistry, input::InputKey, input::InputRegistry},
 };
 
+/// Result of applying fixes.
+#[derive(Debug)]
+pub struct FixResult {
+    /// Number of fixes that were successfully applied.
+    pub applied_count: usize,
+    /// Number of fixes that failed to apply.
+    pub failed_count: usize,
+}
+
 /// Apply all fixes associated with findings, filtered by the specified fix mode.
 pub fn apply_fixes(
     fix_mode: FixMode,
     results: &FindingRegistry,
     registry: &InputRegistry,
-) -> Result<()> {
+) -> Result<FixResult> {
     let mut fixes_by_input: HashMap<&InputKey, Vec<(&Fix, &Finding)>> = HashMap::new();
     let mut total_fixes = 0;
     for finding in results.fixable_findings() {
@@ -46,12 +55,16 @@ pub fn apply_fixes(
         } else {
             anstream::eprintln!("No fixes available to apply.");
         }
-        return Ok(());
+        return Ok(FixResult {
+            applied_count: 0,
+            failed_count: 0,
+        });
     }
 
     // Process each file
     let mut applied_fixes = Vec::new();
     let mut failed_fixes = Vec::new();
+    let mut total_applied = 0;
 
     for (input_key, fixes) in &fixes_by_input {
         let InputKey::Local(local) = input_key else {
@@ -72,6 +85,7 @@ pub fn apply_fixes(
                 Ok(new_document) => {
                     current_document = new_document;
                     file_applied_fixes.push((finding.ident, fix, finding));
+                    total_applied += 1;
                 }
                 Err(e) => {
                     // If the fix fails on modified content, it might be due to conflicts
@@ -101,7 +115,10 @@ pub fn apply_fixes(
         print_summary(&applied_fixes, &failed_fixes);
     }
 
-    Ok(())
+    Ok(FixResult {
+        applied_count: total_applied,
+        failed_count: failed_fixes.len(),
+    })
 }
 
 fn print_summary(

crates/zizmor/src/registry.rs

@@ -199,6 +199,30 @@ impl<'a> FindingRegistry<'a> {
         })
     }
 
+    /// Checks if all findings have at least one fix matching the given fix mode.
+    ///
+    /// Returns true if every finding has at least one applicable fix based on the mode,
+    /// meaning no manual intervention would be required if all fixes are applied successfully.
+    pub(crate) fn all_findings_have_applicable_fixes(&self, fix_mode: crate::FixMode) -> bool {
+        use crate::finding::FixDisposition;
+
+        if self.findings.is_empty() {
+            return true;
+        }
+
+        self.findings.iter().all(|finding| {
+            finding.fixes.iter().any(|fix| {
+                let disposition_matches = match fix_mode {
+                    crate::FixMode::Safe => matches!(fix.disposition, FixDisposition::Safe),
+                    crate::FixMode::UnsafeOnly => matches!(fix.disposition, FixDisposition::Unsafe),
+                    crate::FixMode::All => true,
+                };
+
+                disposition_matches && matches!(fix.key, InputKey::Local(_))
+            })
+        })
+    }
+
     /// All ignored findings.
     pub(crate) fn ignored(&self) -> &[Finding<'a>] {
         &self.ignored

docs/release-notes.md

@@ -9,6 +9,14 @@ of `zizmor`.
 
 ## Next (UNRELEASED)
 
+### Enhancements 🌱
+
+* When running in `--fix` mode and all fixes are successfully applied,
+  `zizmor` now has similar [exit code] behavior as the `--no-exit-codes`
+  and `--format=sarif` flags (#1242)
+
+    Many thanks to @cnaples79 for implementing this improvement!
+
 ## 1.15.2
 
 ### Bug Fixes 🐛
@@ -1161,3 +1169,5 @@ This is one of `zizmor`'s bigger recent releases! Key enhancements include:
 [ref-version-mismatch]: ./audits.md#ref-version-mismatch
 [dependabot-execution]: ./audits.md#dependabot-execution
 [dependabot-cooldown]: ./audits.md#dependabot-cooldown
+
+[exit code]: ./usage.md#exit-codes

docs/usage.md

@@ -393,18 +393,8 @@ annotations.
 
 ## Exit codes
 
-!!! note
-
-    Exit codes 11 and above are **not used** if `--no-exit-codes` or
-    `--format sarif` is passed.
-
-!!! warning "Removal"
-
-    Versions of zizmor prior to `v1.14.0` used exit code `10` to indicate
-    the highest finding having "unknown" severity. This exit code is
-    no longer used as of `v1.14.0`.
-
-`zizmor` uses various exit codes to summarize the results of a run:
+`zizmor` will exit with a variety of exit codes, depending on how
+you invoke it and what happens during the run. In general:
 
 | Code | Meaning |
 | ---- | ------- |
@@ -418,6 +408,36 @@ annotations.
 
 All other exit codes are currently reserved.
 
+!!! warning "Removal"
+
+    Versions of zizmor prior to `v1.14.0` used exit code `10` to indicate
+    the highest finding having "unknown" severity. This exit code is
+    no longer used as of `v1.14.0`.
+
+Beyond this general table, the following modes of operation also
+change the exit code behavior:
+
+* If you run with `--no-exit-codes`, `zizmor` will **not** use exit codes 11
+  and above.
+* If you use `--format=sarif`, `zizmor` will **not** use exit codes 11 and
+  above.
+
+    !!! tip "Why?"
+
+        Because SARIF consumers generally don't expect SARIF producers
+        (like `zizmor`) to exit with a non-zero code *except* in the case
+        of an internal failure. SARIF consumers expect semantic results
+        to be communicated within the SARIF itself.
+
+* If you run with `--fix` *and* all findings are successfully auto-fixed,
+  `zizmor` will **not** use exit codes 11 and above.
+
+    !!! tip "Why?"
+
+        Because the higher exit codes indicate that action needs to be taken,
+        but a successful application of all fixes means that no action
+        is required.
+
 ## Using personas
 
 !!! tip