Skip to content

Commit 50d9473

Browse files
borsbors
committed
Auto merge of #3808 - mikerite:useless-format-suggestions, r=oli-obk
Fix `useless_format` suggestions
2 parents 2141ebf + 0182a66 commit 50d9473

File tree

5 files changed

+104
-52
lines changed

5 files changed

+104
-52
lines changed

‎ci/base-tests.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ rustup override set nightly
5959
# avoid loop spam and allow cmds with exit status != 0
6060
set +ex
6161

62-
for file in `find tests -not -path "tests/ui/format.rs" -not -path "tests/ui/formatting.rs" -not -path "tests/ui/empty_line_after_outer_attribute.rs" -not -path "tests/ui/double_parens.rs" -not -path "tests/ui/doc.rs" -not -path "tests/ui/unused_unit.rs" | grep "\.rs$"` ; do
62+
for file in `find tests -not -path "tests/ui/formatting.rs" -not -path "tests/ui/empty_line_after_outer_attribute.rs" -not -path "tests/ui/double_parens.rs" -not -path "tests/ui/doc.rs" -not -path "tests/ui/unused_unit.rs" | grep "\.rs$"` ; do
6363
rustfmt ${file} --check
6464
if [ $? -ne 0 ]; then
6565
echo "${file} needs reformatting!"

‎clippy_lints/src/format.rs

+23-17
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@ use crate::utils::{
55
};
66
use if_chain::if_chain;
77
use rustc::hir::*;
8-
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
8+
use rustc::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass};
99
use rustc::ty;
1010
use rustc::{declare_tool_lint, lint_array};
1111
use rustc_errors::Applicability;
1212
use syntax::ast::LitKind;
13+
use syntax::source_map::Span;
1314

1415
/// **What it does:** Checks for the use of `format!("string literal with no
1516
/// argument")` and `format!("{}", foo)` where `foo` is a string.
@@ -82,14 +83,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
8283
}
8384
};
8485

85-
span_lint_and_then(cx, USELESS_FORMAT, span, "useless use of `format!`", |db| {
86-
db.span_suggestion(
87-
expr.span,
88-
message,
89-
sugg,
90-
Applicability::MachineApplicable,
91-
);
92-
});
86+
span_useless_format(cx, span, message, sugg);
9387
}
9488
}
9589
},
@@ -98,14 +92,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
9892
if let ExprKind::Tup(ref tup) = matchee.node {
9993
if tup.is_empty() {
10094
let sugg = format!("{}.to_string()", snippet(cx, expr.span, "<expr>").into_owned());
101-
span_lint_and_then(cx, USELESS_FORMAT, span, "useless use of `format!`", |db| {
102-
db.span_suggestion(
103-
span,
104-
"consider using .to_string()",
105-
sugg,
106-
Applicability::MachineApplicable, // snippet
107-
);
108-
});
95+
span_useless_format(cx, span, "consider using .to_string()", sugg);
10996
}
11097
}
11198
},
@@ -115,6 +102,25 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
115102
}
116103
}
117104

105+
fn span_useless_format<'a, 'tcx: 'a, T: LintContext<'tcx>>(cx: &'a T, span: Span, help: &str, mut sugg: String) {
106+
let to_replace = span.source_callsite();
107+
108+
// The callsite span contains the statement semicolon for some reason.
109+
let snippet = snippet(cx, to_replace, "..");
110+
if snippet.ends_with(';') {
111+
sugg.push(';');
112+
}
113+
114+
span_lint_and_then(cx, USELESS_FORMAT, span, "useless use of `format!`", |db| {
115+
db.span_suggestion(
116+
to_replace,
117+
help,
118+
sugg,
119+
Applicability::MachineApplicable, // snippet
120+
);
121+
});
122+
}
123+
118124
/// Checks if the expressions matches `&[""]`
119125
fn check_single_piece(expr: &Expr) -> bool {
120126
if_chain! {

‎tests/ui/format.fixed

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// run-rustfix
2+
3+
#![allow(clippy::print_literal)]
4+
#![warn(clippy::useless_format)]
5+
6+
struct Foo(pub String);
7+
8+
macro_rules! foo {
9+
($($t:tt)*) => (Foo(format!($($t)*)))
10+
}
11+
12+
fn main() {
13+
"foo".to_string();
14+
15+
"foo".to_string();
16+
format!("{:?}", "foo"); // don't warn about debug
17+
format!("{:8}", "foo");
18+
format!("{:width$}", "foo", width = 8);
19+
"foo".to_string(); // warn when the format makes no difference
20+
"foo".to_string(); // warn when the format makes no difference
21+
format!("foo {}", "bar");
22+
format!("{} bar", "foo");
23+
24+
let arg: String = "".to_owned();
25+
arg.to_string();
26+
format!("{:?}", arg); // don't warn about debug
27+
format!("{:8}", arg);
28+
format!("{:width$}", arg, width = 8);
29+
arg.to_string(); // warn when the format makes no difference
30+
arg.to_string(); // warn when the format makes no difference
31+
format!("foo {}", arg);
32+
format!("{} bar", arg);
33+
34+
// we don’t want to warn for non-string args, see #697
35+
format!("{}", 42);
36+
format!("{:?}", 42);
37+
format!("{:+}", 42);
38+
format!("foo {}", 42);
39+
format!("{} bar", 42);
40+
41+
// we only want to warn about `format!` itself
42+
println!("foo");
43+
println!("{}", "foo");
44+
println!("foo {}", "foo");
45+
println!("{}", 42);
46+
println!("foo {}", 42);
47+
48+
// A format! inside a macro should not trigger a warning
49+
foo!("should not warn");
50+
51+
// precision on string means slicing without panicking on size:
52+
format!("{:.1}", "foo"); // could be "foo"[..1]
53+
format!("{:.10}", "foo"); // could not be "foo"[..10]
54+
format!("{:.prec$}", "foo", prec = 1);
55+
format!("{:.prec$}", "foo", prec = 10);
56+
57+
42.to_string();
58+
let x = std::path::PathBuf::from("/bar/foo/qux");
59+
x.display().to_string();
60+
}

‎tests/ui/format.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// run-rustfix
2+
13
#![allow(clippy::print_literal)]
24
#![warn(clippy::useless_format)]
35

‎tests/ui/format.stderr

+18-34
Original file line numberDiff line numberDiff line change
@@ -1,74 +1,58 @@
11
error: useless use of `format!`
2-
--> $DIR/format.rs:11:5
2+
--> $DIR/format.rs:13:5
33
|
44
LL | format!("foo");
5-
| ^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string()`
5+
| ^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string();`
66
|
77
= note: `-D clippy::useless-format` implied by `-D warnings`
88

99
error: useless use of `format!`
10-
--> $DIR/format.rs:13:5
10+
--> $DIR/format.rs:15:5
1111
|
1212
LL | format!("{}", "foo");
13-
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string()`
14-
|
15-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
13+
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string();`
1614

1715
error: useless use of `format!`
18-
--> $DIR/format.rs:17:5
16+
--> $DIR/format.rs:19:5
1917
|
2018
LL | format!("{:+}", "foo"); // warn when the format makes no difference
21-
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string()`
22-
|
23-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
19+
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string();`
2420

2521
error: useless use of `format!`
26-
--> $DIR/format.rs:18:5
22+
--> $DIR/format.rs:20:5
2723
|
2824
LL | format!("{:<}", "foo"); // warn when the format makes no difference
29-
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string()`
30-
|
31-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
25+
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string();`
3226

3327
error: useless use of `format!`
34-
--> $DIR/format.rs:23:5
28+
--> $DIR/format.rs:25:5
3529
|
3630
LL | format!("{}", arg);
37-
| ^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string()`
38-
|
39-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
31+
| ^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string();`
4032

4133
error: useless use of `format!`
42-
--> $DIR/format.rs:27:5
34+
--> $DIR/format.rs:29:5
4335
|
4436
LL | format!("{:+}", arg); // warn when the format makes no difference
45-
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string()`
46-
|
47-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
37+
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string();`
4838

4939
error: useless use of `format!`
50-
--> $DIR/format.rs:28:5
40+
--> $DIR/format.rs:30:5
5141
|
5242
LL | format!("{:<}", arg); // warn when the format makes no difference
53-
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string()`
54-
|
55-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
43+
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string();`
5644

5745
error: useless use of `format!`
58-
--> $DIR/format.rs:55:5
46+
--> $DIR/format.rs:57:5
5947
|
6048
LL | format!("{}", 42.to_string());
61-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `to_string()` is enough: `42.to_string()`
62-
|
63-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
49+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `to_string()` is enough: `42.to_string();`
6450

6551
error: useless use of `format!`
66-
--> $DIR/format.rs:57:5
52+
--> $DIR/format.rs:59:5
6753
|
6854
LL | format!("{}", x.display().to_string());
69-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `to_string()` is enough: `x.display().to_string()`
70-
|
71-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
55+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `to_string()` is enough: `x.display().to_string();`
7256

7357
error: aborting due to 9 previous errors
7458

0 commit comments

Comments
 (0)