Skip to content

Commit 8f43c70

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 8f43c70

File tree

7 files changed

+296
-20
lines changed

7 files changed

+296
-20
lines changed

‎clippy_lints/src/write/empty_string.rs‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,48 @@
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+
let mut applicability = Applicability::MaybeIncorrect;
26+
let mut span = format_args.span;
27+
28+
// If the macro call is `println!`
29+
// or there isn't a comment in the span of macro call, we provide an auto-fix suggestion.
30+
if !is_writeln || span_extract_comments(cx.sess().source_map(), macro_call.span).is_empty() {
31+
applicability = Applicability::MachineApplicable;
32+
33+
let closing_paren = cx.sess().source_map().span_extend_to_prev_char_before(
34+
macro_call.span.shrink_to_hi(),
35+
')',
36+
false,
37+
);
38+
span = format_args.span.with_hi(closing_paren.lo());
39+
40+
if is_writeln {
41+
span = expand_past_previous_comma(cx, span);
42+
}
43+
}
44+
45+
diag.span_suggestion(span, "remove the empty string", "", applicability);
3546
},
3647
);
3748
}

‎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: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,38 @@ fn main() {
1919
//~^ println_empty_string
2020
}
2121
}
22+
23+
#[rustfmt::skip]
24+
fn issue_16167() {
25+
println!();
26+
//~^ println_empty_string
27+
match "a" {
28+
_ => println!(),
29+
//~^ println_empty_string
30+
}
31+
32+
eprintln!();
33+
//~^^ println_empty_string
34+
match "a" {
35+
_ => eprintln!(), // tab
36+
//~^ println_empty_string
37+
}
38+
39+
//~v println_empty_string
40+
println!(
41+
);
42+
match "a" {
43+
//~v println_empty_string
44+
_ => println!(
45+
),
46+
}
47+
48+
//~v println_empty_string
49+
eprintln!(
50+
);
51+
match "a" {
52+
//~v println_empty_string
53+
_ => eprintln!(
54+
),
55+
}
56+
}

‎tests/ui/println_empty_string.rs‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,54 @@ fn main() {
1919
//~^ println_empty_string
2020
}
2121
}
22+
23+
#[rustfmt::skip]
24+
fn issue_16167() {
25+
println!(""/* ,,, */);
26+
//~^ println_empty_string
27+
match "a" {
28+
_ => println!("" ,),
29+
//~^ println_empty_string
30+
}
31+
32+
eprintln!(""
33+
,);
34+
//~^^ println_empty_string
35+
match "a" {
36+
_ => eprintln!("" ,), // tab
37+
//~^ println_empty_string
38+
}
39+
40+
//~v println_empty_string
41+
println!(
42+
"\
43+
\
44+
" /* comment ,,,
45+
, */ ,
46+
);
47+
match "a" {
48+
//~v println_empty_string
49+
_ => println!(
50+
"\
51+
\
52+
"
53+
//, other comment
54+
,
55+
),
56+
}
57+
58+
//~v println_empty_string
59+
eprintln!(
60+
"\
61+
\
62+
",
63+
);
64+
match "a" {
65+
//~v println_empty_string
66+
_ => eprintln!(
67+
"\
68+
\
69+
", // , comment
70+
),
71+
}
72+
}

‎tests/ui/println_empty_string.stderr‎

Lines changed: 89 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,93 @@ 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:25:5
38+
|
39+
LL | println!(""/* ,,, */);
40+
| ^^^^^^^^^-----------^
41+
| |
42+
| help: remove the empty string
43+
44+
error: empty string literal in `println!`
45+
--> tests/ui/println_empty_string.rs:28:14
46+
|
47+
LL | _ => println!("" ,),
48+
| ^^^^^^^^^--------^
49+
| |
50+
| help: remove the empty string
51+
52+
error: empty string literal in `eprintln!`
53+
--> tests/ui/println_empty_string.rs:32:5
54+
|
55+
LL | eprintln!(""
56+
| _____^ -
57+
| |_______________|
58+
LL | || ,);
59+
| ||_________-^
60+
| |__________|
61+
| help: remove the empty string
62+
63+
error: empty string literal in `eprintln!`
64+
--> tests/ui/println_empty_string.rs:36:14
65+
|
66+
LL | _ => eprintln!("" ,), // tab
67+
| ^^^^^^^^^^-------^
68+
| |
69+
| help: remove the empty string
70+
71+
error: empty string literal in `println!`
72+
--> tests/ui/println_empty_string.rs:41:5
73+
|
74+
LL | / println!(
75+
LL | |/ "\
76+
LL | || \
77+
LL | || " /* comment ,,,
78+
LL | || , */ ,
79+
LL | || );
80+
| ||____-^
81+
| |____|
82+
| help: remove the empty string
83+
84+
error: empty string literal in `println!`
85+
--> tests/ui/println_empty_string.rs:49:14
86+
|
87+
LL | _ => println!(
88+
| _______________^
89+
LL | |/ "\
90+
LL | || \
91+
LL | || "
92+
LL | || //, other comment
93+
LL | || ,
94+
LL | || ),
95+
| ||________-^
96+
| |________|
97+
| help: remove the empty string
98+
99+
error: empty string literal in `eprintln!`
100+
--> tests/ui/println_empty_string.rs:59:5
101+
|
102+
LL | / eprintln!(
103+
LL | |/ "\
104+
LL | || \
105+
LL | || ",
106+
LL | || );
107+
| ||____-^
108+
| |____|
109+
| help: remove the empty string
110+
111+
error: empty string literal in `eprintln!`
112+
--> tests/ui/println_empty_string.rs:66:14
113+
|
114+
LL | _ => eprintln!(
115+
| _______________^
116+
LL | |/ "\
117+
LL | || \
118+
LL | || ", // , comment
119+
LL | || ),
120+
| ||________-^
121+
| |________|
122+
| help: remove the empty string
123+
124+
error: aborting due to 12 previous errors
37125

‎tests/ui/writeln_empty_string_unfixable.rs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
//@no-rustfix
2+
#![allow(unused_must_use)]
3+
#![warn(clippy::writeln_empty_string)]
4+
5+
use std::io::Write;
6+
7+
fn issue_16251() {
8+
let mut v = Vec::new();
9+
10+
writeln!(v, /* , */ "");
11+
//~^ writeln_empty_string
12+
13+
writeln!(v, "" /* , */);
14+
//~^ writeln_empty_string
15+
16+
writeln!(v, /* comment */ "");
17+
//~^ writeln_empty_string
18+
19+
writeln!(v, "" /* comment */);
20+
//~^ writeln_empty_string
21+
22+
writeln!(v, /* comment, comma */ "");
23+
//~^ writeln_empty_string
24+
25+
writeln!(v, "" /* comment, comma */);
26+
//~^ writeln_empty_string
27+
28+
writeln!(/* */ v, "");
29+
//~^ writeln_empty_string
30+
}

‎tests/ui/writeln_empty_string_unfixable.stderr‎

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

0 commit comments

Comments
 (0)