Show errors for attempted fixes only when passed --verbose (#15237)

The default logging level for diagnostics includes logs written using
the `log` crate with level `error`, `warn`, and `info`. An unsuccessful
fix attached to a diagnostic via `try_set_fix` or `try_set_optional_fix`
was logged at level `error`. Note that the user would see these messages
even without passing `--fix`, and possibly also on lines with `noqa`
comments.

This PR changes the logging level here to a `debug`. We also found
ad-hoc instances of error logging in the implementations of several
rules, and have replaced those with either a `debug` or call to
`try_set{_optional}_fix`.

Closes #15229

crates/ruff/tests/integration_test.rs

@@ -2126,3 +2126,67 @@ unfixable = ["RUF"]
 
     Ok(())
 }
+
+#[test]
+fn verbose_show_failed_fix_errors() {
+    let mut cmd = RuffCheck::default()
+        .args(["--select", "UP006", "--preview", "-v"])
+        .build();
+
+    insta::with_settings!(
+            {
+                // the logs have timestamps we need to remove
+                filters => vec![(
+                    r"\[[\d:-]+]",
+                    ""
+                )]
+            },{
+    assert_cmd_snapshot!(cmd
+            .pass_stdin("import typing\nCallable = 'abc'\ndef g() -> typing.Callable: ..."),
+                @r###"
+    success: false
+    exit_code: 1
+    ----- stdout -----
+    -:3:12: UP006 Use `collections.abc.Callable` instead of `typing.Callable` for type annotation
+      |
+    1 | import typing
+    2 | Callable = 'abc'
+    3 | def g() -> typing.Callable: ...
+      |            ^^^^^^^^^^^^^^^ UP006
+      |
+      = help: Replace with `collections.abc.Callable`
+
+    Found 1 error.
+
+    ----- stderr -----
+    [ruff::resolve][DEBUG] Isolated mode, not reading any pyproject.toml
+    [ruff_diagnostics::diagnostic][DEBUG] Failed to create fix for NonPEP585Annotation: Unable to insert `Callable` into scope due to name conflict
+    "###);        }
+        );
+}
+
+#[test]
+fn no_verbose_hide_failed_fix_errors() {
+    let mut cmd = RuffCheck::default()
+        .args(["--select", "UP006", "--preview"])
+        .build();
+    assert_cmd_snapshot!(cmd
+        .pass_stdin("import typing\nCallable = 'abc'\ndef g() -> typing.Callable: ..."),
+            @r###"
+    success: false
+    exit_code: 1
+    ----- stdout -----
+    -:3:12: UP006 Use `collections.abc.Callable` instead of `typing.Callable` for type annotation
+      |
+    1 | import typing
+    2 | Callable = 'abc'
+    3 | def g() -> typing.Callable: ...
+      |            ^^^^^^^^^^^^^^^ UP006
+      |
+      = help: Replace with `collections.abc.Callable`
+
+    Found 1 error.
+
+    ----- stderr -----
+    "###);
+}

crates/ruff_diagnostics/src/diagnostic.rs

@@ -1,5 +1,5 @@
 use anyhow::Result;
-use log::error;
+use log::debug;
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
@@ -56,7 +56,7 @@ impl Diagnostic {
     pub fn try_set_fix(&mut self, func: impl FnOnce() -> Result<Fix>) {
         match func() {
             Ok(fix) => self.fix = Some(fix),
-            Err(err) => error!("Failed to create fix for {}: {}", self.kind.name, err),
+            Err(err) => debug!("Failed to create fix for {}: {}", self.kind.name, err),
         }
     }
 
@@ -67,7 +67,7 @@ impl Diagnostic {
         match func() {
             Ok(None) => {}
             Ok(Some(fix)) => self.fix = Some(fix),
-            Err(err) => error!("Failed to create fix for {}: {}", self.kind.name, err),
+            Err(err) => debug!("Failed to create fix for {}: {}", self.kind.name, err),
         }
     }
 

crates/ruff_linter/src/rules/flake8_simplify/rules/ast_with.rs

@@ -1,5 +1,5 @@
+use anyhow::bail;
 use ast::Expr;
-use log::error;
 
 use ruff_diagnostics::{Diagnostic, Fix};
 use ruff_diagnostics::{FixAvailability, Violation};
@@ -170,26 +170,30 @@ pub(crate) fn multiple_with_statements(
             .comment_ranges()
             .intersects(TextRange::new(with_stmt.start(), with_stmt.body[0].start()))
         {
-            match fix_with::fix_multiple_with_statements(
-                checker.locator(),
-                checker.stylist(),
-                with_stmt,
-            ) {
-                Ok(edit) => {
-                    if edit.content().map_or(true, |content| {
-                        fits(
-                            content,
-                            with_stmt.into(),
-                            checker.locator(),
-                            checker.settings.pycodestyle.max_line_length,
-                            checker.settings.tab_size,
-                        )
-                    }) {
-                        diagnostic.set_fix(Fix::unsafe_edit(edit));
+            diagnostic.try_set_optional_fix(|| {
+                match fix_with::fix_multiple_with_statements(
+                    checker.locator(),
+                    checker.stylist(),
+                    with_stmt,
+                ) {
+                    Ok(edit) => {
+                        if edit.content().map_or(true, |content| {
+                            fits(
+                                content,
+                                with_stmt.into(),
+                                checker.locator(),
+                                checker.settings.pycodestyle.max_line_length,
+                                checker.settings.tab_size,
+                            )
+                        }) {
+                            Ok(Some(Fix::unsafe_edit(edit)))
+                        } else {
+                            Ok(None)
+                        }
                     }
+                    Err(err) => bail!("Failed to collapse `with`: {err}"),
                 }
-                Err(err) => error!("Failed to fix nested with: {err}"),
-            }
+            });
         }
         checker.diagnostics.push(diagnostic);
     }

crates/ruff_linter/src/rules/flake8_simplify/rules/collapsible_if.rs

@@ -2,7 +2,6 @@ use std::borrow::Cow;
 
 use anyhow::{bail, Result};
 use libcst_native::ParenthesizedNode;
-use log::error;
 
 use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
@@ -118,22 +117,26 @@ pub(crate) fn nested_if_statements(
         nested_if.start(),
         nested_if.body()[0].start(),
     )) {
-        match collapse_nested_if(checker.locator(), checker.stylist(), nested_if) {
-            Ok(edit) => {
-                if edit.content().map_or(true, |content| {
-                    fits(
-                        content,
-                        (&nested_if).into(),
-                        checker.locator(),
-                        checker.settings.pycodestyle.max_line_length,
-                        checker.settings.tab_size,
-                    )
-                }) {
-                    diagnostic.set_fix(Fix::unsafe_edit(edit));
+        diagnostic.try_set_optional_fix(|| {
+            match collapse_nested_if(checker.locator(), checker.stylist(), nested_if) {
+                Ok(edit) => {
+                    if edit.content().map_or(true, |content| {
+                        fits(
+                            content,
+                            (&nested_if).into(),
+                            checker.locator(),
+                            checker.settings.pycodestyle.max_line_length,
+                            checker.settings.tab_size,
+                        )
+                    }) {
+                        Ok(Some(Fix::unsafe_edit(edit)))
+                    } else {
+                        Ok(None)
+                    }
                 }
+                Err(err) => bail!("Failed to collapse `if`: {err}"),
             }
-            Err(err) => error!("Failed to fix nested if: {err}"),
-        }
+        });
     }
     checker.diagnostics.push(diagnostic);
 }

crates/ruff_linter/src/rules/pyflakes/rules/invalid_literal_comparisons.rs

@@ -1,4 +1,4 @@
-use log::error;
+use anyhow::{bail, Error};
 
 use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
@@ -94,24 +94,29 @@ pub(crate) fn invalid_literal_comparison(
             if lazy_located.is_none() {
                 lazy_located = Some(locate_cmp_ops(expr, checker.tokens()));
             }
-            if let Some(located_op) = lazy_located.as_ref().and_then(|located| located.get(index)) {
-                assert_eq!(located_op.op, *op);
-                if let Some(content) = match located_op.op {
-                    CmpOp::Is => Some("==".to_string()),
-                    CmpOp::IsNot => Some("!=".to_string()),
-                    node => {
-                        error!("Failed to fix invalid comparison: {node:?}");
-                        None
+            diagnostic.try_set_optional_fix(|| {
+                if let Some(located_op) =
+                    lazy_located.as_ref().and_then(|located| located.get(index))
+                {
+                    assert_eq!(located_op.op, *op);
+                    if let Ok(content) = match located_op.op {
+                        CmpOp::Is => Ok::<String, Error>("==".to_string()),
+                        CmpOp::IsNot => Ok("!=".to_string()),
+                        node => {
+                            bail!("Failed to fix invalid comparison: {node:?}")
+                        }
+                    } {
+                        Ok(Some(Fix::safe_edit(Edit::range_replacement(
+                            content,
+                            located_op.range,
+                        ))))
+                    } else {
+                        Ok(None)
                     }
-                } {
-                    diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
-                        content,
-                        located_op.range,
-                    )));
+                } else {
+                    bail!("Failed to fix invalid comparison due to missing op")
                 }
-            } else {
-                error!("Failed to fix invalid comparison due to missing op");
-            }
+            });
             checker.diagnostics.push(diagnostic);
         }
         left = right;

crates/ruff_linter/src/rules/pyupgrade/rules/deprecated_mock_import.rs

@@ -3,7 +3,7 @@ use libcst_native::{
     AsName, AssignTargetExpression, Attribute, Dot, Expression, Import, ImportAlias, ImportFrom,
     ImportNames, Name, NameOrAttribute, ParenthesizableWhitespace,
 };
-use log::error;
+use log::debug;
 
 use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
@@ -286,7 +286,7 @@ pub(crate) fn deprecated_mock_import(checker: &mut Checker, stmt: &Stmt) {
                     match format_import(stmt, indent, checker.locator(), checker.stylist()) {
                         Ok(content) => Some(content),
                         Err(e) => {
-                            error!("Failed to rewrite `mock` import: {e}");
+                            debug!("Failed to rewrite `mock` import: {e}");
                             None
                         }
                     }