[flake8-logging-format] Add auto-fix for f-string logging calls (G004)

crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G004.py

@@ -17,3 +17,50 @@
 # Don't trigger for t-strings
 info(t"{name}")
 info(t"{__name__}")
+
+count = 5
+total = 9
+directory_path = "/home/hamir/ruff/crates/ruff_linter/resources/test/"
+logging.info(f"{count} out of {total} files in {directory_path} checked")
+
+
+
+x = 99
+fmt = "08d"
+logger.info(f"{x:{'08d'}}")
+logger.info(f"{x:>10} {x:{fmt}}")
+
+logging.info(f"")
+logging.info(f"This message doesn't have any variables.")
+
+obj = {"key": "value"}
+logging.info(f"Object: {obj!r}")
+
+items_count = 3
+logging.warning(f"Items: {items_count:d}")
+
+data = {"status": "active"}
+logging.info(f"Processing {len(data)} items")
+logging.info(f"Status: {data.get('status', 'unknown').upper()}")
+
+
+result = 123
+logging.info(f"Calculated result: {result + 100}")
+
+temperature = 123
+logging.info(f"Temperature: {temperature:.1f}°C")
+
+class FilePath:
+    def __init__(self, name: str):
+        self.name = name
+
+logging.info(f"No changes made to {file_path.name}.")
+
+user = "tron"
+balance = 123.45
+logging.error(f"Error {404}: User {user} has insufficient balance ${balance:.2f}")
+
+import logging
+
+x = 1
+logging.error(f"{x} -> %s", x)

crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G004_arg_order.py

@@ -0,0 +1,10 @@
+"""Test f-string argument order."""
+
+import logging
+
+logger = logging.getLogger(__name__)
+
+X = 1
+Y = 2
+logger.error(f"{X} -> %s", Y)
+logger.error(f"{Y} -> %s", X)

crates/ruff_linter/src/preview.rs

@@ -40,6 +40,11 @@ pub(crate) const fn is_bad_version_info_in_non_stub_enabled(settings: &LinterSet
     settings.preview.is_enabled()
 }
 
+/// <https://github.com/astral-sh/ruff/pull/19303>
+pub(crate) const fn is_fix_f_string_logging_enabled(settings: &LinterSettings) -> bool {
+    settings.preview.is_enabled()
+}
+
 // https://github.com/astral-sh/ruff/pull/16719
 pub(crate) const fn is_fix_manual_dict_comprehension_enabled(settings: &LinterSettings) -> bool {
     settings.preview.is_enabled()

crates/ruff_linter/src/rules/flake8_logging_format/mod.rs

@@ -22,6 +22,7 @@ mod tests {
     #[test_case(Path::new("G002.py"))]
     #[test_case(Path::new("G003.py"))]
     #[test_case(Path::new("G004.py"))]
+    #[test_case(Path::new("G004_arg_order.py"))]
     #[test_case(Path::new("G010.py"))]
     #[test_case(Path::new("G101_1.py"))]
     #[test_case(Path::new("G101_2.py"))]
@@ -48,4 +49,24 @@ mod tests {
         assert_diagnostics!(snapshot, diagnostics);
         Ok(())
     }
+
+    #[test_case(Rule::LoggingFString, Path::new("G004.py"))]
+    #[test_case(Rule::LoggingFString, Path::new("G004_arg_order.py"))]
+    fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
+        let snapshot = format!(
+            "preview__{}_{}",
+            rule_code.noqa_code(),
+            path.to_string_lossy()
+        );
+        let diagnostics = test_path(
+            Path::new("flake8_logging_format").join(path).as_path(),
+            &settings::LinterSettings {
+                logger_objects: vec!["logging_setup.logger".to_string()],
+                preview: settings::types::PreviewMode::Enabled,
+                ..settings::LinterSettings::for_rule(rule_code)
+            },
+        )?;
+        assert_diagnostics!(snapshot, diagnostics);
+        Ok(())
+    }
 }

crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs

@@ -1,16 +1,99 @@
-use ruff_python_ast::{self as ast, Arguments, Expr, Keyword, Operator};
+use ruff_python_ast::InterpolatedStringElement;
+use ruff_python_ast::{self as ast, Arguments, Expr, Keyword, Operator, StringFlags};
 use ruff_python_semantic::analyze::logging;
 use ruff_python_stdlib::logging::LoggingLevel;
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
+use crate::preview::is_fix_f_string_logging_enabled;
 use crate::registry::Rule;
 use crate::rules::flake8_logging_format::violations::{
     LoggingExcInfo, LoggingExtraAttrClash, LoggingFString, LoggingPercentFormat,
     LoggingRedundantExcInfo, LoggingStringConcat, LoggingStringFormat, LoggingWarn,
 };
 use crate::{Edit, Fix};
 
+fn logging_f_string(
+    checker: &Checker,
+    msg: &Expr,
+    f_string: &ast::ExprFString,
+    arguments: &Arguments,
+    msg_pos: usize,
+) {
+    // Report the diagnostic up-front so we can attach a fix later only when preview is enabled.
+    let mut diagnostic = checker.report_diagnostic(LoggingFString, msg.range());
+
+    // Preview gate for the automatic fix.
+    if !is_fix_f_string_logging_enabled(checker.settings()) {
+        return;
+    }
+
+    // If there are existing positional arguments after the message, bail out.
+    // This could indicate a mistake or complex usage we shouldn't try to fix.
+    if arguments.args.len() > msg_pos + 1 {
+        return;
+    }
+
+    let mut format_string = String::new();
+    let mut args: Vec<&str> = Vec::new();
+
+    // Try to reuse the first part's quote style when building the replacement.
+    // Default to double quotes if we can't determine it.
+    let quote_str = f_string
+        .value
+        .f_strings()
+        .next()
+        .map(|f| f.flags.quote_str())
+        .unwrap_or("\"");
+
+    for f in f_string.value.f_strings() {
+        for element in &f.elements {
+            match element {
+                InterpolatedStringElement::Literal(lit) => {
+                    // If the literal text contains a '%' placeholder, bail out: mixing
+                    // f-string interpolation with '%' placeholders is ambiguous for our
+                    // automatic conversion, so don't offer a fix for this case.
+                    if lit.value.as_ref().contains('%') {
+                        return;
+                    }
+                    format_string.push_str(lit.value.as_ref());
+                }
+                InterpolatedStringElement::Interpolation(interpolated) => {
+                    if interpolated.format_spec.is_some()
+                        || !matches!(
+                            interpolated.conversion,
+                            ruff_python_ast::ConversionFlag::None
+                        )
+                    {
+                        return;
+                    }
+                    match interpolated.expression.as_ref() {
+                        Expr::Name(name) => {
+                            format_string.push_str("%s");
+                            args.push(name.id.as_str());
+                        }
+                        _ => return,
+                    }
+                }
+            }
+        }
+    }
+
+    if args.is_empty() {
+        return;
+    }
+
+    let replacement = format!(
+        "{q}{format_string}{q}, {args}",
+        q = quote_str,
+        format_string = format_string,
+        args = args.join(", ")
+    );
+
+    let fix = Fix::safe_edit(Edit::range_replacement(replacement, msg.range()));
+    diagnostic.set_fix(fix);
+}
+
 /// Returns `true` if the attribute is a reserved attribute on the `logging` module's `LogRecord`
 /// class.
 fn is_reserved_attr(attr: &str) -> bool {
@@ -42,7 +125,7 @@ fn is_reserved_attr(attr: &str) -> bool {
 }
 
 /// Check logging messages for violations.
-fn check_msg(checker: &Checker, msg: &Expr) {
+fn check_msg(checker: &Checker, msg: &Expr, arguments: &Arguments, msg_pos: usize) {
     match msg {
         // Check for string concatenation and percent format.
         Expr::BinOp(ast::ExprBinOp { op, .. }) => match op {
@@ -55,8 +138,10 @@ fn check_msg(checker: &Checker, msg: &Expr) {
             _ => {}
         },
         // Check for f-strings.
-        Expr::FString(_) => {
-            checker.report_diagnostic_if_enabled(LoggingFString, msg.range());
+        Expr::FString(f_string) => {
+            if checker.is_rule_enabled(Rule::LoggingFString) {
+                logging_f_string(checker, msg, f_string, arguments, msg_pos);
+            }
         }
         // Check for .format() calls.
         Expr::Call(ast::ExprCall { func, .. }) => {
@@ -168,7 +253,7 @@ pub(crate) fn logging_call(checker: &Checker, call: &ast::ExprCall) {
     // G001, G002, G003, G004
     let msg_pos = usize::from(matches!(logging_call_type, LoggingCallType::LogCall));
     if let Some(format_arg) = call.arguments.find_argument_value("msg", msg_pos) {
-        check_msg(checker, format_arg);
+        check_msg(checker, format_arg, &call.arguments, msg_pos);
     }
 
     // G010