Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve non_fmt_panic lint. #82113

Merged
merged 9 commits into from
Feb 23, 2021
31 changes: 30 additions & 1 deletion compiler/rustc_lint/src/non_fmt_panic.rs
Original file line number Diff line number Diff line change
@@ -72,18 +72,38 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
// Find the span of the argument to `panic!()`, before expansion in the
// case of `panic!(some_macro!())`.
let mut arg_span = arg.span;
let mut arg_macro = None;
while !span.contains(arg_span) {
let expn = arg_span.ctxt().outer_expn_data();
if expn.is_root() {
break;
}
arg_macro = expn.macro_def_id;
arg_span = expn.call_site;
}

cx.struct_span_lint(NON_FMT_PANIC, arg_span, |lint| {
let mut l = lint.build("panic message is not a string literal");
l.note("this is no longer accepted in Rust 2021");
if span.contains(arg_span) {
if !span.contains(arg_span) {
// No clue where this argument is coming from.
l.emit();
return;
}
if arg_macro.map_or(false, |id| cx.tcx.is_diagnostic_item(sym::format_macro, id)) {
// A case of `panic!(format!(..))`.
l.note("the panic!() macro supports formatting, so there's no need for the format!() macro here");
if let Some(inner) = find_inner_span(cx, arg_span) {
l.multipart_suggestion(
"remove the `format!(..)` macro call",
vec![
(arg_span.until(inner), "".into()),
(inner.between(arg_span.shrink_to_hi()), "".into()),
],
Applicability::MachineApplicable,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this doesn't actually work due to a bug in cargo: rust-lang/rustfix#141

);
}
} else {
l.span_suggestion_verbose(
arg_span.shrink_to_lo(),
"add a \"{}\" format string to Display the message",
@@ -186,6 +206,15 @@ fn check_panic_str<'tcx>(
}
}

/// Given the span of `some_macro!(args)`, gives the span of `args`.
fn find_inner_span<'tcx>(cx: &LateContext<'tcx>, span: Span) -> Option<Span> {
let snippet = cx.sess().parse_sess.source_map().span_to_snippet(span).ok()?;
Some(span.from_inner(InnerSpan {
start: snippet.find(&['(', '{', '['][..])? + 1,
end: snippet.rfind(&[')', '}', ']'][..])?,
}))
}

fn panic_call<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>) -> (Span, Symbol) {
let mut expn = f.span.ctxt().outer_expn_data();

1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
@@ -554,6 +554,7 @@ symbols! {
format_args,
format_args_capture,
format_args_nl,
format_macro,
freeze,
freg,
frem_fast,
1 change: 1 addition & 0 deletions library/alloc/src/macros.rs
Original file line number Diff line number Diff line change
@@ -107,6 +107,7 @@ macro_rules! vec {
/// ```
#[macro_export]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_diagnostic_item = "format_macro"]
macro_rules! format {
($($arg:tt)*) => {{
let res = $crate::fmt::format($crate::__export::format_args!($($arg)*));