Extract logic to FixableStatistics struct

crates/ruff/src/linter.rs

@@ -415,6 +415,7 @@ fn diagnostics_to_messages(
 
 /// Generate `Diagnostic`s from source code content, iteratively autofixing
 /// until stable.
+#[allow(clippy::too_many_arguments)]
 pub fn lint_fix<'a>(
     contents: &'a str,
     path: &Path,

crates/ruff/src/settings/flags.rs

@@ -1,12 +1,20 @@
-use ruff_diagnostics::Applicability;
-
 #[derive(Debug, Copy, Clone, Hash, is_macro::Is)]
 pub enum FixMode {
     Generate(SuggestedFixes),
     Apply(SuggestedFixes),
     Diff(SuggestedFixes),
 }
 
+impl FixMode {
+    pub fn suggested_fixes(&self) -> &SuggestedFixes {
+        match self {
+            FixMode::Generate(suggested) => suggested,
+            FixMode::Apply(suggested) => suggested,
+            FixMode::Diff(suggested) => suggested,
+        }
+    }
+}
+
 #[derive(Debug, Copy, Clone, Hash, is_macro::Is)]
 pub enum SuggestedFixes {
     Apply,

crates/ruff/src/test.rs

@@ -10,7 +10,7 @@ use ruff_python_parser::lexer::LexResult;
 
 use rustc_hash::FxHashMap;
 
-use ruff_diagnostics::{Applicability, AutofixKind, Diagnostic};
+use ruff_diagnostics::{AutofixKind, Diagnostic};
 use ruff_python_ast::PySourceType;
 use ruff_python_codegen::Stylist;
 use ruff_python_index::Indexer;

crates/ruff_cli/src/diagnostics.rs

@@ -26,7 +26,7 @@ use ruff::registry::Rule;
 use ruff::settings::{flags, AllSettings, Settings};
 use ruff::source_kind::SourceKind;
 use ruff::{fs, IOError};
-use ruff_diagnostics::{Applicability, Diagnostic};
+use ruff_diagnostics::Diagnostic;
 use ruff_macros::CacheKey;
 use ruff_python_ast::imports::ImportMap;
 use ruff_python_ast::PySourceType;
@@ -331,7 +331,7 @@ pub(crate) fn lint_path(
                                 }
                             }
                         }
-                        flags::FixMode::Generate(fix_unsafe) => {}
+                        flags::FixMode::Generate(_) => {}
                     }
                 }
                 (result, fixed)

crates/ruff_cli/src/lib.rs

@@ -242,9 +242,10 @@ pub fn check(args: CheckArgs, log_level: LogLevel) -> Result<ExitStatus> {
     //   [`Applicability::Automatic`] fixes.
 
     // TODO: can't fix this until @zanieb changes the CLI
-    let fix_suggested = match fix_unsafe {
-        true => SuggestedFixes::Apply,
-        false => SuggestedFixes::Disable,
+    let fix_suggested = if fix_unsafe {
+        SuggestedFixes::Apply
+    } else {
+        SuggestedFixes::Disable
     };
 
     let autofix = if cli.diff {

crates/ruff_cli/src/printer.rs

@@ -20,7 +20,7 @@ use ruff::message::{
 };
 use ruff::notify_user;
 use ruff::registry::{AsRule, Rule};
-use ruff::settings::flags;
+use ruff::settings::flags::{self, SuggestedFixes};
 use ruff::settings::types::SerializationFormat;
 
 use crate::diagnostics::Diagnostics;
@@ -119,14 +119,11 @@ impl Printer {
                     writeln!(writer, "Found {remaining} error{s}.")?;
                 }
 
-                if let Some((safe_remaining, unsafe_remaining)) =
-                    applicable_fixes(self.autofix_level, diagnostics)
-                {
-                    writeln!(
-                        writer,
-                        "{}",
-                        display_violations(safe_remaining, unsafe_remaining)
-                    )?;
+                let fixables =
+                    FixableStatistics::new(diagnostics, self.autofix_level.suggested_fixes());
+
+                if !fixables.is_empty() {
+                    writeln!(writer, "{}", fixables.violation_string())?;
                 }
             } else {
                 let fixed = diagnostics
@@ -174,6 +171,7 @@ impl Printer {
         }
 
         let context = EmitterContext::new(&diagnostics.source_kind);
+        let fixables = FixableStatistics::new(diagnostics, self.autofix_level.suggested_fixes());
 
         match self.format {
             SerializationFormat::Json => {
@@ -187,9 +185,7 @@ impl Printer {
             }
             SerializationFormat::Text => {
                 TextEmitter::default()
-                    .with_show_fix_status(
-                        applicable_fixes(self.autofix_level, diagnostics).is_some(),
-                    )
+                    .with_show_fix_status(fixables.fixes_are_applicable())
                     .with_show_fix_diff(self.flags.contains(Flags::SHOW_FIX_DIFF))
                     .with_show_source(self.flags.contains(Flags::SHOW_SOURCE))
                     .emit(writer, &diagnostics.messages, &context)?;
@@ -207,9 +203,7 @@ impl Printer {
             SerializationFormat::Grouped => {
                 GroupedEmitter::default()
                     .with_show_source(self.flags.contains(Flags::SHOW_SOURCE))
-                    .with_show_fix_status(
-                        applicable_fixes(self.autofix_level, diagnostics).is_some(),
-                    )
+                    .with_show_fix_status(fixables.fixes_are_applicable())
                     .emit(writer, &diagnostics.messages, &context)?;
 
                 if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) {
@@ -359,14 +353,16 @@ impl Printer {
             );
         }
 
+        let fixables = FixableStatistics::new(diagnostics, self.autofix_level.suggested_fixes());
+
         if !diagnostics.messages.is_empty() {
             if self.log_level >= LogLevel::Default {
                 writeln!(writer)?;
             }
 
             let context = EmitterContext::new(&diagnostics.source_kind);
             TextEmitter::default()
-                .with_show_fix_status(applicable_fixes(self.autofix_level, diagnostics).is_some())
+                .with_show_fix_status(fixables.fixes_are_applicable())
                 .with_show_source(self.flags.contains(Flags::SHOW_SOURCE))
                 .emit(writer, &diagnostics.messages, &context)?;
         }
@@ -389,58 +385,55 @@ fn num_digits(n: usize) -> usize {
         .max(1)
 }
 
-/// Return the number of safe ([`Applicability::Automatic`]) and suggested ([`Applicability::Suggested`])
-/// fixes generated from a given [`Diagnostics`], contingent upon the mode/applicability levels
-/// passed by the user. Returns [`None`] if there are no applicable fixes.
-fn applicable_fixes(
-    autofix_level: flags::FixMode,
-    diagnostics: &Diagnostics,
-) -> Option<(u32, u32)> {
-    let mut safe_remaining = 0;
-    let mut unsafe_remaining = 0;
-    for message in diagnostics.messages.iter() {
-        if let Some(fix) = &message.fix {
-            if fix.applicability() == Applicability::Suggested {
-                unsafe_remaining += 1;
-            } else if fix.applicability() == Applicability::Automatic {
-                safe_remaining += 1;
-            }
-        }
-    }
-
-    // If we're in apply mode, calculate the status based on the applicability level of the
-    // fixes in question. If the user passed `--fix --suggested`, we can safely say that all fixes
-    // should've been applied in this pass, so we do not want to show the status (We're occasionally
-    // unable to determine whether a specific violation is fixable without trying to fix it, so
-    // if autofix is not enabled, we may inadvertently indicate that a rule is fixable.).
-    //
-    // If the user passed `--fix`, suggested fixes (if any were generated) would not have been
-    // applied. If this is the case, the fix status message will depend on the types of fixes that
-    // remain, so we'll have to return both.
-    //
-    // If we're NOT in apply mode, we want to only show a status if any fixes, regardless of
-    // safety, exist.
-    match autofix_level {
-        flags::FixMode::Apply(fix_suggested) => {
-            if matches!(fix_suggested, flags::SuggestedFixes::Apply) {
-                None
-            } else {
-                if safe_remaining > 0 || unsafe_remaining > 0 {
-                    Some((safe_remaining, unsafe_remaining))
-                } else {
-                    None
-                }
-            }
-        }
-        _ => {
-            if safe_remaining > 0 || unsafe_remaining > 0 {
-                Some((safe_remaining, unsafe_remaining))
-            } else {
-                None
-            }
-        }
-    }
-}
+//fn applicable_fixes(
+//    autofix_level: flags::FixMode,
+//    diagnostics: &Diagnostics,
+//) -> Option<FixableStatistics> {
+//    let mut safe_remaining = 0;
+//    let mut unsafe_remaining = 0;
+//    for message in diagnostics.messages.iter() {
+//        if let Some(fix) = &message.fix {
+//            if fix.applicability() == Applicability::Suggested {
+//                unsafe_remaining += 1;
+//            } else if fix.applicability() == Applicability::Automatic {
+//                safe_remaining += 1;
+//            }
+//        }
+//    }
+
+//    // If we're in apply mode, calculate the status based on the applicability level of the
+//    // fixes in question. If the user passed `--fix --suggested`, we can safely say that all fixes
+//    // should've been applied in this pass, so we do not want to show the status (We're occasionally
+//    // unable to determine whether a specific violation is fixable without trying to fix it, so
+//    // if autofix is not enabled, we may inadvertently indicate that a rule is fixable.).
+//    //
+//    // If the user passed `--fix`, suggested fixes (if any were generated) would not have been
+//    // applied. If this is the case, the fix status message will depend on the types of fixes that
+//    // remain, so we'll have to return both.
+//    //
+//    // If we're NOT in apply mode, we want to only show a status if any fixes, regardless of
+//    // safety, exist.
+//    match autofix_level {
+//        flags::FixMode::Apply(fix_suggested) => {
+//            if matches!(fix_suggested, flags::SuggestedFixes::Apply) {
+//                None
+//            } else {
+//                if safe_remaining > 0 || unsafe_remaining > 0 {
+//                    Some((safe_remaining, unsafe_remaining))
+//                } else {
+//                    None
+//                }
+//            }
+//        }
+//        _ => {
+//            if safe_remaining > 0 || unsafe_remaining > 0 {
+//                Some((safe_remaining, unsafe_remaining))
+//            } else {
+//                None
+//            }
+//        }
+//    }
+//}
 
 fn print_fix_summary(writer: &mut dyn Write, fixed: &FxHashMap<String, FixTable>) -> Result<()> {
     let total = fixed
@@ -483,28 +476,72 @@ fn print_fix_summary(writer: &mut dyn Write, fixed: &FxHashMap<String, FixTable>
     Ok(())
 }
 
-/// Build the displayed fix status message depending on the types of the remaining fixes.
-fn display_violations(safe_remaining: u32, unsafe_remaining: u32) -> String {
-    let prefix = format!("[{}]", "*".cyan());
-    let mut fix_status = prefix;
+/// Contains the number of [`Applicability::Automatic`] and [`Applicability::Suggested`] fixes
+struct FixableStatistics<'a> {
+    automatic: u32,
+    suggested: u32,
+    apply_suggested: &'a SuggestedFixes,
+}
+
+impl<'a> FixableStatistics<'a> {
+    fn new(diagnostics: &Diagnostics, apply_suggested: &'a SuggestedFixes) -> Self {
+        let mut automatic = 0;
+        let mut suggested = 0;
+
+        for message in &diagnostics.messages {
+            if let Some(fix) = &message.fix {
+                if fix.applicability() == Applicability::Suggested {
+                    suggested += 1;
+                } else if fix.applicability() == Applicability::Automatic {
+                    automatic += 1;
+                }
+            }
+        }
+
+        Self {
+            automatic,
+            suggested,
+            apply_suggested,
+        }
+    }
+
+    fn fixes_are_applicable(&self) -> bool {
+        match self.apply_suggested {
+            SuggestedFixes::Apply => self.automatic > 0 || self.suggested > 0,
+            SuggestedFixes::Disable => self.automatic > 0,
+        }
+    }
 
-    if safe_remaining > 0 {
-        fix_status =
-            format!("{fix_status} {safe_remaining} potentially fixable with the --fix option.");
+    /// Returns [`true`] if there aren't any fixes to be displayed
+    fn is_empty(&self) -> bool {
+        self.automatic == 0 && self.suggested == 0
     }
 
-    if unsafe_remaining > 0 {
-        let (line_break, extra_prefix) = if safe_remaining > 0 {
-            ("\n", format!("[{}]", "*".cyan()))
-        } else {
-            ("", String::new())
-        };
+    /// Build the displayed fix status message depending on the types of the remaining fixes.
+    fn violation_string(&self) -> String {
+        let prefix = format!("[{}]", "*".cyan());
+        let mut fix_status = prefix;
+
+        if self.automatic > 0 {
+            fix_status = format!(
+                "{fix_status} {} potentially fixable with the --fix option.",
+                self.automatic
+            );
+        }
+
+        if self.suggested > 0 {
+            let (line_break, extra_prefix) = if self.automatic > 0 {
+                ("\n", format!("[{}]", "*".cyan()))
+            } else {
+                ("", String::new())
+            };
 
-        let total = safe_remaining + unsafe_remaining;
-        fix_status = format!(
+            let total = self.automatic + self.suggested;
+            fix_status = format!(
             "{fix_status}{line_break}{extra_prefix} {total} potentially fixable with the --fix --unsafe options."
         );
-    }
+        }
 
-    fix_status
+        fix_status
+    }
 }

crates/ruff_diagnostics/src/fix.rs

@@ -5,7 +5,10 @@ use ruff_text_size::TextSize;
 
 use crate::edit::Edit;
 
-/// Indicates confidence in the correctness of a suggested fix.
+/// Indicates confidence in the correctness of a suggested fix. Rust internally allows comparison
+/// of enum values based on their order (see Rust's [enum
+/// documentation](https://doc.rust-lang.org/reference/items/enumerations.html)). This allows us to
+/// apply [`Fix`]es based on their [`Applicability`].
 #[derive(Default, Copy, Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum Applicability {