Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF065_1.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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()
}
Loading