OpenE2K
/
rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #78088 - fusion-engineering-forks:panic-fmt-lint, r=estebank 

Add lint for panic!("{}")

This adds a lint that warns about `panic!("{}")`.

`panic!(msg)` invocations with a single argument use their argument as panic payload literally, without using it as a format string. The same holds for `assert!(expr, msg)`.

This lints checks if `msg` is a string literal (after expansion), and warns in case it contained braces. It suggests to insert `"{}", ` to use the message literally, or to add arguments to use it as a format string.

![image](https://user-images.githubusercontent.com/783247/96643867-79eb1080-1328-11eb-8d4e-a5586837c70a.png)

This lint is also a good starting point for adding warnings about `panic!(not_a_string)` later, once [`panic_any()`](https://github.com/rust-lang/rust/pull/74622) becomes a stable alternative.
This commit is contained in:
bors 2020-11-20 03:40:20 +00:00
parent 4ec27e4b79 a125ef2e8e
commit 74285eb3a8
22 changed files with 362 additions and 165 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Cargo.lock
View File

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

52
compiler/rustc_builtin_macros/src/assert.rs
View File

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

3
compiler/rustc_expand/src/mbe/macro_rules.rs
View File

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

1
compiler/rustc_lint/Cargo.toml
View File

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

3
compiler/rustc_lint/src/lib.rs
View File

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

150
compiler/rustc_lint/src/panic_fmt.rs Normal file
View File

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

4
compiler/rustc_passes/src/diagnostic_items.rs
View File

@ -113,6 +113,10 @@ fn collect<'tcx>(tcx: TyCtxt<'tcx>) -> FxHashMap<Symbol, DefId> {
}
}
for m in tcx.hir().krate().exported_macros {
collector.observe_item(m.attrs, m.hir_id);
}
collector.items
}

5
compiler/rustc_span/src/symbol.rs
View File

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

4
library/core/src/macros/mod.rs
View File

@ -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 */ }};

2
library/core/tests/nonzero.rs
View File

@ -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 { ... })"),
}
}

1
library/std/src/macros.rs
View File

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

10
src/test/mir-opt/inst_combine_deref.do_not_miscompile.InstCombine.diff
View File

@ -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(<ZST>)) }
// + span: $DIR/inst_combine_deref.rs:60:5: 60:23
// + literal: Const { ty: fn(&'static str) -> ! {core::panicking::panic}, val: Value(Scalar(<ZST>)) }
// 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 })

6
src/test/ui/auxiliary/fancy-panic.rs Normal file
View File

@ -0,0 +1,6 @@
#[macro_export]
macro_rules! fancy_panic {
($msg:expr) => {
panic!($msg)
};
}

1
src/test/ui/fmt/format-args-capture.rs
View File

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

1
src/test/ui/macros/macro-comma-behavior-rpass.rs
View File

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

31
src/test/ui/panic-brace.rs Normal file
View File

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

107
src/test/ui/panic-brace.stderr Normal file
View File

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

7
src/tools/clippy/clippy_lints/src/assertions_on_constants.rs
View File

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

3
src/tools/clippy/clippy_lints/src/lib.rs
View File

@ -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),

46
src/tools/clippy/clippy_lints/src/panic_unimplemented.rs
View File

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

61
src/tools/clippy/tests/ui/panic.rs
View File

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

28
src/tools/clippy/tests/ui/panic.stderr
View File

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