Skip to content

Commit

Permalink
Support free functions in disallowed-methods lint
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
phlip9 committed Feb 4, 2021
1 parent 357c6a7 commit 7b7e3ca
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 27 deletions.
52 changes: 34 additions & 18 deletions clippy_lints/src/disallowed_method.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -50,22 +68,20 @@ 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("::");

span_lint(
cx,
DISALLOWED_METHOD,
expr.span,
&format!("use of a disallowed method `{}`", method),
&format!("use of a disallowed method `{}`", func_path_string),
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion tests/ui-toml/toml_disallowed_method/clippy.toml
Original file line number Diff line number Diff line change
@@ -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"]
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
}
16 changes: 11 additions & 5 deletions tests/ui-toml/toml_disallowed_method/conf_disallowed_method.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 7b7e3ca

Please sign in to comment.