Skip to content

Commit 675fd68

Browse files
relaxcnrelaxcn
committed
Fix println_empty_string suggestion caused error when there is a , after arg.
Make `writeln_empty_string` don't provide an auto-fix suggestion when there is a comment in the macro call span.
1 parent 6110c80 commit 675fd68

9 files changed

+295
-20
lines changed

‎clippy_lints/src/write/empty_string.rs‎

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,43 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::macros::MacroCall;
33
use clippy_utils::source::expand_past_previous_comma;
4-
use clippy_utils::sym;
4+
use clippy_utils::{span_extract_comments, sym};
55
use rustc_ast::{FormatArgs, FormatArgsPiece};
66
use rustc_errors::Applicability;
7-
use rustc_lint::LateContext;
7+
use rustc_lint::{LateContext, LintContext};
88

99
use super::{PRINTLN_EMPTY_STRING, WRITELN_EMPTY_STRING};
1010

1111
pub(super) fn check(cx: &LateContext<'_>, format_args: &FormatArgs, macro_call: &MacroCall, name: &str) {
1212
if let [FormatArgsPiece::Literal(sym::LF)] = &format_args.template[..] {
13-
let mut span = format_args.span;
14-
15-
let lint = if name == "writeln" {
16-
span = expand_past_previous_comma(cx, span);
17-
18-
WRITELN_EMPTY_STRING
19-
} else {
20-
PRINTLN_EMPTY_STRING
21-
};
13+
let is_writeln = name == "writeln";
2214

2315
span_lint_and_then(
2416
cx,
25-
lint,
17+
if is_writeln {
18+
WRITELN_EMPTY_STRING
19+
} else {
20+
PRINTLN_EMPTY_STRING
21+
},
2622
macro_call.span,
2723
format!("empty string literal in `{name}!`"),
2824
|diag| {
29-
diag.span_suggestion(
30-
span,
31-
"remove the empty string",
32-
String::new(),
33-
Applicability::MachineApplicable,
34-
);
25+
if span_extract_comments(cx.sess().source_map(), macro_call.span).is_empty() {
26+
let closing_paren = cx.sess().source_map().span_extend_to_prev_char_before(
27+
macro_call.span.shrink_to_hi(),
28+
')',
29+
false,
30+
);
31+
let mut span = format_args.span.with_hi(closing_paren.lo());
32+
if is_writeln {
33+
span = expand_past_previous_comma(cx, span);
34+
}
35+
36+
diag.span_suggestion(span, "remove the empty string", "", Applicability::MachineApplicable);
37+
} else {
38+
// If there is a comment in the span of macro call, we don't provide an auto-fix suggestion.
39+
diag.span_note(format_args.span, "remove the empty string");
40+
}
3541
},
3642
);
3743
}

‎tests/ui/crashes/ice-10148.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: empty string literal in `println!`
22
--> tests/ui/crashes/ice-10148.rs:8:5
33
|
44
LL | println!(with_span!(""something ""));
5-
| ^^^^^^^^^^^^^^^^^^^^-----------^^^^^
5+
| ^^^^^^^^^^^^^^^^^^^^---------------^
66
| |
77
| help: remove the empty string
88
|

‎tests/ui/println_empty_string.fixed‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,23 @@ fn main() {
1919
//~^ println_empty_string
2020
}
2121
}
22+
23+
#[rustfmt::skip]
24+
fn issue_16167() {
25+
//~v println_empty_string
26+
println!(
27+
);
28+
29+
match "a" {
30+
_ => println!(), // there is a space between "" and comma
31+
//~^ println_empty_string
32+
}
33+
34+
eprintln!(); // there is a tab between "" and comma
35+
//~^ println_empty_string
36+
37+
match "a" {
38+
_ => eprintln!(), // tab and space between "" and comma
39+
//~^ println_empty_string
40+
}
41+
}

‎tests/ui/println_empty_string.rs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,27 @@ fn main() {
1919
//~^ println_empty_string
2020
}
2121
}
22+
23+
#[rustfmt::skip]
24+
fn issue_16167() {
25+
//~v println_empty_string
26+
println!(
27+
"\
28+
\
29+
"
30+
,
31+
);
32+
33+
match "a" {
34+
_ => println!("" ,), // there is a space between "" and comma
35+
//~^ println_empty_string
36+
}
37+
38+
eprintln!("" ,); // there is a tab between "" and comma
39+
//~^ println_empty_string
40+
41+
match "a" {
42+
_ => eprintln!("" ,), // tab and space between "" and comma
43+
//~^ println_empty_string
44+
}
45+
}

‎tests/ui/println_empty_string.stderr‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,42 @@ LL | _ => eprintln!(""),
3333
| |
3434
| help: remove the empty string
3535

36-
error: aborting due to 4 previous errors
36+
error: empty string literal in `println!`
37+
--> tests/ui/println_empty_string.rs:26:5
38+
|
39+
LL | / println!(
40+
LL | |/ "\
41+
LL | || \
42+
LL | || "
43+
LL | || ,
44+
LL | || );
45+
| ||____-^
46+
| |____|
47+
| help: remove the empty string
48+
49+
error: empty string literal in `println!`
50+
--> tests/ui/println_empty_string.rs:34:14
51+
|
52+
LL | _ => println!("" ,), // there is a space between "" and comma
53+
| ^^^^^^^^^----^
54+
| |
55+
| help: remove the empty string
56+
57+
error: empty string literal in `eprintln!`
58+
--> tests/ui/println_empty_string.rs:38:5
59+
|
60+
LL | eprintln!("" ,); // there is a tab between "" and comma
61+
| ^^^^^^^^^^-------^
62+
| |
63+
| help: remove the empty string
64+
65+
error: empty string literal in `eprintln!`
66+
--> tests/ui/println_empty_string.rs:42:14
67+
|
68+
LL | _ => eprintln!("" ,), // tab and space between "" and comma
69+
| ^^^^^^^^^^--------^
70+
| |
71+
| help: remove the empty string
72+
73+
error: aborting due to 8 previous errors
3774

‎tests/ui/println_empty_string_unfixable.rs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#![allow(clippy::match_single_binding)]
2+
3+
// If there is a comment in the span of macro call, we don't provide an auto-fix suggestion.
4+
#[rustfmt::skip]
5+
fn issue_16167() {
6+
//~v println_empty_string
7+
println!("" /* comment */);
8+
//~v println_empty_string
9+
eprintln!("" /* comment */);
10+
11+
//~v println_empty_string
12+
println!( // comment
13+
"");
14+
//~v println_empty_string
15+
eprintln!( // comment
16+
"");
17+
18+
//~v println_empty_string
19+
println!("", /* comment */);
20+
21+
//~v println_empty_string
22+
println!(
23+
"\
24+
\
25+
",
26+
27+
// there is a comment in the macro span regardless of its position
28+
29+
);
30+
}

‎tests/ui/println_empty_string_unfixable.stderr‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
error: empty string literal in `println!`
2+
--> tests/ui/println_empty_string_unfixable.rs:7:5
3+
|
4+
LL | println!("" /* comment */);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
note: remove the empty string
8+
--> tests/ui/println_empty_string_unfixable.rs:7:14
9+
|
10+
LL | println!("" /* comment */);
11+
| ^^
12+
= note: `-D clippy::println-empty-string` implied by `-D warnings`
13+
= help: to override `-D warnings` add `#[allow(clippy::println_empty_string)]`
14+
15+
error: empty string literal in `eprintln!`
16+
--> tests/ui/println_empty_string_unfixable.rs:9:5
17+
|
18+
LL | eprintln!("" /* comment */);
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
20+
|
21+
note: remove the empty string
22+
--> tests/ui/println_empty_string_unfixable.rs:9:15
23+
|
24+
LL | eprintln!("" /* comment */);
25+
| ^^
26+
27+
error: empty string literal in `println!`
28+
--> tests/ui/println_empty_string_unfixable.rs:12:5
29+
|
30+
LL | / println!( // comment
31+
LL | | "");
32+
| |___________________^
33+
|
34+
note: remove the empty string
35+
--> tests/ui/println_empty_string_unfixable.rs:13:17
36+
|
37+
LL | "");
38+
| ^^
39+
40+
error: empty string literal in `eprintln!`
41+
--> tests/ui/println_empty_string_unfixable.rs:15:5
42+
|
43+
LL | / eprintln!( // comment
44+
LL | | "");
45+
| |___________________^
46+
|
47+
note: remove the empty string
48+
--> tests/ui/println_empty_string_unfixable.rs:16:17
49+
|
50+
LL | "");
51+
| ^^
52+
53+
error: empty string literal in `println!`
54+
--> tests/ui/println_empty_string_unfixable.rs:19:5
55+
|
56+
LL | println!("", /* comment */);
57+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
58+
|
59+
note: remove the empty string
60+
--> tests/ui/println_empty_string_unfixable.rs:19:14
61+
|
62+
LL | println!("", /* comment */);
63+
| ^^
64+
65+
error: empty string literal in `println!`
66+
--> tests/ui/println_empty_string_unfixable.rs:22:5
67+
|
68+
LL | / println!(
69+
LL | | "\
70+
LL | | \
71+
LL | | ",
72+
... |
73+
LL | | );
74+
| |_____^
75+
|
76+
note: remove the empty string
77+
--> tests/ui/println_empty_string_unfixable.rs:23:9
78+
|
79+
LL | / "\
80+
LL | | \
81+
LL | | ",
82+
| |_____________^
83+
84+
error: aborting due to 6 previous errors
85+

‎tests/ui/writeln_empty_string_unfixable.rs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
#![allow(unused_must_use)]
2+
#![warn(clippy::writeln_empty_string)]
3+
4+
use std::io::Write;
5+
6+
// If there is a comment in the span of macro call, we don't provide an auto-fix suggestion.
7+
#[rustfmt::skip]
8+
fn issue_16251() {
9+
let mut v = Vec::new();
10+
11+
writeln!(v, /* comment */ "");
12+
//~^ writeln_empty_string
13+
14+
writeln!(v, "" /* comment */);
15+
//~^ writeln_empty_string
16+
17+
//~v writeln_empty_string
18+
writeln!(v,
19+
"\
20+
\
21+
"
22+
23+
// there is a comment in the macro span regardless of its position
24+
25+
);
26+
}

‎tests/ui/writeln_empty_string_unfixable.stderr‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
error: empty string literal in `writeln!`
2+
--> tests/ui/writeln_empty_string_unfixable.rs:11:5
3+
|
4+
LL | writeln!(v, /* comment */ "");
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
note: remove the empty string
8+
--> tests/ui/writeln_empty_string_unfixable.rs:11:31
9+
|
10+
LL | writeln!(v, /* comment */ "");
11+
| ^^
12+
= note: `-D clippy::writeln-empty-string` implied by `-D warnings`
13+
= help: to override `-D warnings` add `#[allow(clippy::writeln_empty_string)]`
14+
15+
error: empty string literal in `writeln!`
16+
--> tests/ui/writeln_empty_string_unfixable.rs:14:5
17+
|
18+
LL | writeln!(v, "" /* comment */);
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
20+
|
21+
note: remove the empty string
22+
--> tests/ui/writeln_empty_string_unfixable.rs:14:17
23+
|
24+
LL | writeln!(v, "" /* comment */);
25+
| ^^
26+
27+
error: empty string literal in `writeln!`
28+
--> tests/ui/writeln_empty_string_unfixable.rs:18:5
29+
|
30+
LL | / writeln!(v,
31+
LL | | "\
32+
LL | | \
33+
LL | | "
34+
... |
35+
LL | | );
36+
| |_____^
37+
|
38+
note: remove the empty string
39+
--> tests/ui/writeln_empty_string_unfixable.rs:19:9
40+
|
41+
LL | / "\
42+
LL | | \
43+
LL | | "
44+
| |_____________^
45+
46+
error: aborting due to 3 previous errors
47+

0 commit comments

Comments
 (0)