[refurb] Mark fixes that remove unknown separators as unsafe (FURB105) - #27200
Merged
Merged
Conversation
|
ntBre
approved these changes
Jul 27, 2026
ntBre
left a comment
Contributor
There was a problem hiding this comment.
Thanks! I was initially worried that this is quite a restrictive check, but I think trying to make it more permissive isn't worth the complexity it would introduce. For example, we could try to use our typing module to make cases like this safe:
def p(sep: str): print(1, sep=sep)
def p(sep: None): print(1, sep=sep)
def p(sep: str | None): print(1, sep=sep)but then we'd need separate handling for a case like:
def foo():
sep = ""
print(1, sep=sep)and other cases. Given that literals are probably the most common case and that this doesn't show any negative ecosystem impact, I think this is an acceptable tradeoff.
We can try to expand the check again when we get access to ty's type inference.
refurb] Mark fixes that remove unknown separators as unsafe (FURB105)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #20919.
The main learning point after working on this was, that python validates sep even when it is unused;
removing an unknown value can suppress a TypeError; therefore only known-valid separators receive a safe fix.
FURB105 can suggest removing an unused
separgument from aprintcall. The fix was previously considered safe unlessevaluating the separator had observable side effects.
Python still validates
sepeven when the call only prints one value. If the separator is neitherNonenor a string,the original call raises a
TypeError. Removing an unknown separator can therefore change the program's behavior evenwhen evaluating its expression has no side effects.
This change makes the fix conservative: removing
sepis considered safe when its value is statically known to be astring literal or
None. Other separator expressions retain the FURB105 diagnostic, but their fix is marked unsafe.The fixtures cover both a known-valid
Noneseparator and a separator supplied through an unknown variable. The ruledocumentation and snapshots are updated to reflect the safety distinction.
Test Plan
cargo test -p ruff_linter --lib rule_printemptystring_path_new_furb105_py_expectscargo fmt --all --check