From a616f8267ed9b9c45cdef81346e98aae06a70990 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 1 Feb 2021 22:30:09 +0100 Subject: [PATCH 1/6] Add lint for `panic!(123)` which is not accepted in Rust 2021. This extends the `panic_fmt` lint to warn for all cases where the first argument cannot be interpreted as a format string, as will happen in Rust 2021. It suggests to add `"{}", ` to format the message as a string. In the case of `std::panic!()`, it also suggests the recently stabilized `std::panic::panic_any()` function as an alternative. It renames the lint to `non_fmt_panic` to match the lint naming guidelines. --- compiler/rustc_lint/src/lib.rs | 6 +- compiler/rustc_lint/src/non_fmt_panic.rs | 197 ++++++++++++++++++ compiler/rustc_lint/src/panic_fmt.rs | 155 -------------- src/test/ui/fmt/format-args-capture.rs | 2 +- .../ui/macros/macro-comma-behavior-rpass.rs | 2 +- .../ui/{panic-brace.rs => non-fmt-panic.rs} | 12 +- ...anic-brace.stderr => non-fmt-panic.stderr} | 116 +++++++++-- 7 files changed, 310 insertions(+), 180 deletions(-) create mode 100644 compiler/rustc_lint/src/non_fmt_panic.rs delete mode 100644 compiler/rustc_lint/src/panic_fmt.rs rename src/test/ui/{panic-brace.rs => non-fmt-panic.rs} (70%) rename src/test/ui/{panic-brace.stderr => non-fmt-panic.stderr} (52%) diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 6f44436e2a010..638b73c27a8d7 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -55,8 +55,8 @@ mod late; mod levels; mod methods; mod non_ascii_idents; +mod non_fmt_panic; mod nonstandard_style; -mod panic_fmt; mod passes; mod redundant_semicolon; mod traits; @@ -81,8 +81,8 @@ use builtin::*; use internal::*; use methods::*; use non_ascii_idents::*; +use non_fmt_panic::NonPanicFmt; use nonstandard_style::*; -use panic_fmt::PanicFmt; use redundant_semicolon::*; use traits::*; use types::*; @@ -169,7 +169,7 @@ macro_rules! late_lint_passes { ClashingExternDeclarations: ClashingExternDeclarations::new(), DropTraitConstraints: DropTraitConstraints, TemporaryCStringAsPtr: TemporaryCStringAsPtr, - PanicFmt: PanicFmt, + NonPanicFmt: NonPanicFmt, ] ); }; diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs new file mode 100644 index 0000000000000..e98297b692c92 --- /dev/null +++ b/compiler/rustc_lint/src/non_fmt_panic.rs @@ -0,0 +1,197 @@ +use crate::{LateContext, LateLintPass, LintContext}; +use rustc_ast as ast; +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}; + +declare_lint! { + /// The `non_fmt_panic` lint detects `panic!(..)` invocations where the first + /// argument is not a formatting string. + /// + /// ### Example + /// + /// ```rust,no_run + /// panic!("{}"); + /// panic!(123); + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// In Rust 2018 and earlier, `panic!(x)` directly uses `x` as the message. + /// That means that `panic!("{}")` panics with the message `"{}"` instead + /// of using it as a formatting string, and `panic!(123)` will panic with + /// an `i32` as message. + /// + /// Rust 2021 always interprets the first argument as format string. + NON_FMT_PANIC, + Warn, + "detect single-argument panic!() invocations in which the argument is not a format string", + report_in_external_macro +} + +declare_lint_pass!(NonPanicFmt => [NON_FMT_PANIC]); + +impl<'tcx> LateLintPass<'tcx> for NonPanicFmt { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) { + if let hir::ExprKind::Call(f, [arg]) = &expr.kind { + if let &ty::FnDef(def_id, _) = cx.typeck_results().expr_ty(f).kind() { + if Some(def_id) == cx.tcx.lang_items().begin_panic_fn() + || Some(def_id) == cx.tcx.lang_items().panic_fn() + || Some(def_id) == cx.tcx.lang_items().panic_str() + { + if let Some(id) = f.span.ctxt().outer_expn_data().macro_def_id { + if cx.tcx.is_diagnostic_item(sym::std_panic_2015_macro, id) + || cx.tcx.is_diagnostic_item(sym::core_panic_2015_macro, id) + { + check_panic(cx, f, arg); + } + } + } + } + } + } +} + +fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tcx hir::Expr<'tcx>) { + if let hir::ExprKind::Lit(lit) = &arg.kind { + if let ast::LitKind::Str(sym, _) = lit.node { + // The argument is a string literal. + check_panic_str(cx, f, arg, &sym.as_str()); + return; + } + } + + // The argument is *not* a string literal. + + let (span, panic) = panic_call(cx, f); + + 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) { + l.span_suggestion_verbose( + 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, + ); + } + } + l.emit(); + }); +} + +fn check_panic_str<'tcx>( + cx: &LateContext<'tcx>, + f: &'tcx hir::Expr<'tcx>, + arg: &'tcx hir::Expr<'tcx>, + fmt: &str, +) { + if !fmt.contains(&['{', '}'][..]) { + // No brace, no problem. + return; + } + + let fmt_span = arg.span.source_callsite(); + + let (snippet, style) = match cx.sess().parse_sess.source_map().span_to_snippet(fmt_span) { + Ok(snippet) => { + // Count the number of `#`s between the `r` and `"`. + let style = snippet.strip_prefix('r').and_then(|s| s.find('"')); + (Some(snippet), style) + } + Err(_) => (None, None), + }; + + let mut fmt_parser = + 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); + + if n_arguments > 0 && fmt_parser.errors.is_empty() { + let arg_spans: Vec<_> = match &fmt_parser.arg_places[..] { + [] => vec![fmt_span], + v => v.iter().map(|span| fmt_span.from_inner(*span)).collect(), + }; + cx.struct_span_lint(NON_FMT_PANIC, arg_spans, |lint| { + let mut l = lint.build(match n_arguments { + 1 => "panic message contains an unused formatting placeholder", + _ => "panic message contains unused formatting placeholders", + }); + l.note("this message is not used as a format string when given without arguments, but will be in Rust 2021"); + if span.contains(arg.span) { + l.span_suggestion( + arg.span.shrink_to_hi(), + &format!("add the missing argument{}", pluralize!(n_arguments)), + ", ...".into(), + Applicability::HasPlaceholders, + ); + l.span_suggestion( + arg.span.shrink_to_lo(), + "or add a \"{}\" format string to use the message literally", + "\"{}\", ".into(), + Applicability::MachineApplicable, + ); + } + l.emit(); + }); + } else { + let brace_spans: Option> = + snippet.filter(|s| s.starts_with('"') || s.starts_with("r#")).map(|s| { + s.char_indices() + .filter(|&(_, c)| c == '{' || c == '}') + .map(|(i, _)| fmt_span.from_inner(InnerSpan { start: i, end: i + 1 })) + .collect() + }); + let msg = match &brace_spans { + Some(v) if v.len() == 1 => "panic message contains a brace", + _ => "panic message contains braces", + }; + cx.struct_span_lint(NON_FMT_PANIC, brace_spans.unwrap_or(vec![span]), |lint| { + let mut l = lint.build(msg); + l.note("this message is not used as a format string, but will be in Rust 2021"); + if span.contains(arg.span) { + l.span_suggestion( + arg.span.shrink_to_lo(), + "add a \"{}\" format string to use the message literally", + "\"{}\", ".into(), + Applicability::MachineApplicable, + ); + } + l.emit(); + }); + } +} + +fn panic_call<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>) -> (Span, Symbol) { + let mut expn = f.span.ctxt().outer_expn_data(); + + let mut panic_macro = kw::Empty; + + // Unwrap more levels of macro expansion, as panic_2015!() + // was likely expanded from panic!() and possibly from + // [debug_]assert!(). + for &i in + &[sym::std_panic_macro, sym::core_panic_macro, sym::assert_macro, sym::debug_assert_macro] + { + let parent = expn.call_site.ctxt().outer_expn_data(); + if parent.macro_def_id.map_or(false, |id| cx.tcx.is_diagnostic_item(i, id)) { + expn = parent; + panic_macro = i; + } + } + + (expn.call_site, panic_macro) +} diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs deleted file mode 100644 index 4a6aca72acbbe..0000000000000 --- a/compiler/rustc_lint/src/panic_fmt.rs +++ /dev/null @@ -1,155 +0,0 @@ -use crate::{LateContext, LateLintPass, LintContext}; -use rustc_ast as ast; -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, InnerSpan}; - -declare_lint! { - /// The `panic_fmt` lint detects `panic!("..")` with `{` or `}` in the string literal. - /// - /// ### Example - /// - /// ```rust,no_run - /// panic!("{}"); - /// ``` - /// - /// {{produces}} - /// - /// ### Explanation - /// - /// In Rust 2018 and earlier, `panic!("{}")` panics with the message `"{}"`, - /// as a `panic!()` invocation with a single argument does not use `format_args!()`. - /// Rust 2021 interprets this string as format string, which breaks this. - PANIC_FMT, - Warn, - "detect braces in single-argument panic!() invocations", - report_in_external_macro -} - -declare_lint_pass!(PanicFmt => [PANIC_FMT]); - -impl<'tcx> LateLintPass<'tcx> for PanicFmt { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) { - if let hir::ExprKind::Call(f, [arg]) = &expr.kind { - if let &ty::FnDef(def_id, _) = cx.typeck_results().expr_ty(f).kind() { - if Some(def_id) == cx.tcx.lang_items().begin_panic_fn() - || Some(def_id) == cx.tcx.lang_items().panic_fn() - { - check_panic(cx, f, arg); - } - } - } - } -} - -fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tcx hir::Expr<'tcx>) { - if let hir::ExprKind::Lit(lit) = &arg.kind { - if let ast::LitKind::Str(sym, _) = lit.node { - let mut expn = f.span.ctxt().outer_expn_data(); - if let Some(id) = expn.macro_def_id { - if cx.tcx.is_diagnostic_item(sym::std_panic_2015_macro, id) - || cx.tcx.is_diagnostic_item(sym::core_panic_2015_macro, id) - { - let fmt = sym.as_str(); - if !fmt.contains(&['{', '}'][..]) { - return; - } - - let fmt_span = arg.span.source_callsite(); - - let (snippet, style) = - match cx.sess().parse_sess.source_map().span_to_snippet(fmt_span) { - Ok(snippet) => { - // Count the number of `#`s between the `r` and `"`. - let style = snippet.strip_prefix('r').and_then(|s| s.find('"')); - (Some(snippet), style) - } - Err(_) => (None, None), - }; - - let mut fmt_parser = - Parser::new(fmt.as_ref(), style, snippet.clone(), false, ParseMode::Format); - let n_arguments = - (&mut fmt_parser).filter(|a| matches!(a, Piece::NextArgument(_))).count(); - - // Unwrap more levels of macro expansion, as panic_2015!() - // was likely expanded from panic!() and possibly from - // [debug_]assert!(). - for &assert in &[ - sym::std_panic_macro, - sym::core_panic_macro, - sym::assert_macro, - sym::debug_assert_macro, - ] { - let parent = expn.call_site.ctxt().outer_expn_data(); - if parent - .macro_def_id - .map_or(false, |id| cx.tcx.is_diagnostic_item(assert, id)) - { - expn = parent; - } - } - - if n_arguments > 0 && fmt_parser.errors.is_empty() { - let arg_spans: Vec<_> = match &fmt_parser.arg_places[..] { - [] => vec![fmt_span], - v => v.iter().map(|span| fmt_span.from_inner(*span)).collect(), - }; - cx.struct_span_lint(PANIC_FMT, arg_spans, |lint| { - let mut l = lint.build(match n_arguments { - 1 => "panic message contains an unused formatting placeholder", - _ => "panic message contains unused formatting placeholders", - }); - l.note("this message is not used as a format string when given without arguments, but will be in a future Rust edition"); - if expn.call_site.contains(arg.span) { - l.span_suggestion( - arg.span.shrink_to_hi(), - &format!("add the missing argument{}", pluralize!(n_arguments)), - ", ...".into(), - Applicability::HasPlaceholders, - ); - l.span_suggestion( - arg.span.shrink_to_lo(), - "or add a \"{}\" format string to use the message literally", - "\"{}\", ".into(), - Applicability::MachineApplicable, - ); - } - l.emit(); - }); - } else { - let brace_spans: Option> = snippet - .filter(|s| s.starts_with('"') || s.starts_with("r#")) - .map(|s| { - s.char_indices() - .filter(|&(_, c)| c == '{' || c == '}') - .map(|(i, _)| { - fmt_span.from_inner(InnerSpan { start: i, end: i + 1 }) - }) - .collect() - }); - let msg = match &brace_spans { - Some(v) if v.len() == 1 => "panic message contains a brace", - _ => "panic message contains braces", - }; - cx.struct_span_lint(PANIC_FMT, brace_spans.unwrap_or(vec![expn.call_site]), |lint| { - let mut l = lint.build(msg); - l.note("this message is not used as a format string, but will be in a future Rust edition"); - if expn.call_site.contains(arg.span) { - l.span_suggestion( - arg.span.shrink_to_lo(), - "add a \"{}\" format string to use the message literally", - "\"{}\", ".into(), - Applicability::MachineApplicable, - ); - } - l.emit(); - }); - } - } - } - } - } -} diff --git a/src/test/ui/fmt/format-args-capture.rs b/src/test/ui/fmt/format-args-capture.rs index 4e3fa9a3c589a..d5886a13558c6 100644 --- a/src/test/ui/fmt/format-args-capture.rs +++ b/src/test/ui/fmt/format-args-capture.rs @@ -31,7 +31,7 @@ fn panic_with_single_argument_does_not_get_formatted() { // RFC #2795 suggests that this may need to change so that captured arguments are formatted. // For stability reasons this will need to part of an edition change. - #[allow(panic_fmt)] + #[allow(non_fmt_panic)] let msg = std::panic::catch_unwind(|| { panic!("{foo}"); }).unwrap_err(); diff --git a/src/test/ui/macros/macro-comma-behavior-rpass.rs b/src/test/ui/macros/macro-comma-behavior-rpass.rs index c46274d59b658..fd2c590ae5fd6 100644 --- a/src/test/ui/macros/macro-comma-behavior-rpass.rs +++ b/src/test/ui/macros/macro-comma-behavior-rpass.rs @@ -57,7 +57,7 @@ fn writeln_1arg() { // // (Example: Issue #48042) #[test] -#[allow(panic_fmt)] +#[allow(non_fmt_panic)] fn to_format_or_not_to_format() { // ("{}" is the easiest string to test because if this gets // sent to format_args!, it'll simply fail to compile. diff --git a/src/test/ui/panic-brace.rs b/src/test/ui/non-fmt-panic.rs similarity index 70% rename from src/test/ui/panic-brace.rs rename to src/test/ui/non-fmt-panic.rs index 754dcc287d0f9..25c53316e1290 100644 --- a/src/test/ui/panic-brace.rs +++ b/src/test/ui/non-fmt-panic.rs @@ -13,19 +13,27 @@ fn main() { core::panic!("Hello {}"); //~ WARN panic message contains an unused formatting placeholder assert!(false, "{:03x} {test} bla"); //~^ WARN panic message contains unused formatting placeholders + assert!(false, S); + //~^ WARN panic message is not a string literal debug_assert!(false, "{{}} bla"); //~ WARN panic message contains braces - panic!(C); // No warning (yet) - panic!(S); // No warning (yet) + panic!(C); //~ WARN panic message is not a string literal + panic!(S); //~ WARN panic message is not a string literal + std::panic!(123); //~ WARN panic message is not a string literal + core::panic!(&*"abc"); //~ WARN panic message is not a string literal panic!(concat!("{", "}")); //~ WARN panic message contains an unused formatting placeholder panic!(concat!("{", "{")); //~ WARN panic message contains braces fancy_panic::fancy_panic!("test {} 123"); //~^ WARN panic message contains an unused formatting placeholder + fancy_panic::fancy_panic!(S); + //~^ 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 { ($e:expr) => (); } panic!("{}"); // OK + panic!(S); // OK } diff --git a/src/test/ui/panic-brace.stderr b/src/test/ui/non-fmt-panic.stderr similarity index 52% rename from src/test/ui/panic-brace.stderr rename to src/test/ui/non-fmt-panic.stderr index 93808891c3c37..45187c518c423 100644 --- a/src/test/ui/panic-brace.stderr +++ b/src/test/ui/non-fmt-panic.stderr @@ -1,35 +1,35 @@ warning: panic message contains a brace - --> $DIR/panic-brace.rs:11:29 + --> $DIR/non-fmt-panic.rs:11:29 | LL | panic!("here's a brace: {"); | ^ | - = note: `#[warn(panic_fmt)]` on by default - = note: this message is not used as a format string, but will be in a future Rust edition + = note: `#[warn(non_fmt_panic)]` on by default + = note: this message is not used as a format string, but will be in Rust 2021 help: add a "{}" format string to use the message literally | LL | panic!("{}", "here's a brace: {"); | ^^^^^ warning: panic message contains a brace - --> $DIR/panic-brace.rs:12:31 + --> $DIR/non-fmt-panic.rs:12:31 | LL | std::panic!("another one: }"); | ^ | - = note: this message is not used as a format string, but will be in a future Rust edition + = note: this message is not used as a format string, but will be in Rust 2021 help: add a "{}" format string to use the message literally | LL | std::panic!("{}", "another one: }"); | ^^^^^ warning: panic message contains an unused formatting placeholder - --> $DIR/panic-brace.rs:13:25 + --> $DIR/non-fmt-panic.rs:13:25 | LL | core::panic!("Hello {}"); | ^^ | - = note: this message is not used as a format string when given without arguments, but will be in a future Rust edition + = note: this message is not used as a format string when given without arguments, but will be in Rust 2021 help: add the missing argument | LL | core::panic!("Hello {}", ...); @@ -40,12 +40,12 @@ LL | core::panic!("{}", "Hello {}"); | ^^^^^ warning: panic message contains unused formatting placeholders - --> $DIR/panic-brace.rs:14:21 + --> $DIR/non-fmt-panic.rs:14:21 | LL | assert!(false, "{:03x} {test} bla"); | ^^^^^^ ^^^^^^ | - = note: this message is not used as a format string when given without arguments, but will be in a future Rust edition + = note: this message is not used as a format string when given without arguments, but will be in Rust 2021 help: add the missing arguments | LL | assert!(false, "{:03x} {test} bla", ...); @@ -55,25 +55,97 @@ help: or add a "{}" format string to use the message literally LL | assert!(false, "{}", "{:03x} {test} bla"); | ^^^^^ +warning: panic message is not a string literal + --> $DIR/non-fmt-panic.rs:16:20 + | +LL | assert!(false, S); + | ^ + | + = note: this is no longer accepted in Rust 2021 +help: add a "{}" format string to Display the message + | +LL | assert!(false, "{}", S); + | ^^^^^ + warning: panic message contains braces - --> $DIR/panic-brace.rs:16:27 + --> $DIR/non-fmt-panic.rs:18:27 | LL | debug_assert!(false, "{{}} bla"); | ^^^^ | - = note: this message is not used as a format string, but will be in a future Rust edition + = note: this message is not used as a format string, but will be in Rust 2021 help: add a "{}" format string to use the message literally | LL | debug_assert!(false, "{}", "{{}} bla"); | ^^^^^ +warning: panic message is not a string literal + --> $DIR/non-fmt-panic.rs:19:12 + | +LL | panic!(C); + | ^ + | + = note: this is no longer accepted in Rust 2021 +help: add a "{}" format string to Display the message + | +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 + | +LL | panic!(S); + | ^ + | + = note: this is no longer accepted in Rust 2021 +help: add a "{}" format string to Display the message + | +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 + | +LL | std::panic!(123); + | ^^^ + | + = note: this is no longer accepted in Rust 2021 +help: add a "{}" format string to Display the message + | +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 + | +LL | core::panic!(&*"abc"); + | ^^^^^^^ + | + = note: this is no longer accepted in Rust 2021 +help: add a "{}" format string to Display the message + | +LL | core::panic!("{}", &*"abc"); + | ^^^^^ + warning: panic message contains an unused formatting placeholder - --> $DIR/panic-brace.rs:19:12 + --> $DIR/non-fmt-panic.rs:23:12 | LL | panic!(concat!("{", "}")); | ^^^^^^^^^^^^^^^^^ | - = note: this message is not used as a format string when given without arguments, but will be in a future Rust edition + = note: this message is not used as a format string when given without arguments, but will be in Rust 2021 help: add the missing argument | LL | panic!(concat!("{", "}"), ...); @@ -84,24 +156,32 @@ LL | panic!("{}", concat!("{", "}")); | ^^^^^ warning: panic message contains braces - --> $DIR/panic-brace.rs:20:5 + --> $DIR/non-fmt-panic.rs:24:5 | LL | panic!(concat!("{", "{")); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: this message is not used as a format string, but will be in a future Rust edition + = note: this message is not used as a format string, but will be in Rust 2021 help: add a "{}" format string to use the message literally | LL | panic!("{}", concat!("{", "{")); | ^^^^^ warning: panic message contains an unused formatting placeholder - --> $DIR/panic-brace.rs:22:37 + --> $DIR/non-fmt-panic.rs:26:37 | LL | fancy_panic::fancy_panic!("test {} 123"); | ^^ | - = note: this message is not used as a format string when given without arguments, but will be in a future Rust edition + = note: this message is not used as a format string when given without arguments, but will be in Rust 2021 + +warning: panic message is not a string literal + --> $DIR/non-fmt-panic.rs:29:31 + | +LL | fancy_panic::fancy_panic!(S); + | ^ + | + = note: this is no longer accepted in Rust 2021 -warning: 8 warnings emitted +warning: 14 warnings emitted From 34d5ac25c565a772c5974ab3b332644a9eff60f8 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 2 Feb 2021 00:17:51 +0100 Subject: [PATCH 2/6] Make panic/assert calls in rustc compatible with Rust 2021. --- compiler/rustc_ast/src/mut_visit.rs | 2 +- compiler/rustc_errors/src/lib.rs | 4 ++-- compiler/rustc_middle/src/util/bug.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 90d31d4801f92..024d9687f3119 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -28,7 +28,7 @@ pub trait ExpectOne { impl ExpectOne for SmallVec { fn expect_one(self, err: &'static str) -> A::Item { - assert!(self.len() == 1, err); + assert!(self.len() == 1, "{}", err); self.into_iter().next().unwrap() } } diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index e184e929b0745..aa88233209940 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -901,7 +901,7 @@ impl HandlerInner { fn span_bug(&mut self, sp: impl Into, msg: &str) -> ! { self.emit_diag_at_span(Diagnostic::new(Bug, msg), sp); - panic!(ExplicitBug); + panic::panic_any(ExplicitBug); } fn emit_diag_at_span(&mut self, mut diag: Diagnostic, sp: impl Into) { @@ -955,7 +955,7 @@ impl HandlerInner { fn bug(&mut self, msg: &str) -> ! { self.emit_diagnostic(&Diagnostic::new(Bug, msg)); - panic!(ExplicitBug); + panic::panic_any(ExplicitBug); } fn delay_as_bug(&mut self, diagnostic: Diagnostic) { diff --git a/compiler/rustc_middle/src/util/bug.rs b/compiler/rustc_middle/src/util/bug.rs index e79adcdb54598..791d5060fe5c7 100644 --- a/compiler/rustc_middle/src/util/bug.rs +++ b/compiler/rustc_middle/src/util/bug.rs @@ -3,7 +3,7 @@ use crate::ty::{tls, TyCtxt}; use rustc_span::{MultiSpan, Span}; use std::fmt; -use std::panic::Location; +use std::panic::{panic_any, Location}; #[cold] #[inline(never)] @@ -32,7 +32,7 @@ fn opt_span_bug_fmt>( match (tcx, span) { (Some(tcx), Some(span)) => tcx.sess.diagnostic().span_bug(span, &msg), (Some(tcx), None) => tcx.sess.diagnostic().bug(&msg), - (None, _) => panic!(msg), + (None, _) => panic_any(msg), } }); unreachable!(); From e9ad5be0f72f012c463c7796b9620df07b78bce6 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 2 Feb 2021 00:40:17 +0100 Subject: [PATCH 3/6] Allow/fix non_fmt_panic in tests. --- library/term/src/terminfo/parm/tests.rs | 12 +++--- library/test/src/tests.rs | 2 +- src/test/ui-fulldeps/issue-15149.rs | 6 +-- src/test/ui/consts/const-eval/const_panic.rs | 1 + .../ui/consts/const-eval/const_panic.stderr | 40 +++++++++---------- src/test/ui/drop/dynamic-drop-async.rs | 2 +- src/test/ui/drop/dynamic-drop.rs | 4 +- src/test/ui/macros/assert-macro-owned.rs | 2 + src/test/ui/mir/mir_drop_order.rs | 2 +- src/test/ui/panics/explicit-panic-msg.rs | 1 + src/test/ui/panics/panic-macro-any-wrapped.rs | 2 + src/test/ui/panics/panic-macro-any.rs | 1 + src/test/ui/panics/while-panic.rs | 2 +- 13 files changed, 42 insertions(+), 35 deletions(-) diff --git a/library/term/src/terminfo/parm/tests.rs b/library/term/src/terminfo/parm/tests.rs index b975bd2d19882..1cc0967c8f42e 100644 --- a/library/term/src/terminfo/parm/tests.rs +++ b/library/term/src/terminfo/parm/tests.rs @@ -77,15 +77,15 @@ fn test_comparison_ops() { for &(op, bs) in v.iter() { let s = format!("%{{1}}%{{2}}%{}%d", op); let res = expand(s.as_bytes(), &[], &mut Variables::new()); - assert!(res.is_ok(), res.unwrap_err()); + assert!(res.is_ok(), "{}", res.unwrap_err()); assert_eq!(res.unwrap(), vec![b'0' + bs[0]]); let s = format!("%{{1}}%{{1}}%{}%d", op); let res = expand(s.as_bytes(), &[], &mut Variables::new()); - assert!(res.is_ok(), res.unwrap_err()); + assert!(res.is_ok(), "{}", res.unwrap_err()); assert_eq!(res.unwrap(), vec![b'0' + bs[1]]); let s = format!("%{{2}}%{{1}}%{}%d", op); let res = expand(s.as_bytes(), &[], &mut Variables::new()); - assert!(res.is_ok(), res.unwrap_err()); + assert!(res.is_ok(), "{}", res.unwrap_err()); assert_eq!(res.unwrap(), vec![b'0' + bs[2]]); } } @@ -95,13 +95,13 @@ fn test_conditionals() { let mut vars = Variables::new(); let s = b"\\E[%?%p1%{8}%<%t3%p1%d%e%p1%{16}%<%t9%p1%{8}%-%d%e38;5;%p1%d%;m"; let res = expand(s, &[Number(1)], &mut vars); - assert!(res.is_ok(), res.unwrap_err()); + assert!(res.is_ok(), "{}", res.unwrap_err()); assert_eq!(res.unwrap(), "\\E[31m".bytes().collect::>()); let res = expand(s, &[Number(8)], &mut vars); - assert!(res.is_ok(), res.unwrap_err()); + assert!(res.is_ok(), "{}", res.unwrap_err()); assert_eq!(res.unwrap(), "\\E[90m".bytes().collect::>()); let res = expand(s, &[Number(42)], &mut vars); - assert!(res.is_ok(), res.unwrap_err()); + assert!(res.is_ok(), "{}", res.unwrap_err()); assert_eq!(res.unwrap(), "\\E[38;5;42m".bytes().collect::>()); } diff --git a/library/test/src/tests.rs b/library/test/src/tests.rs index 99e12c973c4a2..f0586d510dbdb 100644 --- a/library/test/src/tests.rs +++ b/library/test/src/tests.rs @@ -199,7 +199,7 @@ fn test_should_panic_bad_message() { fn test_should_panic_non_string_message_type() { use crate::tests::TrFailedMsg; fn f() { - panic!(1i32); + std::panic::panic_any(1i32); } let expected = "foobar"; let failed_msg = format!( diff --git a/src/test/ui-fulldeps/issue-15149.rs b/src/test/ui-fulldeps/issue-15149.rs index c80628aabc83f..c7ef5ad70a114 100644 --- a/src/test/ui-fulldeps/issue-15149.rs +++ b/src/test/ui-fulldeps/issue-15149.rs @@ -50,7 +50,7 @@ fn test() { .output().unwrap(); assert!(child_output.status.success(), - format!("child assertion failed\n child stdout:\n {}\n child stderr:\n {}", - str::from_utf8(&child_output.stdout).unwrap(), - str::from_utf8(&child_output.stderr).unwrap())); + "child assertion failed\n child stdout:\n {}\n child stderr:\n {}", + str::from_utf8(&child_output.stdout).unwrap(), + str::from_utf8(&child_output.stderr).unwrap()); } diff --git a/src/test/ui/consts/const-eval/const_panic.rs b/src/test/ui/consts/const-eval/const_panic.rs index e9d66477d60a5..8ae8376ae4a6f 100644 --- a/src/test/ui/consts/const-eval/const_panic.rs +++ b/src/test/ui/consts/const-eval/const_panic.rs @@ -1,4 +1,5 @@ #![feature(const_panic)] +#![allow(non_fmt_panic)] #![crate_type = "lib"] const MSG: &str = "hello"; diff --git a/src/test/ui/consts/const-eval/const_panic.stderr b/src/test/ui/consts/const-eval/const_panic.stderr index 713be5b662d54..74907a0b49518 100644 --- a/src/test/ui/consts/const-eval/const_panic.stderr +++ b/src/test/ui/consts/const-eval/const_panic.stderr @@ -1,10 +1,10 @@ error: any use of this value will cause an error - --> $DIR/const_panic.rs:6:15 + --> $DIR/const_panic.rs:7:15 | LL | const Z: () = std::panic!("cheese"); | --------------^^^^^^^^^^^^^^^^^^^^^- | | - | the evaluated program panicked at 'cheese', $DIR/const_panic.rs:6:15 + | the evaluated program panicked at 'cheese', $DIR/const_panic.rs:7:15 | = note: `#[deny(const_err)]` on by default = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! @@ -12,108 +12,108 @@ LL | const Z: () = std::panic!("cheese"); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: any use of this value will cause an error - --> $DIR/const_panic.rs:10:16 + --> $DIR/const_panic.rs:11:16 | LL | const Z2: () = std::panic!(); | ---------------^^^^^^^^^^^^^- | | - | the evaluated program panicked at 'explicit panic', $DIR/const_panic.rs:10:16 + | the evaluated program panicked at 'explicit panic', $DIR/const_panic.rs:11:16 | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #71800 = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: any use of this value will cause an error - --> $DIR/const_panic.rs:14:15 + --> $DIR/const_panic.rs:15:15 | LL | const Y: () = std::unreachable!(); | --------------^^^^^^^^^^^^^^^^^^^- | | - | the evaluated program panicked at 'internal error: entered unreachable code', $DIR/const_panic.rs:14:15 + | the evaluated program panicked at 'internal error: entered unreachable code', $DIR/const_panic.rs:15:15 | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #71800 = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: any use of this value will cause an error - --> $DIR/const_panic.rs:18:15 + --> $DIR/const_panic.rs:19:15 | LL | const X: () = std::unimplemented!(); | --------------^^^^^^^^^^^^^^^^^^^^^- | | - | the evaluated program panicked at 'not implemented', $DIR/const_panic.rs:18:15 + | the evaluated program panicked at 'not implemented', $DIR/const_panic.rs:19:15 | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #71800 = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: any use of this value will cause an error - --> $DIR/const_panic.rs:22:15 + --> $DIR/const_panic.rs:23:15 | LL | const W: () = std::panic!(MSG); | --------------^^^^^^^^^^^^^^^^- | | - | the evaluated program panicked at 'hello', $DIR/const_panic.rs:22:15 + | the evaluated program panicked at 'hello', $DIR/const_panic.rs:23:15 | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #71800 = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: any use of this value will cause an error - --> $DIR/const_panic.rs:26:20 + --> $DIR/const_panic.rs:27:20 | LL | const Z_CORE: () = core::panic!("cheese"); | -------------------^^^^^^^^^^^^^^^^^^^^^^- | | - | the evaluated program panicked at 'cheese', $DIR/const_panic.rs:26:20 + | the evaluated program panicked at 'cheese', $DIR/const_panic.rs:27:20 | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #71800 = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: any use of this value will cause an error - --> $DIR/const_panic.rs:30:21 + --> $DIR/const_panic.rs:31:21 | LL | const Z2_CORE: () = core::panic!(); | --------------------^^^^^^^^^^^^^^- | | - | the evaluated program panicked at 'explicit panic', $DIR/const_panic.rs:30:21 + | the evaluated program panicked at 'explicit panic', $DIR/const_panic.rs:31:21 | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #71800 = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: any use of this value will cause an error - --> $DIR/const_panic.rs:34:20 + --> $DIR/const_panic.rs:35:20 | LL | const Y_CORE: () = core::unreachable!(); | -------------------^^^^^^^^^^^^^^^^^^^^- | | - | the evaluated program panicked at 'internal error: entered unreachable code', $DIR/const_panic.rs:34:20 + | the evaluated program panicked at 'internal error: entered unreachable code', $DIR/const_panic.rs:35:20 | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #71800 = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: any use of this value will cause an error - --> $DIR/const_panic.rs:38:20 + --> $DIR/const_panic.rs:39:20 | LL | const X_CORE: () = core::unimplemented!(); | -------------------^^^^^^^^^^^^^^^^^^^^^^- | | - | the evaluated program panicked at 'not implemented', $DIR/const_panic.rs:38:20 + | the evaluated program panicked at 'not implemented', $DIR/const_panic.rs:39:20 | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #71800 = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: any use of this value will cause an error - --> $DIR/const_panic.rs:42:20 + --> $DIR/const_panic.rs:43:20 | LL | const W_CORE: () = core::panic!(MSG); | -------------------^^^^^^^^^^^^^^^^^- | | - | the evaluated program panicked at 'hello', $DIR/const_panic.rs:42:20 + | the evaluated program panicked at 'hello', $DIR/const_panic.rs:43:20 | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #71800 diff --git a/src/test/ui/drop/dynamic-drop-async.rs b/src/test/ui/drop/dynamic-drop-async.rs index a952fe8e76e81..cb6d58a23d936 100644 --- a/src/test/ui/drop/dynamic-drop-async.rs +++ b/src/test/ui/drop/dynamic-drop-async.rs @@ -82,7 +82,7 @@ impl Allocator { self.cur_ops.set(self.cur_ops.get() + 1); if self.cur_ops.get() == self.failing_op { - panic!(InjectedFailure); + panic::panic_any(InjectedFailure); } } } diff --git a/src/test/ui/drop/dynamic-drop.rs b/src/test/ui/drop/dynamic-drop.rs index ddccee20e12a6..e28bedb982dd9 100644 --- a/src/test/ui/drop/dynamic-drop.rs +++ b/src/test/ui/drop/dynamic-drop.rs @@ -46,7 +46,7 @@ impl Allocator { self.cur_ops.set(self.cur_ops.get() + 1); if self.cur_ops.get() == self.failing_op { - panic!(InjectedFailure); + panic::panic_any(InjectedFailure); } let mut data = self.data.borrow_mut(); @@ -67,7 +67,7 @@ impl<'a> Drop for Ptr<'a> { self.1.cur_ops.set(self.1.cur_ops.get() + 1); if self.1.cur_ops.get() == self.1.failing_op { - panic!(InjectedFailure); + panic::panic_any(InjectedFailure); } } } diff --git a/src/test/ui/macros/assert-macro-owned.rs b/src/test/ui/macros/assert-macro-owned.rs index b50fe65c0150f..2846f2a1f8353 100644 --- a/src/test/ui/macros/assert-macro-owned.rs +++ b/src/test/ui/macros/assert-macro-owned.rs @@ -2,6 +2,8 @@ // error-pattern:panicked at 'test-assert-owned' // ignore-emscripten no processes +#![allow(non_fmt_panic)] + fn main() { assert!(false, "test-assert-owned".to_string()); } diff --git a/src/test/ui/mir/mir_drop_order.rs b/src/test/ui/mir/mir_drop_order.rs index 2949437b1e4b6..22c804abf5cc8 100644 --- a/src/test/ui/mir/mir_drop_order.rs +++ b/src/test/ui/mir/mir_drop_order.rs @@ -38,7 +38,7 @@ fn main() { assert_eq!(get(), vec![0, 2, 3, 1]); let _ = std::panic::catch_unwind(|| { - (d(4), &d(5), d(6), &d(7), panic!(InjectedFailure)); + (d(4), &d(5), d(6), &d(7), panic::panic_any(InjectedFailure)); }); // here, the temporaries (5/7) live until the end of the diff --git a/src/test/ui/panics/explicit-panic-msg.rs b/src/test/ui/panics/explicit-panic-msg.rs index 1789e2e62c8b2..bfcc12cd186bd 100644 --- a/src/test/ui/panics/explicit-panic-msg.rs +++ b/src/test/ui/panics/explicit-panic-msg.rs @@ -1,5 +1,6 @@ #![allow(unused_assignments)] #![allow(unused_variables)] +#![allow(non_fmt_panic)] // run-fail // error-pattern:wooooo diff --git a/src/test/ui/panics/panic-macro-any-wrapped.rs b/src/test/ui/panics/panic-macro-any-wrapped.rs index 80c87c6f32c4b..95ae6ffe8be02 100644 --- a/src/test/ui/panics/panic-macro-any-wrapped.rs +++ b/src/test/ui/panics/panic-macro-any-wrapped.rs @@ -2,6 +2,8 @@ // error-pattern:panicked at 'Box' // ignore-emscripten no processes +#![allow(non_fmt_panic)] + fn main() { panic!(Box::new(612_i64)); } diff --git a/src/test/ui/panics/panic-macro-any.rs b/src/test/ui/panics/panic-macro-any.rs index ffc7114c1f5f2..d2a7ba3713a51 100644 --- a/src/test/ui/panics/panic-macro-any.rs +++ b/src/test/ui/panics/panic-macro-any.rs @@ -3,6 +3,7 @@ // ignore-emscripten no processes #![feature(box_syntax)] +#![allow(non_fmt_panic)] fn main() { panic!(box 413 as Box); diff --git a/src/test/ui/panics/while-panic.rs b/src/test/ui/panics/while-panic.rs index 857f65a225228..3c6ee8fa3155e 100644 --- a/src/test/ui/panics/while-panic.rs +++ b/src/test/ui/panics/while-panic.rs @@ -5,7 +5,7 @@ // ignore-emscripten no processes fn main() { - panic!({ + panic!("{}", { while true { panic!("giraffe") } From 753b0b0b80238a24da87137b67040286dfb82c4d Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 2 Feb 2021 10:30:50 +0100 Subject: [PATCH 4/6] Update panic!() documentation about non-string panics. --- library/core/src/macros/panic.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/library/core/src/macros/panic.md b/library/core/src/macros/panic.md index a02e74d5e5a4d..6e502426df906 100644 --- a/library/core/src/macros/panic.md +++ b/library/core/src/macros/panic.md @@ -10,22 +10,23 @@ tests. `panic!` is closely tied with the `unwrap` method of both `panic!` when they are set to [`None`] or [`Err`] variants. This macro is used to inject panic into a Rust thread, causing the thread to -panic entirely. Each thread's panic can be reaped as the [`Box`]`<`[`Any`]`>` type, -and the single-argument form of the `panic!` macro will be the value which -is transmitted. +panic entirely. This macro panics with a string and uses the [`format!`] syntax +for building the message. + +Each thread's panic can be reaped as the [`Box`]`<`[`Any`]`>` type, +which contains either a `&str` or `String` for regular `panic!()` invocations. +To panic with a value of another other type, [`panic_any`] can be used. [`Result`] enum is often a better solution for recovering from errors than using the `panic!` macro. This macro should be used to avoid proceeding using incorrect values, such as from external sources. Detailed information about error handling is found in the [book]. -The multi-argument form of this macro panics with a string and has the -[`format!`] syntax for building a string. - See also the macro [`compile_error!`], for raising errors during compilation. [ounwrap]: Option::unwrap [runwrap]: Result::unwrap +[`panic_any`]: ../std/panic/fn.panic_any.html [`Box`]: ../std/boxed/struct.Box.html [`Any`]: crate::any::Any [`format!`]: ../std/macro.format.html @@ -42,6 +43,6 @@ program with code `101`. # #![allow(unreachable_code)] panic!(); panic!("this is a terrible mistake!"); -panic!(4); // panic with the value of 4 to be collected elsewhere panic!("this is a {} {message}", "fancy", message = "message"); +std::panic::panic_any(4); // panic with the value of 4 to be collected elsewhere ``` From 3f3eb89547522be44308c66c5152c65e1a822bbe Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 2 Feb 2021 20:24:42 +0100 Subject: [PATCH 5/6] Fix/allow non_fmt_panic in clippy tests. --- src/tools/clippy/tests/missing-test-files.rs | 14 ++++++-------- .../clippy/tests/ui/assertions_on_constants.rs | 2 ++ .../tests/ui/assertions_on_constants.stderr | 18 +++++++++--------- .../clippy/tests/ui/fallible_impl_from.rs | 2 +- .../clippy/tests/ui/fallible_impl_from.stderr | 4 ++-- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/tools/clippy/tests/missing-test-files.rs b/src/tools/clippy/tests/missing-test-files.rs index d87bb4be3c3f9..9cef7438d225c 100644 --- a/src/tools/clippy/tests/missing-test-files.rs +++ b/src/tools/clippy/tests/missing-test-files.rs @@ -9,14 +9,12 @@ fn test_missing_tests() { if !missing_files.is_empty() { assert!( false, - format!( - "Didn't see a test file for the following files:\n\n{}\n", - missing_files - .iter() - .map(|s| format!("\t{}", s)) - .collect::>() - .join("\n") - ) + "Didn't see a test file for the following files:\n\n{}\n", + missing_files + .iter() + .map(|s| format!("\t{}", s)) + .collect::>() + .join("\n") ); } } diff --git a/src/tools/clippy/tests/ui/assertions_on_constants.rs b/src/tools/clippy/tests/ui/assertions_on_constants.rs index 60d721c2f2049..e989de6540456 100644 --- a/src/tools/clippy/tests/ui/assertions_on_constants.rs +++ b/src/tools/clippy/tests/ui/assertions_on_constants.rs @@ -1,3 +1,5 @@ +#![allow(non_fmt_panic)] + macro_rules! assert_const { ($len:expr) => { assert!($len > 0); diff --git a/src/tools/clippy/tests/ui/assertions_on_constants.stderr b/src/tools/clippy/tests/ui/assertions_on_constants.stderr index 8f09c8ce9d52a..c66fdf093f514 100644 --- a/src/tools/clippy/tests/ui/assertions_on_constants.stderr +++ b/src/tools/clippy/tests/ui/assertions_on_constants.stderr @@ -1,5 +1,5 @@ error: `assert!(true)` will be optimized out by the compiler - --> $DIR/assertions_on_constants.rs:9:5 + --> $DIR/assertions_on_constants.rs:11:5 | LL | assert!(true); | ^^^^^^^^^^^^^^ @@ -9,7 +9,7 @@ LL | assert!(true); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `assert!(false)` should probably be replaced - --> $DIR/assertions_on_constants.rs:10:5 + --> $DIR/assertions_on_constants.rs:12:5 | LL | assert!(false); | ^^^^^^^^^^^^^^^ @@ -18,7 +18,7 @@ LL | assert!(false); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `assert!(true)` will be optimized out by the compiler - --> $DIR/assertions_on_constants.rs:11:5 + --> $DIR/assertions_on_constants.rs:13:5 | LL | assert!(true, "true message"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -27,7 +27,7 @@ LL | assert!(true, "true message"); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `assert!(false, "false message")` should probably be replaced - --> $DIR/assertions_on_constants.rs:12:5 + --> $DIR/assertions_on_constants.rs:14:5 | LL | assert!(false, "false message"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -36,7 +36,7 @@ LL | assert!(false, "false message"); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `assert!(false, msg.to_uppercase())` should probably be replaced - --> $DIR/assertions_on_constants.rs:15:5 + --> $DIR/assertions_on_constants.rs:17:5 | LL | assert!(false, msg.to_uppercase()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -45,7 +45,7 @@ LL | assert!(false, msg.to_uppercase()); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `assert!(true)` will be optimized out by the compiler - --> $DIR/assertions_on_constants.rs:18:5 + --> $DIR/assertions_on_constants.rs:20:5 | LL | assert!(B); | ^^^^^^^^^^^ @@ -54,7 +54,7 @@ LL | assert!(B); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `assert!(false)` should probably be replaced - --> $DIR/assertions_on_constants.rs:21:5 + --> $DIR/assertions_on_constants.rs:23:5 | LL | assert!(C); | ^^^^^^^^^^^ @@ -63,7 +63,7 @@ LL | assert!(C); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `assert!(false, "C message")` should probably be replaced - --> $DIR/assertions_on_constants.rs:22:5 + --> $DIR/assertions_on_constants.rs:24:5 | LL | assert!(C, "C message"); | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -72,7 +72,7 @@ LL | assert!(C, "C message"); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `debug_assert!(true)` will be optimized out by the compiler - --> $DIR/assertions_on_constants.rs:24:5 + --> $DIR/assertions_on_constants.rs:26:5 | LL | debug_assert!(true); | ^^^^^^^^^^^^^^^^^^^^ diff --git a/src/tools/clippy/tests/ui/fallible_impl_from.rs b/src/tools/clippy/tests/ui/fallible_impl_from.rs index 679f4a7dc357d..5d5af4e463297 100644 --- a/src/tools/clippy/tests/ui/fallible_impl_from.rs +++ b/src/tools/clippy/tests/ui/fallible_impl_from.rs @@ -36,7 +36,7 @@ impl From> for Invalid { fn from(s: Option) -> Invalid { let s = s.unwrap(); if !s.is_empty() { - panic!(42); + panic!("42"); } else if s.parse::().unwrap() != 42 { panic!("{:?}", s); } diff --git a/src/tools/clippy/tests/ui/fallible_impl_from.stderr b/src/tools/clippy/tests/ui/fallible_impl_from.stderr index ab976b947b356..f787b30bdabc5 100644 --- a/src/tools/clippy/tests/ui/fallible_impl_from.stderr +++ b/src/tools/clippy/tests/ui/fallible_impl_from.stderr @@ -59,8 +59,8 @@ note: potential failure(s) LL | let s = s.unwrap(); | ^^^^^^^^^^ LL | if !s.is_empty() { -LL | panic!(42); - | ^^^^^^^^^^^ +LL | panic!("42"); + | ^^^^^^^^^^^^^ LL | } else if s.parse::().unwrap() != 42 { | ^^^^^^^^^^^^^^^^^^^^^^^^^ LL | panic!("{:?}", s); From 0870c154b63319df131de822d85dcebcbba080af Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 3 Feb 2021 10:55:33 +0100 Subject: [PATCH 6/6] Suggest panic!("{}", ..) instead of panic!(..) clippy::expect_fun_call. --- src/tools/clippy/clippy_lints/src/methods/mod.rs | 2 +- src/tools/clippy/tests/ui/expect_fun_call.fixed | 10 +++++----- src/tools/clippy/tests/ui/expect_fun_call.stderr | 10 +++++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index a17c5996293e9..4ee423b383b0f 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -2183,7 +2183,7 @@ fn lint_expect_fun_call( span_replace_word, &format!("use of `{}` followed by a function call", name), "try this", - format!("unwrap_or_else({} {{ panic!({}) }})", closure_args, arg_root_snippet), + format!("unwrap_or_else({} {{ panic!(\"{{}}\", {}) }})", closure_args, arg_root_snippet), applicability, ); } diff --git a/src/tools/clippy/tests/ui/expect_fun_call.fixed b/src/tools/clippy/tests/ui/expect_fun_call.fixed index f3d8a941a92bc..a756d1cf50659 100644 --- a/src/tools/clippy/tests/ui/expect_fun_call.fixed +++ b/src/tools/clippy/tests/ui/expect_fun_call.fixed @@ -74,12 +74,12 @@ fn main() { "foo" } - Some("foo").unwrap_or_else(|| { panic!(get_string()) }); - Some("foo").unwrap_or_else(|| { panic!(get_string()) }); - Some("foo").unwrap_or_else(|| { panic!(get_string()) }); + Some("foo").unwrap_or_else(|| { panic!("{}", get_string()) }); + Some("foo").unwrap_or_else(|| { panic!("{}", get_string()) }); + Some("foo").unwrap_or_else(|| { panic!("{}", get_string()) }); - Some("foo").unwrap_or_else(|| { panic!(get_static_str()) }); - Some("foo").unwrap_or_else(|| { panic!(get_non_static_str(&0).to_string()) }); + Some("foo").unwrap_or_else(|| { panic!("{}", get_static_str()) }); + Some("foo").unwrap_or_else(|| { panic!("{}", get_non_static_str(&0).to_string()) }); } //Issue #3839 diff --git a/src/tools/clippy/tests/ui/expect_fun_call.stderr b/src/tools/clippy/tests/ui/expect_fun_call.stderr index a492e2df89d47..6dc796f5cee37 100644 --- a/src/tools/clippy/tests/ui/expect_fun_call.stderr +++ b/src/tools/clippy/tests/ui/expect_fun_call.stderr @@ -34,31 +34,31 @@ error: use of `expect` followed by a function call --> $DIR/expect_fun_call.rs:77:21 | LL | Some("foo").expect(&get_string()); - | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!(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:78:21 | LL | Some("foo").expect(get_string().as_ref()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!(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 | LL | Some("foo").expect(get_string().as_str()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!(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:81:21 | LL | Some("foo").expect(get_static_str()); - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!(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:82:21 | LL | Some("foo").expect(get_non_static_str(&0)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!(get_non_static_str(&0).to_string()) })` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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:86:16