From f36b51b2eb219c7ca60eb4da528bb6ba08c948a4 Mon Sep 17 00:00:00 2001 From: Dan Date: Fri, 14 Nov 2025 17:45:48 -0500 Subject: [PATCH 1/2] fix-21458 --- .../resources/test/fixtures/ruff/RUF065_0.py | 15 ++++++ .../ruff/rules/logging_eager_conversion.rs | 46 +++++++++++++++++-- ...ules__ruff__tests__RUF065_RUF065_0.py.snap | 2 + 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF065_0.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF065_0.py index 879fb186bf543b..985ff09b4e904c 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF065_0.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF065_0.py @@ -69,3 +69,18 @@ def str(s): return f"str = {s}" info("Hex: %s", hex(42)) log(logging.INFO, "Hex: %s", hex(255)) +# Complex conversion specifiers that make oct() and hex() necessary +# These should NOT be flagged because the behavior differs between %s and %#o/%#x + +# %06s with oct() - zero-pad flag with width (should NOT be flagged) +logging.warning("%06s", oct(123)) + +# % s with oct() - blank sign flag (should NOT be flagged) +logging.warning("% s", oct(123)) + +# %+s with oct() - sign char flag (should NOT be flagged) +logging.warning("%+s", oct(123)) + +# %.3s with hex() - precision (should NOT be flagged) +logging.warning("%.3s", hex(123)) + diff --git a/crates/ruff_linter/src/rules/ruff/rules/logging_eager_conversion.rs b/crates/ruff_linter/src/rules/ruff/rules/logging_eager_conversion.rs index 32c4755229d6d2..6d7f0ddb624105 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/logging_eager_conversion.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/logging_eager_conversion.rs @@ -2,7 +2,9 @@ use std::str::FromStr; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::{self as ast, Expr}; -use ruff_python_literal::cformat::{CFormatPart, CFormatString, CFormatType}; +use ruff_python_literal::cformat::{ + CConversionFlags, CFormatPart, CFormatSpec, CFormatString, CFormatType, +}; use ruff_python_literal::format::FormatConversion; use ruff_text_size::Ranged; @@ -194,8 +196,10 @@ pub(crate) fn logging_eager_conversion(checker: &Checker, call: &ast::ExprCall) ); } // %s with oct() - suggest using %#o instead + // Skip if the conversion specifier has complex flags or precision that change behavior FormatConversion::Str - if checker.semantic().match_builtin_expr(func.as_ref(), "oct") => + if checker.semantic().match_builtin_expr(func.as_ref(), "oct") + && !has_complex_conversion_specifier(spec) => { checker.report_diagnostic( LoggingEagerConversion { @@ -206,8 +210,10 @@ pub(crate) fn logging_eager_conversion(checker: &Checker, call: &ast::ExprCall) ); } // %s with hex() - suggest using %#x instead + // Skip if the conversion specifier has complex flags or precision that change behavior FormatConversion::Str - if checker.semantic().match_builtin_expr(func.as_ref(), "hex") => + if checker.semantic().match_builtin_expr(func.as_ref(), "hex") + && !has_complex_conversion_specifier(spec) => { checker.report_diagnostic( LoggingEagerConversion { @@ -222,3 +228,37 @@ pub(crate) fn logging_eager_conversion(checker: &Checker, call: &ast::ExprCall) } } } + +/// Check if a conversion specifier has complex flags or precision that make `oct()` or `hex()` necessary. +/// +/// Returns `true` if any of these conditions are met: +/// - Flag `0` (zero-pad) is used, flag `-` (left-adjust) is not used, and minimum width is specified +/// - Flag ` ` (blank sign) is used +/// - Flag `+` (sign char) is used +/// - Precision is specified +fn has_complex_conversion_specifier(spec: &CFormatSpec) -> bool { + // Flag `0` is used, flag `-` is not used, and minimum width is specified + if spec.flags.contains(CConversionFlags::ZERO_PAD) + && !spec.flags.contains(CConversionFlags::LEFT_ADJUST) + && spec.min_field_width.is_some() + { + return true; + } + + // Flag ` ` (blank sign) is used + if spec.flags.contains(CConversionFlags::BLANK_SIGN) { + return true; + } + + // Flag `+` (sign char) is used + if spec.flags.contains(CConversionFlags::SIGN_CHAR) { + return true; + } + + // Precision is specified + if spec.precision.is_some() { + return true; + } + + false +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF065_RUF065_0.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF065_RUF065_0.py.snap index 9ac438216a1a72..99a9f33f5ea402 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF065_RUF065_0.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF065_RUF065_0.py.snap @@ -216,4 +216,6 @@ RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` inste 69 | info("Hex: %s", hex(42)) 70 | log(logging.INFO, "Hex: %s", hex(255)) | ^^^^^^^^ +71 | +72 | # Complex conversion specifiers that make oct() and hex() necessary | From 2b1ecc2742d1197827f0128114f1daa73964107e Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 18 Nov 2025 12:54:48 -0800 Subject: [PATCH 2/2] clean up comments and condense helper logic --- .../resources/test/fixtures/ruff/RUF065_0.py | 15 ----------- .../resources/test/fixtures/ruff/RUF065_1.py | 16 ++++++++++++ .../ruff/rules/logging_eager_conversion.rs | 26 ++++--------------- ...ules__ruff__tests__RUF065_RUF065_0.py.snap | 2 -- 4 files changed, 21 insertions(+), 38 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF065_0.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF065_0.py index 985ff09b4e904c..879fb186bf543b 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF065_0.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF065_0.py @@ -69,18 +69,3 @@ def str(s): return f"str = {s}" info("Hex: %s", hex(42)) log(logging.INFO, "Hex: %s", hex(255)) -# Complex conversion specifiers that make oct() and hex() necessary -# These should NOT be flagged because the behavior differs between %s and %#o/%#x - -# %06s with oct() - zero-pad flag with width (should NOT be flagged) -logging.warning("%06s", oct(123)) - -# % s with oct() - blank sign flag (should NOT be flagged) -logging.warning("% s", oct(123)) - -# %+s with oct() - sign char flag (should NOT be flagged) -logging.warning("%+s", oct(123)) - -# %.3s with hex() - precision (should NOT be flagged) -logging.warning("%.3s", hex(123)) - diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF065_1.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF065_1.py index 048a5be48cf16c..c5071f8ac7dda3 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF065_1.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF065_1.py @@ -16,3 +16,19 @@ # str() with single keyword argument - should be flagged (equivalent to str("!")) logging.warning("%s", str(object="!")) + +# Complex conversion specifiers that make oct() and hex() necessary +# These should NOT be flagged because the behavior differs between %s and %#o/%#x +# https://github.com/astral-sh/ruff/issues/21458 + +# %06s with oct() - zero-pad flag with width (should NOT be flagged) +logging.warning("%06s", oct(123)) + +# % s with oct() - blank sign flag (should NOT be flagged) +logging.warning("% s", oct(123)) + +# %+s with oct() - sign char flag (should NOT be flagged) +logging.warning("%+s", oct(123)) + +# %.3s with hex() - precision (should NOT be flagged) +logging.warning("%.3s", hex(123)) diff --git a/crates/ruff_linter/src/rules/ruff/rules/logging_eager_conversion.rs b/crates/ruff_linter/src/rules/ruff/rules/logging_eager_conversion.rs index 6d7f0ddb624105..920f6c73a5e644 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/logging_eager_conversion.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/logging_eager_conversion.rs @@ -196,7 +196,6 @@ pub(crate) fn logging_eager_conversion(checker: &Checker, call: &ast::ExprCall) ); } // %s with oct() - suggest using %#o instead - // Skip if the conversion specifier has complex flags or precision that change behavior FormatConversion::Str if checker.semantic().match_builtin_expr(func.as_ref(), "oct") && !has_complex_conversion_specifier(spec) => @@ -210,7 +209,6 @@ pub(crate) fn logging_eager_conversion(checker: &Checker, call: &ast::ExprCall) ); } // %s with hex() - suggest using %#x instead - // Skip if the conversion specifier has complex flags or precision that change behavior FormatConversion::Str if checker.semantic().match_builtin_expr(func.as_ref(), "hex") && !has_complex_conversion_specifier(spec) => @@ -237,28 +235,14 @@ pub(crate) fn logging_eager_conversion(checker: &Checker, call: &ast::ExprCall) /// - Flag `+` (sign char) is used /// - Precision is specified fn has_complex_conversion_specifier(spec: &CFormatSpec) -> bool { - // Flag `0` is used, flag `-` is not used, and minimum width is specified - if spec.flags.contains(CConversionFlags::ZERO_PAD) - && !spec.flags.contains(CConversionFlags::LEFT_ADJUST) + if spec.flags.intersects(CConversionFlags::ZERO_PAD) + && !spec.flags.intersects(CConversionFlags::LEFT_ADJUST) && spec.min_field_width.is_some() { return true; } - // Flag ` ` (blank sign) is used - if spec.flags.contains(CConversionFlags::BLANK_SIGN) { - return true; - } - - // Flag `+` (sign char) is used - if spec.flags.contains(CConversionFlags::SIGN_CHAR) { - return true; - } - - // Precision is specified - if spec.precision.is_some() { - return true; - } - - false + spec.flags + .intersects(CConversionFlags::BLANK_SIGN | CConversionFlags::SIGN_CHAR) + || spec.precision.is_some() } diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF065_RUF065_0.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF065_RUF065_0.py.snap index 99a9f33f5ea402..9ac438216a1a72 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF065_RUF065_0.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF065_RUF065_0.py.snap @@ -216,6 +216,4 @@ RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` inste 69 | info("Hex: %s", hex(42)) 70 | log(logging.INFO, "Hex: %s", hex(255)) | ^^^^^^^^ -71 | -72 | # Complex conversion specifiers that make oct() and hex() necessary |