diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs index e98297b692c92..bfe37ce6959e7 100644 --- a/compiler/rustc_lint/src/non_fmt_panic.rs +++ b/compiler/rustc_lint/src/non_fmt_panic.rs @@ -69,23 +69,65 @@ 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!())`. + // 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) { + 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((open, close, _)) = find_delimiters(cx, arg_span) { + l.multipart_suggestion( + "remove the `format!(..)` macro call", + vec![ + (arg_span.until(open.shrink_to_hi()), "".into()), + (close.until(arg_span.shrink_to_hi()), "".into()), + ], + Applicability::MachineApplicable, + ); + } + } else { 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), - "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(); @@ -175,6 +217,19 @@ fn check_panic_str<'tcx>( } } +/// 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()?; + 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) { 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 4ed0262bf2cfe..01820656c370d 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -560,6 +560,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..6a64587a2237f 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")] +#[cfg_attr(not(test), rustc_diagnostic_item = "format_macro")] macro_rules! format { ($($arg:tt)*) => {{ let res = $crate::fmt::format($crate::__export::format_args!($($arg)*)); diff --git a/src/test/ui/non-fmt-panic.rs b/src/test/ui/non-fmt-panic.rs index 25c53316e1290..c80a90b3eaaac 100644 --- a/src/test/ui/non-fmt-panic.rs +++ b/src/test/ui/non-fmt-panic.rs @@ -29,6 +29,17 @@ 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 + + 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 45187c518c423..7a333b3e76abe 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 @@ -183,5 +183,66 @@ 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: 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: 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