diff --git a/Cargo.lock b/Cargo.lock index 928d19b1e2c3f..4c84e7bd8c9f1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3831,6 +3831,7 @@ dependencies = [ "rustc_hir", "rustc_index", "rustc_middle", + "rustc_parse_format", "rustc_session", "rustc_span", "rustc_target", diff --git a/compiler/rustc_builtin_macros/src/assert.rs b/compiler/rustc_builtin_macros/src/assert.rs index 5bfd8a2bf561c..bb6d3f6a0076c 100644 --- a/compiler/rustc_builtin_macros/src/assert.rs +++ b/compiler/rustc_builtin_macros/src/assert.rs @@ -1,8 +1,8 @@ use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_ast::ptr::P; -use rustc_ast::token::{self, TokenKind}; -use rustc_ast::tokenstream::{DelimSpan, TokenStream, TokenTree}; +use rustc_ast::token; +use rustc_ast::tokenstream::{DelimSpan, TokenStream}; use rustc_ast::{self as ast, *}; use rustc_ast_pretty::pprust; use rustc_expand::base::*; @@ -26,31 +26,39 @@ pub fn expand_assert<'cx>( // `core::panic` and `std::panic` are different macros, so we use call-site // context to pick up whichever is currently in scope. let sp = cx.with_call_site_ctxt(sp); - let tokens = custom_message.unwrap_or_else(|| { - TokenStream::from(TokenTree::token( - TokenKind::lit( - token::Str, + + let panic_call = if let Some(tokens) = custom_message { + // Pass the custom message to panic!(). + cx.expr( + sp, + ExprKind::MacCall(MacCall { + path: Path::from_ident(Ident::new(sym::panic, sp)), + args: P(MacArgs::Delimited( + DelimSpan::from_single(sp), + MacDelimiter::Parenthesis, + tokens, + )), + prior_type_ascription: None, + }), + ) + } else { + // Pass our own message directly to $crate::panicking::panic(), + // because it might contain `{` and `}` that should always be + // passed literally. + cx.expr_call_global( + sp, + cx.std_path(&[sym::panicking, sym::panic]), + vec![cx.expr_str( + DUMMY_SP, Symbol::intern(&format!( "assertion failed: {}", pprust::expr_to_string(&cond_expr).escape_debug() )), - None, - ), - DUMMY_SP, - )) - }); - let args = P(MacArgs::Delimited(DelimSpan::from_single(sp), MacDelimiter::Parenthesis, tokens)); - let panic_call = MacCall { - path: Path::from_ident(Ident::new(sym::panic, sp)), - args, - prior_type_ascription: None, + )], + ) }; - let if_expr = cx.expr_if( - sp, - cx.expr(sp, ExprKind::Unary(UnOp::Not, cond_expr)), - cx.expr(sp, ExprKind::MacCall(panic_call)), - None, - ); + let if_expr = + cx.expr_if(sp, cx.expr(sp, ExprKind::Unary(UnOp::Not, cond_expr)), panic_call, None); MacEager::expr(if_expr) } diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index a074af0189a28..66463eeb90713 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -1173,7 +1173,8 @@ fn quoted_tt_to_string(tt: &mbe::TokenTree) -> String { mbe::TokenTree::MetaVar(_, name) => format!("${}", name), mbe::TokenTree::MetaVarDecl(_, name, kind) => format!("${}:{}", name, kind), _ => panic!( - "unexpected mbe::TokenTree::{{Sequence or Delimited}} \ + "{}", + "unexpected mbe::TokenTree::{Sequence or Delimited} \ in follow set checker" ), } diff --git a/compiler/rustc_lint/Cargo.toml b/compiler/rustc_lint/Cargo.toml index 760a8e385d680..c56eb09b63471 100644 --- a/compiler/rustc_lint/Cargo.toml +++ b/compiler/rustc_lint/Cargo.toml @@ -20,3 +20,4 @@ rustc_feature = { path = "../rustc_feature" } rustc_index = { path = "../rustc_index" } rustc_session = { path = "../rustc_session" } rustc_trait_selection = { path = "../rustc_trait_selection" } +rustc_parse_format = { path = "../rustc_parse_format" } diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 24bfdad970a1c..81549be4b0915 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -55,6 +55,7 @@ mod levels; mod methods; mod non_ascii_idents; mod nonstandard_style; +mod panic_fmt; mod passes; mod redundant_semicolon; mod traits; @@ -80,6 +81,7 @@ use internal::*; use methods::*; use non_ascii_idents::*; use nonstandard_style::*; +use panic_fmt::PanicFmt; use redundant_semicolon::*; use traits::*; use types::*; @@ -166,6 +168,7 @@ macro_rules! late_lint_passes { ClashingExternDeclarations: ClashingExternDeclarations::new(), DropTraitConstraints: DropTraitConstraints, TemporaryCStringAsPtr: TemporaryCStringAsPtr, + PanicFmt: PanicFmt, ] ); }; diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs new file mode 100644 index 0000000000000..0d2b20989b0c3 --- /dev/null +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -0,0 +1,150 @@ +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 + /// + /// `panic!("{}")` panics with the message `"{}"`, as a `panic!()` invocation + /// with a single argument does not use `format_args!()`. + /// A future edition of Rust will interpret this string as format string, + /// which would break 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_macro, id) + || cx.tcx.is_diagnostic_item(sym::core_panic_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 another level of macro expansion if this panic!() + // was expanded from assert!() or debug_assert!(). + for &assert in &[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/compiler/rustc_passes/src/diagnostic_items.rs b/compiler/rustc_passes/src/diagnostic_items.rs index 0f4aa72d5c47b..5a087c41f58b9 100644 --- a/compiler/rustc_passes/src/diagnostic_items.rs +++ b/compiler/rustc_passes/src/diagnostic_items.rs @@ -113,6 +113,10 @@ fn collect<'tcx>(tcx: TyCtxt<'tcx>) -> FxHashMap { } } + for m in tcx.hir().krate().exported_macros { + collector.observe_item(m.attrs, m.hir_id); + } + collector.items } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 3a2a3adce35c9..338ff005995d5 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -267,6 +267,7 @@ symbols! { asm, assert, assert_inhabited, + assert_macro, assert_receiver_is_total_eq, assert_uninit_valid, assert_zero_valid, @@ -393,6 +394,7 @@ symbols! { copysignf64, core, core_intrinsics, + core_panic_macro, cosf32, cosf64, crate_id, @@ -416,6 +418,7 @@ symbols! { dead_code, dealloc, debug, + debug_assert_macro, debug_assertions, debug_struct, debug_trait, @@ -789,6 +792,7 @@ symbols! { panic_runtime, panic_str, panic_unwind, + panicking, param_attrs, parent_trait, partial_cmp, @@ -1064,6 +1068,7 @@ symbols! { staticlib, std, std_inject, + std_panic_macro, stmt, stmt_expr_attributes, stop_after_dataflow, diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index fe3eff04b4ae5..0416a7614a3f6 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -2,6 +2,7 @@ #[macro_export] #[allow_internal_unstable(core_panic, const_caller_location)] #[stable(feature = "core", since = "1.6.0")] +#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "core_panic_macro")] macro_rules! panic { () => ( $crate::panic!("explicit panic") @@ -162,6 +163,7 @@ macro_rules! assert_ne { /// ``` #[macro_export] #[stable(feature = "rust1", since = "1.0.0")] +#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "debug_assert_macro")] macro_rules! debug_assert { ($($arg:tt)*) => (if $crate::cfg!(debug_assertions) { $crate::assert!($($arg)*); }) } @@ -1215,6 +1217,8 @@ pub(crate) mod builtin { #[stable(feature = "rust1", since = "1.0.0")] #[rustc_builtin_macro] #[macro_export] + #[cfg_attr(not(bootstrap), rustc_diagnostic_item = "assert_macro")] + #[allow_internal_unstable(core_panic)] macro_rules! assert { ($cond:expr $(,)?) => {{ /* compiler built-in */ }}; ($cond:expr, $($arg:tt)+) => {{ /* compiler built-in */ }}; diff --git a/library/core/tests/nonzero.rs b/library/core/tests/nonzero.rs index ca449b4350ede..b66c482c5e5e3 100644 --- a/library/core/tests/nonzero.rs +++ b/library/core/tests/nonzero.rs @@ -85,7 +85,7 @@ fn test_match_option_string() { let five = "Five".to_string(); match Some(five) { Some(s) => assert_eq!(s, "Five"), - None => panic!("unexpected None while matching on Some(String { ... })"), + None => panic!("{}", "unexpected None while matching on Some(String { ... })"), } } diff --git a/library/std/src/macros.rs b/library/std/src/macros.rs index 57649d6f8f252..de072e83dfc41 100644 --- a/library/std/src/macros.rs +++ b/library/std/src/macros.rs @@ -8,6 +8,7 @@ #[macro_export] #[stable(feature = "rust1", since = "1.0.0")] #[allow_internal_unstable(libstd_sys_internals)] +#[cfg_attr(not(any(bootstrap, test)), rustc_diagnostic_item = "std_panic_macro")] macro_rules! panic { () => ({ $crate::panic!("explicit panic") }); ($msg:expr $(,)?) => ({ $crate::rt::begin_panic($msg) }); diff --git a/src/test/mir-opt/inst_combine_deref.do_not_miscompile.InstCombine.diff b/src/test/mir-opt/inst_combine_deref.do_not_miscompile.InstCombine.diff index 23c18bde2262b..dac9ec3b443d7 100644 --- a/src/test/mir-opt/inst_combine_deref.do_not_miscompile.InstCombine.diff +++ b/src/test/mir-opt/inst_combine_deref.do_not_miscompile.InstCombine.diff @@ -10,7 +10,7 @@ let mut _8: bool; // in scope 0 at $DIR/inst_combine_deref.rs:60:5: 60:23 let mut _9: bool; // in scope 0 at $DIR/inst_combine_deref.rs:60:13: 60:21 let mut _10: i32; // in scope 0 at $DIR/inst_combine_deref.rs:60:13: 60:15 - let mut _11: !; // in scope 0 at $SRC_DIR/std/src/macros.rs:LL:COL + let mut _11: !; // in scope 0 at $DIR/inst_combine_deref.rs:60:5: 60:23 scope 1 { debug x => _1; // in scope 1 at $DIR/inst_combine_deref.rs:55:9: 55:10 let _2: i32; // in scope 1 at $DIR/inst_combine_deref.rs:56:9: 56:10 @@ -69,11 +69,11 @@ } bb2: { - StorageLive(_11); // scope 4 at $SRC_DIR/std/src/macros.rs:LL:COL - begin_panic::<&str>(const "assertion failed: *y == 99"); // scope 4 at $SRC_DIR/std/src/macros.rs:LL:COL + StorageLive(_11); // scope 4 at $DIR/inst_combine_deref.rs:60:5: 60:23 + core::panicking::panic(const "assertion failed: *y == 99"); // scope 4 at $DIR/inst_combine_deref.rs:60:5: 60:23 // mir::Constant - // + span: $SRC_DIR/std/src/macros.rs:LL:COL - // + literal: Const { ty: fn(&str) -> ! {std::rt::begin_panic::<&str>}, val: Value(Scalar()) } + // + span: $DIR/inst_combine_deref.rs:60:5: 60:23 + // + literal: Const { ty: fn(&'static str) -> ! {core::panicking::panic}, val: Value(Scalar()) } // ty::Const // + ty: &str // + val: Value(Slice { data: Allocation { bytes: [97, 115, 115, 101, 114, 116, 105, 111, 110, 32, 102, 97, 105, 108, 101, 100, 58, 32, 42, 121, 32, 61, 61, 32, 57, 57], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [67108863], len: Size { raw: 26 } }, size: Size { raw: 26 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 26 }) diff --git a/src/test/ui/auxiliary/fancy-panic.rs b/src/test/ui/auxiliary/fancy-panic.rs new file mode 100644 index 0000000000000..e5a25a171fbe0 --- /dev/null +++ b/src/test/ui/auxiliary/fancy-panic.rs @@ -0,0 +1,6 @@ +#[macro_export] +macro_rules! fancy_panic { + ($msg:expr) => { + panic!($msg) + }; +} diff --git a/src/test/ui/fmt/format-args-capture.rs b/src/test/ui/fmt/format-args-capture.rs index 9348bb46dfe34..4e3fa9a3c589a 100644 --- a/src/test/ui/fmt/format-args-capture.rs +++ b/src/test/ui/fmt/format-args-capture.rs @@ -31,6 +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)] 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 32cf59294e760..e5e656de6fa7f 100644 --- a/src/test/ui/macros/macro-comma-behavior-rpass.rs +++ b/src/test/ui/macros/macro-comma-behavior-rpass.rs @@ -57,6 +57,7 @@ fn writeln_1arg() { // // (Example: Issue #48042) #[test] +#[allow(panic_fmt)] 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/panic-brace.rs new file mode 100644 index 0000000000000..754dcc287d0f9 --- /dev/null +++ b/src/test/ui/panic-brace.rs @@ -0,0 +1,31 @@ +// build-pass (FIXME(62277): should be check-pass) +// aux-build:fancy-panic.rs + +extern crate fancy_panic; + +const C: &str = "abc {}"; +static S: &str = "{bla}"; + +#[allow(unreachable_code)] +fn main() { + panic!("here's a brace: {"); //~ WARN panic message contains a brace + std::panic!("another one: }"); //~ WARN panic message contains a brace + core::panic!("Hello {}"); //~ WARN panic message contains an unused formatting placeholder + assert!(false, "{:03x} {test} bla"); + //~^ WARN panic message contains unused formatting placeholders + debug_assert!(false, "{{}} bla"); //~ WARN panic message contains braces + panic!(C); // No warning (yet) + panic!(S); // No warning (yet) + 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 + + // Check that the lint only triggers for std::panic and core::panic, + // not any panic macro: + macro_rules! panic { + ($e:expr) => (); + } + panic!("{}"); // OK +} diff --git a/src/test/ui/panic-brace.stderr b/src/test/ui/panic-brace.stderr new file mode 100644 index 0000000000000..93808891c3c37 --- /dev/null +++ b/src/test/ui/panic-brace.stderr @@ -0,0 +1,107 @@ +warning: panic message contains a brace + --> $DIR/panic-brace.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 +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 + | +LL | std::panic!("another one: }"); + | ^ + | + = note: this message is not used as a format string, but will be in a future Rust edition +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 + | +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 +help: add the missing argument + | +LL | core::panic!("Hello {}", ...); + | ^^^^^ +help: or add a "{}" format string to use the message literally + | +LL | core::panic!("{}", "Hello {}"); + | ^^^^^ + +warning: panic message contains unused formatting placeholders + --> $DIR/panic-brace.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 +help: add the missing arguments + | +LL | assert!(false, "{:03x} {test} bla", ...); + | ^^^^^ +help: or add a "{}" format string to use the message literally + | +LL | assert!(false, "{}", "{:03x} {test} bla"); + | ^^^^^ + +warning: panic message contains braces + --> $DIR/panic-brace.rs:16:27 + | +LL | debug_assert!(false, "{{}} bla"); + | ^^^^ + | + = note: this message is not used as a format string, but will be in a future Rust edition +help: add a "{}" format string to use the message literally + | +LL | debug_assert!(false, "{}", "{{}} bla"); + | ^^^^^ + +warning: panic message contains an unused formatting placeholder + --> $DIR/panic-brace.rs:19: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 +help: add the missing argument + | +LL | panic!(concat!("{", "}"), ...); + | ^^^^^ +help: or add a "{}" format string to use the message literally + | +LL | panic!("{}", concat!("{", "}")); + | ^^^^^ + +warning: panic message contains braces + --> $DIR/panic-brace.rs:20:5 + | +LL | panic!(concat!("{", "{")); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this message is not used as a format string, but will be in a future Rust edition +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 + | +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 + +warning: 8 warnings emitted + diff --git a/src/tools/clippy/clippy_lints/src/assertions_on_constants.rs b/src/tools/clippy/clippy_lints/src/assertions_on_constants.rs index a2ccb0369c4a4..a52f0997d439d 100644 --- a/src/tools/clippy/clippy_lints/src/assertions_on_constants.rs +++ b/src/tools/clippy/clippy_lints/src/assertions_on_constants.rs @@ -129,8 +129,11 @@ fn match_assert_with_message<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) if let ExprKind::Block(ref block, _) = arms[0].body.kind; if block.stmts.is_empty(); if let Some(block_expr) = &block.expr; - if let ExprKind::Block(ref inner_block, _) = block_expr.kind; - if let Some(begin_panic_call) = &inner_block.expr; + // inner block is optional. unwarp it if it exists, or use the expression as is otherwise. + if let Some(begin_panic_call) = match block_expr.kind { + ExprKind::Block(ref inner_block, _) => &inner_block.expr, + _ => &block.expr, + }; // function call if let Some(args) = match_panic_call(cx, begin_panic_call); if args.len() == 1; diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 126852df502eb..19bf67d80c428 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -788,7 +788,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL, &panic_in_result_fn::PANIC_IN_RESULT_FN, &panic_unimplemented::PANIC, - &panic_unimplemented::PANIC_PARAMS, &panic_unimplemented::TODO, &panic_unimplemented::UNIMPLEMENTED, &panic_unimplemented::UNREACHABLE, @@ -1499,7 +1498,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(&option_env_unwrap::OPTION_ENV_UNWRAP), LintId::of(&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL), - LintId::of(&panic_unimplemented::PANIC_PARAMS), LintId::of(&partialeq_ne_impl::PARTIALEQ_NE_IMPL), LintId::of(&precedence::PRECEDENCE), LintId::of(&ptr::CMP_NULL), @@ -1666,7 +1664,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST), LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES), - LintId::of(&panic_unimplemented::PANIC_PARAMS), LintId::of(&ptr::CMP_NULL), LintId::of(&ptr::PTR_ARG), LintId::of(&ptr_eq::PTR_EQ), diff --git a/src/tools/clippy/clippy_lints/src/panic_unimplemented.rs b/src/tools/clippy/clippy_lints/src/panic_unimplemented.rs index 3d888fe732573..8b10d07164712 100644 --- a/src/tools/clippy/clippy_lints/src/panic_unimplemented.rs +++ b/src/tools/clippy/clippy_lints/src/panic_unimplemented.rs @@ -1,30 +1,10 @@ -use crate::utils::{is_direct_expn_of, is_expn_of, match_panic_call, span_lint}; +use crate::utils::{is_expn_of, match_panic_call, span_lint}; use if_chain::if_chain; -use rustc_ast::ast::LitKind; -use rustc_hir::{Expr, ExprKind}; +use rustc_hir::Expr; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::Span; -declare_clippy_lint! { - /// **What it does:** Checks for missing parameters in `panic!`. - /// - /// **Why is this bad?** Contrary to the `format!` family of macros, there are - /// two forms of `panic!`: if there are no parameters given, the first argument - /// is not a format string and used literally. So while `format!("{}")` will - /// fail to compile, `panic!("{}")` will not. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```no_run - /// panic!("This `panic!` is probably missing a parameter there: {}"); - /// ``` - pub PANIC_PARAMS, - style, - "missing parameters in `panic!` calls" -} - declare_clippy_lint! { /// **What it does:** Checks for usage of `panic!`. /// @@ -89,11 +69,11 @@ declare_clippy_lint! { "`unreachable!` should not be present in production code" } -declare_lint_pass!(PanicUnimplemented => [PANIC_PARAMS, UNIMPLEMENTED, UNREACHABLE, TODO, PANIC]); +declare_lint_pass!(PanicUnimplemented => [UNIMPLEMENTED, UNREACHABLE, TODO, PANIC]); impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let Some(params) = match_panic_call(cx, expr) { + if let Some(_) = match_panic_call(cx, expr) { let span = get_outer_span(expr); if is_expn_of(expr.span, "unimplemented").is_some() { span_lint( @@ -113,7 +93,6 @@ impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented { ); } else if is_expn_of(expr.span, "panic").is_some() { span_lint(cx, PANIC, span, "`panic` should not be present in production code"); - match_panic(params, expr, cx); } } } @@ -132,20 +111,3 @@ fn get_outer_span(expr: &Expr<'_>) -> Span { } } } - -fn match_panic(params: &[Expr<'_>], expr: &Expr<'_>, cx: &LateContext<'_>) { - if_chain! { - if let ExprKind::Lit(ref lit) = params[0].kind; - if is_direct_expn_of(expr.span, "panic").is_some(); - if let LitKind::Str(ref string, _) = lit.node; - let string = string.as_str().replace("{{", "").replace("}}", ""); - if let Some(par) = string.find('{'); - if string[par..].contains('}'); - if params[0].span.source_callee().is_none(); - if params[0].span.lo() != params[0].span.hi(); - then { - span_lint(cx, PANIC_PARAMS, params[0].span, - "you probably are missing some parameter in your format string"); - } - } -} diff --git a/src/tools/clippy/tests/ui/panic.rs b/src/tools/clippy/tests/ui/panic.rs deleted file mode 100644 index 6e004aa9a924f..0000000000000 --- a/src/tools/clippy/tests/ui/panic.rs +++ /dev/null @@ -1,61 +0,0 @@ -#![warn(clippy::panic_params)] -#![allow(clippy::assertions_on_constants)] -fn missing() { - if true { - panic!("{}"); - } else if false { - panic!("{:?}"); - } else { - assert!(true, "here be missing values: {}"); - } - - panic!("{{{this}}}"); -} - -fn ok_single() { - panic!("foo bar"); -} - -fn ok_inner() { - // Test for #768 - assert!("foo bar".contains(&format!("foo {}", "bar"))); -} - -fn ok_multiple() { - panic!("{}", "This is {ok}"); -} - -fn ok_bracket() { - match 42 { - 1337 => panic!("{so is this"), - 666 => panic!("so is this}"), - _ => panic!("}so is that{"), - } -} - -const ONE: u32 = 1; - -fn ok_nomsg() { - assert!({ 1 == ONE }); - assert!(if 1 == ONE { ONE == 1 } else { false }); -} - -fn ok_escaped() { - panic!("{{ why should this not be ok? }}"); - panic!(" or {{ that ?"); - panic!(" or }} this ?"); - panic!(" {or {{ that ?"); - panic!(" }or }} this ?"); - panic!("{{ test }"); - panic!("{case }}"); -} - -fn main() { - missing(); - ok_single(); - ok_multiple(); - ok_bracket(); - ok_inner(); - ok_nomsg(); - ok_escaped(); -} diff --git a/src/tools/clippy/tests/ui/panic.stderr b/src/tools/clippy/tests/ui/panic.stderr deleted file mode 100644 index 1f8ff8ccf5575..0000000000000 --- a/src/tools/clippy/tests/ui/panic.stderr +++ /dev/null @@ -1,28 +0,0 @@ -error: you probably are missing some parameter in your format string - --> $DIR/panic.rs:5:16 - | -LL | panic!("{}"); - | ^^^^ - | - = note: `-D clippy::panic-params` implied by `-D warnings` - -error: you probably are missing some parameter in your format string - --> $DIR/panic.rs:7:16 - | -LL | panic!("{:?}"); - | ^^^^^^ - -error: you probably are missing some parameter in your format string - --> $DIR/panic.rs:9:23 - | -LL | assert!(true, "here be missing values: {}"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: you probably are missing some parameter in your format string - --> $DIR/panic.rs:12:12 - | -LL | panic!("{{{this}}}"); - | ^^^^^^^^^^^^ - -error: aborting due to 4 previous errors -