Skip to content

Conversation

@danparizher
Copy link
Contributor

Summary

Fixed RUF065 (logging-eager-conversion) to only flag str() calls when they perform a simple conversion that can be safely removed. The rule now ignores str() calls with no arguments, multiple arguments, starred arguments, or keyword unpacking, preventing false positives.

Fixes #21315

Problem Analysis

The RUF065 rule was incorrectly flagging all str() calls in logging statements, even when str() was performing actual conversion work beyond simple type coercion. Specifically, the rule flagged:

  • str() with no arguments - which returns an empty string
  • str(b"data", "utf-8") with multiple arguments - which performs encoding conversion
  • str(*args) with starred arguments - which unpacks arguments
  • str(**kwargs) with keyword unpacking - which passes keyword arguments

These cases cannot be safely removed because str() is doing meaningful work (encoding conversion, argument unpacking, etc.), not just redundant type conversion.

The root cause was that the rule only checked if the function was str() without validating the call signature. It didn't distinguish between simple str(value) conversions (which can be removed) and more complex str() calls that perform actual work.

Approach

The fix adds validation to the str() detection logic in logging_eager_conversion.rs:

  1. Check argument count: Only flag str() calls with exactly one positional argument (str_call_args.args.len() == 1)
  2. Check for starred arguments: Ensure the single argument is not starred (!str_call_args.args[0].is_starred_expr())
  3. Check for keyword arguments: Ensure there are no keyword arguments (str_call_args.keywords.is_empty())

This ensures the rule only flags cases like str(value) where str() is truly redundant and can be removed, while ignoring cases where str() performs actual conversion work.

The fix maintains backward compatibility - all existing valid test cases continue to be flagged correctly, while the new edge cases are properly ignored.

if checker.semantic().match_builtin_expr(func.as_ref(), "str")
&& str_call_args.args.len() == 1
&& !str_call_args.args[0].is_starred_expr()
&& str_call_args.keywords.is_empty() =>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logging.warning("%s", str(object="!")), with one keyword argument, should still be flagged.

@danparizher
Copy link
Contributor Author

I added a test case for str(object="!"), and the new logic seems to work in isolation, but for some reason isn't being detected in the test file. I would appreciate it if someone could help me diagnose the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RUF065 should ignore str when not used for conversion

2 participants