Update CLI to respect fix applicability (#7769)

Rebase of https://github.com/astral-sh/ruff/pull/5119 authored by
@evanrittenhouse with additional refinements.

## Changes

- Adds `--unsafe-fixes` / `--no-unsafe-fixes` flags to `ruff check`
- Violations with unsafe fixes are not shown as fixable unless opted-in
- Fix applicability is respected now
    - `Applicability::Never` fixes are no longer applied
    - `Applicability::Sometimes` fixes require opt-in
    - `Applicability::Always` fixes are unchanged
- Hints for availability of `--unsafe-fixes` added to `ruff check`
output

## Examples

Check hints at hidden unsafe fixes
```
❯ ruff check example.py --no-cache --select F601,W292
example.py:1:14: F601 Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
```

We could add an indicator for which violations have hidden fixes in the
future.

Check treats unsafe fixes as applicable with opt-in
```
❯ ruff check example.py --no-cache --select F601,W292 --unsafe-fixes
example.py:1:14: F601 [*] Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 2 fixable with the --fix option.
```

Also can be enabled in the config file

```
❯ cat ruff.toml
unsafe-fixes = true
```

And opted-out per invocation

```
❯ ruff check example.py --no-cache --select F601,W292 --no-unsafe-fixes
example.py:1:14: F601 Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
```

Diff does not include unsafe fixes
```
❯ ruff check example.py --no-cache --select F601,W292 --diff
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
 x = {'a': 1, 'a': 1}
-print(('foo'))
+print(('foo'))
\ No newline at end of file

Would fix 1 error.
```

Unless there is opt-in
```
❯ ruff check example.py --no-cache --select F601,W292 --diff --unsafe-fixes
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
-x = {'a': 1}
-print(('foo'))
+x = {'a': 1, 'a': 1}
+print(('foo'))
\ No newline at end of file

Would fix 2 errors.
```

https://github.com/astral-sh/ruff/pull/7790 will improve the diff
messages following this pull request

Similarly, `--fix` and `--fix-only` require the `--unsafe-fixes` flag to
apply unsafe fixes.

## Related

Replaces #5119
Closes https://github.com/astral-sh/ruff/issues/4185
Closes https://github.com/astral-sh/ruff/issues/7214
Closes https://github.com/astral-sh/ruff/issues/4845
Closes https://github.com/astral-sh/ruff/issues/3863
Addresses https://github.com/astral-sh/ruff/issues/6835
Addresses https://github.com/astral-sh/ruff/issues/7019
Needs follow-up https://github.com/astral-sh/ruff/issues/6962
Needs follow-up https://github.com/astral-sh/ruff/issues/4845
Needs follow-up https://github.com/astral-sh/ruff/issues/7436
Needs follow-up https://github.com/astral-sh/ruff/issues/7025
Needs follow-up https://github.com/astral-sh/ruff/issues/6434
Follow-up #7790 
Follow-up https://github.com/astral-sh/ruff/pull/7792

---------

Co-authored-by: Evan Rittenhouse <evanrittenhouse@gmail.com>

crates/ruff_cli/src/args.rs

@@ -9,6 +9,7 @@ use ruff_linter::logging::LogLevel;
 use ruff_linter::registry::Rule;
 use ruff_linter::settings::types::{
     FilePattern, PatternPrefixPair, PerFileIgnore, PreviewMode, PythonVersion, SerializationFormat,
+    UnsafeFixes,
 };
 use ruff_linter::{RuleParser, RuleSelector, RuleSelectorParser};
 use ruff_workspace::configuration::{Configuration, RuleSelection};
@@ -76,12 +77,18 @@ pub enum Command {
 pub struct CheckCommand {
     /// List of files or directories to check.
     pub files: Vec<PathBuf>,
-    /// Attempt to automatically fix lint violations.
-    /// Use `--no-fix` to disable.
+    /// Apply fixes to resolve lint violations.
+    /// Use `--no-fix` to disable or `--unsafe-fixes` to include unsafe fixes.
     #[arg(long, overrides_with("no_fix"))]
     fix: bool,
     #[clap(long, overrides_with("fix"), hide = true)]
     no_fix: bool,
+    /// Include fixes that may not retain the original intent of the code.
+    /// Use `--no-unsafe-fixes` to disable.
+    #[arg(long, overrides_with("no_unsafe_fixes"))]
+    unsafe_fixes: bool,
+    #[arg(long, overrides_with("unsafe_fixes"), hide = true)]
+    no_unsafe_fixes: bool,
     /// Show violations with source code.
     /// Use `--no-show-source` to disable.
     #[arg(long, overrides_with("no_show_source"))]
@@ -100,8 +107,8 @@ pub struct CheckCommand {
     /// Run in watch mode by re-running whenever files change.
     #[arg(short, long)]
     pub watch: bool,
-    /// Fix any fixable lint violations, but don't report on leftover violations. Implies `--fix`.
-    /// Use `--no-fix-only` to disable.
+    /// Apply fixes to resolve lint violations, but don't report on leftover violations. Implies `--fix`.
+    /// Use `--no-fix-only` to disable or `--unsafe-fixes` to include unsafe fixes.
     #[arg(long, overrides_with("no_fix_only"))]
     fix_only: bool,
     #[clap(long, overrides_with("fix_only"), hide = true)]
@@ -497,6 +504,8 @@ impl CheckCommand {
                 cache_dir: self.cache_dir,
                 fix: resolve_bool_arg(self.fix, self.no_fix),
                 fix_only: resolve_bool_arg(self.fix_only, self.no_fix_only),
+                unsafe_fixes: resolve_bool_arg(self.unsafe_fixes, self.no_unsafe_fixes)
+                    .map(UnsafeFixes::from),
                 force_exclude: resolve_bool_arg(self.force_exclude, self.no_force_exclude),
                 output_format: self.output_format.or(self.format),
                 show_fixes: resolve_bool_arg(self.show_fixes, self.no_show_fixes),
@@ -599,6 +608,7 @@ pub struct CliOverrides {
     pub cache_dir: Option<PathBuf>,
     pub fix: Option<bool>,
     pub fix_only: Option<bool>,
+    pub unsafe_fixes: Option<UnsafeFixes>,
     pub force_exclude: Option<bool>,
     pub output_format: Option<SerializationFormat>,
     pub show_fixes: Option<bool>,
@@ -624,6 +634,9 @@ impl ConfigurationTransformer for CliOverrides {
         if let Some(fix_only) = &self.fix_only {
             config.fix_only = Some(*fix_only);
         }
+        if self.unsafe_fixes.is_some() {
+            config.unsafe_fixes = self.unsafe_fixes;
+        }
         config.lint.rule_selections.push(RuleSelection {
             select: self.select.clone(),
             ignore: self

crates/ruff_cli/src/cache.rs

@@ -338,6 +338,7 @@ pub(crate) fn init(path: &Path) -> Result<()> {
 #[cfg(test)]
 mod tests {
     use filetime::{set_file_mtime, FileTime};
+    use ruff_linter::settings::types::UnsafeFixes;
     use std::env::temp_dir;
     use std::fs;
     use std::io;
@@ -410,6 +411,7 @@ mod tests {
                     Some(&cache),
                     flags::Noqa::Enabled,
                     flags::FixMode::Generate,
+                    UnsafeFixes::Enabled,
                 )
                 .unwrap();
                 if diagnostics
@@ -455,6 +457,7 @@ mod tests {
                 Some(&cache),
                 flags::Noqa::Enabled,
                 flags::FixMode::Generate,
+                UnsafeFixes::Enabled,
             )
             .unwrap();
         }
@@ -712,6 +715,7 @@ mod tests {
                 Some(cache),
                 flags::Noqa::Enabled,
                 flags::FixMode::Generate,
+                UnsafeFixes::Enabled,
             )
         }
     }

crates/ruff_cli/src/commands/check.rs

@@ -11,6 +11,7 @@ use itertools::Itertools;
 use log::{debug, error, warn};
 #[cfg(not(target_family = "wasm"))]
 use rayon::prelude::*;
+use ruff_linter::settings::types::UnsafeFixes;
 use rustc_hash::FxHashMap;
 
 use ruff_diagnostics::Diagnostic;
@@ -36,6 +37,7 @@ pub(crate) fn check(
     cache: flags::Cache,
     noqa: flags::Noqa,
     fix_mode: flags::FixMode,
+    unsafe_fixes: UnsafeFixes,
 ) -> Result<Diagnostics> {
     // Collect all the Python files to check.
     let start = Instant::now();
@@ -119,7 +121,16 @@ pub(crate) fn check(
                         }
                     });
 
-                    lint_path(path, package, &settings.linter, cache, noqa, fix_mode).map_err(|e| {
+                    lint_path(
+                        path,
+                        package,
+                        &settings.linter,
+                        cache,
+                        noqa,
+                        fix_mode,
+                        unsafe_fixes,
+                    )
+                    .map_err(|e| {
                         (Some(path.to_owned()), {
                             let mut error = e.to_string();
                             for cause in e.chain() {
@@ -199,9 +210,10 @@ fn lint_path(
     cache: Option<&Cache>,
     noqa: flags::Noqa,
     fix_mode: flags::FixMode,
+    unsafe_fixes: UnsafeFixes,
 ) -> Result<Diagnostics> {
     let result = catch_unwind(|| {
-        crate::diagnostics::lint_path(path, package, settings, cache, noqa, fix_mode)
+        crate::diagnostics::lint_path(path, package, settings, cache, noqa, fix_mode, unsafe_fixes)
     });
 
     match result {
@@ -233,6 +245,8 @@ mod test {
     use std::os::unix::fs::OpenOptionsExt;
 
     use anyhow::Result;
+
+    use ruff_linter::settings::types::UnsafeFixes;
     use rustc_hash::FxHashMap;
     use tempfile::TempDir;
 
@@ -285,6 +299,7 @@ mod test {
             flags::Cache::Disabled,
             flags::Noqa::Disabled,
             flags::FixMode::Generate,
+            UnsafeFixes::Enabled,
         )
         .unwrap();
         let mut output = Vec::new();

crates/ruff_cli/src/diagnostics.rs

@@ -11,6 +11,7 @@ use anyhow::{Context, Result};
 use colored::Colorize;
 use filetime::FileTime;
 use log::{debug, error, warn};
+use ruff_linter::settings::types::UnsafeFixes;
 use rustc_hash::FxHashMap;
 
 use ruff_diagnostics::Diagnostic;
@@ -168,6 +169,7 @@ pub(crate) fn lint_path(
     cache: Option<&Cache>,
     noqa: flags::Noqa,
     fix_mode: flags::FixMode,
+    unsafe_fixes: UnsafeFixes,
 ) -> Result<Diagnostics> {
     // Check the cache.
     // TODO(charlie): `fixer::Mode::Apply` and `fixer::Mode::Diff` both have
@@ -244,8 +246,15 @@ pub(crate) fn lint_path(
             result,
             transformed,
             fixed,
-        }) = lint_fix(path, package, noqa, settings, &source_kind, source_type)
-        {
+        }) = lint_fix(
+            path,
+            package,
+            noqa,
+            unsafe_fixes,
+            settings,
+            &source_kind,
+            source_type,
+        ) {
             if !fixed.is_empty() {
                 match fix_mode {
                     flags::FixMode::Apply => transformed.write(&mut File::create(path)?)?,
@@ -355,6 +364,7 @@ pub(crate) fn lint_stdin(
             path.unwrap_or_else(|| Path::new("-")),
             package,
             noqa,
+            settings.unsafe_fixes,
             &settings.linter,
             &source_kind,
             source_type,

crates/ruff_cli/src/lib.rs

@@ -10,7 +10,7 @@ use log::warn;
 use notify::{recommended_watcher, RecursiveMode, Watcher};
 
 use ruff_linter::logging::{set_up_logging, LogLevel};
-use ruff_linter::settings::flags;
+use ruff_linter::settings::flags::FixMode;
 use ruff_linter::settings::types::SerializationFormat;
 use ruff_linter::{fs, warn_user, warn_user_once};
 use ruff_workspace::Settings;
@@ -228,6 +228,7 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
     let Settings {
         fix,
         fix_only,
+        unsafe_fixes,
         output_format,
         show_fixes,
         show_source,
@@ -236,17 +237,20 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
 
     // Fix rules are as follows:
     // - By default, generate all fixes, but don't apply them to the filesystem.
-    // - If `--fix` or `--fix-only` is set, always apply fixes to the filesystem (or
+    // - If `--fix` or `--fix-only` is set, apply applicable fixes to the filesystem (or
     //   print them to stdout, if we're reading from stdin).
-    // - If `--diff` or `--fix-only` are set, don't print any violations (only
-    //   fixes).
+    // - If `--diff` or `--fix-only` are set, don't print any violations (only applicable fixes)
+    // - By default, applicable fixes only include [`Applicablility::Automatic`], but if
+    //   `--unsafe-fixes` is set, then [`Applicablility::Suggested`] fixes are included.
+
     let fix_mode = if cli.diff {
-        flags::FixMode::Diff
+        FixMode::Diff
     } else if fix || fix_only {
-        flags::FixMode::Apply
+        FixMode::Apply
     } else {
-        flags::FixMode::Generate
+        FixMode::Generate
     };
+
     let cache = !cli.no_cache;
     let noqa = !cli.ignore_noqa;
     let mut printer_flags = PrinterFlags::empty();
@@ -290,7 +294,13 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
         return Ok(ExitStatus::Success);
     }
 
-    let printer = Printer::new(output_format, log_level, fix_mode, printer_flags);
+    let printer = Printer::new(
+        output_format,
+        log_level,
+        fix_mode,
+        unsafe_fixes,
+        printer_flags,
+    );
 
     if cli.watch {
         if output_format != SerializationFormat::Text {
@@ -318,6 +328,7 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
             cache.into(),
             noqa.into(),
             fix_mode,
+            unsafe_fixes,
         )?;
         printer.write_continuously(&mut writer, &messages)?;
 
@@ -350,6 +361,7 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
                         cache.into(),
                         noqa.into(),
                         fix_mode,
+                        unsafe_fixes,
                     )?;
                     printer.write_continuously(&mut writer, &messages)?;
                 }
@@ -376,13 +388,14 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
                 cache.into(),
                 noqa.into(),
                 fix_mode,
+                unsafe_fixes,
             )?
         };
 
         // Always try to print violations (the printer itself may suppress output),
         // unless we're writing fixes via stdin (in which case, the transformed
         // source code goes to stdout).
-        if !(is_stdin && matches!(fix_mode, flags::FixMode::Apply | flags::FixMode::Diff)) {
+        if !(is_stdin && matches!(fix_mode, FixMode::Apply | FixMode::Diff)) {
             if cli.statistics {
                 printer.write_statistics(&diagnostics, &mut writer)?;
             } else {

crates/ruff_cli/src/printer.rs

@@ -19,8 +19,8 @@ use ruff_linter::message::{
 };
 use ruff_linter::notify_user;
 use ruff_linter::registry::{AsRule, Rule};
-use ruff_linter::settings::flags;
-use ruff_linter::settings::types::SerializationFormat;
+use ruff_linter::settings::flags::{self};
+use ruff_linter::settings::types::{SerializationFormat, UnsafeFixes};
 
 use crate::diagnostics::Diagnostics;
 
@@ -73,6 +73,7 @@ pub(crate) struct Printer {
     format: SerializationFormat,
     log_level: LogLevel,
     fix_mode: flags::FixMode,
+    unsafe_fixes: UnsafeFixes,
     flags: Flags,
 }
 
@@ -81,12 +82,14 @@ impl Printer {
         format: SerializationFormat,
         log_level: LogLevel,
         fix_mode: flags::FixMode,
+        unsafe_fixes: UnsafeFixes,
         flags: Flags,
     ) -> Self {
         Self {
             format,
             log_level,
             fix_mode,
+            unsafe_fixes,
             flags,
         }
     }
@@ -118,19 +121,8 @@ impl Printer {
                     writeln!(writer, "Found {remaining} error{s}.")?;
                 }
 
-                if show_fix_status(self.fix_mode) {
-                    let num_fixable = diagnostics
-                        .messages
-                        .iter()
-                        .filter(|message| message.fix.is_some())
-                        .count();
-                    if num_fixable > 0 {
-                        writeln!(
-                            writer,
-                            "[{}] {num_fixable} potentially fixable with the --fix option.",
-                            "*".cyan(),
-                        )?;
-                    }
+                if let Some(fixables) = FixableSummary::try_from(diagnostics, self.unsafe_fixes) {
+                    writeln!(writer, "{fixables}")?;
                 }
             } else {
                 let fixed = diagnostics
@@ -178,6 +170,7 @@ impl Printer {
         }
 
         let context = EmitterContext::new(&diagnostics.notebook_indexes);
+        let fixables = FixableSummary::try_from(diagnostics, self.unsafe_fixes);
 
         match self.format {
             SerializationFormat::Json => {
@@ -191,9 +184,10 @@ impl Printer {
             }
             SerializationFormat::Text => {
                 TextEmitter::default()
-                    .with_show_fix_status(show_fix_status(self.fix_mode))
+                    .with_show_fix_status(show_fix_status(self.fix_mode, fixables.as_ref()))
                     .with_show_fix_diff(self.flags.intersects(Flags::SHOW_FIX_DIFF))
                     .with_show_source(self.flags.intersects(Flags::SHOW_SOURCE))
+                    .with_unsafe_fixes(self.unsafe_fixes)
                     .emit(writer, &diagnostics.messages, &context)?;
 
                 if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) {
@@ -209,7 +203,8 @@ impl Printer {
             SerializationFormat::Grouped => {
                 GroupedEmitter::default()
                     .with_show_source(self.flags.intersects(Flags::SHOW_SOURCE))
-                    .with_show_fix_status(show_fix_status(self.fix_mode))
+                    .with_show_fix_status(show_fix_status(self.fix_mode, fixables.as_ref()))
+                    .with_unsafe_fixes(self.unsafe_fixes)
                     .emit(writer, &diagnostics.messages, &context)?;
 
                 if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) {
@@ -359,6 +354,8 @@ impl Printer {
             );
         }
 
+        let fixables = FixableSummary::try_from(diagnostics, self.unsafe_fixes);
+
         if !diagnostics.messages.is_empty() {
             if self.log_level >= LogLevel::Default {
                 writeln!(writer)?;
@@ -366,8 +363,9 @@ impl Printer {
 
             let context = EmitterContext::new(&diagnostics.notebook_indexes);
             TextEmitter::default()
-                .with_show_fix_status(show_fix_status(self.fix_mode))
+                .with_show_fix_status(show_fix_status(self.fix_mode, fixables.as_ref()))
                 .with_show_source(self.flags.intersects(Flags::SHOW_SOURCE))
+                .with_unsafe_fixes(self.unsafe_fixes)
                 .emit(writer, &diagnostics.messages, &context)?;
         }
         writer.flush()?;
@@ -390,13 +388,13 @@ fn num_digits(n: usize) -> usize {
 }
 
 /// Return `true` if the [`Printer`] should indicate that a rule is fixable.
-const fn show_fix_status(fix_mode: flags::FixMode) -> bool {
+fn show_fix_status(fix_mode: flags::FixMode, fixables: Option<&FixableSummary>) -> bool {
     // If we're in application mode, avoid indicating that a rule is fixable.
     // If the specific violation were truly fixable, it would've been fixed in
     // this pass! (We're occasionally unable to determine whether a specific
     // violation is fixable without trying to fix it, so if fix is not
     // enabled, we may inadvertently indicate that a rule is fixable.)
-    !fix_mode.is_apply()
+    (!fix_mode.is_apply()) && fixables.is_some_and(FixableSummary::any_applicable_fixes)
 }
 
 fn print_fix_summary(writer: &mut dyn Write, fixed: &FxHashMap<String, FixTable>) -> Result<()> {
@@ -439,3 +437,80 @@ fn print_fix_summary(writer: &mut dyn Write, fixed: &FxHashMap<String, FixTable>
     }
     Ok(())
 }
+
+/// Summarizes [applicable][ruff_diagnostics::Applicability] fixes.
+#[derive(Debug)]
+struct FixableSummary {
+    applicable: u32,
+    unapplicable: u32,
+    unsafe_fixes: UnsafeFixes,
+}
+
+impl FixableSummary {
+    fn try_from(diagnostics: &Diagnostics, unsafe_fixes: UnsafeFixes) -> Option<Self> {
+        let mut applicable = 0;
+        let mut unapplicable = 0;
+
+        for message in &diagnostics.messages {
+            if let Some(fix) = &message.fix {
+                if fix.applies(unsafe_fixes.required_applicability()) {
+                    applicable += 1;
+                } else {
+                    unapplicable += 1;
+                }
+            }
+        }
+
+        if applicable == 0 && unapplicable == 0 {
+            None
+        } else {
+            Some(Self {
+                applicable,
+                unapplicable,
+                unsafe_fixes,
+            })
+        }
+    }
+
+    fn any_applicable_fixes(&self) -> bool {
+        self.applicable > 0
+    }
+}
+
+impl Display for FixableSummary {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let fix_prefix = format!("[{}]", "*".cyan());
+
+        if self.unsafe_fixes.is_enabled() {
+            write!(
+                f,
+                "{fix_prefix} {} fixable with the --fix option.",
+                self.applicable
+            )
+        } else {
+            if self.applicable > 0 && self.unapplicable > 0 {
+                let es = if self.unapplicable == 1 { "" } else { "es" };
+                write!(
+                    f,
+                    "{fix_prefix} {} fixable with the `--fix` option ({} hidden fix{es} can be enabled with the `--unsafe-fixes` option).",
+                    self.applicable, self.unapplicable
+                )
+            } else if self.applicable > 0 {
+                // Only applicable fixes
+                write!(
+                    f,
+                    "{fix_prefix} {} fixable with the `--fix` option.",
+                    self.applicable,
+                )
+            } else {
+                // Only unapplicable fixes
+                let es = if self.unapplicable == 1 { "" } else { "es" };
+                write!(
+                    f,
+                    "{} hidden fix{es} can be enabled with the `--unsafe-fixes` option.",
+                    self.unapplicable
+                )
+            }
+        }
+    }
+}