From 293db0d33c06f1642af51df0afcb6f5e0fd0eb10 Mon Sep 17 00:00:00 2001 From: Ariel Davis Date: Wed, 8 Sep 2021 21:12:02 -0400 Subject: [PATCH] Allow giving reasons for disallowed_methods --- clippy_lints/src/disallowed_method.rs | 84 +++++++++++-------- clippy_lints/src/lib.rs | 4 +- clippy_lints/src/utils/conf.rs | 10 ++- .../toml_disallowed_method/clippy.toml | 7 +- .../conf_disallowed_method.stderr | 8 +- 5 files changed, 69 insertions(+), 44 deletions(-) diff --git a/clippy_lints/src/disallowed_method.rs b/clippy_lints/src/disallowed_method.rs index 7069cb4198c..1167b26c8f1 100644 --- a/clippy_lints/src/disallowed_method.rs +++ b/clippy_lints/src/disallowed_method.rs @@ -1,33 +1,44 @@ -use clippy_utils::diagnostics::span_lint; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::fn_def_id; -use rustc_data_structures::fx::FxHashSet; -use rustc_hir::{def::Res, def_id::DefId, Crate, Expr}; +use rustc_hir::{def::Res, def_id::DefIdMap, Crate, Expr}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::Symbol; + +use crate::utils::conf; declare_clippy_lint! { /// ### What it does /// Denies the configured methods and functions in clippy.toml /// /// ### Why is this bad? - /// Some methods are undesirable in certain contexts, - /// and it's beneficial to lint for them as needed. + /// Some methods are undesirable in certain contexts, and it's beneficial to + /// lint for them as needed. /// /// ### Example /// An example clippy.toml configuration: /// ```toml /// # clippy.toml - /// disallowed-methods = ["std::vec::Vec::leak", "std::time::Instant::now"] + /// disallowed-methods = [ + /// # Can use a string as the path of the disallowed method. + /// "std::boxed::Box::new", + /// # Can also use an inline table with a `path` key. + /// { path = "std::time::Instant::now" }, + /// # When using an inline table, can add a `reason` for why the method + /// # is disallowed. + /// { path = "std::vec::Vec::leak", reason = "no leaking memory" }, + /// ] /// ``` /// /// ```rust,ignore /// // Example code where clippy issues a warning /// let xs = vec![1, 2, 3, 4]; /// xs.leak(); // Vec::leak is disallowed in the config. + /// // The diagnostic contains the message "no leaking memory". /// /// let _now = Instant::now(); // Instant::now is disallowed in the config. + /// + /// let _box = Box::new(3); // Box::new is disallowed in the config. /// ``` /// /// Use instead: @@ -43,18 +54,15 @@ declare_clippy_lint! { #[derive(Clone, Debug)] pub struct DisallowedMethod { - disallowed: FxHashSet>, - def_ids: FxHashSet<(DefId, Vec)>, + conf_disallowed: Vec, + disallowed: DefIdMap>, } impl DisallowedMethod { - pub fn new(disallowed: &FxHashSet) -> Self { + pub fn new(conf_disallowed: Vec) -> Self { Self { - disallowed: disallowed - .iter() - .map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::>()) - .collect(), - def_ids: FxHashSet::default(), + conf_disallowed, + disallowed: DefIdMap::default(), } } } @@ -63,32 +71,36 @@ impl_lint_pass!(DisallowedMethod => [DISALLOWED_METHOD]); impl<'tcx> LateLintPass<'tcx> for DisallowedMethod { fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) { - for path in &self.disallowed { - let segs = path.iter().map(ToString::to_string).collect::>(); - if let Res::Def(_, id) = clippy_utils::path_to_res(cx, &segs.iter().map(String::as_str).collect::>()) - { - self.def_ids.insert((id, path.clone())); + for conf in &self.conf_disallowed { + let (path, reason) = match conf { + conf::DisallowedMethod::Simple(path) => (path, None), + conf::DisallowedMethod::WithReason { path, reason } => ( + path, + reason.as_ref().map(|reason| format!("{} (from clippy.toml)", reason)), + ), + }; + let segs: Vec<_> = path.split("::").collect(); + if let Res::Def(_, id) = clippy_utils::path_to_res(cx, &segs) { + self.disallowed.insert(id, reason); } } } fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let Some(def_id) = fn_def_id(cx, expr) { - if self.def_ids.iter().any(|(id, _)| def_id == *id) { - let func_path = cx.get_def_path(def_id); - let func_path_string = func_path - .into_iter() - .map(Symbol::to_ident_string) - .collect::>() - .join("::"); - - span_lint( - cx, - DISALLOWED_METHOD, - expr.span, - &format!("use of a disallowed method `{}`", func_path_string), - ); + let def_id = match fn_def_id(cx, expr) { + Some(def_id) => def_id, + None => return, + }; + let reason = match self.disallowed.get(&def_id) { + Some(reason) => reason, + None => return, + }; + let func_path = cx.tcx.def_path_str(def_id); + let msg = format!("use of a disallowed method `{}`", func_path); + span_lint_and_then(cx, DISALLOWED_METHOD, expr.span, &msg, |diag| { + if let Some(reason) = reason { + diag.note(reason); } - } + }); } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 92a13be81f4..9544309ae4f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -2105,8 +2105,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(float_equality_without_abs::FloatEqualityWithoutAbs)); store.register_late_pass(|| Box::new(semicolon_if_nothing_returned::SemicolonIfNothingReturned)); store.register_late_pass(|| Box::new(async_yields_async::AsyncYieldsAsync)); - let disallowed_methods = conf.disallowed_methods.iter().cloned().collect::>(); - store.register_late_pass(move || Box::new(disallowed_method::DisallowedMethod::new(&disallowed_methods))); + let disallowed_methods = conf.disallowed_methods.clone(); + store.register_late_pass(move || Box::new(disallowed_method::DisallowedMethod::new(disallowed_methods.clone()))); store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86AttSyntax)); store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86IntelSyntax)); store.register_late_pass(|| Box::new(undropped_manually_drops::UndroppedManuallyDrops)); diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 8651fa6fcf9..8b087507b11 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -15,6 +15,14 @@ pub struct Rename { pub rename: String, } +/// A single disallowed method, used by the `DISALLOWED_METHOD` lint. +#[derive(Clone, Debug, Deserialize)] +#[serde(untagged)] +pub enum DisallowedMethod { + Simple(String), + WithReason { path: String, reason: Option }, +} + /// Conf with parse errors #[derive(Default)] pub struct TryConf { @@ -243,7 +251,7 @@ define_Conf! { /// Lint: DISALLOWED_METHOD. /// /// The list of disallowed methods, written as fully qualified paths. - (disallowed_methods: Vec = Vec::new()), + (disallowed_methods: Vec = Vec::new()), /// Lint: DISALLOWED_TYPE. /// /// The list of disallowed types, written as fully qualified paths. diff --git a/tests/ui-toml/toml_disallowed_method/clippy.toml b/tests/ui-toml/toml_disallowed_method/clippy.toml index a3245da6825..f1d4a4619c5 100644 --- a/tests/ui-toml/toml_disallowed_method/clippy.toml +++ b/tests/ui-toml/toml_disallowed_method/clippy.toml @@ -1,5 +1,8 @@ disallowed-methods = [ + # just a string is shorthand for path only "std::iter::Iterator::sum", - "regex::Regex::is_match", - "regex::Regex::new" + # can give path and reason with an inline table + { path = "regex::Regex::is_match", reason = "no matching allowed" }, + # can use an inline table but omit reason + { path = "regex::Regex::new" }, ] diff --git a/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.stderr b/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.stderr index 2b628c67fa7..38123220a43 100644 --- a/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.stderr +++ b/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.stderr @@ -1,4 +1,4 @@ -error: use of a disallowed method `regex::re_unicode::Regex::new` +error: use of a disallowed method `regex::Regex::new` --> $DIR/conf_disallowed_method.rs:7:14 | LL | let re = Regex::new(r"ab.*c").unwrap(); @@ -6,13 +6,15 @@ LL | let re = Regex::new(r"ab.*c").unwrap(); | = note: `-D clippy::disallowed-method` implied by `-D warnings` -error: use of a disallowed method `regex::re_unicode::Regex::is_match` +error: use of a disallowed method `regex::Regex::is_match` --> $DIR/conf_disallowed_method.rs:8:5 | LL | re.is_match("abc"); | ^^^^^^^^^^^^^^^^^^ + | + = note: no matching allowed (from clippy.toml) -error: use of a disallowed method `core::iter::traits::iterator::Iterator::sum` +error: use of a disallowed method `std::iter::Iterator::sum` --> $DIR/conf_disallowed_method.rs:11:5 | LL | a.iter().sum::();