diff --git a/CHANGELOG.md b/CHANGELOG.md index 3deece422..9e233837b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -815,6 +815,7 @@ All notable changes to this project will be documented in this file. [`trivial_regex`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#trivial_regex [`type_complexity`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#type_complexity [`unicode_not_nfc`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unicode_not_nfc +[`unimplemented`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unimplemented [`unit_arg`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unit_arg [`unit_cmp`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unit_cmp [`unnecessary_cast`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unnecessary_cast diff --git a/README.md b/README.md index 9472156ec..506615014 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 258 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html) +[There are 259 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 7864e90c1..aaf2ef573 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -177,7 +177,7 @@ pub mod non_expressive_names; pub mod ok_if_let; pub mod open_options; pub mod overflow_check_conditional; -pub mod panic; +pub mod panic_unimplemented; pub mod partialeq_ne_impl; pub mod precedence; pub mod ptr; @@ -352,7 +352,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box escape::Pass{too_large_for_stack: conf.too_large_for_stack}); reg.register_early_lint_pass(box misc_early::MiscEarly); reg.register_late_lint_pass(box array_indexing::ArrayIndexing); - reg.register_late_lint_pass(box panic::Pass); + reg.register_late_lint_pass(box panic_unimplemented::Pass); reg.register_late_lint_pass(box strings::StringLitAsBytes); reg.register_late_lint_pass(box derive::Derive); reg.register_late_lint_pass(box types::CharLitAsU8); @@ -434,6 +434,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { methods::WRONG_PUB_SELF_CONVENTION, misc::FLOAT_CMP_CONST, missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS, + panic_unimplemented::UNIMPLEMENTED, shadow::SHADOW_REUSE, shadow::SHADOW_SAME, shadow::SHADOW_UNRELATED, @@ -626,7 +627,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { ok_if_let::IF_LET_SOME_RESULT, open_options::NONSENSICAL_OPEN_OPTIONS, overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL, - panic::PANIC_PARAMS, + panic_unimplemented::PANIC_PARAMS, partialeq_ne_impl::PARTIALEQ_NE_IMPL, precedence::PRECEDENCE, ptr::CMP_NULL, @@ -748,7 +749,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { non_expressive_names::JUST_UNDERSCORES_AND_DIGITS, non_expressive_names::MANY_SINGLE_CHAR_NAMES, ok_if_let::IF_LET_SOME_RESULT, - panic::PANIC_PARAMS, + panic_unimplemented::PANIC_PARAMS, ptr::CMP_NULL, ptr::PTR_ARG, question_mark::QUESTION_MARK, diff --git a/clippy_lints/src/panic.rs b/clippy_lints/src/panic.rs deleted file mode 100644 index bd44b8d9b..000000000 --- a/clippy_lints/src/panic.rs +++ /dev/null @@ -1,58 +0,0 @@ -use rustc::hir::*; -use rustc::lint::*; -use syntax::ast::LitKind; -use utils::{is_direct_expn_of, match_def_path, opt_def_id, paths, resolve_node, span_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:** -/// ```rust -/// panic!("This `panic!` is probably missing a parameter there: {}"); -/// ``` -declare_clippy_lint! { - pub PANIC_PARAMS, - style, - "missing parameters in `panic!` calls" -} - -#[allow(missing_copy_implementations)] -pub struct Pass; - -impl LintPass for Pass { - fn get_lints(&self) -> LintArray { - lint_array!(PANIC_PARAMS) - } -} - -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { - fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { - if_chain! { - if let ExprBlock(ref block, _) = expr.node; - if let Some(ref ex) = block.expr; - if let ExprCall(ref fun, ref params) = ex.node; - if params.len() == 2; - if let ExprPath(ref qpath) = fun.node; - if let Some(fun_def_id) = opt_def_id(resolve_node(cx, qpath, fun.hir_id)); - if match_def_path(cx.tcx, fun_def_id, &paths::BEGIN_PANIC); - if let ExprLit(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, - "you probably are missing some parameter in your format string"); - } - } - } -} diff --git a/clippy_lints/src/panic_unimplemented.rs b/clippy_lints/src/panic_unimplemented.rs new file mode 100644 index 000000000..b257f5b3b --- /dev/null +++ b/clippy_lints/src/panic_unimplemented.rs @@ -0,0 +1,102 @@ +use rustc::hir::*; +use rustc::lint::*; +use syntax::ast::LitKind; +use syntax::ptr::P; +use syntax::ext::quote::rt::Span; +use utils::{is_direct_expn_of, is_expn_of, match_def_path, opt_def_id, paths, resolve_node, span_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:** +/// ```rust +/// panic!("This `panic!` is probably missing a parameter there: {}"); +/// ``` +declare_clippy_lint! { + pub PANIC_PARAMS, + style, + "missing parameters in `panic!` calls" +} + +/// **What it does:** Checks for usage of `unimplemented!`. +/// +/// **Why is this bad?** This macro should not be present in production code +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// unimplemented!(); +/// ``` +declare_clippy_lint! { + pub UNIMPLEMENTED, + restriction, + "`unimplemented!` should not be present in production code" +} + +#[allow(missing_copy_implementations)] +pub struct Pass; + +impl LintPass for Pass { + fn get_lints(&self) -> LintArray { + lint_array!(PANIC_PARAMS, UNIMPLEMENTED) + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { + if_chain! { + if let ExprBlock(ref block, _) = expr.node; + if let Some(ref ex) = block.expr; + if let ExprCall(ref fun, ref params) = ex.node; + if let ExprPath(ref qpath) = fun.node; + if let Some(fun_def_id) = opt_def_id(resolve_node(cx, qpath, fun.hir_id)); + if match_def_path(cx.tcx, 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); + } + } + } + } +} + +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 + } + } +} + +fn match_panic(params: &P<[Expr]>, expr: &Expr, cx: &LateContext) { + if_chain! { + if let ExprLit(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, + "you probably are missing some parameter in your format string"); + } + } +} diff --git a/tests/ui/panic.rs b/tests/ui/panic_unimplemented.rs similarity index 88% rename from tests/ui/panic.rs rename to tests/ui/panic_unimplemented.rs index d833d2651..33050633f 100644 --- a/tests/ui/panic.rs +++ b/tests/ui/panic_unimplemented.rs @@ -1,7 +1,7 @@ -#![warn(panic_params)] +#![warn(panic_params, unimplemented)] fn missing() { if true { @@ -53,6 +53,12 @@ fn ok_escaped() { panic!("{case }}"); } +fn unimplemented() { + let a = 2; + unimplemented!(); + let b = a + 2; +} + fn main() { missing(); ok_single(); @@ -61,4 +67,5 @@ fn main() { ok_inner(); ok_nomsg(); ok_escaped(); + unimplemented(); } diff --git a/tests/ui/panic.stderr b/tests/ui/panic_unimplemented.stderr similarity index 60% rename from tests/ui/panic.stderr rename to tests/ui/panic_unimplemented.stderr index 165c33cac..3bf5589c4 100644 --- a/tests/ui/panic.stderr +++ b/tests/ui/panic_unimplemented.stderr @@ -1,5 +1,5 @@ error: you probably are missing some parameter in your format string - --> $DIR/panic.rs:8:16 + --> $DIR/panic_unimplemented.rs:8:16 | 8 | panic!("{}"); | ^^^^ @@ -7,22 +7,30 @@ error: you probably are missing some parameter in your format string = note: `-D panic-params` implied by `-D warnings` error: you probably are missing some parameter in your format string - --> $DIR/panic.rs:10:16 + --> $DIR/panic_unimplemented.rs:10:16 | 10 | panic!("{:?}"); | ^^^^^^ error: you probably are missing some parameter in your format string - --> $DIR/panic.rs:12:23 + --> $DIR/panic_unimplemented.rs:12:23 | 12 | assert!(true, "here be missing values: {}"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: you probably are missing some parameter in your format string - --> $DIR/panic.rs:15:12 + --> $DIR/panic_unimplemented.rs:15:12 | 15 | panic!("{{{this}}}"); | ^^^^^^^^^^^^ -error: aborting due to 4 previous errors +error: `unimplemented` should not be present in production code + --> $DIR/panic_unimplemented.rs:58:5 + | +58 | unimplemented!(); + | ^^^^^^^^^^^^^^^^^ + | + = note: `-D unimplemented` implied by `-D warnings` + +error: aborting due to 5 previous errors