From 6b44662669cf1680fe097e593eae20ca5dbed2ee Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 20 Oct 2020 22:25:42 +0200 Subject: [PATCH] Parse the format string for the panic_fmt lint for better warnings. --- Cargo.lock | 1 + compiler/rustc_lint/Cargo.toml | 1 + compiler/rustc_lint/src/panic_fmt.rs | 42 ++++++++++++++++++++++------ src/test/ui/panic-brace.rs | 2 +- src/test/ui/panic-brace.stderr | 22 +++++++-------- 5 files changed, 47 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 928d19b1e2c..4c84e7bd8c9 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_lint/Cargo.toml b/compiler/rustc_lint/Cargo.toml index 760a8e385d6..c56eb09b634 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/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs index 288e1d61bbf..cff50ff9912 100644 --- a/compiler/rustc_lint/src/panic_fmt.rs +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -3,6 +3,7 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_middle::ty; +use rustc_parse_format::{ParseMode, Parser, Piece}; use rustc_span::sym; declare_lint! { @@ -52,13 +53,28 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc if cx.tcx.is_diagnostic_item(sym::std_panic_macro, id) || cx.tcx.is_diagnostic_item(sym::core_panic_macro, id) { - let s = sym.as_str(); - if !s.contains(&['{', '}'][..]) { + let fmt = sym.as_str(); + if !fmt.contains(&['{', '}'][..]) { return; } - let s = s.replace("{{", "").replace("}}", ""); - let looks_like_placeholder = - s.find('{').map_or(false, |i| s[i + 1..].contains('}')); + + 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, 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] { @@ -70,15 +86,23 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc expn = parent; } } - if looks_like_placeholder { - cx.struct_span_lint(PANIC_FMT, arg.span.source_callsite(), |lint| { - let mut l = lint.build("panic message contains an unused formatting placeholder"); + + 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 version"); if expn.call_site.contains(arg.span) { l.span_suggestion( arg.span.shrink_to_hi(), "add the missing argument(s)", - ", argument".into(), + ", ...".into(), Applicability::HasPlaceholders, ); l.span_suggestion( diff --git a/src/test/ui/panic-brace.rs b/src/test/ui/panic-brace.rs index 6ab5fafee88..e74b6ad96c2 100644 --- a/src/test/ui/panic-brace.rs +++ b/src/test/ui/panic-brace.rs @@ -5,6 +5,6 @@ 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} bla"); //~ 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 a brace } diff --git a/src/test/ui/panic-brace.stderr b/src/test/ui/panic-brace.stderr index 00b005a59d8..23ae31d00eb 100644 --- a/src/test/ui/panic-brace.stderr +++ b/src/test/ui/panic-brace.stderr @@ -24,35 +24,35 @@ LL | std::panic!("{}", "another one: }"); | ^^^^^ warning: panic message contains an unused formatting placeholder - --> $DIR/panic-brace.rs:7:18 + --> $DIR/panic-brace.rs:7: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 version help: add the missing argument(s) | -LL | core::panic!("Hello {}", argument); - | ^^^^^^^^^^ +LL | core::panic!("Hello {}", ...); + | ^^^^^ help: or add a "{}" format string to use the message literally | LL | core::panic!("{}", "Hello {}"); | ^^^^^ -warning: panic message contains an unused formatting placeholder - --> $DIR/panic-brace.rs:8:20 +warning: panic message contains unused formatting placeholders + --> $DIR/panic-brace.rs:8:21 | -LL | assert!(false, "{:03x} bla"); - | ^^^^^^^^^^^^ +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 version help: add the missing argument(s) | -LL | assert!(false, "{:03x} bla", argument); - | ^^^^^^^^^^ +LL | assert!(false, "{:03x} {test} bla", ...); + | ^^^^^ help: or add a "{}" format string to use the message literally | -LL | assert!(false, "{}", "{:03x} bla"); +LL | assert!(false, "{}", "{:03x} {test} bla"); | ^^^^^ warning: panic message contains a brace