Skip to content

Commit

Permalink
Improve code documentation and catch more cases
Browse files Browse the repository at this point in the history
  • Loading branch information
@tjkuson
tjkuson committed Sep 23, 2023
1 parent 5a2fb01 commit 7bf84c8
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 18 deletions.
4 changes: 2 additions & 2 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB105.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
print("", "", sep="", end="bar")
print("", sep="", end="bar")
print(sep="", end="bar")
print("", "foo", sep="")
print("foo", "", sep="")

# OK.

Expand All @@ -19,5 +21,3 @@
print("", "")
print("", "foo")
print("foo", "")
print("", "foo", sep="")
print("foo", "", sep="")
34 changes: 25 additions & 9 deletions crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,48 +63,57 @@ impl Violation for PrintEmptyString {

/// 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"]))
{
// If the print call does not have precisely one positional argument,
// do not trigger (more than one positional empty string argument is
// not equivalent to no positional arguments due to the `sep` argument).
// Check if the `sep` keyword argument is an empty string.
let sep_value_is_empty_string = call
.arguments
.find_keyword("sep")
.map_or(false, |keyword| is_const_empty_string(&keyword.value));

// If the print call does not have precisely one positional argument,
// do not trigger unless the `sep` keyword argument is an empty string.
if call.arguments.args.len() != 1 && !sep_value_is_empty_string {
return;
}
// Check if the positional arguments is are all empty string.
let is_empty = call.arguments.args.iter().all(is_const_empty_string);

if is_empty {
// 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))
{
// Find the index of the `sep` keyword argument, if it exists.
let sep_index = call.arguments.keywords.iter().position(|keyword| {
keyword
.arg
.clone()
.is_some_and(|arg| arg.to_string() == "sep")
});

let suggestion = 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(),
);

if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::suggested(Edit::replacement(
suggestion,
call.start(),
call.end(),
)));
}

checker.diagnostics.push(diagnostic);
}
}
Expand All @@ -122,19 +131,26 @@ fn is_const_empty_string(expr: &Expr) -> bool {
}

/// Generate a suggestion to replace a `print` call with `print` call with no
/// positional arguments, and no `sep` keyword argument.
/// empty string positional arguments, and no `sep` keyword argument.
fn generate_suggestion(
call: &ast::ExprCall,
sep_index: Option<usize>,
generator: Generator,
) -> String {
// Clone the call so that we can mutate it.
let mut suggestion = call.clone();
// Remove all positional arguments.
suggestion.arguments.args.clear();

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

generator.expr(&suggestion.into())
}
56 changes: 49 additions & 7 deletions ...ter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB105_FURB105.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ FURB105.py:11:1: FURB105 [*] Called `print` with an empty string and a redundant
11 |+print(end="bar")
12 12 | print("", sep="", end="bar")
13 13 | print(sep="", end="bar")
14 14 |
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
|
Expand All @@ -195,6 +195,7 @@ FURB105.py:12:1: FURB105 [*] Called `print` with an empty string and a redundant
12 | print("", sep="", end="bar")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
13 | print(sep="", end="bar")
14 | print("", "foo", sep="")
|
= help: Remove empty string positional argument and redundant separator

Expand All @@ -205,17 +206,17 @@ FURB105.py:12:1: FURB105 [*] Called `print` with an empty string and a redundant
12 |-print("", sep="", end="bar")
12 |+print(end="bar")
13 13 | print(sep="", end="bar")
14 14 |
15 15 | # OK.
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
|
11 | print("", "", sep="", end="bar")
12 | print("", sep="", end="bar")
13 | print(sep="", end="bar")
| ^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
14 |
15 | # OK.
14 | print("", "foo", sep="")
15 | print("foo", "", sep="")
|
= help: Remove empty string positional argument and redundant separator

Expand All @@ -225,8 +226,49 @@ FURB105.py:13:1: FURB105 [*] Called `print` with an empty string and a redundant
12 12 | print("", sep="", end="bar")
13 |-print(sep="", end="bar")
13 |+print(end="bar")
14 14 |
15 15 | # OK.
14 14 | print("", "foo", sep="")
15 15 | print("foo", "", sep="")
16 16 |

FURB105.py:14:1: FURB105 [*] Called `print` with an empty string and a redundant separator, use `print("foo")` instead
|
12 | print("", sep="", end="bar")
13 | print(sep="", end="bar")
14 | print("", "foo", sep="")
| ^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
15 | print("foo", "", sep="")
|
= help: Remove empty string positional argument and redundant separator

ℹ Suggested fix
11 11 | print("", "", sep="", end="bar")
12 12 | print("", sep="", end="bar")
13 13 | print(sep="", end="bar")
14 |-print("", "foo", sep="")
14 |+print("foo")
15 15 | print("foo", "", sep="")
16 16 |
17 17 | # OK.

FURB105.py:15:1: FURB105 [*] Called `print` with an empty string and a redundant separator, use `print("foo")` instead
|
13 | print(sep="", end="bar")
14 | print("", "foo", sep="")
15 | print("foo", "", sep="")
| ^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
16 |
17 | # OK.
|
= help: Remove empty string positional argument and redundant separator

ℹ Suggested fix
12 12 | print("", sep="", end="bar")
13 13 | print(sep="", end="bar")
14 14 | print("", "foo", sep="")
15 |-print("foo", "", sep="")
15 |+print("foo")
16 16 |
17 17 | # OK.
18 18 |


0 comments on commit 7bf84c8

Please sign in to comment.