From 16999d389addae588c8a09f2ef89c73609108f97 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Thu, 4 Sep 2025 17:17:06 +0900 Subject: [PATCH] flake8-logging-format: G004 fix handles grouping parens and preserves r-prefix/quotes --- .../fixtures/flake8_logging_format/G004.py | 5 ++ .../rules/logging_call.rs | 31 +++++++---- ...flake8_logging_format__tests__G004.py.snap | 34 ++++++++++++ ..._format__tests__preview__G004_G004.py.snap | 52 +++++++++++++++++++ 4 files changed, 113 insertions(+), 9 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G004.py b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G004.py index fcfa953a32..21ec343634 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G004.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_logging_format/G004.py @@ -64,3 +64,8 @@ import logging x = 1 logging.error(f"{x} -> %s", x) + +# https://github.com/astral-sh/ruff/issues/20151 +logging.warning(fr"\'{x}") +logging.warning(f"{x}" "!") +logging.warning((f"{x}")) diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs b/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs index 23d842efeb..11a8bcb6dd 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs +++ b/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs @@ -1,6 +1,6 @@ -use ruff_python_ast::InterpolatedStringElement; +use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_ast::{self as ast, Arguments, Expr, Keyword, Operator, StringFlags}; - +use ruff_python_ast::{FStringPart, InterpolatedStringElement}; use ruff_python_semantic::analyze::logging; use ruff_python_stdlib::logging::LoggingLevel; use ruff_text_size::{Ranged, TextRange}; @@ -38,28 +38,31 @@ fn logging_f_string( let mut format_string = String::new(); let mut args: Vec<&str> = Vec::new(); + let f_string_flags = f_string.value.f_strings().next().map(|f| f.flags); + let is_raw = f_string_flags.is_some_and(|flags| flags.prefix().is_raw()); + // 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 .iter() .map(|part| match part { - ast::FStringPart::Literal(literal) => literal.flags.quote_str(), - ast::FStringPart::FString(f) => f.flags.quote_str(), + FStringPart::Literal(literal) => literal.flags.quote_str(), + FStringPart::FString(f) => f.flags.quote_str(), }) .next() .unwrap_or("\""); for part in &f_string.value { match part { - ast::FStringPart::Literal(literal) => { + FStringPart::Literal(literal) => { let literal_text = literal.as_str(); if literal_text.contains('%') { return; } format_string.push_str(literal_text); } - ast::FStringPart::FString(f) => { + FStringPart::FString(f) => { for element in &f.elements { match element { InterpolatedStringElement::Literal(lit) => { @@ -99,13 +102,23 @@ fn logging_f_string( } let replacement = format!( - "{q}{format_string}{q}, {args}", + "{prefix}{q}{format_string}{q}, {args}", + prefix = if is_raw { "r" } else { "" }, q = quote_str, - format_string = format_string, args = args.join(", ") ); - let fix = Fix::safe_edit(Edit::range_replacement(replacement, msg.range())); + // Replace the full message argument, including redundant grouping parentheses + // around the f-string expression (e.g., `logging.warning((f"{Ellipsis}"))`). + let replace_range = parenthesized_range( + msg.into(), + (arguments).into(), + checker.comment_ranges(), + checker.source(), + ) + .unwrap_or_else(|| msg.range()); + + let fix = Fix::safe_edit(Edit::range_replacement(replacement, replace_range)); diagnostic.set_fix(fix); } diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G004.py.snap b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G004.py.snap index f02d019ad5..59e6271617 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G004.py.snap +++ b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__G004.py.snap @@ -217,5 +217,39 @@ G004 Logging statement uses f-string 65 | x = 1 66 | logging.error(f"{x} -> %s", x) | ^^^^^^^^^^^^ +67 | +68 | # https://github.com/astral-sh/ruff/issues/20151 + | +help: Convert to lazy `%` formatting + +G004 Logging statement uses f-string + --> G004.py:69:17 + | +68 | # https://github.com/astral-sh/ruff/issues/20151 +69 | logging.warning(fr"\'{x}") + | ^^^^^^^^^ +70 | logging.warning(f"{x}" "!") +71 | logging.warning((f"{x}")) + | +help: Convert to lazy `%` formatting + +G004 Logging statement uses f-string + --> G004.py:70:17 + | +68 | # https://github.com/astral-sh/ruff/issues/20151 +69 | logging.warning(fr"\'{x}") +70 | logging.warning(f"{x}" "!") + | ^^^^^^^^^^ +71 | logging.warning((f"{x}")) + | +help: Convert to lazy `%` formatting + +G004 Logging statement uses f-string + --> G004.py:71:18 + | +69 | logging.warning(fr"\'{x}") +70 | logging.warning(f"{x}" "!") +71 | logging.warning((f"{x}")) + | ^^^^^^ | help: Convert to lazy `%` formatting diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__preview__G004_G004.py.snap b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__preview__G004_G004.py.snap index 92a9ff3139..26aeacd7a3 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__preview__G004_G004.py.snap +++ b/crates/ruff_linter/src/rules/flake8_logging_format/snapshots/ruff_linter__rules__flake8_logging_format__tests__preview__G004_G004.py.snap @@ -273,5 +273,57 @@ G004 Logging statement uses f-string 65 | x = 1 66 | logging.error(f"{x} -> %s", x) | ^^^^^^^^^^^^ +67 | +68 | # https://github.com/astral-sh/ruff/issues/20151 | help: Convert to lazy `%` formatting + +G004 [*] Logging statement uses f-string + --> G004.py:69:17 + | +68 | # https://github.com/astral-sh/ruff/issues/20151 +69 | logging.warning(fr"\'{x}") + | ^^^^^^^^^ +70 | logging.warning(f"{x}" "!") +71 | logging.warning((f"{x}")) + | +help: Convert to lazy `%` formatting +66 | logging.error(f"{x} -> %s", x) +67 | +68 | # https://github.com/astral-sh/ruff/issues/20151 + - logging.warning(fr"\'{x}") +69 + logging.warning(r"\'%s", x) +70 | logging.warning(f"{x}" "!") +71 | logging.warning((f"{x}")) + +G004 [*] Logging statement uses f-string + --> G004.py:70:17 + | +68 | # https://github.com/astral-sh/ruff/issues/20151 +69 | logging.warning(fr"\'{x}") +70 | logging.warning(f"{x}" "!") + | ^^^^^^^^^^ +71 | logging.warning((f"{x}")) + | +help: Convert to lazy `%` formatting +67 | +68 | # https://github.com/astral-sh/ruff/issues/20151 +69 | logging.warning(fr"\'{x}") + - logging.warning(f"{x}" "!") +70 + logging.warning("%s!", x) +71 | logging.warning((f"{x}")) + +G004 [*] Logging statement uses f-string + --> G004.py:71:18 + | +69 | logging.warning(fr"\'{x}") +70 | logging.warning(f"{x}" "!") +71 | logging.warning((f"{x}")) + | ^^^^^^ + | +help: Convert to lazy `%` formatting +68 | # https://github.com/astral-sh/ruff/issues/20151 +69 | logging.warning(fr"\'{x}") +70 | logging.warning(f"{x}" "!") + - logging.warning((f"{x}")) +71 + logging.warning("%s", x)