From 77794e91e219b85663cf693d35f677e564151420 Mon Sep 17 00:00:00 2001 From: "Michael A. Plikk" Date: Wed, 23 May 2018 16:43:05 +0200 Subject: [PATCH 1/4] Create lint for unimplemented!() --- CHANGELOG.md | 1 + README.md | 2 +- clippy_lints/src/lib.rs | 2 ++ clippy_lints/src/panic.rs | 56 ++++++++++++++++++++++++++++++--------- tests/ui/panic.rs | 7 ++++- tests/ui/panic.stderr | 11 +++++++- 6 files changed, 63 insertions(+), 16 deletions(-) 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..9dd9a364b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -627,6 +627,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { open_options::NONSENSICAL_OPEN_OPTIONS, overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL, panic::PANIC_PARAMS, + panic::UNIMPLEMENTED, partialeq_ne_impl::PARTIALEQ_NE_IMPL, precedence::PRECEDENCE, ptr::CMP_NULL, @@ -749,6 +750,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { non_expressive_names::MANY_SINGLE_CHAR_NAMES, ok_if_let::IF_LET_SOME_RESULT, panic::PANIC_PARAMS, + panic::UNIMPLEMENTED, ptr::CMP_NULL, ptr::PTR_ARG, question_mark::QUESTION_MARK, diff --git a/clippy_lints/src/panic.rs b/clippy_lints/src/panic.rs index bd44b8d9b..7f1b6775b 100644 --- a/clippy_lints/src/panic.rs +++ b/clippy_lints/src/panic.rs @@ -1,7 +1,8 @@ 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}; +use syntax::ptr::P; +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!`. /// @@ -22,12 +23,28 @@ declare_clippy_lint! { "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, + style, + "`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) + lint_array!(PANIC_PARAMS, UNIMPLEMENTED) } } @@ -37,22 +54,35 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { 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(); + if params.len() == 2; then { - span_lint(cx, PANIC_PARAMS, params[0].span, - "you probably are missing some parameter in your format string"); + if is_expn_of(expr.span, "unimplemented").is_some() { + span_lint(cx, UNIMPLEMENTED, expr.span, + "`unimplemented` should not be present in production code"); + } else { + match_panic(params, expr, cx); + } } } } } + +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.rs index d833d2651..56d06f239 100644 --- a/tests/ui/panic.rs +++ b/tests/ui/panic.rs @@ -1,7 +1,7 @@ -#![warn(panic_params)] +#![warn(panic_params, unimplemented)] fn missing() { if true { @@ -53,6 +53,10 @@ fn ok_escaped() { panic!("{case }}"); } +fn unimplemented() { + unimplemented!(); +} + fn main() { missing(); ok_single(); @@ -61,4 +65,5 @@ fn main() { ok_inner(); ok_nomsg(); ok_escaped(); + unimplemented(); } diff --git a/tests/ui/panic.stderr b/tests/ui/panic.stderr index 165c33cac..786a20c03 100644 --- a/tests/ui/panic.stderr +++ b/tests/ui/panic.stderr @@ -24,5 +24,14 @@ error: you probably are missing some parameter in your format string 15 | panic!("{{{this}}}"); | ^^^^^^^^^^^^ -error: aborting due to 4 previous errors +error: `unimplemented` should not be present in production code + --> $DIR/panic.rs:57:5 + | +57 | unimplemented!(); + | ^^^^^^^^^^^^^^^^^ + | + = note: `-D unimplemented` implied by `-D warnings` + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error: aborting due to 5 previous errors From 88c3c2f1c2b47862656c71973dc356422d03ba8b Mon Sep 17 00:00:00 2001 From: "Michael A. Plikk" Date: Thu, 24 May 2018 08:59:54 +0200 Subject: [PATCH 2/4] Rename panic files to panic_unimplemented --- clippy_lints/src/lib.rs | 12 ++++++------ .../src/{panic.rs => panic_unimplemented.rs} | 0 tests/ui/{panic.rs => panic_unimplemented.rs} | 0 .../ui/{panic.stderr => panic_unimplemented.stderr} | 10 +++++----- 4 files changed, 11 insertions(+), 11 deletions(-) rename clippy_lints/src/{panic.rs => panic_unimplemented.rs} (100%) rename tests/ui/{panic.rs => panic_unimplemented.rs} (100%) rename tests/ui/{panic.stderr => panic_unimplemented.stderr} (83%) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 9dd9a364b..1f9b2512d 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); @@ -626,8 +626,8 @@ 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_unimplemented::PANIC_PARAMS, + panic_unimplemented::UNIMPLEMENTED, partialeq_ne_impl::PARTIALEQ_NE_IMPL, precedence::PRECEDENCE, ptr::CMP_NULL, @@ -749,8 +749,8 @@ 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_unimplemented::PANIC_PARAMS, + panic_unimplemented::UNIMPLEMENTED, ptr::CMP_NULL, ptr::PTR_ARG, question_mark::QUESTION_MARK, diff --git a/clippy_lints/src/panic.rs b/clippy_lints/src/panic_unimplemented.rs similarity index 100% rename from clippy_lints/src/panic.rs rename to clippy_lints/src/panic_unimplemented.rs diff --git a/tests/ui/panic.rs b/tests/ui/panic_unimplemented.rs similarity index 100% rename from tests/ui/panic.rs rename to tests/ui/panic_unimplemented.rs diff --git a/tests/ui/panic.stderr b/tests/ui/panic_unimplemented.stderr similarity index 83% rename from tests/ui/panic.stderr rename to tests/ui/panic_unimplemented.stderr index 786a20c03..534bfbae8 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,25 +7,25 @@ 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: `unimplemented` should not be present in production code - --> $DIR/panic.rs:57:5 + --> $DIR/panic_unimplemented.rs:57:5 | 57 | unimplemented!(); | ^^^^^^^^^^^^^^^^^ From dc8d29be4ac405e61b6c403b0ea09809e55b8efa Mon Sep 17 00:00:00 2001 From: "Michael A. Plikk" Date: Thu, 24 May 2018 16:30:26 +0200 Subject: [PATCH 3/4] Allow unimplemented in other tests --- tests/run-pass/ice_exacte_size.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/run-pass/ice_exacte_size.rs b/tests/run-pass/ice_exacte_size.rs index 914153c64..d99734810 100644 --- a/tests/run-pass/ice_exacte_size.rs +++ b/tests/run-pass/ice_exacte_size.rs @@ -6,6 +6,7 @@ struct Foo; impl Iterator for Foo { type Item = (); + #[allow(unimplemented)] fn next(&mut self) -> Option<()> { let _ = self.len() == 0; unimplemented!() From 1f10dd26069d94488bbcb9669e4b60c66e295c60 Mon Sep 17 00:00:00 2001 From: "Michael A. Plikk" Date: Thu, 24 May 2018 19:26:04 +0200 Subject: [PATCH 4/4] Fix note on macro outside current crate. Changed group to restricted --- clippy_lints/src/lib.rs | 3 +-- clippy_lints/src/panic_unimplemented.rs | 18 ++++++++++++++++-- tests/run-pass/ice_exacte_size.rs | 1 - tests/ui/panic_unimplemented.rs | 2 ++ tests/ui/panic_unimplemented.stderr | 5 ++--- 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1f9b2512d..aaf2ef573 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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, @@ -627,7 +628,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { open_options::NONSENSICAL_OPEN_OPTIONS, overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL, panic_unimplemented::PANIC_PARAMS, - panic_unimplemented::UNIMPLEMENTED, partialeq_ne_impl::PARTIALEQ_NE_IMPL, precedence::PRECEDENCE, ptr::CMP_NULL, @@ -750,7 +750,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { non_expressive_names::MANY_SINGLE_CHAR_NAMES, ok_if_let::IF_LET_SOME_RESULT, panic_unimplemented::PANIC_PARAMS, - panic_unimplemented::UNIMPLEMENTED, ptr::CMP_NULL, ptr::PTR_ARG, question_mark::QUESTION_MARK, diff --git a/clippy_lints/src/panic_unimplemented.rs b/clippy_lints/src/panic_unimplemented.rs index 7f1b6775b..b257f5b3b 100644 --- a/clippy_lints/src/panic_unimplemented.rs +++ b/clippy_lints/src/panic_unimplemented.rs @@ -2,6 +2,7 @@ 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!`. @@ -35,7 +36,7 @@ declare_clippy_lint! { /// ``` declare_clippy_lint! { pub UNIMPLEMENTED, - style, + restriction, "`unimplemented!` should not be present in production code" } @@ -60,7 +61,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { if params.len() == 2; then { if is_expn_of(expr.span, "unimplemented").is_some() { - span_lint(cx, UNIMPLEMENTED, expr.span, + 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); @@ -70,6 +72,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } } +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; diff --git a/tests/run-pass/ice_exacte_size.rs b/tests/run-pass/ice_exacte_size.rs index d99734810..914153c64 100644 --- a/tests/run-pass/ice_exacte_size.rs +++ b/tests/run-pass/ice_exacte_size.rs @@ -6,7 +6,6 @@ struct Foo; impl Iterator for Foo { type Item = (); - #[allow(unimplemented)] fn next(&mut self) -> Option<()> { let _ = self.len() == 0; unimplemented!() diff --git a/tests/ui/panic_unimplemented.rs b/tests/ui/panic_unimplemented.rs index 56d06f239..33050633f 100644 --- a/tests/ui/panic_unimplemented.rs +++ b/tests/ui/panic_unimplemented.rs @@ -54,7 +54,9 @@ fn ok_escaped() { } fn unimplemented() { + let a = 2; unimplemented!(); + let b = a + 2; } fn main() { diff --git a/tests/ui/panic_unimplemented.stderr b/tests/ui/panic_unimplemented.stderr index 534bfbae8..3bf5589c4 100644 --- a/tests/ui/panic_unimplemented.stderr +++ b/tests/ui/panic_unimplemented.stderr @@ -25,13 +25,12 @@ error: you probably are missing some parameter in your format string | ^^^^^^^^^^^^ error: `unimplemented` should not be present in production code - --> $DIR/panic_unimplemented.rs:57:5 + --> $DIR/panic_unimplemented.rs:58:5 | -57 | unimplemented!(); +58 | unimplemented!(); | ^^^^^^^^^^^^^^^^^ | = note: `-D unimplemented` implied by `-D warnings` - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to 5 previous errors