From 7b7e3ca5116983f5485bc9c8a5b2abf490900241 Mon Sep 17 00:00:00 2001 From: Philip Hayes Date: Fri, 29 Jan 2021 22:18:56 -0800 Subject: [PATCH] Support free functions in disallowed-methods lint In other words, support: `disallowed_methods = ["alloc::vec::Vec::new"]` (a free function) in addition to `disallowed_methods = ["alloc::vec::Vec::leak"]` (a method). Improve the documentation to clarify that users must specify the full qualified path for each disallowed function, which can be confusing for reexports. Include an example `clippy.toml`. Simplify the actual lint pass so we can reuse `utils::fn_def_id`. --- clippy_lints/src/disallowed_method.rs | 52 ++++++++++++------- clippy_lints/src/utils/conf.rs | 2 +- .../toml_disallowed_method/clippy.toml | 2 +- .../conf_disallowed_method.rs | 3 +- .../conf_disallowed_method.stderr | 16 ++++-- 5 files changed, 48 insertions(+), 27 deletions(-) diff --git a/clippy_lints/src/disallowed_method.rs b/clippy_lints/src/disallowed_method.rs index 581c3242e374..56dc6d18a58f 100644 --- a/clippy_lints/src/disallowed_method.rs +++ b/clippy_lints/src/disallowed_method.rs @@ -1,29 +1,47 @@ -use crate::utils::span_lint; +use crate::utils::{fn_def_id, span_lint}; use rustc_data_structures::fx::FxHashSet; -use rustc_hir::{Expr, ExprKind}; +use rustc_hir::Expr; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::Symbol; declare_clippy_lint! { - /// **What it does:** Lints for specific trait methods defined in clippy.toml + /// **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 would be beneficial to lint for them as needed. + /// and it's beneficial to lint for them as needed. /// - /// **Known problems:** None. + /// **Known problems:** Currently, you must write each function as a + /// fully-qualified path. This lint doesn't support aliases or reexported + /// names; be aware that many types in `std` are actually reexports. + /// + /// For example, if you want to disallow `Duration::as_secs`, your clippy.toml + /// configuration would look like + /// `disallowed-methods = ["core::time::Duration::as_secs"]` and not + /// `disallowed-methods = ["std::time::Duration::as_secs"]` as you might expect. /// /// **Example:** /// + /// An example clippy.toml configuration: + /// ```toml + /// # clippy.toml + /// disallowed-methods = ["alloc::vec::Vec::leak", "std::time::Instant::now"] + /// ``` + /// /// ```rust,ignore - /// // example code where clippy issues a warning - /// foo.bad_method(); // Foo::bad_method is disallowed in the configuration + /// // Example code where clippy issues a warning + /// let xs = vec![1, 2, 3, 4]; + /// xs.leak(); // Vec::leak is disallowed in the config. + /// + /// let _now = Instant::now(); // Instant::now is disallowed in the config. /// ``` + /// /// Use instead: /// ```rust,ignore - /// // example code which does not raise clippy warning - /// goodStruct.bad_method(); // GoodStruct::bad_method is not disallowed + /// // Example code which does not raise clippy warning + /// let mut xs = Vec::new(); // Vec::new is _not_ disallowed in the config. + /// xs.push(123); // Vec::push is _not_ disallowed in the config. /// ``` pub DISALLOWED_METHOD, nursery, @@ -50,14 +68,12 @@ impl_lint_pass!(DisallowedMethod => [DISALLOWED_METHOD]); impl<'tcx> LateLintPass<'tcx> for DisallowedMethod { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::MethodCall(_path, _, _args, _) = &expr.kind { - let def_id = cx.typeck_results().type_dependent_def_id(expr.hir_id).unwrap(); - - let method_call = cx.get_def_path(def_id); - if self.disallowed.contains(&method_call) { - let method = method_call - .iter() - .map(|s| s.to_ident_string()) + if let Some(def_id) = fn_def_id(cx, expr) { + let func_path = cx.get_def_path(def_id); + if self.disallowed.contains(&func_path) { + let func_path_string = func_path + .into_iter() + .map(Symbol::to_ident_string) .collect::>() .join("::"); @@ -65,7 +81,7 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedMethod { cx, DISALLOWED_METHOD, expr.span, - &format!("use of a disallowed method `{}`", method), + &format!("use of a disallowed method `{}`", func_path_string), ); } } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index b5a8300376c1..1fb99e04de3b 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -169,7 +169,7 @@ define_Conf! { (max_fn_params_bools, "max_fn_params_bools": u64, 3), /// Lint: WILDCARD_IMPORTS. Whether to allow certain wildcard imports (prelude, super in tests). (warn_on_all_wildcard_imports, "warn_on_all_wildcard_imports": bool, false), - /// Lint: DISALLOWED_METHOD. The list of blacklisted methods to lint about. NB: `bar` is not here since it has legitimate uses + /// Lint: DISALLOWED_METHOD. The list of disallowed methods, written as fully qualified paths. (disallowed_methods, "disallowed_methods": Vec, Vec::::new()), /// Lint: UNREADABLE_LITERAL. Should the fraction of a decimal be linted to include separators. (unreadable_literal_lint_fractions, "unreadable_literal_lint_fractions": bool, true), diff --git a/tests/ui-toml/toml_disallowed_method/clippy.toml b/tests/ui-toml/toml_disallowed_method/clippy.toml index a1f515e443dc..c0df3b6e8af5 100644 --- a/tests/ui-toml/toml_disallowed_method/clippy.toml +++ b/tests/ui-toml/toml_disallowed_method/clippy.toml @@ -1 +1 @@ -disallowed-methods = ["core::iter::traits::iterator::Iterator::sum", "regex::re_unicode::Regex::is_match"] +disallowed-methods = ["core::iter::traits::iterator::Iterator::sum", "regex::re_unicode::Regex::is_match", "regex::re_unicode::Regex::new"] diff --git a/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.rs b/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.rs index 3d3f0729abd8..1901a99377ec 100644 --- a/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.rs +++ b/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.rs @@ -4,10 +4,9 @@ extern crate regex; use regex::Regex; fn main() { - let a = vec![1, 2, 3, 4]; let re = Regex::new(r"ab.*c").unwrap(); - re.is_match("abc"); + let a = vec![1, 2, 3, 4]; a.iter().sum::(); } 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 ed91b5a6796d..2b628c67fa75 100644 --- a/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.stderr +++ b/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.stderr @@ -1,16 +1,22 @@ +error: use of a disallowed method `regex::re_unicode::Regex::new` + --> $DIR/conf_disallowed_method.rs:7:14 + | +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` - --> $DIR/conf_disallowed_method.rs:10:5 + --> $DIR/conf_disallowed_method.rs:8:5 | LL | re.is_match("abc"); | ^^^^^^^^^^^^^^^^^^ - | - = note: `-D clippy::disallowed-method` implied by `-D warnings` error: use of a disallowed method `core::iter::traits::iterator::Iterator::sum` - --> $DIR/conf_disallowed_method.rs:12:5 + --> $DIR/conf_disallowed_method.rs:11:5 | LL | a.iter().sum::(); | ^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors