From ef778e796502691ebfb1f3dd5c03248aea6d2ac8 Mon Sep 17 00:00:00 2001
From: Mara Bos <m-ou.se@m-ou.se>
Date: Sun, 14 Feb 2021 18:14:23 +0100
Subject: [PATCH 1/9] Fix span in non_fmt_panic for panic!(some_macro!()).

---
 compiler/rustc_lint/src/non_fmt_panic.rs | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs
index e98297b692c92..0c5456cf619d1 100644
--- a/compiler/rustc_lint/src/non_fmt_panic.rs
+++ b/compiler/rustc_lint/src/non_fmt_panic.rs
@@ -69,19 +69,30 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
 
     let (span, panic) = panic_call(cx, f);
 
-    cx.struct_span_lint(NON_FMT_PANIC, arg.span, |lint| {
+    // Find the span of the argument to `panic!()`, before expansion in the
+    // case of `panic!(some_macro!())`.
+    let mut arg_span = arg.span;
+    while !span.contains(arg_span) {
+        let expn = arg_span.ctxt().outer_expn_data();
+        if expn.is_root() {
+            break;
+        }
+        arg_span = expn.call_site;
+    }
+
+    cx.struct_span_lint(NON_FMT_PANIC, arg_span, |lint| {
         let mut l = lint.build("panic message is not a string literal");
         l.note("this is no longer accepted in Rust 2021");
-        if span.contains(arg.span) {
+        if span.contains(arg_span) {
             l.span_suggestion_verbose(
-                arg.span.shrink_to_lo(),
+                arg_span.shrink_to_lo(),
                 "add a \"{}\" format string to Display the message",
                 "\"{}\", ".into(),
                 Applicability::MaybeIncorrect,
             );
             if panic == sym::std_panic_macro {
                 l.span_suggestion_verbose(
-                    span.until(arg.span),
+                    span.until(arg_span),
                     "or use std::panic::panic_any instead",
                     "std::panic::panic_any(".into(),
                     Applicability::MachineApplicable,

From 1abc1e2a43c116a87bd430fa14a86e67617b861a Mon Sep 17 00:00:00 2001
From: Mara Bos <m-ou.se@m-ou.se>
Date: Sun, 14 Feb 2021 18:17:34 +0100
Subject: [PATCH 2/9] Add test for non_fmt_panic lint for
 panic!(some_macro!()).

---
 src/test/ui/non-fmt-panic.rs     |  6 ++++++
 src/test/ui/non-fmt-panic.stderr | 18 +++++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/test/ui/non-fmt-panic.rs b/src/test/ui/non-fmt-panic.rs
index 25c53316e1290..ff9e3b497f23d 100644
--- a/src/test/ui/non-fmt-panic.rs
+++ b/src/test/ui/non-fmt-panic.rs
@@ -29,6 +29,12 @@ fn main() {
     fancy_panic::fancy_panic!(S);
     //~^ WARN panic message is not a string literal
 
+    macro_rules! a {
+        () => { 123 };
+    }
+
+    panic!(a!()); //~ WARN panic message is not a string literal
+
     // Check that the lint only triggers for std::panic and core::panic,
     // not any panic macro:
     macro_rules! panic {
diff --git a/src/test/ui/non-fmt-panic.stderr b/src/test/ui/non-fmt-panic.stderr
index 45187c518c423..2f33f04cb19f6 100644
--- a/src/test/ui/non-fmt-panic.stderr
+++ b/src/test/ui/non-fmt-panic.stderr
@@ -183,5 +183,21 @@ LL |     fancy_panic::fancy_panic!(S);
    |
    = note: this is no longer accepted in Rust 2021
 
-warning: 14 warnings emitted
+warning: panic message is not a string literal
+  --> $DIR/non-fmt-panic.rs:36:12
+   |
+LL |     panic!(a!());
+   |            ^^^^
+   |
+   = note: this is no longer accepted in Rust 2021
+help: add a "{}" format string to Display the message
+   |
+LL |     panic!("{}", a!());
+   |            ^^^^^
+help: or use std::panic::panic_any instead
+   |
+LL |     std::panic::panic_any(a!());
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+
+warning: 15 warnings emitted
 

From a428ab17abc310c17ca13eeb6f74b3c5bddff940 Mon Sep 17 00:00:00 2001
From: Mara Bos <m-ou.se@m-ou.se>
Date: Sun, 14 Feb 2021 18:52:47 +0100
Subject: [PATCH 3/9] Improve suggestion for panic!(format!(..)).

---
 compiler/rustc_lint/src/non_fmt_panic.rs | 31 +++++++++++++++++++++++-
 compiler/rustc_span/src/symbol.rs        |  1 +
 library/alloc/src/macros.rs              |  1 +
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs
index 0c5456cf619d1..7432f476d7cd0 100644
--- a/compiler/rustc_lint/src/non_fmt_panic.rs
+++ b/compiler/rustc_lint/src/non_fmt_panic.rs
@@ -72,18 +72,38 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
     // Find the span of the argument to `panic!()`, before expansion in the
     // case of `panic!(some_macro!())`.
     let mut arg_span = arg.span;
+    let mut arg_macro = None;
     while !span.contains(arg_span) {
         let expn = arg_span.ctxt().outer_expn_data();
         if expn.is_root() {
             break;
         }
+        arg_macro = expn.macro_def_id;
         arg_span = expn.call_site;
     }
 
     cx.struct_span_lint(NON_FMT_PANIC, arg_span, |lint| {
         let mut l = lint.build("panic message is not a string literal");
         l.note("this is no longer accepted in Rust 2021");
-        if span.contains(arg_span) {
+        if !span.contains(arg_span) {
+            // No clue where this argument is coming from.
+            l.emit();
+            return;
+        }
+        if arg_macro.map_or(false, |id| cx.tcx.is_diagnostic_item(sym::format_macro, id)) {
+            // A case of `panic!(format!(..))`.
+            l.note("the panic!() macro supports formatting, so there's no need for the format!() macro here");
+            if let Some(inner) = find_inner_span(cx, arg_span) {
+                l.multipart_suggestion(
+                    "remove the `format!(..)` macro call",
+                    vec![
+                        (arg_span.until(inner), "".into()),
+                        (inner.between(arg_span.shrink_to_hi()), "".into()),
+                    ],
+                    Applicability::MachineApplicable,
+                );
+            }
+        } else {
             l.span_suggestion_verbose(
                 arg_span.shrink_to_lo(),
                 "add a \"{}\" format string to Display the message",
@@ -186,6 +206,15 @@ fn check_panic_str<'tcx>(
     }
 }
 
+/// Given the span of `some_macro!(args)`, gives the span of `args`.
+fn find_inner_span<'tcx>(cx: &LateContext<'tcx>, span: Span) -> Option<Span> {
+    let snippet = cx.sess().parse_sess.source_map().span_to_snippet(span).ok()?;
+    Some(span.from_inner(InnerSpan {
+        start: snippet.find(&['(', '{', '['][..])? + 1,
+        end: snippet.rfind(&[')', '}', ']'][..])?,
+    }))
+}
+
 fn panic_call<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>) -> (Span, Symbol) {
     let mut expn = f.span.ctxt().outer_expn_data();
 
diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs
index 20e4f7262acb9..831e82559adcc 100644
--- a/compiler/rustc_span/src/symbol.rs
+++ b/compiler/rustc_span/src/symbol.rs
@@ -554,6 +554,7 @@ symbols! {
         format_args,
         format_args_capture,
         format_args_nl,
+        format_macro,
         freeze,
         freg,
         frem_fast,
diff --git a/library/alloc/src/macros.rs b/library/alloc/src/macros.rs
index a64a8b32ad77f..88a6cec3a8342 100644
--- a/library/alloc/src/macros.rs
+++ b/library/alloc/src/macros.rs
@@ -107,6 +107,7 @@ macro_rules! vec {
 /// ```
 #[macro_export]
 #[stable(feature = "rust1", since = "1.0.0")]
+#[rustc_diagnostic_item = "format_macro"]
 macro_rules! format {
     ($($arg:tt)*) => {{
         let res = $crate::fmt::format($crate::__export::format_args!($($arg)*));

From 9f97a0b364bcbb261f05d6ac62436b6802bfa80b Mon Sep 17 00:00:00 2001
From: Mara Bos <m-ou.se@m-ou.se>
Date: Sun, 14 Feb 2021 18:53:10 +0100
Subject: [PATCH 4/9] Add test for panic!(format!(..)) lint.

---
 src/test/ui/non-fmt-panic.rs     |  2 ++
 src/test/ui/non-fmt-panic.stderr | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/test/ui/non-fmt-panic.rs b/src/test/ui/non-fmt-panic.rs
index ff9e3b497f23d..c20df5ffd10a0 100644
--- a/src/test/ui/non-fmt-panic.rs
+++ b/src/test/ui/non-fmt-panic.rs
@@ -35,6 +35,8 @@ fn main() {
 
     panic!(a!()); //~ WARN panic message is not a string literal
 
+    panic!(format!("{}", 1)); //~ WARN panic message is not a string literal
+
     // Check that the lint only triggers for std::panic and core::panic,
     // not any panic macro:
     macro_rules! panic {
diff --git a/src/test/ui/non-fmt-panic.stderr b/src/test/ui/non-fmt-panic.stderr
index 2f33f04cb19f6..043c4b0f7506c 100644
--- a/src/test/ui/non-fmt-panic.stderr
+++ b/src/test/ui/non-fmt-panic.stderr
@@ -199,5 +199,18 @@ help: or use std::panic::panic_any instead
 LL |     std::panic::panic_any(a!());
    |     ^^^^^^^^^^^^^^^^^^^^^^
 
-warning: 15 warnings emitted
+warning: panic message is not a string literal
+  --> $DIR/non-fmt-panic.rs:38:12
+   |
+LL |     panic!(format!("{}", 1));
+   |            ^^^^^^^^^^^^^^^^
+   |
+   = note: this is no longer accepted in Rust 2021
+   = note: the panic!() macro supports formatting, so there's no need for the format!() macro here
+help: remove the `format!(..)` macro call
+   |
+LL |     panic!("{}", 1);
+   |           --     --
+
+warning: 16 warnings emitted
 

From 37c532c010cddb8f9283fb50318d8c3816646063 Mon Sep 17 00:00:00 2001
From: Mara Bos <m-ou.se@m-ou.se>
Date: Sun, 14 Feb 2021 19:43:07 +0100
Subject: [PATCH 5/9] Suggest correct replacement for panic![123].

Before this change, the suggestion was `std::panic::panic_any(123]`,
changing the opening brace but not the closing one.
---
 compiler/rustc_lint/src/non_fmt_panic.rs | 48 ++++++++++++++++--------
 src/test/ui/non-fmt-panic.stderr         |  8 ++--
 2 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs
index 7432f476d7cd0..c4627745648d3 100644
--- a/compiler/rustc_lint/src/non_fmt_panic.rs
+++ b/compiler/rustc_lint/src/non_fmt_panic.rs
@@ -93,12 +93,12 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
         if arg_macro.map_or(false, |id| cx.tcx.is_diagnostic_item(sym::format_macro, id)) {
             // A case of `panic!(format!(..))`.
             l.note("the panic!() macro supports formatting, so there's no need for the format!() macro here");
-            if let Some(inner) = find_inner_span(cx, arg_span) {
+            if let Some((open, close, _)) = find_delimiters(cx, arg_span) {
                 l.multipart_suggestion(
                     "remove the `format!(..)` macro call",
                     vec![
-                        (arg_span.until(inner), "".into()),
-                        (inner.between(arg_span.shrink_to_hi()), "".into()),
+                        (arg_span.until(open.shrink_to_hi()), "".into()),
+                        (close.until(arg_span.shrink_to_hi()), "".into()),
                     ],
                     Applicability::MachineApplicable,
                 );
@@ -111,12 +111,20 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
                 Applicability::MaybeIncorrect,
             );
             if panic == sym::std_panic_macro {
-                l.span_suggestion_verbose(
-                    span.until(arg_span),
-                    "or use std::panic::panic_any instead",
-                    "std::panic::panic_any(".into(),
-                    Applicability::MachineApplicable,
-                );
+                if let Some((open, close, del)) = find_delimiters(cx, span) {
+                    l.multipart_suggestion(
+                        "or use std::panic::panic_any instead",
+                        if del == '(' {
+                            vec![(span.until(open), "std::panic::panic_any".into())]
+                        } else {
+                            vec![
+                                (span.until(open.shrink_to_hi()), "std::panic::panic_any(".into()),
+                                (close, ")".into()),
+                            ]
+                        },
+                        Applicability::MachineApplicable,
+                    );
+                }
             }
         }
         l.emit();
@@ -206,13 +214,23 @@ fn check_panic_str<'tcx>(
     }
 }
 
-/// Given the span of `some_macro!(args)`, gives the span of `args`.
-fn find_inner_span<'tcx>(cx: &LateContext<'tcx>, span: Span) -> Option<Span> {
+/// Given the span of `some_macro!(args);`, gives the span of `(` and `)`,
+/// and the type of (opening) delimiter used.
+fn find_delimiters<'tcx>(cx: &LateContext<'tcx>, span: Span) -> Option<(Span, Span, char)> {
     let snippet = cx.sess().parse_sess.source_map().span_to_snippet(span).ok()?;
-    Some(span.from_inner(InnerSpan {
-        start: snippet.find(&['(', '{', '['][..])? + 1,
-        end: snippet.rfind(&[')', '}', ']'][..])?,
-    }))
+    let (open, open_ch) = snippet.char_indices().find(|&(_, c)| "([{".contains(c))?;
+    let close = snippet.rfind(|c| ")]}".contains(c))?;
+    Some((
+        span.from_inner(InnerSpan {
+            start: open,
+            end: open + 1,
+        }),
+        span.from_inner(InnerSpan {
+            start: close,
+            end: close + 1,
+        }),
+        open_ch,
+    ))
 }
 
 fn panic_call<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>) -> (Span, Symbol) {
diff --git a/src/test/ui/non-fmt-panic.stderr b/src/test/ui/non-fmt-panic.stderr
index 043c4b0f7506c..001f066ad212a 100644
--- a/src/test/ui/non-fmt-panic.stderr
+++ b/src/test/ui/non-fmt-panic.stderr
@@ -93,7 +93,7 @@ LL |     panic!("{}", C);
 help: or use std::panic::panic_any instead
    |
 LL |     std::panic::panic_any(C);
-   |     ^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^
 
 warning: panic message is not a string literal
   --> $DIR/non-fmt-panic.rs:20:12
@@ -109,7 +109,7 @@ LL |     panic!("{}", S);
 help: or use std::panic::panic_any instead
    |
 LL |     std::panic::panic_any(S);
-   |     ^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^
 
 warning: panic message is not a string literal
   --> $DIR/non-fmt-panic.rs:21:17
@@ -125,7 +125,7 @@ LL |     std::panic!("{}", 123);
 help: or use std::panic::panic_any instead
    |
 LL |     std::panic::panic_any(123);
-   |     ^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^
 
 warning: panic message is not a string literal
   --> $DIR/non-fmt-panic.rs:22:18
@@ -197,7 +197,7 @@ LL |     panic!("{}", a!());
 help: or use std::panic::panic_any instead
    |
 LL |     std::panic::panic_any(a!());
-   |     ^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^
 
 warning: panic message is not a string literal
   --> $DIR/non-fmt-panic.rs:38:12

From 8ad12bb9b2709678f58e780daca981aafc2810dd Mon Sep 17 00:00:00 2001
From: Mara Bos <m-ou.se@m-ou.se>
Date: Sun, 14 Feb 2021 19:44:57 +0100
Subject: [PATCH 6/9] Add ui tests for panic![123] and panic!{123}.

---
 src/test/ui/non-fmt-panic.rs     |  3 +++
 src/test/ui/non-fmt-panic.stderr | 34 +++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/src/test/ui/non-fmt-panic.rs b/src/test/ui/non-fmt-panic.rs
index c20df5ffd10a0..c80a90b3eaaac 100644
--- a/src/test/ui/non-fmt-panic.rs
+++ b/src/test/ui/non-fmt-panic.rs
@@ -37,6 +37,9 @@ fn main() {
 
     panic!(format!("{}", 1)); //~ WARN panic message is not a string literal
 
+    panic![123]; //~ WARN panic message is not a string literal
+    panic!{123}; //~ WARN panic message is not a string literal
+
     // Check that the lint only triggers for std::panic and core::panic,
     // not any panic macro:
     macro_rules! panic {
diff --git a/src/test/ui/non-fmt-panic.stderr b/src/test/ui/non-fmt-panic.stderr
index 001f066ad212a..7a333b3e76abe 100644
--- a/src/test/ui/non-fmt-panic.stderr
+++ b/src/test/ui/non-fmt-panic.stderr
@@ -212,5 +212,37 @@ help: remove the `format!(..)` macro call
 LL |     panic!("{}", 1);
    |           --     --
 
-warning: 16 warnings emitted
+warning: panic message is not a string literal
+  --> $DIR/non-fmt-panic.rs:40:12
+   |
+LL |     panic![123];
+   |            ^^^
+   |
+   = note: this is no longer accepted in Rust 2021
+help: add a "{}" format string to Display the message
+   |
+LL |     panic!["{}", 123];
+   |            ^^^^^
+help: or use std::panic::panic_any instead
+   |
+LL |     std::panic::panic_any(123);
+   |     ^^^^^^^^^^^^^^^^^^^^^^   ^
+
+warning: panic message is not a string literal
+  --> $DIR/non-fmt-panic.rs:41:12
+   |
+LL |     panic!{123};
+   |            ^^^
+   |
+   = note: this is no longer accepted in Rust 2021
+help: add a "{}" format string to Display the message
+   |
+LL |     panic!{"{}", 123};
+   |            ^^^^^
+help: or use std::panic::panic_any instead
+   |
+LL |     std::panic::panic_any(123);
+   |     ^^^^^^^^^^^^^^^^^^^^^^   ^
+
+warning: 18 warnings emitted
 

From 2a0c42450ec34f2c4b3985c159c499560f1c8270 Mon Sep 17 00:00:00 2001
From: Mara Bos <m-ou.se@m-ou.se>
Date: Sun, 14 Feb 2021 19:51:15 +0100
Subject: [PATCH 7/9] Formatting.

---
 compiler/rustc_lint/src/non_fmt_panic.rs | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs
index c4627745648d3..0c7c8b78845cf 100644
--- a/compiler/rustc_lint/src/non_fmt_panic.rs
+++ b/compiler/rustc_lint/src/non_fmt_panic.rs
@@ -221,14 +221,8 @@ fn find_delimiters<'tcx>(cx: &LateContext<'tcx>, span: Span) -> Option<(Span, Sp
     let (open, open_ch) = snippet.char_indices().find(|&(_, c)| "([{".contains(c))?;
     let close = snippet.rfind(|c| ")]}".contains(c))?;
     Some((
-        span.from_inner(InnerSpan {
-            start: open,
-            end: open + 1,
-        }),
-        span.from_inner(InnerSpan {
-            start: close,
-            end: close + 1,
-        }),
+        span.from_inner(InnerSpan { start: open, end: open + 1 }),
+        span.from_inner(InnerSpan { start: close, end: close + 1 }),
         open_ch,
     ))
 }

From daa371d1891f6833ad08542caeb3cf424483a0f9 Mon Sep 17 00:00:00 2001
From: Mara Bos <m-ou.se@m-ou.se>
Date: Sun, 14 Feb 2021 20:03:13 +0100
Subject: [PATCH 8/9] Only define rustc_diagnostic_item format_macro in
 not(test).

---
 library/alloc/src/macros.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/library/alloc/src/macros.rs b/library/alloc/src/macros.rs
index 88a6cec3a8342..6a64587a2237f 100644
--- a/library/alloc/src/macros.rs
+++ b/library/alloc/src/macros.rs
@@ -107,7 +107,7 @@ macro_rules! vec {
 /// ```
 #[macro_export]
 #[stable(feature = "rust1", since = "1.0.0")]
-#[rustc_diagnostic_item = "format_macro"]
+#[cfg_attr(not(test), rustc_diagnostic_item = "format_macro")]
 macro_rules! format {
     ($($arg:tt)*) => {{
         let res = $crate::fmt::format($crate::__export::format_args!($($arg)*));

From ad93f48d770cc8cfe473e809700201c31550bc68 Mon Sep 17 00:00:00 2001
From: Mara Bos <m-ou.se@m-ou.se>
Date: Wed, 17 Feb 2021 10:51:22 +0100
Subject: [PATCH 9/9] Add comment about how we find the right span in
 non_fmt_panic.

---
 compiler/rustc_lint/src/non_fmt_panic.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs
index 0c7c8b78845cf..bfe37ce6959e7 100644
--- a/compiler/rustc_lint/src/non_fmt_panic.rs
+++ b/compiler/rustc_lint/src/non_fmt_panic.rs
@@ -71,6 +71,9 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
 
     // Find the span of the argument to `panic!()`, before expansion in the
     // case of `panic!(some_macro!())`.
+    // We don't use source_callsite(), because this `panic!(..)` might itself
+    // be expanded from another macro, in which case we want to stop at that
+    // expansion.
     let mut arg_span = arg.span;
     let mut arg_macro = None;
     while !span.contains(arg_span) {