From 4a117b8876600a7e6cdb24d5916711bb16061827 Mon Sep 17 00:00:00 2001 From: jesco-absolut Date: Sat, 4 Jul 2026 02:27:39 -0400 Subject: [PATCH] Mark uncertain FURB105 sep fixes unsafe --- .../resources/test/fixtures/refurb/FURB105.py | 4 ++ .../rules/refurb/rules/print_empty_string.rs | 17 +++--- ...es__refurb__tests__FURB105_FURB105.py.snap | 56 +++++++++++++++---- 3 files changed, 58 insertions(+), 19 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB105.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB105.py index 007d5822dec1ef..51eae1673778cf 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB105.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB105.py @@ -22,6 +22,10 @@ print(f"") print(f"", sep=",") print(f"", end="bar") +print(1, sep=None) + +def p(sep): + print(1, sep=sep) # OK. diff --git a/crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs b/crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs index 9e228bf4837268..ee803ba0e02bf0 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs @@ -1,8 +1,7 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::helpers::{contains_effect, is_empty_f_string}; +use ruff_python_ast::helpers::is_empty_f_string; use ruff_python_ast::{self as ast, Expr}; use ruff_python_codegen::Generator; -use ruff_python_semantic::SemanticModel; use ruff_python_trivia::CommentRanges; use ruff_text_size::Ranged; @@ -33,8 +32,8 @@ use crate::{Applicability, Edit, Fix, FixAvailability, Violation}; /// /// ## Fix safety /// This fix is marked as unsafe if it removes comments or an unused `sep` keyword argument -/// that may have side effects. Removing such arguments may change the program's -/// behavior by skipping the execution of those side effects. +/// that is not known to be a valid separator. Removing such arguments may change the +/// program's behavior by skipping their evaluation or hiding a `TypeError`. /// /// ## References /// - [Python documentation: `print`](https://docs.python.org/3/library/functions.html#print) @@ -98,7 +97,6 @@ pub(crate) fn print_empty_string(checker: &Checker, call: &ast::ExprCall) { EmptyStringFix::from_call( call, Separator::Remove, - checker.semantic(), checker.generator(), checker.comment_ranges(), ) @@ -125,7 +123,6 @@ pub(crate) fn print_empty_string(checker: &Checker, call: &ast::ExprCall) { EmptyStringFix::from_call( call, Separator::Remove, - checker.semantic(), checker.generator(), checker.comment_ranges(), ) @@ -191,7 +188,6 @@ pub(crate) fn print_empty_string(checker: &Checker, call: &ast::ExprCall) { EmptyStringFix::from_call( call, separator, - checker.semantic(), checker.generator(), checker.comment_ranges(), ) @@ -210,6 +206,10 @@ fn is_empty_string(expr: &Expr) -> bool { } } +fn is_known_valid_separator(expr: &Expr) -> bool { + expr.is_string_literal_expr() || expr.is_none_literal_expr() +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum Separator { Remove, @@ -225,7 +225,6 @@ impl EmptyStringFix { fn from_call( call: &ast::ExprCall, separator: Separator, - semantic: &SemanticModel, generator: Generator, comment_ranges: &CommentRanges, ) -> Self { @@ -262,7 +261,7 @@ impl EmptyStringFix { return true; } - if contains_effect(&keyword.value, |id| semantic.has_builtin_binding(id)) { + if !is_known_valid_separator(&keyword.value) { applicability = Applicability::Unsafe; } diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB105_FURB105.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB105_FURB105.py.snap index 15b5cea41c3d40..81628ae9e230fa 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB105_FURB105.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB105_FURB105.py.snap @@ -351,6 +351,7 @@ FURB105 [*] Unnecessary empty string and separator passed to `print` 23 | print(f"", sep=",") | ^^^^^^^^^^^^^^^^^^^ 24 | print(f"", end="bar") +25 | print(1, sep=None) | help: Remove empty string and separator | @@ -367,33 +368,68 @@ FURB105 [*] Unnecessary empty string passed to `print` 23 | print(f"", sep=",") 24 | print(f"", end="bar") | ^^^^^^^^^^^^^^^^^^^^^ -25 | -26 | # OK. +25 | print(1, sep=None) | help: Remove empty string | 23 | print(f"", sep=",") - print(f"", end="bar") 24 + print(end="bar") -25 | +25 | print(1, sep=None) | +FURB105 [*] Unnecessary separator passed to `print` + --> FURB105.py:25:1 + | +23 | print(f"", sep=",") +24 | print(f"", end="bar") +25 | print(1, sep=None) + | ^^^^^^^^^^^^^^^^^^ +26 | +27 | def p(sep): + | +help: Remove separator + | +24 | print(f"", end="bar") + - print(1, sep=None) +25 + print(1) +26 | + | + +FURB105 [*] Unnecessary separator passed to `print` + --> FURB105.py:28:5 + | +27 | def p(sep): +28 | print(1, sep=sep) + | ^^^^^^^^^^^^^^^^^ +29 | +30 | # OK. + | +help: Remove separator + | +27 | def p(sep): + - print(1, sep=sep) +28 + print(1) +29 | + | +note: This is an unsafe fix and may change runtime behavior + FURB105 [*] Unnecessary empty string passed to `print` - --> FURB105.py:42:1 + --> FURB105.py:46:1 | -42 | / print( -43 | | # text -44 | | "" -45 | | ) +46 | / print( +47 | | # text +48 | | "" +49 | | ) | |_^ | help: Remove empty string | -41 | +45 | - print( - # text - "" - ) -42 + print() +46 + print() | note: This is an unsafe fix and may change runtime behavior