From 77b4ce3f5978a49cb8ff2d5a44365db090eb6d46 Mon Sep 17 00:00:00 2001 From: richardhapb Date: Sun, 14 Dec 2025 01:38:39 -0300 Subject: [PATCH] Fix `\n` in logging lazy fix in rule G004 Fixes #21973. When the fix is applied and a `\n` is present, the transformation introduces a newline and results in a syntax error. The fix replaces `\n` with `\\n` to preserve correct string rendering. This change also adds support for `Expr::Call` and `Expr::Attribute` in the fix by replacing the full text range. --- .../fixtures/flake8_logging_format/G004.py | 2 +- .../rules/logging_call.rs | 10 ++++- ...flake8_logging_format__tests__G004.py.snap | 4 +- ..._format__tests__preview__G004_G004.py.snap | 38 +++++++++++++++---- 4 files changed, 43 insertions(+), 11 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..03d5618050 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 @@ -21,7 +21,7 @@ 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") +logging.info(f"{count} out of {total} files in\n{directory_path} checked") 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..757d8a3194 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 @@ -69,7 +69,7 @@ fn logging_f_string( if lit.value.as_ref().contains('%') { return; } - format_string.push_str(lit.value.as_ref()); + format_string.push_str(&lit.value.replace('\n', "\\n")); } InterpolatedStringElement::Interpolation(interpolated) => { if interpolated.format_spec.is_some() @@ -85,6 +85,14 @@ fn logging_f_string( format_string.push_str("%s"); args.push(name.id.as_str()); } + Expr::Call(call) => { + format_string.push_str("%s"); + args.push(checker.locator().slice(call.range)); + } + Expr::Attribute(attr) => { + format_string.push_str("%s"); + args.push(checker.locator().slice(attr.range)); + } _ => return, } } 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..330eb65d98 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 @@ -73,8 +73,8 @@ G004 Logging statement uses f-string | 22 | total = 9 23 | directory_path = "/home/hamir/ruff/crates/ruff_linter/resources/test/" -24 | logging.info(f"{count} out of {total} files in {directory_path} checked") - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +24 | logging.info(f"{count} out of {total} files in\n{directory_path} checked") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | 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..9c9f2d8955 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 @@ -121,15 +121,15 @@ G004 [*] Logging statement uses f-string | 22 | total = 9 23 | directory_path = "/home/hamir/ruff/crates/ruff_linter/resources/test/" -24 | logging.info(f"{count} out of {total} files in {directory_path} checked") - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +24 | logging.info(f"{count} out of {total} files in\n{directory_path} checked") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: Convert to lazy `%` formatting 21 | count = 5 22 | total = 9 23 | directory_path = "/home/hamir/ruff/crates/ruff_linter/resources/test/" - - logging.info(f"{count} out of {total} files in {directory_path} checked") -24 + logging.info("%s out of %s files in %s checked", count, total, directory_path) + - logging.info(f"{count} out of {total} files in\n{directory_path} checked") +24 + logging.info("%s out of %s files in\n%s checked", count, total, directory_path) 25 | 26 | 27 | @@ -201,7 +201,7 @@ G004 Logging statement uses f-string | help: Convert to lazy `%` formatting -G004 Logging statement uses f-string +G004 [*] Logging statement uses f-string --> G004.py:43:14 | 42 | data = {"status": "active"} @@ -210,8 +210,16 @@ G004 Logging statement uses f-string 44 | logging.info(f"Status: {data.get('status', 'unknown').upper()}") | help: Convert to lazy `%` formatting +40 | logging.warning(f"Items: {items_count:d}") +41 | +42 | data = {"status": "active"} + - logging.info(f"Processing {len(data)} items") +43 + logging.info("Processing %s items", len(data)) +44 | logging.info(f"Status: {data.get('status', 'unknown').upper()}") +45 | +46 | -G004 Logging statement uses f-string +G004 [*] Logging statement uses f-string --> G004.py:44:14 | 42 | data = {"status": "active"} @@ -220,6 +228,14 @@ G004 Logging statement uses f-string | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: Convert to lazy `%` formatting +41 | +42 | data = {"status": "active"} +43 | logging.info(f"Processing {len(data)} items") + - logging.info(f"Status: {data.get('status', 'unknown').upper()}") +44 + logging.info("Status: %s", data.get('status', 'unknown').upper()) +45 | +46 | +47 | result = 123 G004 Logging statement uses f-string --> G004.py:48:14 @@ -243,7 +259,7 @@ G004 Logging statement uses f-string | help: Convert to lazy `%` formatting -G004 Logging statement uses f-string +G004 [*] Logging statement uses f-string --> G004.py:57:14 | 55 | self.name = name @@ -254,6 +270,14 @@ G004 Logging statement uses f-string 59 | user = "tron" | help: Convert to lazy `%` formatting +54 | def __init__(self, name: str): +55 | self.name = name +56 | + - logging.info(f"No changes made to {file_path.name}.") +57 + logging.info("No changes made to %s.", file_path.name) +58 | +59 | user = "tron" +60 | balance = 123.45 G004 Logging statement uses f-string --> G004.py:61:15