Skip to content

Commit d2ed70c

Browse files
relaxcnrelaxcn
committed
Fix println_empty_string suggestion caused error when there is a , after arg.
Fix `writeln_empty_string` suggestion caused error when there is a `,` in the comment before arg.
1 parent 40ad1a6 commit d2ed70c

File tree

8 files changed

+305
-4
lines changed

8 files changed

+305
-4
lines changed

‎clippy_lints/src/write/empty_string.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::macros::MacroCall;
3-
use clippy_utils::source::expand_past_previous_comma;
3+
use clippy_utils::source::{expand_past_next_comma_before_end_pos, expand_past_previous_comma_after_begin_pos};
44
use clippy_utils::sym;
55
use rustc_ast::{FormatArgs, FormatArgsPiece};
66
use rustc_errors::Applicability;
@@ -13,10 +13,12 @@ pub(super) fn check(cx: &LateContext<'_>, format_args: &FormatArgs, macro_call:
1313
let mut span = format_args.span;
1414

1515
let lint = if name == "writeln" {
16-
span = expand_past_previous_comma(cx, span);
16+
span = expand_past_previous_comma_after_begin_pos(cx, span, macro_call.span.lo()).unwrap_or(span);
1717

1818
WRITELN_EMPTY_STRING
1919
} else {
20+
span = expand_past_next_comma_before_end_pos(cx, span, macro_call.span.hi()).unwrap_or(span);
21+
2022
PRINTLN_EMPTY_STRING
2123
};
2224

‎clippy_utils/src/source.rs‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -759,6 +759,67 @@ pub fn expand_past_previous_comma(sess: &impl HasSession, span: Span) -> Span {
759759
extended.with_lo(extended.lo() - BytePos(1))
760760
}
761761

762+
/// Expand a span to include a preceding comma (not in comment) after `begin_pos`.
763+
/// ```rust,ignore
764+
/// writeln!(o, /* , */ "") -> writeln!(o, /* , */ "")
765+
/// ^^ ^^^^^^^^^^^^
766+
/// ```
767+
pub fn expand_past_previous_comma_after_begin_pos(
768+
sess: &impl HasSession,
769+
span: Span,
770+
begin_pos: BytePos,
771+
) -> Option<Span> {
772+
if span.lo() <= begin_pos {
773+
return None;
774+
}
775+
776+
let source_map = sess.sess().source_map();
777+
let search_span = span.shrink_to_lo().with_lo(begin_pos);
778+
let Ok(text) = source_map.span_to_snippet(search_span) else {
779+
return None;
780+
};
781+
782+
let mut byte_offset = 0;
783+
for token in tokenize(&text, FrontmatterAllowed::No) {
784+
if token.kind == TokenKind::Comma {
785+
let comma_end_pos =
786+
span.lo() - BytePos(u32::try_from(text.len()).expect("Error casting from usize to u32") - byte_offset);
787+
return Some(span.with_lo(comma_end_pos));
788+
}
789+
byte_offset += token.len;
790+
}
791+
792+
None
793+
}
794+
795+
/// Expand a span to include the following comma (not in comment) before `end_pos`.
796+
/// ```rust,ignore
797+
/// println!("" /* , */ ,) -> println!("" /* , */ ,)
798+
/// ^^ ^^^^^^^^^^^^
799+
/// ```
800+
pub fn expand_past_next_comma_before_end_pos(sess: &impl HasSession, span: Span, end_pos: BytePos) -> Option<Span> {
801+
if span.hi() >= end_pos {
802+
return None;
803+
}
804+
805+
let source_map = sess.sess().source_map();
806+
let search_span = span.shrink_to_hi().with_hi(end_pos);
807+
let Ok(text) = source_map.span_to_snippet(search_span) else {
808+
return None;
809+
};
810+
811+
let mut byte_offset = 0;
812+
for token in tokenize(&text, FrontmatterAllowed::No) {
813+
if token.kind == TokenKind::Comma {
814+
let comma_end_pos = span.hi() + BytePos(byte_offset + token.len);
815+
return Some(span.with_hi(comma_end_pos));
816+
}
817+
byte_offset += token.len;
818+
}
819+
820+
None
821+
}
822+
762823
/// Converts `expr` to a `char` literal if it's a `str` literal containing a single
763824
/// character (or a single byte with `ascii_only`)
764825
pub fn str_literal_to_char_literal(

‎tests/ui/println_empty_string.fixed‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,42 @@ 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+
);
43+
match "a" {
44+
//~v println_empty_string
45+
_ => println!(
46+
47+
),
48+
}
49+
50+
//~v println_empty_string
51+
eprintln!(
52+
53+
);
54+
match "a" {
55+
//~v println_empty_string
56+
_ => eprintln!(
57+
58+
),
59+
}
60+
}

‎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+
",
70+
),
71+
}
72+
}

‎tests/ui/println_empty_string.stderr‎

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,89 @@ 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+
| ||_____________________- help: remove the empty string
80+
LL | | );
81+
| |______^
82+
83+
error: empty string literal in `println!`
84+
--> tests/ui/println_empty_string.rs:49:14
85+
|
86+
LL | _ => println!(
87+
| _______________^
88+
LL | |/ "\
89+
LL | || \
90+
LL | || "
91+
LL | || //, other comment
92+
LL | || ,
93+
| ||_____________________- help: remove the empty string
94+
LL | | ),
95+
| |__________^
96+
97+
error: empty string literal in `eprintln!`
98+
--> tests/ui/println_empty_string.rs:59:5
99+
|
100+
LL | / eprintln!(
101+
LL | |/ "\
102+
LL | || \
103+
LL | || ",
104+
| ||__________________- help: remove the empty string
105+
LL | | );
106+
| |______^
107+
108+
error: empty string literal in `eprintln!`
109+
--> tests/ui/println_empty_string.rs:66:14
110+
|
111+
LL | _ => eprintln!(
112+
| _______________^
113+
LL | |/ "\
114+
LL | || \
115+
LL | || ",
116+
| ||______________________- help: remove the empty string
117+
LL | | ),
118+
| |__________^
119+
120+
error: aborting due to 12 previous errors
37121

‎tests/ui/writeln_empty_string.fixed‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,17 @@ fn main() {
1818
writeln!(v, " ");
1919
write!(v, "");
2020
}
21+
22+
#[rustfmt::skip]
23+
fn issue_16251() {
24+
let mut v = Vec::new();
25+
26+
writeln!(v);
27+
//~^ writeln_empty_string
28+
29+
//~v writeln_empty_string
30+
writeln!(v);
31+
32+
//~v writeln_empty_string
33+
writeln!(v);
34+
}

‎tests/ui/writeln_empty_string.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,21 @@ fn main() {
1818
writeln!(v, " ");
1919
write!(v, "");
2020
}
21+
22+
#[rustfmt::skip]
23+
fn issue_16251() {
24+
let mut v = Vec::new();
25+
26+
writeln!(v, /* , */ "");
27+
//~^ writeln_empty_string
28+
29+
//~v writeln_empty_string
30+
writeln!(v, /* , */
31+
// hi ,
32+
"");
33+
34+
//~v writeln_empty_string
35+
writeln!(v,
36+
// hi ,
37+
/* , */ "");
38+
}

‎tests/ui/writeln_empty_string.stderr‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,37 @@ LL | writeln!(suggestion, "");
1717
| |
1818
| help: remove the empty string
1919

20-
error: aborting due to 2 previous errors
20+
error: empty string literal in `writeln!`
21+
--> tests/ui/writeln_empty_string.rs:26:5
22+
|
23+
LL | writeln!(v, /* , */ "");
24+
| ^^^^^^^^^^------------^
25+
| |
26+
| help: remove the empty string
27+
28+
error: empty string literal in `writeln!`
29+
--> tests/ui/writeln_empty_string.rs:30:5
30+
|
31+
LL | writeln!(v, /* , */
32+
| _____^ -
33+
| |_______________|
34+
LL | || // hi ,
35+
LL | || "");
36+
| ||__________-^
37+
| |___________|
38+
| help: remove the empty string
39+
40+
error: empty string literal in `writeln!`
41+
--> tests/ui/writeln_empty_string.rs:35:5
42+
|
43+
LL | writeln!(v,
44+
| _____^ -
45+
| |_______________|
46+
LL | || // hi ,
47+
LL | || /* , */ "");
48+
| ||__________________-^
49+
| |___________________|
50+
| help: remove the empty string
51+
52+
error: aborting due to 5 previous errors
2153

0 commit comments

Comments
 (0)