fix-21458

This commit is contained in:
Dan 2025-11-14 17:45:48 -05:00
parent 2a2b719f00
commit f36b51b2eb
3 changed files with 60 additions and 3 deletions

View file

@ -69,3 +69,18 @@ log(logging.INFO, "Octal: %s", oct(255))
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))

View file

@ -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
}

View file

@ -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
|