Parse the format string for the panic_fmt lint for better warnings.

This commit is contained in:
Mara Bos 2020-10-20 22:25:42 +02:00
parent 0f193d1a62
commit 6b44662669
5 changed files with 47 additions and 21 deletions

View File

@ -3831,6 +3831,7 @@ dependencies = [
"rustc_hir", "rustc_hir",
"rustc_index", "rustc_index",
"rustc_middle", "rustc_middle",
"rustc_parse_format",
"rustc_session", "rustc_session",
"rustc_span", "rustc_span",
"rustc_target", "rustc_target",

View File

@ -20,3 +20,4 @@ rustc_feature = { path = "../rustc_feature" }
rustc_index = { path = "../rustc_index" } rustc_index = { path = "../rustc_index" }
rustc_session = { path = "../rustc_session" } rustc_session = { path = "../rustc_session" }
rustc_trait_selection = { path = "../rustc_trait_selection" } rustc_trait_selection = { path = "../rustc_trait_selection" }
rustc_parse_format = { path = "../rustc_parse_format" }

View File

@ -3,6 +3,7 @@
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_middle::ty; use rustc_middle::ty;
use rustc_parse_format::{ParseMode, Parser, Piece};
use rustc_span::sym; use rustc_span::sym;
declare_lint! { 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) if cx.tcx.is_diagnostic_item(sym::std_panic_macro, id)
|| cx.tcx.is_diagnostic_item(sym::core_panic_macro, id) || cx.tcx.is_diagnostic_item(sym::core_panic_macro, id)
{ {
let s = sym.as_str(); let fmt = sym.as_str();
if !s.contains(&['{', '}'][..]) { if !fmt.contains(&['{', '}'][..]) {
return; return;
} }
let s = s.replace("{{", "").replace("}}", "");
let looks_like_placeholder = let fmt_span = arg.span.source_callsite();
s.find('{').map_or(false, |i| s[i + 1..].contains('}'));
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!() // Unwrap another level of macro expansion if this panic!()
// was expanded from assert!() or debug_assert!(). // was expanded from assert!() or debug_assert!().
for &assert in &[sym::assert_macro, sym::debug_assert_macro] { 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; expn = parent;
} }
} }
if looks_like_placeholder {
cx.struct_span_lint(PANIC_FMT, arg.span.source_callsite(), |lint| { if n_arguments > 0 && fmt_parser.errors.is_empty() {
let mut l = lint.build("panic message contains an unused formatting placeholder"); 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"); 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) { if expn.call_site.contains(arg.span) {
l.span_suggestion( l.span_suggestion(
arg.span.shrink_to_hi(), arg.span.shrink_to_hi(),
"add the missing argument(s)", "add the missing argument(s)",
", argument".into(), ", ...".into(),
Applicability::HasPlaceholders, Applicability::HasPlaceholders,
); );
l.span_suggestion( l.span_suggestion(

View File

@ -5,6 +5,6 @@ fn main() {
panic!("here's a brace: {"); //~ WARN panic message contains a brace panic!("here's a brace: {"); //~ WARN panic message contains a brace
std::panic!("another one: }"); //~ 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 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 debug_assert!(false, "{{}} bla"); //~ WARN panic message contains a brace
} }

View File

@ -24,35 +24,35 @@ LL | std::panic!("{}", "another one: }");
| ^^^^^ | ^^^^^
warning: panic message contains an unused formatting placeholder warning: panic message contains an unused formatting placeholder
--> $DIR/panic-brace.rs:7:18 --> $DIR/panic-brace.rs:7:25
| |
LL | core::panic!("Hello {}"); 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 = 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) 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 help: or add a "{}" format string to use the message literally
| |
LL | core::panic!("{}", "Hello {}"); LL | core::panic!("{}", "Hello {}");
| ^^^^^ | ^^^^^
warning: panic message contains an unused formatting placeholder warning: panic message contains unused formatting placeholders
--> $DIR/panic-brace.rs:8:20 --> $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 = 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) 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 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 warning: panic message contains a brace