diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index 2eb95ebffbf..30ad022f476 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -47,7 +47,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { return; } match expr.node { - // `format!("{}", foo)` expansion ExprKind::Call(ref fun, ref args) => { if_chain! { @@ -58,12 +57,24 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { if check_single_piece(&args[0]); if let Some(format_arg) = get_single_string_arg(cx, &args[1]); if check_unformatted(&args[2]); + if let ExprKind::AddrOf(_, ref format_arg) = format_arg.node; then { - let sugg = format!("{}.to_string()", snippet(cx, format_arg, "").into_owned()); + let (message, sugg) = if_chain! { + if let ExprKind::MethodCall(ref path, ref span, ref expr) = format_arg.node; + if path.ident.as_interned_str() == "to_string"; + then { + ("`to_string()` is enough", + snippet(cx, format_arg.span, "").to_string()) + } else { + ("consider using .to_string()", + format!("{}.to_string()", snippet(cx, format_arg.span, ""))) + } + }; + span_lint_and_then(cx, USELESS_FORMAT, span, "useless use of `format!`", |db| { db.span_suggestion_with_applicability( expr.span, - "consider using .to_string()", + message, sugg, Applicability::MachineApplicable, ); @@ -114,9 +125,9 @@ fn check_single_piece(expr: &Expr) -> bool { /// ::std::fmt::Display::fmt)], /// } /// ``` -/// and that type of `__arg0` is `&str` or `String` -/// then returns the span of first element of the matched tuple -fn get_single_string_arg(cx: &LateContext<'_, '_>, expr: &Expr) -> Option { +/// and that the type of `__arg0` is `&str` or `String`, +/// then returns the span of first element of the matched tuple. +fn get_single_string_arg<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr) -> Option<&'a Expr> { if_chain! { if let ExprKind::AddrOf(_, ref expr) = expr.node; if let ExprKind::Match(ref match_expr, ref arms, _) = expr.node; @@ -135,7 +146,7 @@ fn get_single_string_arg(cx: &LateContext<'_, '_>, expr: &Expr) -> Option let ty = walk_ptrs_ty(cx.tables.pat_ty(&pat[0])); if ty.sty == ty::Str || match_type(cx, ty, &paths::STRING) { if let ExprKind::Tup(ref values) = match_expr.node { - return Some(values[0].span); + return Some(&values[0]); } } } @@ -162,9 +173,12 @@ fn check_unformatted(expr: &Expr) -> bool { if let ExprKind::Struct(_, ref fields, _) = exprs[0].node; if let Some(format_field) = fields.iter().find(|f| f.ident.name == "format"); if let ExprKind::Struct(_, ref fields, _) = format_field.expr.node; - if let Some(align_field) = fields.iter().find(|f| f.ident.name == "width"); - if let ExprKind::Path(ref qpath) = align_field.expr.node; - if last_path_segment(qpath).ident.name == "Implied"; + if let Some(width_field) = fields.iter().find(|f| f.ident.name == "width"); + if let ExprKind::Path(ref width_qpath) = width_field.expr.node; + if last_path_segment(width_qpath).ident.name == "Implied"; + if let Some(precision_field) = fields.iter().find(|f| f.ident.name == "precision"); + if let ExprKind::Path(ref precision_path) = precision_field.expr.node; + if last_path_segment(precision_path).ident.name == "Implied"; then { return true; } diff --git a/tests/ui/format.rs b/tests/ui/format.rs index 8f31d92ac3c..858c9fc8de5 100644 --- a/tests/ui/format.rs +++ b/tests/ui/format.rs @@ -14,6 +14,7 @@ fn main() { format!("{}", "foo"); format!("{:?}", "foo"); // don't warn about debug format!("{:8}", "foo"); + format!("{:width$}", "foo", width = 8); format!("{:+}", "foo"); // warn when the format makes no difference format!("{:<}", "foo"); // warn when the format makes no difference format!("foo {}", "bar"); @@ -23,6 +24,7 @@ fn main() { format!("{}", arg); format!("{:?}", arg); // don't warn about debug format!("{:8}", arg); + format!("{:width$}", arg, width = 8); format!("{:+}", arg); // warn when the format makes no difference format!("{:<}", arg); // warn when the format makes no difference format!("foo {}", arg); @@ -44,4 +46,14 @@ fn main() { // A format! inside a macro should not trigger a warning foo!("should not warn"); + + // precision on string means slicing without panicking on size: + format!("{:.1}", "foo"); // could be "foo"[..1] + format!("{:.10}", "foo"); // could not be "foo"[..10] + format!("{:.prec$}", "foo", prec = 1); + format!("{:.prec$}", "foo", prec = 10); + + format!("{}", 42.to_string()); + let x = std::path::PathBuf::from("/bar/foo/qux"); + format!("{}", x.display().to_string()); } diff --git a/tests/ui/format.stderr b/tests/ui/format.stderr index ca6ef905396..520c1b79433 100644 --- a/tests/ui/format.stderr +++ b/tests/ui/format.stderr @@ -14,45 +14,61 @@ error: useless use of `format!` | = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) -error: useless use of `format!` - --> $DIR/format.rs:17:5 - | -17 | format!("{:+}", "foo"); // warn when the format makes no difference - | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string()` - | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - error: useless use of `format!` --> $DIR/format.rs:18:5 | -18 | format!("{:<}", "foo"); // warn when the format makes no difference +18 | format!("{:+}", "foo"); // warn when the format makes no difference | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string()` | = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: useless use of `format!` - --> $DIR/format.rs:23:5 + --> $DIR/format.rs:19:5 | -23 | format!("{}", arg); +19 | format!("{:<}", "foo"); // warn when the format makes no difference + | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string()` + | + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error: useless use of `format!` + --> $DIR/format.rs:24:5 + | +24 | format!("{}", arg); | ^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string()` | = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: useless use of `format!` - --> $DIR/format.rs:26:5 + --> $DIR/format.rs:28:5 | -26 | format!("{:+}", arg); // warn when the format makes no difference +28 | format!("{:+}", arg); // warn when the format makes no difference | ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string()` | = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: useless use of `format!` - --> $DIR/format.rs:27:5 + --> $DIR/format.rs:29:5 | -27 | format!("{:<}", arg); // warn when the format makes no difference +29 | format!("{:<}", arg); // warn when the format makes no difference | ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string()` | = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) -error: aborting due to 7 previous errors +error: useless use of `format!` + --> $DIR/format.rs:56:5 + | +56 | format!("{}", 42.to_string()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `to_string()` is enough: `42.to_string()` + | + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error: useless use of `format!` + --> $DIR/format.rs:58:5 + | +58 | format!("{}", x.display().to_string()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `to_string()` is enough: `x.display().to_string()` + | + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error: aborting due to 9 previous errors