diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 53143800f763..8e0794ec8e34 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -311,6 +311,7 @@ pub fn register_pre_expansion_lints( ); store.register_pre_expansion_pass(Some(session), true, false, box attrs::DeprecatedCfgAttribute); store.register_pre_expansion_pass(Some(session), true, false, box dbg_macro::DbgMacro); + store.register_pre_expansion_pass(Some(session), true, false, box panic_unimplemented::PanicUnimplemented); } #[doc(hidden)] @@ -487,7 +488,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { ); reg.register_late_lint_pass(box escape::BoxedLocal{too_large_for_stack: conf.too_large_for_stack}); reg.register_early_lint_pass(box misc_early::MiscEarlyLints); - reg.register_late_lint_pass(box panic_unimplemented::PanicUnimplemented); reg.register_late_lint_pass(box strings::StringLitAsBytes); reg.register_late_lint_pass(box derive::Derive); reg.register_late_lint_pass(box types::CharLitAsU8); diff --git a/clippy_lints/src/panic_unimplemented.rs b/clippy_lints/src/panic_unimplemented.rs index bee1ec46f553..2bfe267726d5 100644 --- a/clippy_lints/src/panic_unimplemented.rs +++ b/clippy_lints/src/panic_unimplemented.rs @@ -1,11 +1,9 @@ -use crate::utils::{is_direct_expn_of, is_expn_of, match_def_path, paths, resolve_node, span_lint}; +use crate::utils::span_lint; use if_chain::if_chain; -use rustc::hir::*; -use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass}; use rustc::{declare_lint_pass, declare_tool_lint}; -use syntax::ast::LitKind; -use syntax::ptr::P; -use syntax_pos::Span; +use syntax::ast; +use syntax::parse::{parser, token}; declare_clippy_lint! { /// **What it does:** Checks for missing parameters in `panic!`. @@ -44,54 +42,41 @@ declare_clippy_lint! { declare_lint_pass!(PanicUnimplemented => [PANIC_PARAMS, UNIMPLEMENTED]); -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PanicUnimplemented { - fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { - if_chain! { - if let ExprKind::Block(ref block, _) = expr.node; - if let Some(ref ex) = block.expr; - if let ExprKind::Call(ref fun, ref params) = ex.node; - if let ExprKind::Path(ref qpath) = fun.node; - if let Some(fun_def_id) = resolve_node(cx, qpath, fun.hir_id).opt_def_id(); - if match_def_path(cx, fun_def_id, &paths::BEGIN_PANIC); - if params.len() == 2; - then { - if is_expn_of(expr.span, "unimplemented").is_some() { - let span = get_outer_span(expr); - span_lint(cx, UNIMPLEMENTED, span, - "`unimplemented` should not be present in production code"); - } else { - match_panic(params, expr, cx); +impl EarlyLintPass for PanicUnimplemented { + fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &ast::Mac) { + if mac.node.path == sym!(unimplemented) { + span_lint( + cx, + UNIMPLEMENTED, + mac.span, + "`unimplemented` should not be present in production code", + ); + } else if mac.node.path == sym!(panic) || mac.node.path == sym!(assert) { + let tts = mac.node.tts.clone(); + let mut parser = parser::Parser::new(&cx.sess.parse_sess, tts, None, false, false, None); + + if mac.node.path == sym!(assert) { + if parser.parse_expr().map_err(|mut err| err.cancel()).is_err() { + return; } - } - } - } -} -fn get_outer_span(expr: &Expr) -> Span { - if_chain! { - if let Some(first) = expr.span.ctxt().outer_expn_info(); - if let Some(second) = first.call_site.ctxt().outer_expn_info(); - then { - second.call_site - } else { - expr.span - } - } -} + if parser.expect(&token::Comma).map_err(|mut err| err.cancel()).is_err() { + return; + } + } -fn match_panic(params: &P<[Expr]>, expr: &Expr, cx: &LateContext<'_, '_>) { - if_chain! { - if let ExprKind::Lit(ref lit) = params[0].node; - 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, + if_chain! { + if let Ok((string, _)) = parser.parse_str(); + let span = parser.prev_span; + let string = string.as_str().replace("{{", "").replace("}}", ""); + if let Some(par) = string.find('{'); + if string[par..].contains('}'); + if parser.expect(&token::Comma).map_err(|mut err| err.cancel()).is_err(); + then { + span_lint(cx, PANIC_PARAMS, span, "you probably are missing some parameter in your format string"); + } + } } } } diff --git a/tests/ui/panic_unimplemented.rs b/tests/ui/panic_unimplemented.rs index 92290da8a6ac..5925c4f17681 100644 --- a/tests/ui/panic_unimplemented.rs +++ b/tests/ui/panic_unimplemented.rs @@ -50,10 +50,18 @@ fn ok_escaped() { panic!("{case }}"); } +macro_rules! outer_macro { + ($e:expr) => { + $e; + }; +} + fn unimplemented() { let a = 2; unimplemented!(); let b = a + 2; + unimplemented!("foo"); + outer_macro!(unimplemented!()); } fn main() { diff --git a/tests/ui/panic_unimplemented.stderr b/tests/ui/panic_unimplemented.stderr index 588fa187b4ab..6b2720db2996 100644 --- a/tests/ui/panic_unimplemented.stderr +++ b/tests/ui/panic_unimplemented.stderr @@ -25,12 +25,18 @@ LL | panic!("{{{this}}}"); | ^^^^^^^^^^^^ error: `unimplemented` should not be present in production code - --> $DIR/panic_unimplemented.rs:55:5 + --> $DIR/panic_unimplemented.rs:61:5 | LL | unimplemented!(); - | ^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ | = note: `-D clippy::unimplemented` implied by `-D warnings` -error: aborting due to 5 previous errors +error: `unimplemented` should not be present in production code + --> $DIR/panic_unimplemented.rs:63:5 + | +LL | unimplemented!("foo"); + | ^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors