[refurb] Mark fixes that remove unknown separators as unsafe (FURB105)

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.
 

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

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