Skip to content

Commit 79e8182

Browse files
oli-obkoli-obk
authored
Merge pull request #2690 from mrecachinas/feature/print-string-literal
Fix alignment/precision/width false positives for print/write_literal
2 parents c5b39a5 + a317bc9 commit 79e8182

File tree

5 files changed

+150
-113
lines changed

5 files changed

+150
-113
lines changed

‎clippy_lints/src/write.rs

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use syntax::ptr;
77
use syntax::symbol::InternedString;
88
use syntax_pos::Span;
99
use utils::{is_expn_of, match_def_path, match_path, resolve_node, span_lint, span_lint_and_sugg};
10-
use utils::{opt_def_id, paths};
10+
use utils::{opt_def_id, paths, last_path_segment};
1111

1212
/// **What it does:** This lint warns when you use `println!("")` to
1313
/// print a newline.
@@ -266,7 +266,6 @@ fn check_print_variants<'a, 'tcx>(
266266
};
267267

268268
span_lint(cx, PRINT_STDOUT, span, &format!("use of `{}!`", name));
269-
270269
if_chain! {
271270
// ensure we're calling Arguments::new_v1
272271
if args.len() == 1;
@@ -339,7 +338,9 @@ where
339338
F: Fn(Span),
340339
{
341340
if_chain! {
342-
if args.len() > 1;
341+
if args.len() >= 2;
342+
343+
// the match statement
343344
if let ExprAddrOf(_, ref match_expr) = args[1].node;
344345
if let ExprMatch(ref matchee, ref arms, _) = match_expr.node;
345346
if let ExprTup(ref tup) = matchee.node;
@@ -355,15 +356,31 @@ where
355356
if let ExprLit(_) = tup_val.node;
356357

357358
// next, check the corresponding match arm body to ensure
358-
// this is "{}", or DISPLAY_FMT_METHOD
359+
// this is DISPLAY_FMT_METHOD
359360
if let ExprCall(_, ref body_args) = arm_body_exprs[idx].node;
360361
if body_args.len() == 2;
361362
if let ExprPath(ref body_qpath) = body_args[1].node;
362363
if let Some(fun_def_id) = opt_def_id(resolve_node(cx, body_qpath, body_args[1].hir_id));
363-
if match_def_path(cx.tcx, fun_def_id, &paths::DISPLAY_FMT_METHOD) ||
364-
match_def_path(cx.tcx, fun_def_id, &paths::DEBUG_FMT_METHOD);
364+
if match_def_path(cx.tcx, fun_def_id, &paths::DISPLAY_FMT_METHOD);
365365
then {
366-
lint_fn(tup_val.span);
366+
if args.len() == 2 {
367+
lint_fn(tup_val.span);
368+
}
369+
370+
// ensure the format str has no options (e.g., width, precision, alignment, etc.)
371+
// and is just "{}"
372+
if_chain! {
373+
if args.len() == 3;
374+
if let ExprAddrOf(_, ref format_expr) = args[2].node;
375+
if let ExprArray(ref format_exprs) = format_expr.node;
376+
if format_exprs.len() >= 1;
377+
if let ExprStruct(_, ref fields, _) = format_exprs[idx].node;
378+
if let Some(format_field) = fields.iter().find(|f| f.name.node == "format");
379+
if check_unformatted(&format_field.expr);
380+
then {
381+
lint_fn(tup_val.span);
382+
}
383+
}
367384
}
368385
}
369386
}
@@ -438,3 +455,33 @@ fn is_in_debug_impl(cx: &LateContext, expr: &Expr) -> bool {
438455
}
439456
false
440457
}
458+
459+
/// Checks if the expression matches
460+
/// ```rust,ignore
461+
/// &[_ {
462+
/// format: _ {
463+
/// width: _::Implied,
464+
/// ...
465+
/// },
466+
/// ...,
467+
/// }]
468+
/// ```
469+
pub fn check_unformatted(format_field: &Expr) -> bool {
470+
if_chain! {
471+
if let ExprStruct(_, ref fields, _) = format_field.node;
472+
if let Some(width_field) = fields.iter().find(|f| f.name.node == "width");
473+
if let ExprPath(ref qpath) = width_field.expr.node;
474+
if last_path_segment(qpath).name == "Implied";
475+
if let Some(align_field) = fields.iter().find(|f| f.name.node == "align");
476+
if let ExprPath(ref qpath) = align_field.expr.node;
477+
if last_path_segment(qpath).name == "Unknown";
478+
if let Some(precision_field) = fields.iter().find(|f| f.name.node == "precision");
479+
if let ExprPath(ref qpath_precision) = precision_field.expr.node;
480+
if last_path_segment(qpath_precision).name == "Implied";
481+
then {
482+
return true;
483+
}
484+
}
485+
486+
false
487+
}

‎tests/ui/print_literal.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,23 @@ fn main() {
99
let world = "world";
1010
println!("Hello {}", world);
1111
println!("3 in hex is {:X}", 3);
12+
println!("2 + 1 = {:.4}", 3);
13+
println!("2 + 1 = {:5.4}", 3);
14+
println!("Debug test {:?}", "hello, world");
15+
println!("{0:8} {1:>8}", "hello", "world");
16+
println!("{1:8} {0:>8}", "hello", "world");
17+
println!("{foo:8} {bar:>8}", foo="hello", bar="world");
18+
println!("{bar:8} {foo:>8}", foo="hello", bar="world");
19+
println!("{number:>width$}", number=1, width=6);
20+
println!("{number:>0width$}", number=1, width=6);
1221

1322
// these should throw warnings
23+
println!("{} of {:b} people know binary, the other half doesn't", 1, 2);
1424
print!("Hello {}", "world");
1525
println!("Hello {} {}", world, "world");
1626
println!("Hello {}", "world");
1727
println!("10 / 4 is {}", 2.5);
1828
println!("2 + 1 = {}", 3);
19-
println!("2 + 1 = {:.4}", 3);
20-
println!("2 + 1 = {:5.4}", 3);
21-
println!("Debug test {:?}", "hello, world");
2229

2330
// positional args don't change the fact
2431
// that we're using a literal -- this should

‎tests/ui/print_literal.stderr

Lines changed: 34 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,100 +1,88 @@
11
error: printing a literal with an empty format string
2-
--> $DIR/print_literal.rs:14:24
2+
--> $DIR/print_literal.rs:23:71
33
|
4-
14 | print!("Hello {}", "world");
5-
| ^^^^^^^
4+
23 | println!("{} of {:b} people know binary, the other half doesn't", 1, 2);
5+
| ^
66
|
77
= note: `-D print-literal` implied by `-D warnings`
88

99
error: printing a literal with an empty format string
10-
--> $DIR/print_literal.rs:15:36
10+
--> $DIR/print_literal.rs:24:24
1111
|
12-
15 | println!("Hello {} {}", world, "world");
12+
24 | print!("Hello {}", "world");
13+
| ^^^^^^^
14+
15+
error: printing a literal with an empty format string
16+
--> $DIR/print_literal.rs:25:36
17+
|
18+
25 | println!("Hello {} {}", world, "world");
1319
| ^^^^^^^
1420

1521
error: printing a literal with an empty format string
16-
--> $DIR/print_literal.rs:16:26
22+
--> $DIR/print_literal.rs:26:26
1723
|
18-
16 | println!("Hello {}", "world");
24+
26 | println!("Hello {}", "world");
1925
| ^^^^^^^
2026

2127
error: printing a literal with an empty format string
22-
--> $DIR/print_literal.rs:17:30
28+
--> $DIR/print_literal.rs:27:30
2329
|
24-
17 | println!("10 / 4 is {}", 2.5);
30+
27 | println!("10 / 4 is {}", 2.5);
2531
| ^^^
2632

2733
error: printing a literal with an empty format string
28-
--> $DIR/print_literal.rs:18:28
34+
--> $DIR/print_literal.rs:28:28
2935
|
30-
18 | println!("2 + 1 = {}", 3);
36+
28 | println!("2 + 1 = {}", 3);
3137
| ^
3238

3339
error: printing a literal with an empty format string
34-
--> $DIR/print_literal.rs:19:31
35-
|
36-
19 | println!("2 + 1 = {:.4}", 3);
37-
| ^
38-
39-
error: printing a literal with an empty format string
40-
--> $DIR/print_literal.rs:20:32
41-
|
42-
20 | println!("2 + 1 = {:5.4}", 3);
43-
| ^
44-
45-
error: printing a literal with an empty format string
46-
--> $DIR/print_literal.rs:21:33
47-
|
48-
21 | println!("Debug test {:?}", "hello, world");
49-
| ^^^^^^^^^^^^^^
50-
51-
error: printing a literal with an empty format string
52-
--> $DIR/print_literal.rs:26:25
40+
--> $DIR/print_literal.rs:33:25
5341
|
54-
26 | println!("{0} {1}", "hello", "world");
42+
33 | println!("{0} {1}", "hello", "world");
5543
| ^^^^^^^
5644

5745
error: printing a literal with an empty format string
58-
--> $DIR/print_literal.rs:26:34
46+
--> $DIR/print_literal.rs:33:34
5947
|
60-
26 | println!("{0} {1}", "hello", "world");
48+
33 | println!("{0} {1}", "hello", "world");
6149
| ^^^^^^^
6250

6351
error: printing a literal with an empty format string
64-
--> $DIR/print_literal.rs:27:25
52+
--> $DIR/print_literal.rs:34:25
6553
|
66-
27 | println!("{1} {0}", "hello", "world");
54+
34 | println!("{1} {0}", "hello", "world");
6755
| ^^^^^^^
6856

6957
error: printing a literal with an empty format string
70-
--> $DIR/print_literal.rs:27:34
58+
--> $DIR/print_literal.rs:34:34
7159
|
72-
27 | println!("{1} {0}", "hello", "world");
60+
34 | println!("{1} {0}", "hello", "world");
7361
| ^^^^^^^
7462

7563
error: printing a literal with an empty format string
76-
--> $DIR/print_literal.rs:30:33
64+
--> $DIR/print_literal.rs:37:33
7765
|
78-
30 | println!("{foo} {bar}", foo="hello", bar="world");
66+
37 | println!("{foo} {bar}", foo="hello", bar="world");
7967
| ^^^^^^^
8068

8169
error: printing a literal with an empty format string
82-
--> $DIR/print_literal.rs:30:46
70+
--> $DIR/print_literal.rs:37:46
8371
|
84-
30 | println!("{foo} {bar}", foo="hello", bar="world");
72+
37 | println!("{foo} {bar}", foo="hello", bar="world");
8573
| ^^^^^^^
8674

8775
error: printing a literal with an empty format string
88-
--> $DIR/print_literal.rs:31:33
76+
--> $DIR/print_literal.rs:38:33
8977
|
90-
31 | println!("{bar} {foo}", foo="hello", bar="world");
78+
38 | println!("{bar} {foo}", foo="hello", bar="world");
9179
| ^^^^^^^
9280

9381
error: printing a literal with an empty format string
94-
--> $DIR/print_literal.rs:31:46
82+
--> $DIR/print_literal.rs:38:46
9583
|
96-
31 | println!("{bar} {foo}", foo="hello", bar="world");
84+
38 | println!("{bar} {foo}", foo="hello", bar="world");
9785
| ^^^^^^^
9886

99-
error: aborting due to 16 previous errors
87+
error: aborting due to 14 previous errors
10088

‎tests/ui/write_literal.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,29 @@ use std::io::Write;
66
fn main() {
77
let mut v = Vec::new();
88

9-
// These should be fine
9+
// these should be fine
1010
write!(&mut v, "Hello");
1111
writeln!(&mut v, "Hello");
1212
let world = "world";
1313
writeln!(&mut v, "Hello {}", world);
1414
writeln!(&mut v, "3 in hex is {:X}", 3);
15+
writeln!(&mut v, "2 + 1 = {:.4}", 3);
16+
writeln!(&mut v, "2 + 1 = {:5.4}", 3);
17+
writeln!(&mut v, "Debug test {:?}", "hello, world");
18+
writeln!(&mut v, "{0:8} {1:>8}", "hello", "world");
19+
writeln!(&mut v, "{1:8} {0:>8}", "hello", "world");
20+
writeln!(&mut v, "{foo:8} {bar:>8}", foo="hello", bar="world");
21+
writeln!(&mut v, "{bar:8} {foo:>8}", foo="hello", bar="world");
22+
writeln!(&mut v, "{number:>width$}", number=1, width=6);
23+
writeln!(&mut v, "{number:>0width$}", number=1, width=6);
1524

16-
// These should throw warnings
25+
// these should throw warnings
26+
writeln!(&mut v, "{} of {:b} people know binary, the other half doesn't", 1, 2);
1727
write!(&mut v, "Hello {}", "world");
1828
writeln!(&mut v, "Hello {} {}", world, "world");
1929
writeln!(&mut v, "Hello {}", "world");
2030
writeln!(&mut v, "10 / 4 is {}", 2.5);
2131
writeln!(&mut v, "2 + 1 = {}", 3);
22-
writeln!(&mut v, "2 + 1 = {:.4}", 3);
23-
writeln!(&mut v, "2 + 1 = {:5.4}", 3);
24-
writeln!(&mut v, "Debug test {:?}", "hello, world");
2532

2633
// positional args don't change the fact
2734
// that we're using a literal -- this should
@@ -30,6 +37,6 @@ fn main() {
3037
writeln!(&mut v, "{1} {0}", "hello", "world");
3138

3239
// named args shouldn't change anything either
33-
writeln!(&mut v, "{foo} {bar}", foo = "hello", bar = "world");
34-
writeln!(&mut v, "{bar} {foo}", foo = "hello", bar = "world");
40+
writeln!(&mut v, "{foo} {bar}", foo="hello", bar="world");
41+
writeln!(&mut v, "{bar} {foo}", foo="hello", bar="world");
3542
}

0 commit comments

Comments
 (0)