Merge pull request #3256 from rust-lang-nursery/fix-2796

Fix 2796 and 3020

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, "<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, "<arg>").to_string())
+                                } else {
+                                    ("consider using .to_string()",
+                                    format!("{}.to_string()", snippet(cx, format_arg.span, "<arg>")))
+                                }
+                            };
+
                             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`
+/// and that the type of `__arg0` is `&str` or `String`,
-/// then returns the span of first element of the matched tuple
+/// then returns the span of first element of the matched tuple.
-fn get_single_string_arg(cx: &LateContext<'_, '_>, expr: &Expr) -> Option<Span> {
+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<Span>
             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 Some(width_field) = fields.iter().find(|f| f.ident.name == "width");
-        if let ExprKind::Path(ref qpath) = align_field.expr.node;
+        if let ExprKind::Path(ref width_qpath) = width_field.expr.node;
-        if last_path_segment(qpath).ident.name == "Implied";
+        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;
         }

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());
 }

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