about summary refs log tree commit diff
diff options
context:
space:
mode:
authorPhilip Hayes <philiphayes@fb.com>2021-01-29 22:18:56 -0800
committerPhilip Hayes <philiphayes@fb.com>2021-02-04 11:28:55 -0800
commit7b7e3ca5116983f5485bc9c8a5b2abf490900241 (patch)
treed6be0008e91e1502a4757435a824699b44a6eaed
parent357c6a7e2739da8c8d0a68a5e9e0867983af7cbe (diff)
downloadrust-7b7e3ca5116983f5485bc9c8a5b2abf490900241.tar.gz
rust-7b7e3ca5116983f5485bc9c8a5b2abf490900241.zip
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`.
-rw-r--r--clippy_lints/src/disallowed_method.rs52
-rw-r--r--clippy_lints/src/utils/conf.rs2
-rw-r--r--tests/ui-toml/toml_disallowed_method/clippy.toml2
-rw-r--r--tests/ui-toml/toml_disallowed_method/conf_disallowed_method.rs3
-rw-r--r--tests/ui-toml/toml_disallowed_method/conf_disallowed_method.stderr16
5 files changed, 48 insertions, 27 deletions
diff --git a/clippy_lints/src/disallowed_method.rs b/clippy_lints/src/disallowed_method.rs
index 581c3242e37..56dc6d18a58 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::<Vec<_>>()
                     .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 b5a8300376c..1fb99e04de3 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<String>, Vec::<String>::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 a1f515e443d..c0df3b6e8af 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 3d3f0729abd..1901a99377e 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::<i32>();
 }
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 ed91b5a6796..2b628c67fa7 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::<i32>();
    |     ^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 2 previous errors
+error: aborting due to 3 previous errors