Skip to content

Commit 8282a3a

Browse files
committed
Fix problem in PANIC_PARAMS with inner format!
1 parent 251c3ee commit 8282a3a

File tree

3 files changed

+36
-13
lines changed

3 files changed

+36
-13
lines changed

src/panic.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use rustc::lint::*;
22
use rustc_front::hir::*;
33
use syntax::ast::LitKind;
4-
use utils::{span_lint, in_external_macro, match_path, BEGIN_UNWIND};
4+
use utils::{span_lint, is_direct_expn_of, match_path, BEGIN_UNWIND};
55

66
/// **What it does:** This lint checks for missing parameters in `panic!`.
77
///
@@ -28,24 +28,20 @@ impl LintPass for PanicPass {
2828
impl LateLintPass for PanicPass {
2929
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
3030
if_let_chain! {[
31-
in_external_macro(cx, expr.span),
3231
let ExprBlock(ref block) = expr.node,
3332
let Some(ref ex) = block.expr,
3433
let ExprCall(ref fun, ref params) = ex.node,
3534
params.len() == 2,
3635
let ExprPath(None, ref path) = fun.node,
3736
match_path(path, &BEGIN_UNWIND),
3837
let ExprLit(ref lit) = params[0].node,
38+
is_direct_expn_of(cx, params[0].span, "panic").is_some(),
3939
let LitKind::Str(ref string, _) = lit.node,
4040
let Some(par) = string.find('{'),
41-
string[par..].contains('}'),
42-
let Some(sp) = cx.sess().codemap()
43-
.with_expn_info(expr.span.expn_id,
44-
|info| info.map(|i| i.call_site))
41+
string[par..].contains('}')
4542
], {
46-
47-
span_lint(cx, PANIC_PARAMS, sp,
48-
"You probably are missing some parameter in your `panic!` call");
43+
span_lint(cx, PANIC_PARAMS, params[0].span,
44+
"you probably are missing some parameter in your format string");
4945
}}
5046
}
5147
}

src/utils/mod.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,7 @@ fn parse_attrs<F: FnMut(u64)>(sess: &Session, attrs: &[ast::Attribute], name: &'
604604
}
605605

606606
/// Return the pre-expansion span if is this comes from an expansion of the macro `name`.
607+
/// See also `is_direct_expn_of`.
607608
pub fn is_expn_of(cx: &LateContext, mut span: Span, name: &str) -> Option<Span> {
608609
loop {
609610
let span_name_span = cx.tcx
@@ -619,6 +620,25 @@ pub fn is_expn_of(cx: &LateContext, mut span: Span, name: &str) -> Option<Span>
619620
}
620621
}
621622

623+
/// Return the pre-expansion span if is this directly comes from an expansion of the macro `name`.
624+
/// The difference with `is_expn_of` is that in
625+
/// ```rust,ignore
626+
/// foo!(bar!(42));
627+
/// ```
628+
/// `42` is considered expanded from `foo!` and `bar!` by `is_expn_of` but only `bar!` by
629+
/// `is_direct_expn_of`.
630+
pub fn is_direct_expn_of(cx: &LateContext, span: Span, name: &str) -> Option<Span> {
631+
let span_name_span = cx.tcx
632+
.sess
633+
.codemap()
634+
.with_expn_info(span.expn_id, |expn| expn.map(|ei| (ei.callee.name(), ei.call_site)));
635+
636+
match span_name_span {
637+
Some((mac_name, new_span)) if mac_name.as_str() == name => Some(new_span),
638+
_ => None,
639+
}
640+
}
641+
622642
/// Returns index of character after first CamelCase component of `s`
623643
pub fn camel_case_until(s: &str) -> usize {
624644
let mut iter = s.char_indices();

tests/compile-fail/panic.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,32 @@
11
#![feature(plugin)]
22
#![plugin(clippy)]
33

4-
#[deny(panic_params)]
4+
#![deny(panic_params)]
55

66
fn missing() {
77
if true {
8-
panic!("{}"); //~ERROR: You probably are missing some parameter
8+
panic!("{}"); //~ERROR: you probably are missing some parameter
9+
} else if false {
10+
panic!("{:?}"); //~ERROR: you probably are missing some parameter
911
} else {
10-
panic!("{:?}"); //~ERROR: You probably are missing some parameter
12+
assert!(true, "here be missing values: {}"); //~ERROR you probably are missing some parameter
1113
}
1214
}
1315

1416
fn ok_single() {
1517
panic!("foo bar");
1618
}
1719

20+
fn ok_inner() {
21+
// Test for #768
22+
assert!("foo bar".contains(&format!("foo {}", "bar")));
23+
}
24+
1825
fn ok_multiple() {
1926
panic!("{}", "This is {ok}");
2027
}
2128

2229
fn ok_bracket() {
23-
// the match is just here because of #759, it serves no other purpose for the lint
2430
match 42 {
2531
1337 => panic!("{so is this"),
2632
666 => panic!("so is this}"),
@@ -33,4 +39,5 @@ fn main() {
3339
ok_single();
3440
ok_multiple();
3541
ok_bracket();
42+
ok_inner();
3643
}

0 commit comments

Comments
 (0)