Rollup merge of rust-lang#85075 - ptrojahn:panic_warning, r=jackh726

Improve "panic message is not a string literal" warning This warning always referenced panic! even in case of an assert. Related to rust-lang#84656
Dylan-DPC-zz · May 9, 2021 · b5176ce · b5176ce
2 parents 86f9fd1 + 8f14592
commit b5176ce
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 9 deletions.
diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs
@@ -4,7 +4,7 @@ use rustc_errors::{pluralize, Applicability};
 use rustc_hir as hir;
 use rustc_middle::ty;
 use rustc_parse_format::{ParseMode, Parser, Piece};
-use rustc_span::{sym, symbol::kw, InnerSpan, Span, Symbol};
+use rustc_span::{hygiene, sym, symbol::kw, symbol::SymbolStr, InnerSpan, Span, Symbol};
 
 declare_lint! {
     /// The `non_fmt_panic` lint detects `panic!(..)` invocations where the first
@@ -67,7 +67,7 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
 
     // The argument is *not* a string literal.
 
-    let (span, panic) = panic_call(cx, f);
+    let (span, panic, symbol_str) = panic_call(cx, f);
 
     // Find the span of the argument to `panic!()`, before expansion in the
     // case of `panic!(some_macro!())`.
@@ -95,7 +95,7 @@ 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");
+            l.note(format!("the {}!() macro supports formatting, so there's no need for the format!() macro here", symbol_str).as_str());
             if let Some((open, close, _)) = find_delimiters(cx, arg_span) {
                 l.multipart_suggestion(
                     "remove the `format!(..)` macro call",
@@ -160,7 +160,7 @@ fn check_panic_str<'tcx>(
         Parser::new(fmt.as_ref(), style, snippet.clone(), false, ParseMode::Format);
     let n_arguments = (&mut fmt_parser).filter(|a| matches!(a, Piece::NextArgument(_))).count();
 
-    let (span, _) = panic_call(cx, f);
+    let (span, _, _) = panic_call(cx, f);
 
     if n_arguments > 0 && fmt_parser.errors.is_empty() {
         let arg_spans: Vec<_> = match &fmt_parser.arg_places[..] {
@@ -230,7 +230,7 @@ fn find_delimiters<'tcx>(cx: &LateContext<'tcx>, span: Span) -> Option<(Span, Sp
     ))
 }
 
-fn panic_call<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>) -> (Span, Symbol) {
+fn panic_call<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>) -> (Span, Symbol, SymbolStr) {
     let mut expn = f.span.ctxt().outer_expn_data();
 
     let mut panic_macro = kw::Empty;
@@ -248,5 +248,10 @@ fn panic_call<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>) -> (Span,
         }
     }
 
-    (expn.call_site, panic_macro)
+    let macro_symbol = if let hygiene::ExpnKind::Macro(_, symbol) = expn.kind {
+        symbol
+    } else {
+        Symbol::intern("panic")
+    };
+    (expn.call_site, panic_macro, macro_symbol.as_str())
 }
diff --git a/src/test/ui/non-fmt-panic.rs b/src/test/ui/non-fmt-panic.rs
@@ -36,6 +36,8 @@ fn main() {
     panic!(a!()); //~ WARN panic message is not a string literal
 
     panic!(format!("{}", 1)); //~ WARN panic message is not a string literal
+    assert!(false, format!("{}", 1)); //~ WARN panic message is not a string literal
+    debug_assert!(false, 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

diff --git a/src/test/ui/non-fmt-panic.stderr b/src/test/ui/non-fmt-panic.stderr
@@ -213,7 +213,33 @@ LL |     panic!("{}", 1);
    |           --     --
 
 warning: panic message is not a string literal
-  --> $DIR/non-fmt-panic.rs:40:12
+  --> $DIR/non-fmt-panic.rs:39:20
+   |
+LL |     assert!(false, format!("{}", 1));
+   |                    ^^^^^^^^^^^^^^^^
+   |
+   = note: this is no longer accepted in Rust 2021
+   = note: the assert!() macro supports formatting, so there's no need for the format!() macro here
+help: remove the `format!(..)` macro call
+   |
+LL |     assert!(false, "{}", 1);
+   |                   --     --
+
+warning: panic message is not a string literal
+  --> $DIR/non-fmt-panic.rs:40:26
+   |
+LL |     debug_assert!(false, format!("{}", 1));
+   |                          ^^^^^^^^^^^^^^^^
+   |
+   = note: this is no longer accepted in Rust 2021
+   = note: the debug_assert!() macro supports formatting, so there's no need for the format!() macro here
+help: remove the `format!(..)` macro call
+   |
+LL |     debug_assert!(false, "{}", 1);
+   |                         --     --
+
+warning: panic message is not a string literal
+  --> $DIR/non-fmt-panic.rs:42:12
    |
 LL |     panic![123];
    |            ^^^
@@ -229,7 +255,7 @@ LL |     std::panic::panic_any(123);
    |     ^^^^^^^^^^^^^^^^^^^^^^   ^
 
 warning: panic message is not a string literal
-  --> $DIR/non-fmt-panic.rs:41:12
+  --> $DIR/non-fmt-panic.rs:43:12
    |
 LL |     panic!{123};
    |            ^^^
@@ -244,5 +270,5 @@ help: or use std::panic::panic_any instead
 LL |     std::panic::panic_any(123);
    |     ^^^^^^^^^^^^^^^^^^^^^^   ^
 
-warning: 18 warnings emitted
+warning: 20 warnings emitted