bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Add lint for panic!(123) which is not accepted in Rust 2021.

Browse source 
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.
This commit is contained in:
Mara Bos 2021-02-01 22:30:09 +01:00
parent 120b2a704a
commit a616f8267e
7 changed files with 310 additions and 180 deletions
Download patch file Download diff file Expand all files Collapse all files

6
compiler/rustc_lint/src/lib.rs
View file

@ -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,
]
);
};

197
compiler/rustc_lint/src/non_fmt_panic.rs Normal file
View file

@ -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<Vec<_>> =
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)
}

155
compiler/rustc_lint/src/panic_fmt.rs
View file

@ -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<Vec<_>> = 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();
});
}
}
}
}
}
}

2
src/test/ui/fmt/format-args-capture.rs
View file

@ -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();

2
src/test/ui/macros/macro-comma-behavior-rpass.rs
View file

@ -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.

12
src/test/ui/panic-brace.rs → src/test/ui/non-fmt-panic.rs
View file

@ -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
}

116
src/test/ui/panic-brace.stderr → src/test/ui/non-fmt-panic.stderr
View file

@ -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: 8 warnings emitted
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: 14 warnings emitted