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 048a5be48c..c5071f8ac7 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 @@ logging.warning("%s", str(**{"object": b"\xf0\x9f\x9a\xa8", "encoding": "utf-8"} # 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 32c4755229..920f6c73a5 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; @@ -195,7 +197,8 @@ pub(crate) fn logging_eager_conversion(checker: &Checker, call: &ast::ExprCall) } // %s with oct() - suggest using %#o instead 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 { @@ -207,7 +210,8 @@ pub(crate) fn logging_eager_conversion(checker: &Checker, call: &ast::ExprCall) } // %s with hex() - suggest using %#x instead 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 +226,23 @@ 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 { + if spec.flags.intersects(CConversionFlags::ZERO_PAD) + && !spec.flags.intersects(CConversionFlags::LEFT_ADJUST) + && spec.min_field_width.is_some() + { + return true; + } + + spec.flags + .intersects(CConversionFlags::BLANK_SIGN | CConversionFlags::SIGN_CHAR) + || spec.precision.is_some() +}