Auto merge of rust-lang#8365 - Alexendoo:explicit-write-suggestion, r=camsteffen

Add `explicit_write` suggestions for `write!`s with format args

changelog: Add [`explicit_write`] suggestions for `write!`s with format args

Fixes rust-lang#4542

```rust
writeln!(std::io::stderr(), "macro arg {}", one!()).unwrap();
```

Now suggests:

```
error: use of `writeln!(stderr(), ...).unwrap()`
  --> $DIR/explicit_write.rs:36:9
   |
LL |         writeln!(std::io::stderr(), "macro arg {}", one!()).unwrap();
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("macro arg {}", one!())`
```

---------

r? `@camsteffen` (again, sorry 😛) for the `FormatArgsExpn` change

Before this change `inputs_span` returned a span pointing to just `1` in

```rust
macro_rules! one {
    () => { 1 };
}

`writeln!(std::io::stderr(), "macro arg {}", one!()).unwrap();`
```

And the `source_callsite` of that span didn't include the format string, it was just `one!()`

Author: bors

clippy_lints/src/explicit_write.rs

@@ -1,5 +1,6 @@
-use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
+use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::macros::FormatArgsExpn;
+use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::{is_expn_of, match_function_call, paths};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
@@ -79,28 +80,22 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite {
                         "print".into(),
                     )
                 };
-                let msg = format!("use of `{}.unwrap()`", used);
-                if let [write_output] = *format_args.format_string_parts {
-                    let mut write_output = write_output.to_string();
-                    if write_output.ends_with('\n') {
-                        write_output.pop();
-                    }
-
-                    let sugg = format!("{}{}!(\"{}\")", prefix, sugg_mac, write_output.escape_default());
-                    span_lint_and_sugg(
-                        cx,
-                        EXPLICIT_WRITE,
-                        expr.span,
-                        &msg,
-                        "try this",
-                        sugg,
-                        Applicability::MachineApplicable
-                    );
-                } else {
-                    // We don't have a proper suggestion
-                    let help = format!("consider using `{}{}!` instead", prefix, sugg_mac);
-                    span_lint_and_help(cx, EXPLICIT_WRITE, expr.span, &msg, None, &help);
-                }
+                let mut applicability = Applicability::MachineApplicable;
+                let inputs_snippet = snippet_with_applicability(
+                    cx,
+                    format_args.inputs_span(),
+                    "..",
+                    &mut applicability,
+                );
+                span_lint_and_sugg(
+                    cx,
+                    EXPLICIT_WRITE,
+                    expr.span,
+                    &format!("use of `{}.unwrap()`", used),
+                    "try this",
+                    format!("{}{}!({})", prefix, sugg_mac, inputs_snippet),
+                    applicability,
+                )
             }
         }
     }

clippy_utils/src/macros.rs

@@ -9,7 +9,7 @@ use rustc_hir::intravisit::Visitor;
 use rustc_hir::{self as hir, Expr, ExprKind, HirId, Node, QPath};
 use rustc_lint::LateContext;
 use rustc_span::def_id::DefId;
-use rustc_span::hygiene::{MacroKind, SyntaxContext};
+use rustc_span::hygiene::{self, MacroKind, SyntaxContext};
 use rustc_span::{sym, ExpnData, ExpnId, ExpnKind, Span, Symbol};
 use std::ops::ControlFlow;
 
@@ -306,6 +306,7 @@ fn is_assert_arg(cx: &LateContext<'_>, expr: &Expr<'_>, assert_expn: ExpnId) ->
 }
 
 /// A parsed `format_args!` expansion
+#[derive(Debug)]
 pub struct FormatArgsExpn<'tcx> {
     /// Span of the first argument, the format string
     pub format_string_span: Span,
@@ -465,11 +466,13 @@ impl<'tcx> FormatArgsExpn<'tcx> {
             .collect()
     }
 
-    /// Span of all inputs
+    /// Source callsite span of all inputs
     pub fn inputs_span(&self) -> Span {
         match *self.value_args {
             [] => self.format_string_span,
-            [.., last] => self.format_string_span.to(last.span),
+            [.., last] => self
+                .format_string_span
+                .to(hygiene::walk_chain(last.span, self.format_string_span.ctxt())),
         }
     }
 }

tests/ui/expect_fun_call.fixed

@@ -5,6 +5,12 @@
 
 /// Checks implementation of the `EXPECT_FUN_CALL` lint
 
+macro_rules! one {
+    () => {
+        1
+    };
+}
+
 fn main() {
     struct Foo;
 
@@ -31,6 +37,9 @@ fn main() {
     let with_none_and_as_str: Option<i32> = None;
     with_none_and_as_str.unwrap_or_else(|| panic!("Error {}: fake error", error_code));
 
+    let with_none_and_format_with_macro: Option<i32> = None;
+    with_none_and_format_with_macro.unwrap_or_else(|| panic!("Error {}: fake error", one!()));
+
     let with_ok: Result<(), ()> = Ok(());
     with_ok.expect("error");
 

tests/ui/expect_fun_call.rs

@@ -5,6 +5,12 @@
 
 /// Checks implementation of the `EXPECT_FUN_CALL` lint
 
+macro_rules! one {
+    () => {
+        1
+    };
+}
+
 fn main() {
     struct Foo;
 
@@ -31,6 +37,9 @@ fn main() {
     let with_none_and_as_str: Option<i32> = None;
     with_none_and_as_str.expect(format!("Error {}: fake error", error_code).as_str());
 
+    let with_none_and_format_with_macro: Option<i32> = None;
+    with_none_and_format_with_macro.expect(format!("Error {}: fake error", one!()).as_str());
+
     let with_ok: Result<(), ()> = Ok(());
     with_ok.expect("error");
 

tests/ui/expect_fun_call.stderr

@@ -1,76 +1,82 @@
 error: use of `expect` followed by a function call
-  --> $DIR/expect_fun_call.rs:29:26
+  --> $DIR/expect_fun_call.rs:35:26
    |
 LL |     with_none_and_format.expect(&format!("Error {}: fake error", error_code));
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("Error {}: fake error", error_code))`
    |
    = note: `-D clippy::expect-fun-call` implied by `-D warnings`
 
 error: use of `expect` followed by a function call
-  --> $DIR/expect_fun_call.rs:32:26
+  --> $DIR/expect_fun_call.rs:38:26
    |
 LL |     with_none_and_as_str.expect(format!("Error {}: fake error", error_code).as_str());
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("Error {}: fake error", error_code))`
 
 error: use of `expect` followed by a function call
-  --> $DIR/expect_fun_call.rs:42:25
+  --> $DIR/expect_fun_call.rs:41:37
+   |
+LL |     with_none_and_format_with_macro.expect(format!("Error {}: fake error", one!()).as_str());
+   |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("Error {}: fake error", one!()))`
+
+error: use of `expect` followed by a function call
+  --> $DIR/expect_fun_call.rs:51:25
    |
 LL |     with_err_and_format.expect(&format!("Error {}: fake error", error_code));
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| panic!("Error {}: fake error", error_code))`
 
 error: use of `expect` followed by a function call
-  --> $DIR/expect_fun_call.rs:45:25
+  --> $DIR/expect_fun_call.rs:54:25
    |
 LL |     with_err_and_as_str.expect(format!("Error {}: fake error", error_code).as_str());
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| panic!("Error {}: fake error", error_code))`
 
 error: use of `expect` followed by a function call
-  --> $DIR/expect_fun_call.rs:57:17
+  --> $DIR/expect_fun_call.rs:66:17
    |
 LL |     Some("foo").expect(format!("{} {}", 1, 2).as_ref());
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("{} {}", 1, 2))`
 
 error: use of `expect` followed by a function call
-  --> $DIR/expect_fun_call.rs:78:21
+  --> $DIR/expect_fun_call.rs:87:21
    |
 LL |         Some("foo").expect(&get_string());
    |                     ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!("{}", get_string()) })`
 
 error: use of `expect` followed by a function call
-  --> $DIR/expect_fun_call.rs:79:21
+  --> $DIR/expect_fun_call.rs:88:21
    |
 LL |         Some("foo").expect(get_string().as_ref());
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!("{}", get_string()) })`
 
 error: use of `expect` followed by a function call
-  --> $DIR/expect_fun_call.rs:80:21
+  --> $DIR/expect_fun_call.rs:89:21
    |
 LL |         Some("foo").expect(get_string().as_str());
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!("{}", get_string()) })`
 
 error: use of `expect` followed by a function call
-  --> $DIR/expect_fun_call.rs:82:21
+  --> $DIR/expect_fun_call.rs:91:21
    |
 LL |         Some("foo").expect(get_static_str());
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!("{}", get_static_str()) })`
 
 error: use of `expect` followed by a function call
-  --> $DIR/expect_fun_call.rs:83:21
+  --> $DIR/expect_fun_call.rs:92:21
    |
 LL |         Some("foo").expect(get_non_static_str(&0));
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!("{}", get_non_static_str(&0).to_string()) })`
 
 error: use of `expect` followed by a function call
-  --> $DIR/expect_fun_call.rs:87:16
+  --> $DIR/expect_fun_call.rs:96:16
    |
 LL |     Some(true).expect(&format!("key {}, {}", 1, 2));
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("key {}, {}", 1, 2))`
 
 error: use of `expect` followed by a function call
-  --> $DIR/expect_fun_call.rs:93:17
+  --> $DIR/expect_fun_call.rs:102:17
    |
 LL |         opt_ref.expect(&format!("{:?}", opt_ref));
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("{:?}", opt_ref))`
 
-error: aborting due to 12 previous errors
+error: aborting due to 13 previous errors
 

tests/ui/explicit_write.fixed

@@ -10,6 +10,12 @@ fn stderr() -> String {
     String::new()
 }
 
+macro_rules! one {
+    () => {
+        1
+    };
+}
+
 fn main() {
     // these should warn
     {
@@ -24,6 +30,12 @@ fn main() {
         // including newlines
         println!("test\ntest");
         eprintln!("test\ntest");
+
+        let value = 1;
+        eprintln!("with {}", value);
+        eprintln!("with {} {}", 2, value);
+        eprintln!("with {value}");
+        eprintln!("macro arg {}", one!());
     }
     // these should not warn, different destination
     {

tests/ui/explicit_write.rs

@@ -10,6 +10,12 @@ fn stderr() -> String {
     String::new()
 }
 
+macro_rules! one {
+    () => {
+        1
+    };
+}
+
 fn main() {
     // these should warn
     {
@@ -24,6 +30,12 @@ fn main() {
         // including newlines
         writeln!(std::io::stdout(), "test\ntest").unwrap();
         writeln!(std::io::stderr(), "test\ntest").unwrap();
+
+        let value = 1;
+        writeln!(std::io::stderr(), "with {}", value).unwrap();
+        writeln!(std::io::stderr(), "with {} {}", 2, value).unwrap();
+        writeln!(std::io::stderr(), "with {value}").unwrap();
+        writeln!(std::io::stderr(), "macro arg {}", one!()).unwrap();
     }
     // these should not warn, different destination
     {

tests/ui/explicit_write.stderr

@@ -1,52 +1,76 @@
 error: use of `write!(stdout(), ...).unwrap()`
-  --> $DIR/explicit_write.rs:17:9
+  --> $DIR/explicit_write.rs:23:9
    |
 LL |         write!(std::io::stdout(), "test").unwrap();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `print!("test")`
    |
    = note: `-D clippy::explicit-write` implied by `-D warnings`
 
 error: use of `write!(stderr(), ...).unwrap()`
-  --> $DIR/explicit_write.rs:18:9
+  --> $DIR/explicit_write.rs:24:9
    |
 LL |         write!(std::io::stderr(), "test").unwrap();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprint!("test")`
 
 error: use of `writeln!(stdout(), ...).unwrap()`
-  --> $DIR/explicit_write.rs:19:9
+  --> $DIR/explicit_write.rs:25:9
    |
 LL |         writeln!(std::io::stdout(), "test").unwrap();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `println!("test")`
 
 error: use of `writeln!(stderr(), ...).unwrap()`
-  --> $DIR/explicit_write.rs:20:9
+  --> $DIR/explicit_write.rs:26:9
    |
 LL |         writeln!(std::io::stderr(), "test").unwrap();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("test")`
 
 error: use of `stdout().write_fmt(...).unwrap()`
-  --> $DIR/explicit_write.rs:21:9
+  --> $DIR/explicit_write.rs:27:9
    |
 LL |         std::io::stdout().write_fmt(format_args!("test")).unwrap();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `print!("test")`
 
 error: use of `stderr().write_fmt(...).unwrap()`
-  --> $DIR/explicit_write.rs:22:9
+  --> $DIR/explicit_write.rs:28:9
    |
 LL |         std::io::stderr().write_fmt(format_args!("test")).unwrap();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprint!("test")`
 
 error: use of `writeln!(stdout(), ...).unwrap()`
-  --> $DIR/explicit_write.rs:25:9
+  --> $DIR/explicit_write.rs:31:9
    |
 LL |         writeln!(std::io::stdout(), "test/ntest").unwrap();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `println!("test/ntest")`
 
 error: use of `writeln!(stderr(), ...).unwrap()`
-  --> $DIR/explicit_write.rs:26:9
+  --> $DIR/explicit_write.rs:32:9
    |
 LL |         writeln!(std::io::stderr(), "test/ntest").unwrap();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("test/ntest")`
 
-error: aborting due to 8 previous errors
+error: use of `writeln!(stderr(), ...).unwrap()`
+  --> $DIR/explicit_write.rs:35:9
+   |
+LL |         writeln!(std::io::stderr(), "with {}", value).unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("with {}", value)`
+
+error: use of `writeln!(stderr(), ...).unwrap()`
+  --> $DIR/explicit_write.rs:36:9
+   |
+LL |         writeln!(std::io::stderr(), "with {} {}", 2, value).unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("with {} {}", 2, value)`
+
+error: use of `writeln!(stderr(), ...).unwrap()`
+  --> $DIR/explicit_write.rs:37:9
+   |
+LL |         writeln!(std::io::stderr(), "with {value}").unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("with {value}")`
+
+error: use of `writeln!(stderr(), ...).unwrap()`
+  --> $DIR/explicit_write.rs:38:9
+   |
+LL |         writeln!(std::io::stderr(), "macro arg {}", one!()).unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("macro arg {}", one!())`
+
+error: aborting due to 12 previous errors