Refactor FURB105 into explicit cases (#7634)

## Summary

I was having trouble keeping track of the various cases here, so opted
to refactor into a more explicit `match`.

crates/ruff_linter/resources/test/fixtures/refurb/FURB105.py

@@ -17,6 +17,7 @@ print("foo", "", "bar", sep="")
 print("", *args)
 print("", *args, sep="")
 print("", **kwargs)
+print(sep="\t")
 
 # OK.
 

crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs

@@ -29,7 +29,17 @@ use crate::registry::AsRule;
 /// - [Python documentation: `print`](https://docs.python.org/3/library/functions.html#print)
 #[violation]
 pub struct PrintEmptyString {
-    separator: Option<Separator>,
+    reason: Reason,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum Reason {
+    /// Ex) `print("")`
+    EmptyArgument,
+    /// Ex) `print("foo", sep="\t")`
+    UselessSeparator,
+    /// Ex) `print("", sep="\t")`
+    Both,
 }
 
 impl Violation for PrintEmptyString {
@@ -37,84 +47,135 @@ impl Violation for PrintEmptyString {
 
     #[derive_message_formats]
     fn message(&self) -> String {
-        let PrintEmptyString { separator } = self;
+        let PrintEmptyString { reason } = self;
-        match separator {
+        match reason {
-            None | Some(Separator::Retain) => format!("Unnecessary empty string passed to `print`"),
+            Reason::EmptyArgument => format!("Unnecessary empty string passed to `print`"),
-            Some(Separator::Remove) => {
+            Reason::UselessSeparator => format!("Unnecessary separator passed to `print`"),
-                format!("Unnecessary empty string passed to `print` with redundant separator")
+            Reason::Both => format!("Unnecessary empty string and separator passed to `print`"),
-            }
         }
     }
 
     fn autofix_title(&self) -> Option<String> {
-        let PrintEmptyString { separator } = self;
+        let PrintEmptyString { reason } = self;
-        match separator {
+        match reason {
-            None | Some(Separator::Retain) => Some("Remove empty string".to_string()),
+            Reason::EmptyArgument => Some("Remove empty string".to_string()),
-            Some(Separator::Remove) => Some("Remove empty string and separator".to_string()),
+            Reason::UselessSeparator => Some("Remove separator".to_string()),
+            Reason::Both => Some("Remove empty string and separator".to_string()),
         }
     }
 }
 
 /// FURB105
 pub(crate) fn print_empty_string(checker: &mut Checker, call: &ast::ExprCall) {
-    if checker
+    if !checker
         .semantic()
         .resolve_call_path(&call.func)
         .as_ref()
         .is_some_and(|call_path| matches!(call_path.as_slice(), ["", "print"]))
     {
-        // Ex) `print("", sep="")` or `print("", "", **kwargs)`
+        return;
-        let empty_separator = call
+    }
-            .arguments
+
-            .find_keyword("sep")
+    match &call.arguments.args.as_slice() {
-            .map_or(false, |keyword| is_empty_string(&keyword.value))
+        // Ex) `print(*args)` or `print(*args, sep="\t")`
-            && !call
+        [arg] if arg.is_starred_expr() => {}
+
+        // Ex) `print("")` or `print("", sep="\t")`
+        [arg] if is_empty_string(arg) => {
+            let reason = if call.arguments.find_keyword("sep").is_some() {
+                Reason::Both
+            } else {
+                Reason::EmptyArgument
+            };
+
+            let mut diagnostic = Diagnostic::new(PrintEmptyString { reason }, call.range());
+
+            if checker.patch(diagnostic.kind.rule()) {
+                diagnostic.set_fix(Fix::suggested(Edit::replacement(
+                    generate_suggestion(call, Separator::Remove, checker.generator()),
+                    call.start(),
+                    call.end(),
+                )));
+            }
+
+            checker.diagnostics.push(diagnostic);
+        }
+
+        // Ex) `print(sep="\t")` or `print(obj, sep="\t")`
+        [] | [_] => {
+            // If there's a `sep` argument, remove it, regardless of what it is.
+            if call.arguments.find_keyword("sep").is_some() {
+                let mut diagnostic = Diagnostic::new(
+                    PrintEmptyString {
+                        reason: Reason::UselessSeparator,
+                    },
+                    call.range(),
+                );
+
+                if checker.patch(diagnostic.kind.rule()) {
+                    diagnostic.set_fix(Fix::suggested(Edit::replacement(
+                        generate_suggestion(call, Separator::Remove, checker.generator()),
+                        call.start(),
+                        call.end(),
+                    )));
+                }
+
+                checker.diagnostics.push(diagnostic);
+            }
+        }
+
+        // Ex) `print("foo", "", "bar", sep="")`
+        _ => {
+            // Ignore `**kwargs`.
+            let has_kwargs = call
                 .arguments
                 .keywords
                 .iter()
                 .any(|keyword| keyword.arg.is_none());
+            if has_kwargs {
+                return;
+            }
 
-        // Avoid flagging, e.g., `print("", "", sep="sep")`
+            // Require an empty `sep` argument.
-        if !empty_separator && call.arguments.args.len() != 1 {
+            let empty_separator = call
-            return;
-        }
-
-        // Check if the positional arguments is are all empty strings, or if
-        // there are any empty strings and the `sep` keyword argument is also
-        // an empty string.
-        if call.arguments.args.iter().all(is_empty_string)
-            || (empty_separator && call.arguments.args.iter().any(is_empty_string))
-        {
-            let separator = call
                 .arguments
-                .keywords
+                .find_keyword("sep")
+                .map_or(false, |keyword| is_empty_string(&keyword.value));
+            if !empty_separator {
+                return;
+            }
+
+            // Count the number of empty and non-empty arguments.
+            let empty_arguments = call
+                .arguments
+                .args
                 .iter()
-                .any(|keyword| {
+                .filter(|arg| is_empty_string(arg))
-                    keyword
+                .count();
-                        .arg
+            if empty_arguments == 0 {
-                        .as_ref()
+                return;
-                        .is_some_and(|arg| arg.as_str() == "sep")
+            }
-                })
-                .then(|| {
-                    let is_starred = call.arguments.args.iter().any(Expr::is_starred_expr);
-                    if is_starred {
-                        return Separator::Retain;
-                    }
 
-                    let non_empty = call
+            // If removing the arguments would leave us with one or fewer, then we can remove the
-                        .arguments
+            // separator too.
-                        .args
+            let separator = if call.arguments.args.len() - empty_arguments > 1
-                        .iter()
+                || call.arguments.args.iter().any(Expr::is_starred_expr)
-                        .filter(|arg| !is_empty_string(arg))
+            {
-                        .count();
+                Separator::Retain
-                    if non_empty > 1 {
+            } else {
-                        return Separator::Retain;
+                Separator::Remove
-                    }
+            };
 
-                    Separator::Remove
+            let mut diagnostic = Diagnostic::new(
-                });
+                PrintEmptyString {
-
+                    reason: if separator == Separator::Retain {
-            let mut diagnostic = Diagnostic::new(PrintEmptyString { separator }, call.range());
+                        Reason::EmptyArgument
+                    } else {
+                        Reason::Both
+                    },
+                },
+                call.range(),
+            );
 
             if checker.patch(diagnostic.kind.rule()) {
                 diagnostic.set_fix(Fix::suggested(Edit::replacement(
@@ -134,9 +195,9 @@ fn is_empty_string(expr: &Expr) -> bool {
     matches!(
         expr,
         Expr::Constant(ast::ExprConstant {
-            value: Constant::Str(s),
+            value: Constant::Str(value),
             ..
-        }) if s.is_empty()
+        }) if value.is_empty()
     )
 }
 
@@ -148,18 +209,14 @@ enum Separator {
 
 /// Generate a suggestion to remove the empty string positional argument and
 /// the `sep` keyword argument, if it exists.
-fn generate_suggestion(
+fn generate_suggestion(call: &ast::ExprCall, separator: Separator, generator: Generator) -> String {
-    call: &ast::ExprCall,
-    separator: Option<Separator>,
-    generator: Generator,
-) -> String {
     let mut call = call.clone();
 
     // Remove all empty string positional arguments.
     call.arguments.args.retain(|arg| !is_empty_string(arg));
 
     // Remove the `sep` keyword argument if it exists.
-    if separator == Some(Separator::Remove) {
+    if separator == Separator::Remove {
         call.arguments.keywords.retain(|keyword| {
             keyword
                 .arg

crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB105_FURB105.py.snap

@@ -21,7 +21,7 @@ FURB105.py:3:1: FURB105 [*] Unnecessary empty string passed to `print`
 5 5 | print("", end="bar")
 6 6 | print("", sep=",", end="bar")
 
-FURB105.py:4:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
+FURB105.py:4:1: FURB105 [*] Unnecessary empty string and separator passed to `print`
   |
 3 | print("")
 4 | print("", sep=",")
@@ -62,7 +62,7 @@ FURB105.py:5:1: FURB105 [*] Unnecessary empty string passed to `print`
 7 7 | print(sep="")
 8 8 | print("", sep="")
 
-FURB105.py:6:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
+FURB105.py:6:1: FURB105 [*] Unnecessary empty string and separator passed to `print`
   |
 4 | print("", sep=",")
 5 | print("", end="bar")
@@ -83,7 +83,7 @@ FURB105.py:6:1: FURB105 [*] Unnecessary empty string passed to `print` with redu
 8 8 | print("", sep="")
 9 9 | print("", "", sep="")
 
-FURB105.py:7:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
+FURB105.py:7:1: FURB105 [*] Unnecessary separator passed to `print`
   |
 5 | print("", end="bar")
 6 | print("", sep=",", end="bar")
@@ -92,7 +92,7 @@ FURB105.py:7:1: FURB105 [*] Unnecessary empty string passed to `print` with redu
 8 | print("", sep="")
 9 | print("", "", sep="")
   |
-  = help: Remove empty string and separator
+  = help: Remove separator
 
 ℹ Suggested fix
 4 4 | print("", sep=",")
@@ -104,7 +104,7 @@ FURB105.py:7:1: FURB105 [*] Unnecessary empty string passed to `print` with redu
 9 9 | print("", "", sep="")
 10 10 | print("", "", sep="", end="")
 
-FURB105.py:8:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
+FURB105.py:8:1: FURB105 [*] Unnecessary empty string and separator passed to `print`
    |
  6 | print("", sep=",", end="bar")
  7 | print(sep="")
@@ -125,7 +125,7 @@ FURB105.py:8:1: FURB105 [*] Unnecessary empty string passed to `print` with redu
 10 10 | print("", "", sep="", end="")
 11 11 | print("", "", sep="", end="bar")
 
-FURB105.py:9:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
+FURB105.py:9:1: FURB105 [*] Unnecessary empty string and separator passed to `print`
    |
  7 | print(sep="")
  8 | print("", sep="")
@@ -146,7 +146,7 @@ FURB105.py:9:1: FURB105 [*] Unnecessary empty string passed to `print` with redu
 11 11 | print("", "", sep="", end="bar")
 12 12 | print("", sep="", end="bar")
 
-FURB105.py:10:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
+FURB105.py:10:1: FURB105 [*] Unnecessary empty string and separator passed to `print`
    |
  8 | print("", sep="")
  9 | print("", "", sep="")
@@ -167,7 +167,7 @@ FURB105.py:10:1: FURB105 [*] Unnecessary empty string passed to `print` with red
 12 12 | print("", sep="", end="bar")
 13 13 | print(sep="", end="bar")
 
-FURB105.py:11:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
+FURB105.py:11:1: FURB105 [*] Unnecessary empty string and separator passed to `print`
    |
  9 | print("", "", sep="")
 10 | print("", "", sep="", end="")
@@ -188,7 +188,7 @@ FURB105.py:11:1: FURB105 [*] Unnecessary empty string passed to `print` with red
 13 13 | print(sep="", end="bar")
 14 14 | print("", "foo", sep="")
 
-FURB105.py:12:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
+FURB105.py:12:1: FURB105 [*] Unnecessary empty string and separator passed to `print`
    |
 10 | print("", "", sep="", end="")
 11 | print("", "", sep="", end="bar")
@@ -209,7 +209,7 @@ FURB105.py:12:1: FURB105 [*] Unnecessary empty string passed to `print` with red
 14 14 | print("", "foo", sep="")
 15 15 | print("foo", "", sep="")
 
-FURB105.py:13:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
+FURB105.py:13:1: FURB105 [*] Unnecessary separator passed to `print`
    |
 11 | print("", "", sep="", end="bar")
 12 | print("", sep="", end="bar")
@@ -218,7 +218,7 @@ FURB105.py:13:1: FURB105 [*] Unnecessary empty string passed to `print` with red
 14 | print("", "foo", sep="")
 15 | print("foo", "", sep="")
    |
-   = help: Remove empty string and separator
+   = help: Remove separator
 
 ℹ Suggested fix
 10 10 | print("", "", sep="", end="")
@@ -230,7 +230,7 @@ FURB105.py:13:1: FURB105 [*] Unnecessary empty string passed to `print` with red
 15 15 | print("foo", "", sep="")
 16 16 | print("foo", "", "bar", sep="")
 
-FURB105.py:14:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
+FURB105.py:14:1: FURB105 [*] Unnecessary empty string and separator passed to `print`
    |
 12 | print("", sep="", end="bar")
 13 | print(sep="", end="bar")
@@ -251,7 +251,7 @@ FURB105.py:14:1: FURB105 [*] Unnecessary empty string passed to `print` with red
 16 16 | print("foo", "", "bar", sep="")
 17 17 | print("", *args)
 
-FURB105.py:15:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
+FURB105.py:15:1: FURB105 [*] Unnecessary empty string and separator passed to `print`
    |
 13 | print(sep="", end="bar")
 14 | print("", "foo", sep="")
@@ -300,6 +300,7 @@ FURB105.py:18:1: FURB105 [*] Unnecessary empty string passed to `print`
 18 | print("", *args, sep="")
    | ^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
 19 | print("", **kwargs)
+20 | print(sep="\t")
    |
    = help: Remove empty string
 
@@ -310,8 +311,8 @@ FURB105.py:18:1: FURB105 [*] Unnecessary empty string passed to `print`
 18    |-print("", *args, sep="")
    18 |+print(*args, sep="")
 19 19 | print("", **kwargs)
-20 20 | 
+20 20 | print(sep="\t")
-21 21 | # OK.
+21 21 | 
 
 FURB105.py:19:1: FURB105 [*] Unnecessary empty string passed to `print`
    |
@@ -319,8 +320,7 @@ FURB105.py:19:1: FURB105 [*] Unnecessary empty string passed to `print`
 18 | print("", *args, sep="")
 19 | print("", **kwargs)
    | ^^^^^^^^^^^^^^^^^^^ FURB105
-20 | 
+20 | print(sep="\t")
-21 | # OK.
    |
    = help: Remove empty string
 
@@ -330,8 +330,29 @@ FURB105.py:19:1: FURB105 [*] Unnecessary empty string passed to `print`
 18 18 | print("", *args, sep="")
 19    |-print("", **kwargs)
    19 |+print(**kwargs)
-20 20 | 
+20 20 | print(sep="\t")
-21 21 | # OK.
+21 21 | 
-22 22 | 
+22 22 | # OK.
+
+FURB105.py:20:1: FURB105 [*] Unnecessary separator passed to `print`
+   |
+18 | print("", *args, sep="")
+19 | print("", **kwargs)
+20 | print(sep="\t")
+   | ^^^^^^^^^^^^^^^ FURB105
+21 | 
+22 | # OK.
+   |
+   = help: Remove separator
+
+ℹ Suggested fix
+17 17 | print("", *args)
+18 18 | print("", *args, sep="")
+19 19 | print("", **kwargs)
+20    |-print(sep="\t")
+   20 |+print()
+21 21 | 
+22 22 | # OK.
+23 23 |