[refurb] Implement print-empty-string (FURB105) (#7617)

## Summary

Implement
[`simplify-print`](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/print.py)
as `print-empty-string` (`FURB105`).

Extends the original rule in that it also checks for multiple empty
string positional arguments with an empty string separator.

Related to #1348.

## Test Plan

`cargo test`
This commit is contained in:
Tom Kuson 2023-09-24 05:10:36 +01:00 committed by GitHub
parent 865c89800e
commit 604cf521b5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 535 additions and 0 deletions

View file

@ -0,0 +1,32 @@
# Errors.
print("")
print("", sep=",")
print("", end="bar")
print("", sep=",", end="bar")
print(sep="")
print("", sep="")
print("", "", sep="")
print("", "", sep="", end="")
print("", "", sep="", end="bar")
print("", sep="", end="bar")
print(sep="", end="bar")
print("", "foo", sep="")
print("foo", "", sep="")
print("foo", "", "bar", sep="")
print("", *args)
print("", *args, sep="")
# OK.
print()
print("foo")
print("", "")
print("", "foo")
print("foo", "")
print("", "", sep=",")
print("", "foo", sep=",")
print("foo", "", sep=",")
print("foo", "", "bar", "", sep=",")
print("", "", **kwargs)
print("", **kwargs)

View file

@ -895,6 +895,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::UnsupportedMethodCallOnAll) {
flake8_pyi::rules::unsupported_method_call_on_all(checker, func);
}
if checker.enabled(Rule::PrintEmptyString) {
refurb::rules::print_empty_string(checker, call);
}
if checker.enabled(Rule::QuadraticListSummation) {
ruff::rules::quadratic_list_summation(checker, call);
}

View file

@ -914,6 +914,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Slots, "002") => (RuleGroup::Unspecified, rules::flake8_slots::rules::NoSlotsInNamedtupleSubclass),
// refurb
(Refurb, "105") => (RuleGroup::Preview, rules::refurb::rules::PrintEmptyString),
#[allow(deprecated)]
(Refurb, "113") => (RuleGroup::Nursery, rules::refurb::rules::RepeatedAppend),
#[allow(deprecated)]

View file

@ -20,6 +20,7 @@ mod tests {
#[test_case(Rule::ReimplementedStarmap, Path::new("FURB140.py"))]
#[test_case(Rule::SliceCopy, Path::new("FURB145.py"))]
#[test_case(Rule::UnnecessaryEnumerate, Path::new("FURB148.py"))]
#[test_case(Rule::PrintEmptyString, Path::new("FURB105.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(

View file

@ -1,5 +1,6 @@
pub(crate) use check_and_remove_from_set::*;
pub(crate) use delete_full_slice::*;
pub(crate) use print_empty_string::*;
pub(crate) use reimplemented_starmap::*;
pub(crate) use repeated_append::*;
pub(crate) use slice_copy::*;
@ -7,6 +8,7 @@ pub(crate) use unnecessary_enumerate::*;
mod check_and_remove_from_set;
mod delete_full_slice;
mod print_empty_string;
mod reimplemented_starmap;
mod repeated_append;
mod slice_copy;

View file

@ -0,0 +1,177 @@
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Constant, Expr};
use ruff_python_codegen::Generator;
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
use crate::registry::AsRule;
/// ## What it does
/// Checks for `print` calls with an empty string as the only positional
/// argument.
///
/// ## Why is this bad?
/// Prefer calling `print` without any positional arguments, which is
/// equivalent and more concise.
///
/// ## Example
/// ```python
/// print("")
/// ```
///
/// Use instead:
/// ```python
/// print()
/// ```
///
/// ## References
/// - [Python documentation: `print`](https://docs.python.org/3/library/functions.html#print)
#[violation]
pub struct PrintEmptyString {
separator: Option<Separator>,
}
impl Violation for PrintEmptyString {
const AUTOFIX: AutofixKind = AutofixKind::Sometimes;
#[derive_message_formats]
fn message(&self) -> String {
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<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) {
// Avoid flagging, e.g., `print("", "", **kwargs)`.
if call
.arguments
.keywords
.iter()
.any(|keyword| keyword.arg.is_none())
{
return;
}
if checker
.semantic()
.resolve_call_path(&call.func)
.as_ref()
.is_some_and(|call_path| matches!(call_path.as_slice(), ["", "print"]))
{
// 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_empty_string)
|| (empty_separator && call.arguments.args.iter().any(is_empty_string))
{
let separator = call
.arguments
.keywords
.iter()
.any(|keyword| {
keyword
.arg
.as_ref()
.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
.arguments
.args
.iter()
.filter(|arg| !is_empty_string(arg))
.count();
if non_empty > 1 {
return Separator::Retain;
}
Separator::Remove
});
let mut diagnostic = Diagnostic::new(PrintEmptyString { separator }, call.range());
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::suggested(Edit::replacement(
generate_suggestion(call, separator, checker.generator()),
call.start(),
call.end(),
)));
}
checker.diagnostics.push(diagnostic);
}
}
}
/// Check if an expression is a constant empty string.
fn is_empty_string(expr: &Expr) -> bool {
matches!(
expr,
Expr::Constant(ast::ExprConstant {
value: Constant::Str(s),
..
}) if s.is_empty()
)
}
#[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,
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) {
call.arguments.keywords.retain(|keyword| {
keyword
.arg
.as_ref()
.map_or(true, |arg| arg.as_str() != "sep")
});
}
generator.expr(&call.into())
}

View file

@ -0,0 +1,317 @@
---
source: crates/ruff_linter/src/rules/refurb/mod.rs
---
FURB105.py:3:1: FURB105 [*] Unnecessary empty string passed to `print`
|
1 | # Errors.
2 |
3 | print("")
| ^^^^^^^^^ FURB105
4 | print("", sep=",")
5 | print("", end="bar")
|
= help: Remove empty string
Suggested fix
1 1 | # Errors.
2 2 |
3 |-print("")
3 |+print()
4 4 | print("", sep=",")
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
|
3 | print("")
4 | print("", sep=",")
| ^^^^^^^^^^^^^^^^^^ FURB105
5 | print("", end="bar")
6 | print("", sep=",", end="bar")
|
= help: Remove empty string and separator
Suggested fix
1 1 | # Errors.
2 2 |
3 3 | print("")
4 |-print("", sep=",")
4 |+print()
5 5 | print("", end="bar")
6 6 | print("", sep=",", end="bar")
7 7 | print(sep="")
FURB105.py:5:1: FURB105 [*] Unnecessary empty string passed to `print`
|
3 | print("")
4 | print("", sep=",")
5 | print("", end="bar")
| ^^^^^^^^^^^^^^^^^^^^ FURB105
6 | print("", sep=",", end="bar")
7 | print(sep="")
|
= help: Remove empty string
Suggested fix
2 2 |
3 3 | print("")
4 4 | print("", sep=",")
5 |-print("", end="bar")
5 |+print(end="bar")
6 6 | print("", sep=",", end="bar")
7 7 | print(sep="")
8 8 | print("", sep="")
FURB105.py:6:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
|
4 | print("", sep=",")
5 | print("", end="bar")
6 | print("", sep=",", end="bar")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
7 | print(sep="")
8 | print("", sep="")
|
= help: Remove empty string and separator
Suggested fix
3 3 | print("")
4 4 | print("", sep=",")
5 5 | print("", end="bar")
6 |-print("", sep=",", end="bar")
6 |+print(end="bar")
7 7 | print(sep="")
8 8 | print("", sep="")
9 9 | print("", "", sep="")
FURB105.py:7:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
|
5 | print("", end="bar")
6 | print("", sep=",", end="bar")
7 | print(sep="")
| ^^^^^^^^^^^^^ FURB105
8 | print("", sep="")
9 | print("", "", sep="")
|
= help: Remove empty string and separator
Suggested fix
4 4 | print("", sep=",")
5 5 | print("", end="bar")
6 6 | print("", sep=",", end="bar")
7 |-print(sep="")
7 |+print()
8 8 | print("", sep="")
9 9 | print("", "", sep="")
10 10 | print("", "", sep="", end="")
FURB105.py:8:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
|
6 | print("", sep=",", end="bar")
7 | print(sep="")
8 | print("", sep="")
| ^^^^^^^^^^^^^^^^^ FURB105
9 | print("", "", sep="")
10 | print("", "", sep="", end="")
|
= help: Remove empty string and separator
Suggested fix
5 5 | print("", end="bar")
6 6 | print("", sep=",", end="bar")
7 7 | print(sep="")
8 |-print("", sep="")
8 |+print()
9 9 | print("", "", sep="")
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
|
7 | print(sep="")
8 | print("", sep="")
9 | print("", "", sep="")
| ^^^^^^^^^^^^^^^^^^^^^ FURB105
10 | print("", "", sep="", end="")
11 | print("", "", sep="", end="bar")
|
= help: Remove empty string and separator
Suggested fix
6 6 | print("", sep=",", end="bar")
7 7 | print(sep="")
8 8 | print("", sep="")
9 |-print("", "", sep="")
9 |+print()
10 10 | print("", "", sep="", end="")
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
|
8 | print("", sep="")
9 | print("", "", sep="")
10 | print("", "", sep="", end="")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
11 | print("", "", sep="", end="bar")
12 | print("", sep="", end="bar")
|
= help: Remove empty string and separator
Suggested fix
7 7 | print(sep="")
8 8 | print("", sep="")
9 9 | print("", "", sep="")
10 |-print("", "", sep="", end="")
10 |+print(end="")
11 11 | print("", "", sep="", end="bar")
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
|
9 | print("", "", sep="")
10 | print("", "", sep="", end="")
11 | print("", "", sep="", end="bar")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
12 | print("", sep="", end="bar")
13 | print(sep="", end="bar")
|
= help: Remove empty string and separator
Suggested fix
8 8 | print("", sep="")
9 9 | print("", "", sep="")
10 10 | print("", "", sep="", end="")
11 |-print("", "", sep="", end="bar")
11 |+print(end="bar")
12 12 | print("", sep="", end="bar")
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
|
10 | print("", "", sep="", end="")
11 | print("", "", sep="", end="bar")
12 | print("", sep="", end="bar")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
13 | print(sep="", end="bar")
14 | print("", "foo", sep="")
|
= help: Remove empty string and separator
Suggested fix
9 9 | print("", "", sep="")
10 10 | print("", "", sep="", end="")
11 11 | print("", "", sep="", end="bar")
12 |-print("", sep="", end="bar")
12 |+print(end="bar")
13 13 | print(sep="", end="bar")
14 14 | print("", "foo", sep="")
15 15 | print("foo", "", sep="")
FURB105.py:13:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
|
11 | print("", "", sep="", end="bar")
12 | print("", sep="", end="bar")
13 | print(sep="", end="bar")
| ^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
14 | print("", "foo", sep="")
15 | print("foo", "", sep="")
|
= help: Remove empty string and separator
Suggested fix
10 10 | print("", "", sep="", end="")
11 11 | print("", "", sep="", end="bar")
12 12 | print("", sep="", end="bar")
13 |-print(sep="", end="bar")
13 |+print(end="bar")
14 14 | print("", "foo", sep="")
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
|
12 | print("", sep="", end="bar")
13 | print(sep="", end="bar")
14 | print("", "foo", sep="")
| ^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
15 | print("foo", "", sep="")
16 | print("foo", "", "bar", sep="")
|
= help: Remove empty string and 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 | print("foo", "", "bar", sep="")
17 17 | print("", *args)
FURB105.py:15:1: FURB105 [*] Unnecessary empty string passed to `print` with redundant separator
|
13 | print(sep="", end="bar")
14 | print("", "foo", sep="")
15 | print("foo", "", sep="")
| ^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
16 | print("foo", "", "bar", sep="")
17 | print("", *args)
|
= help: Remove empty string and 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 | print("foo", "", "bar", sep="")
17 17 | print("", *args)
18 18 | print("", *args, sep="")
FURB105.py:16:1: FURB105 [*] Unnecessary empty string passed to `print`
|
14 | print("", "foo", sep="")
15 | print("foo", "", sep="")
16 | print("foo", "", "bar", sep="")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
17 | print("", *args)
18 | print("", *args, sep="")
|
= help: Remove empty string
Suggested fix
13 13 | print(sep="", end="bar")
14 14 | print("", "foo", sep="")
15 15 | print("foo", "", sep="")
16 |-print("foo", "", "bar", sep="")
16 |+print("foo", "bar", sep="")
17 17 | print("", *args)
18 18 | print("", *args, sep="")
19 19 |
FURB105.py:18:1: FURB105 [*] Unnecessary empty string passed to `print`
|
16 | print("foo", "", "bar", sep="")
17 | print("", *args)
18 | print("", *args, sep="")
| ^^^^^^^^^^^^^^^^^^^^^^^^ FURB105
19 |
20 | # OK.
|
= help: Remove empty string
Suggested fix
15 15 | print("foo", "", sep="")
16 16 | print("foo", "", "bar", sep="")
17 17 | print("", *args)
18 |-print("", *args, sep="")
18 |+print(*args, sep="")
19 19 |
20 20 | # OK.
21 21 |

2
ruff.schema.json generated
View file

@ -2217,6 +2217,8 @@
"FLY002",
"FURB",
"FURB1",
"FURB10",
"FURB105",
"FURB11",
"FURB113",
"FURB13",