From 5e805e3092ece8551185c078a9a71c96810dcff7 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 23 Sep 2023 23:42:58 -0400 Subject: [PATCH] Tweaks --- .../rules/refurb/rules/print_empty_string.rs | 146 ++++++++---------- ...es__refurb__tests__FURB105_FURB105.py.snap | 56 +++---- 2 files changed, 93 insertions(+), 109 deletions(-) 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 d7c54f10a5ccda..ebef2eeb7da51d 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 @@ -29,8 +29,7 @@ use crate::registry::AsRule; /// - [Python documentation: `print`](https://docs.python.org/3/library/functions.html#print) #[violation] pub struct PrintEmptyString { - suggestion: String, - redundant_sep: bool, + separator: Option, } impl Violation for PrintEmptyString { @@ -38,99 +37,78 @@ impl Violation for PrintEmptyString { #[derive_message_formats] fn message(&self) -> String { - let PrintEmptyString { - suggestion, - redundant_sep, - } = self; - if redundant_sep == &true { - format!( - "Called `print` with an empty string and a redundant separator, use `{suggestion}` instead", - ) - } else { - format!("Called `print` with an empty string, use `{suggestion}` instead",) + let PrintEmptyString { separator } = self; + match separator { + None | Some(Separator::Retain) => format!("Unnecessary empty string passed to `print`"), + Some(Separator::Remove) => { + format!("Unnecessary empty string passed to `print` with redundant separator") + } } } fn autofix_title(&self) -> Option { - let PrintEmptyString { redundant_sep, .. } = self; - if redundant_sep == &true { - Some("Remove empty string positional argument and redundant separator".to_string()) - } else { - Some("Remove empty string positional argument".to_string()) + let PrintEmptyString { separator } = self; + match separator { + None | Some(Separator::Retain) => Some("Remove empty string".to_string()), + Some(Separator::Remove) => Some("Remove empty string and separator".to_string()), } } } /// FURB105 pub(crate) fn print_empty_string(checker: &mut Checker, call: &ast::ExprCall) { - // Check if the call is to the builtin `print` function. if checker .semantic() .resolve_call_path(&call.func) .as_ref() .is_some_and(|call_path| matches!(call_path.as_slice(), ["", "print"])) { - // For performance reasons, defer assignment to until we know that we - // need to check if the separator is an empty string. - let mut sep_value_is_empty_string = false; - - // If the call does not have only one positional argument, check if the - // `sep` keyword argument is an empty string; if it is not an empty - // string, don't trigger. - if call.arguments.args.len() != 1 { - sep_value_is_empty_string = call - .arguments - .find_keyword("sep") - .map_or(false, |keyword| is_const_empty_string(&keyword.value)); - if !sep_value_is_empty_string { - return; - } + // Ex) `print("", sep="")` + let empty_separator = call + .arguments + .find_keyword("sep") + .map_or(false, |keyword| is_empty_string(&keyword.value)); + + // Avoid flagging, e.g., `print("", "", sep="sep")` + if !empty_separator && call.arguments.args.len() != 1 { + 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_const_empty_string) - || (sep_value_is_empty_string && call.arguments.args.iter().any(is_const_empty_string)) + if call.arguments.args.iter().all(is_empty_string) + || (empty_separator && call.arguments.args.iter().any(is_empty_string)) { - let non_empty_string_args_count = call + let separator = call .arguments - .args + .keywords .iter() - .filter(|arg| !is_const_empty_string(arg)) - .count(); - - let sep_index = if non_empty_string_args_count > 1 { - // If the call has more than one non-empty string positional - // argument, the `sep` keyword argument is NOT redundant. - None - } else { - // Find the index of the `sep` keyword argument, if it exists. - call.arguments.keywords.iter().position(|keyword| { + .any(|keyword| { keyword .arg - .clone() - .is_some_and(|arg| arg.to_string() == "sep") + .as_ref() + .is_some_and(|arg| arg.as_str() == "sep") }) - }; - - let suggestion = if non_empty_string_args_count > 1 { - generate_suggestion(&call.clone(), None, checker.generator()) - } else { - generate_suggestion(&call.clone(), sep_index, checker.generator()) - }; - - let mut diagnostic = Diagnostic::new( - PrintEmptyString { - suggestion: suggestion.clone(), - redundant_sep: sep_index.is_some(), - }, - call.range(), - ); + .then(|| { + let non_empty = call + .arguments + .args + .iter() + .filter(|arg| !is_empty_string(arg)) + .count(); + if non_empty > 1 { + Separator::Retain + } else { + Separator::Remove + } + }); + + let mut diagnostic = Diagnostic::new(PrintEmptyString { separator }, call.range()); if checker.patch(diagnostic.kind.rule()) { diagnostic.set_fix(Fix::suggested(Edit::replacement( - suggestion, + generate_suggestion(call, separator, checker.generator()), call.start(), call.end(), ))); @@ -142,7 +120,7 @@ pub(crate) fn print_empty_string(checker: &mut Checker, call: &ast::ExprCall) { } /// Check if an expression is a constant empty string. -fn is_const_empty_string(expr: &Expr) -> bool { +fn is_empty_string(expr: &Expr) -> bool { matches!( expr, Expr::Constant(ast::ExprConstant { @@ -152,27 +130,33 @@ fn is_const_empty_string(expr: &Expr) -> bool { ) } -/// Generate a suggestion to replace a `print` call with `print` call with no -/// empty string positional arguments, and no `sep` keyword argument. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Separator { + Remove, + Retain, +} + +/// Generate a suggestion to remove the empty string positional argument and +/// the `sep` keyword argument, if it exists. fn generate_suggestion( call: &ast::ExprCall, - sep_index: Option, + separator: Option, generator: Generator, ) -> String { - // Clone the call so that we can mutate it. - let mut suggestion = call.clone(); + let mut call = call.clone(); // Remove all empty string positional arguments. - suggestion - .arguments - .args - .retain(|arg| !is_const_empty_string(arg)); - - // If there is a `sep` keyword argument, remove it too (the separator is - // not needed if there are no objects to separate) by finding its index. - if let Some(index) = sep_index { - suggestion.arguments.keywords.remove(index); + call.arguments.args.retain(|arg| !is_empty_string(arg)); + + // Remove the `sep` keyword argument if it exists. + if separator == Some(Separator::Remove) { + call.arguments.keywords.retain(|keyword| { + keyword + .arg + .as_ref() + .map_or(true, |arg| arg.as_str() != "sep") + }); } - generator.expr(&suggestion.into()) + generator.expr(&call.into()) } 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 668329c18020de..7d9d2904ff5c8f 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 @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/refurb/mod.rs --- -FURB105.py:3:1: FURB105 [*] Called `print` with an empty string, use `print()` instead +FURB105.py:3:1: FURB105 [*] Unnecessary empty string passed to `print` | 1 | # Errors. 2 | @@ -10,7 +10,7 @@ FURB105.py:3:1: FURB105 [*] Called `print` with an empty string, use `print()` i 4 | print("", sep=",") 5 | print("", end="bar") | - = help: Remove empty string positional argument + = help: Remove empty string ℹ Suggested fix 1 1 | # Errors. @@ -21,7 +21,7 @@ FURB105.py:3:1: FURB105 [*] Called `print` with an empty string, use `print()` i 5 5 | print("", end="bar") 6 6 | print("", sep=",", end="bar") -FURB105.py:4:1: FURB105 [*] Called `print` with an empty string and a redundant separator, use `print()` instead +FURB105.py:4:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator | 3 | print("") 4 | print("", sep=",") @@ -29,7 +29,7 @@ FURB105.py:4:1: FURB105 [*] Called `print` with an empty string and a redundant 5 | print("", end="bar") 6 | print("", sep=",", end="bar") | - = help: Remove empty string positional argument and redundant separator + = help: Remove empty string and separator ℹ Suggested fix 1 1 | # Errors. @@ -41,7 +41,7 @@ FURB105.py:4:1: FURB105 [*] Called `print` with an empty string and a redundant 6 6 | print("", sep=",", end="bar") 7 7 | print(sep="") -FURB105.py:5:1: FURB105 [*] Called `print` with an empty string, use `print(end="bar")` instead +FURB105.py:5:1: FURB105 [*] Unnecessary empty string passed to `print` | 3 | print("") 4 | print("", sep=",") @@ -50,7 +50,7 @@ FURB105.py:5:1: FURB105 [*] Called `print` with an empty string, use `print(end= 6 | print("", sep=",", end="bar") 7 | print(sep="") | - = help: Remove empty string positional argument + = help: Remove empty string ℹ Suggested fix 2 2 | @@ -62,7 +62,7 @@ FURB105.py:5:1: FURB105 [*] Called `print` with an empty string, use `print(end= 7 7 | print(sep="") 8 8 | print("", sep="") -FURB105.py:6:1: FURB105 [*] Called `print` with an empty string and a redundant separator, use `print(end="bar")` instead +FURB105.py:6:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator | 4 | print("", sep=",") 5 | print("", end="bar") @@ -71,7 +71,7 @@ FURB105.py:6:1: FURB105 [*] Called `print` with an empty string and a redundant 7 | print(sep="") 8 | print("", sep="") | - = help: Remove empty string positional argument and redundant separator + = help: Remove empty string and separator ℹ Suggested fix 3 3 | print("") @@ -83,7 +83,7 @@ FURB105.py:6:1: FURB105 [*] Called `print` with an empty string and a redundant 8 8 | print("", sep="") 9 9 | print("", "", sep="") -FURB105.py:7:1: FURB105 [*] Called `print` with an empty string and a redundant separator, use `print()` instead +FURB105.py:7:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator | 5 | print("", end="bar") 6 | print("", sep=",", end="bar") @@ -92,7 +92,7 @@ FURB105.py:7:1: FURB105 [*] Called `print` with an empty string and a redundant 8 | print("", sep="") 9 | print("", "", sep="") | - = help: Remove empty string positional argument and redundant separator + = help: Remove empty string and separator ℹ Suggested fix 4 4 | print("", sep=",") @@ -104,7 +104,7 @@ FURB105.py:7:1: FURB105 [*] Called `print` with an empty string and a redundant 9 9 | print("", "", sep="") 10 10 | print("", "", sep="", end="") -FURB105.py:8:1: FURB105 [*] Called `print` with an empty string and a redundant separator, use `print()` instead +FURB105.py:8:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator | 6 | print("", sep=",", end="bar") 7 | print(sep="") @@ -113,7 +113,7 @@ FURB105.py:8:1: FURB105 [*] Called `print` with an empty string and a redundant 9 | print("", "", sep="") 10 | print("", "", sep="", end="") | - = help: Remove empty string positional argument and redundant separator + = help: Remove empty string and separator ℹ Suggested fix 5 5 | print("", end="bar") @@ -125,7 +125,7 @@ FURB105.py:8:1: FURB105 [*] Called `print` with an empty string and a redundant 10 10 | print("", "", sep="", end="") 11 11 | print("", "", sep="", end="bar") -FURB105.py:9:1: FURB105 [*] Called `print` with an empty string and a redundant separator, use `print()` instead +FURB105.py:9:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator | 7 | print(sep="") 8 | print("", sep="") @@ -134,7 +134,7 @@ FURB105.py:9:1: FURB105 [*] Called `print` with an empty string and a redundant 10 | print("", "", sep="", end="") 11 | print("", "", sep="", end="bar") | - = help: Remove empty string positional argument and redundant separator + = help: Remove empty string and separator ℹ Suggested fix 6 6 | print("", sep=",", end="bar") @@ -146,7 +146,7 @@ FURB105.py:9:1: FURB105 [*] Called `print` with an empty string and a redundant 11 11 | print("", "", sep="", end="bar") 12 12 | print("", sep="", end="bar") -FURB105.py:10:1: FURB105 [*] Called `print` with an empty string and a redundant separator, use `print(end="")` instead +FURB105.py:10:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator | 8 | print("", sep="") 9 | print("", "", sep="") @@ -155,7 +155,7 @@ FURB105.py:10:1: FURB105 [*] Called `print` with an empty string and a redundant 11 | print("", "", sep="", end="bar") 12 | print("", sep="", end="bar") | - = help: Remove empty string positional argument and redundant separator + = help: Remove empty string and separator ℹ Suggested fix 7 7 | print(sep="") @@ -167,7 +167,7 @@ FURB105.py:10:1: FURB105 [*] Called `print` with an empty string and a redundant 12 12 | print("", sep="", end="bar") 13 13 | print(sep="", end="bar") -FURB105.py:11:1: FURB105 [*] Called `print` with an empty string and a redundant separator, use `print(end="bar")` instead +FURB105.py:11:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator | 9 | print("", "", sep="") 10 | print("", "", sep="", end="") @@ -176,7 +176,7 @@ FURB105.py:11:1: FURB105 [*] Called `print` with an empty string and a redundant 12 | print("", sep="", end="bar") 13 | print(sep="", end="bar") | - = help: Remove empty string positional argument and redundant separator + = help: Remove empty string and separator ℹ Suggested fix 8 8 | print("", sep="") @@ -188,7 +188,7 @@ FURB105.py:11:1: FURB105 [*] Called `print` with an empty string and a redundant 13 13 | print(sep="", end="bar") 14 14 | print("", "foo", sep="") -FURB105.py:12:1: FURB105 [*] Called `print` with an empty string and a redundant separator, use `print(end="bar")` instead +FURB105.py:12:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator | 10 | print("", "", sep="", end="") 11 | print("", "", sep="", end="bar") @@ -197,7 +197,7 @@ FURB105.py:12:1: FURB105 [*] Called `print` with an empty string and a redundant 13 | print(sep="", end="bar") 14 | print("", "foo", sep="") | - = help: Remove empty string positional argument and redundant separator + = help: Remove empty string and separator ℹ Suggested fix 9 9 | print("", "", sep="") @@ -209,7 +209,7 @@ FURB105.py:12:1: FURB105 [*] Called `print` with an empty string and a redundant 14 14 | print("", "foo", sep="") 15 15 | print("foo", "", sep="") -FURB105.py:13:1: FURB105 [*] Called `print` with an empty string and a redundant separator, use `print(end="bar")` instead +FURB105.py:13:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator | 11 | print("", "", sep="", end="bar") 12 | print("", sep="", end="bar") @@ -218,7 +218,7 @@ FURB105.py:13:1: FURB105 [*] Called `print` with an empty string and a redundant 14 | print("", "foo", sep="") 15 | print("foo", "", sep="") | - = help: Remove empty string positional argument and redundant separator + = help: Remove empty string and separator ℹ Suggested fix 10 10 | print("", "", sep="", end="") @@ -230,7 +230,7 @@ FURB105.py:13:1: FURB105 [*] Called `print` with an empty string and a redundant 15 15 | print("foo", "", sep="") 16 16 | print("foo", "", "bar", sep="") -FURB105.py:14:1: FURB105 [*] Called `print` with an empty string and a redundant separator, use `print("foo")` instead +FURB105.py:14:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator | 12 | print("", sep="", end="bar") 13 | print(sep="", end="bar") @@ -239,7 +239,7 @@ FURB105.py:14:1: FURB105 [*] Called `print` with an empty string and a redundant 15 | print("foo", "", sep="") 16 | print("foo", "", "bar", sep="") | - = help: Remove empty string positional argument and redundant separator + = help: Remove empty string and separator ℹ Suggested fix 11 11 | print("", "", sep="", end="bar") @@ -251,7 +251,7 @@ FURB105.py:14:1: FURB105 [*] Called `print` with an empty string and a redundant 16 16 | print("foo", "", "bar", sep="") 17 17 | -FURB105.py:15:1: FURB105 [*] Called `print` with an empty string and a redundant separator, use `print("foo")` instead +FURB105.py:15:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator | 13 | print(sep="", end="bar") 14 | print("", "foo", sep="") @@ -259,7 +259,7 @@ FURB105.py:15:1: FURB105 [*] Called `print` with an empty string and a redundant | ^^^^^^^^^^^^^^^^^^^^^^^^ FURB105 16 | print("foo", "", "bar", sep="") | - = help: Remove empty string positional argument and redundant separator + = help: Remove empty string and separator ℹ Suggested fix 12 12 | print("", sep="", end="bar") @@ -271,7 +271,7 @@ FURB105.py:15:1: FURB105 [*] Called `print` with an empty string and a redundant 17 17 | 18 18 | # OK. -FURB105.py:16:1: FURB105 [*] Called `print` with an empty string, use `print("foo", "bar", sep="")` instead +FURB105.py:16:1: FURB105 [*] Unnecessary empty string passed to `print` | 14 | print("", "foo", sep="") 15 | print("foo", "", sep="") @@ -280,7 +280,7 @@ FURB105.py:16:1: FURB105 [*] Called `print` with an empty string, use `print("fo 17 | 18 | # OK. | - = help: Remove empty string positional argument + = help: Remove empty string ℹ Suggested fix 13 13 | print(sep="", end="bar")