Fix problem in PANIC_PARAMS with inner `format!`

diff --git a/src/panic.rs b/src/panic.rs
index 7dbcf2a5b30f0cfce336444d8968bb3cc72e02a6..8b9bf9f1f1976ea0f46674e7f50ba3d764035b0e 100644 (file)
--- a/src/panic.rs
+++ b/src/panic.rs
@@ -1,7 +1,7 @@
 use rustc::lint::*;
 use rustc_front::hir::*;
 use syntax::ast::LitKind;
-use utils::{span_lint, in_external_macro, match_path, BEGIN_UNWIND};
+use utils::{span_lint, is_direct_expn_of, match_path, BEGIN_UNWIND};
 
 /// **What it does:** This lint checks for missing parameters in `panic!`.
 ///
@@ -28,7 +28,6 @@ fn get_lints(&self) -> LintArray {
 impl LateLintPass for PanicPass {
     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
         if_let_chain! {[
-            in_external_macro(cx, expr.span),
             let ExprBlock(ref block) = expr.node,
             let Some(ref ex) = block.expr,
             let ExprCall(ref fun, ref params) = ex.node,
@@ -36,16 +35,13 @@ fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
             let ExprPath(None, ref path) = fun.node,
             match_path(path, &BEGIN_UNWIND),
             let ExprLit(ref lit) = params[0].node,
+            is_direct_expn_of(cx, params[0].span, "panic").is_some(),
             let LitKind::Str(ref string, _) = lit.node,
             let Some(par) = string.find('{'),
-            string[par..].contains('}'),
-            let Some(sp) = cx.sess().codemap()
-                             .with_expn_info(expr.span.expn_id,
-                                             |info| info.map(|i| i.call_site))
+            string[par..].contains('}')
         ], {
-
-            span_lint(cx, PANIC_PARAMS, sp,
-                      "You probably are missing some parameter in your `panic!` call");
+            span_lint(cx, PANIC_PARAMS, params[0].span,
+                      "you probably are missing some parameter in your format string");
         }}
     }
 }

diff --git a/src/utils/mod.rs b/src/utils/mod.rs
index 86a5e24efc2e42f7980a75248758d14185bde7ad..3fb52318b6f8ba1ff3b53ad68b092f0e11e54f1e 100644 (file)
--- a/src/utils/mod.rs
+++ b/src/utils/mod.rs
@@ -604,6 +604,7 @@ fn parse_attrs<F: FnMut(u64)>(sess: &Session, attrs: &[ast::Attribute], name: &'
 }
 
 /// Return the pre-expansion span if is this comes from an expansion of the macro `name`.
+/// See also `is_direct_expn_of`.
 pub fn is_expn_of(cx: &LateContext, mut span: Span, name: &str) -> Option<Span> {
     loop {
         let span_name_span = cx.tcx
@@ -619,6 +620,25 @@ pub fn is_expn_of(cx: &LateContext, mut span: Span, name: &str) -> Option<Span>
     }
 }
 
+/// Return the pre-expansion span if is this directly comes from an expansion of the macro `name`.
+/// The difference with `is_expn_of` is that in
+/// ```rust,ignore
+/// foo!(bar!(42));
+/// ```
+/// `42` is considered expanded from `foo!` and `bar!` by `is_expn_of` but only `bar!` by
+/// `is_direct_expn_of`.
+pub fn is_direct_expn_of(cx: &LateContext, span: Span, name: &str) -> Option<Span> {
+    let span_name_span = cx.tcx
+                           .sess
+                           .codemap()
+                           .with_expn_info(span.expn_id, |expn| expn.map(|ei| (ei.callee.name(), ei.call_site)));
+
+    match span_name_span {
+        Some((mac_name, new_span)) if mac_name.as_str() == name => Some(new_span),
+        _ => None,
+    }
+}
+
 /// Returns index of character after first CamelCase component of `s`
 pub fn camel_case_until(s: &str) -> usize {
     let mut iter = s.char_indices();

diff --git a/tests/compile-fail/panic.rs b/tests/compile-fail/panic.rs
index 38fe5aa2c0f39702b97ebdea2e211f7ec8811e34..7e535d69b698ad504120b9c13468d93f28bedc34 100644 (file)
--- a/tests/compile-fail/panic.rs
+++ b/tests/compile-fail/panic.rs
@@ -1,13 +1,15 @@
 #![feature(plugin)]
 #![plugin(clippy)]
 
-#[deny(panic_params)]
+#![deny(panic_params)]
 
 fn missing() {
     if true {
-        panic!("{}"); //~ERROR: You probably are missing some parameter
+        panic!("{}"); //~ERROR: you probably are missing some parameter
+    } else if false {
+        panic!("{:?}"); //~ERROR: you probably are missing some parameter
     } else {
-        panic!("{:?}"); //~ERROR: You probably are missing some parameter
+        assert!(true, "here be missing values: {}"); //~ERROR you probably are missing some parameter
     }
 }
 
@@ -15,12 +17,16 @@ 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() {
-    // the match is just here because of #759, it serves no other purpose for the lint
     match 42 {
         1337 => panic!("{so is this"),
         666 => panic!("so is this}"),
@@ -33,4 +39,5 @@ fn main() {
     ok_single();
     ok_multiple();
     ok_bracket();
+    ok_inner();
 }