Skip to content

implement disallowed trait methods #12194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 73 additions & 6 deletions clippy_lints/src/disallowed_methods.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
use clippy_config::types::DisallowedPath;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::{fn_def_id, get_parent_expr, path_def_id};
use rustc_hir::def_id::DefIdMap;
use rustc_hir::{Expr, ExprKind};
use itertools::Itertools as _;
use rustc_data_structures::unord::UnordMap;
use rustc_hir::def::Res;
use rustc_hir::def_id::{DefId, DefIdMap};
use rustc_hir::{Expr, ExprKind, PrimTy};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self, AdtKind};
use rustc_session::impl_lint_pass;

declare_clippy_lint! {
@@ -29,6 +33,8 @@ declare_clippy_lint! {
/// # When using an inline table, can add a `reason` for why the method
/// # is disallowed.
/// { path = "std::vec::Vec::leak", reason = "no leaking memory" },
/// # Can also use a qualified path to disallow a method on a specific type.
/// "<std::string::String as std::default::Default>::default",
/// ]
/// ```
///
@@ -59,13 +65,16 @@ declare_clippy_lint! {
pub struct DisallowedMethods {
conf_disallowed: Vec<DisallowedPath>,
disallowed: DefIdMap<usize>,
// (Self, TraitMethod)
disallowed_qualified_trait: UnordMap<(Res, DefId), usize>,
}

impl DisallowedMethods {
pub fn new(conf_disallowed: Vec<DisallowedPath>) -> Self {
Self {
conf_disallowed,
disallowed: DefIdMap::default(),
disallowed_qualified_trait: UnordMap::default(),
}
}
}
@@ -75,9 +84,31 @@ impl_lint_pass!(DisallowedMethods => [DISALLOWED_METHODS]);
impl<'tcx> LateLintPass<'tcx> for DisallowedMethods {
fn check_crate(&mut self, cx: &LateContext<'_>) {
for (index, conf) in self.conf_disallowed.iter().enumerate() {
let segs: Vec<_> = conf.path().split("::").collect();
for id in clippy_utils::def_path_def_ids(cx, &segs) {
self.disallowed.insert(id, index);
let path = conf.path();
if let Some(path) = path.strip_prefix('<') {
// a qualified associated item
let Some((tr, method)) = path.split_once(">::") else {
continue;
};
let Some((self_ty, _as, trait_path)) = tr.split_whitespace().next_tuple() else {
continue;
};
let self_segs: Vec<_> = self_ty.split("::").collect();
let self_ress: Vec<_> = clippy_utils::def_path_res(cx, &self_segs);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this does not support things like <[T] as U>::u_method, <&T as U>::u_method etc due to how the lookup in def_path_res works

let mut method_segs: Vec<_> = trait_path.split("::").collect();
method_segs.push(method);
let method_id: Vec<_> = clippy_utils::def_path_def_ids(cx, &method_segs).collect();
for self_res in &self_ress {
for method_id in &method_id {
self.disallowed_qualified_trait.insert((*self_res, *method_id), index);
}
}
} else {
// simple path
let segs: Vec<_> = path.split("::").collect();
for id in clippy_utils::def_path_def_ids(cx, &segs) {
self.disallowed.insert(id, index);
}
}
}
}
@@ -96,7 +127,43 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedMethods {
};
let conf = match self.disallowed.get(&def_id) {
Some(&index) => &self.conf_disallowed[index],
None => return,
None if self.disallowed_qualified_trait.is_empty() => return,
None => {
let self_ty = match expr.kind {
ExprKind::Call(_, _) => {
let typeck = cx.typeck_results();
typeck.expr_ty(expr)
},
ExprKind::MethodCall(_, self_arg, _, _) => {
let typeck = cx.typeck_results();
typeck.expr_ty(self_arg)
},
_ => return,
};

let self_res: Res<rustc_hir::HirId> = match self_ty.kind() {
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) => {
Res::PrimTy(PrimTy::from_name(self_ty.primitive_symbol().unwrap()).unwrap())
},
ty::Str => Res::PrimTy(PrimTy::Str),
ty::Adt(adt, _) => Res::Def(
match adt.adt_kind() {
AdtKind::Struct => rustc_hir::def::DefKind::Struct,
AdtKind::Union => rustc_hir::def::DefKind::Union,
AdtKind::Enum => rustc_hir::def::DefKind::Enum,
},
adt.did(),
),
// FIXME: these other kinds are not currently supported by disallowed_methods due to how
// def_path_ref is implemented
_ => return,
};

match self.disallowed_qualified_trait.get(&(self_res, def_id)) {
Some(&index) => &self.conf_disallowed[index],
None => return,
}
},
};
let msg = format!("use of a disallowed method `{}`", conf.path());
span_lint_and_then(cx, DISALLOWED_METHODS, expr.span, &msg, |diag| {
2 changes: 2 additions & 0 deletions tests/ui-toml/toml_disallowed_methods/clippy.toml
Original file line number Diff line number Diff line change
@@ -14,4 +14,6 @@ disallowed-methods = [
"conf_disallowed_methods::Struct::method",
"conf_disallowed_methods::Trait::provided_method",
"conf_disallowed_methods::Trait::implemented_method",
"<std::string::String as std::default::Default>::default",
"<conf_disallowed_methods::StructBad as conf_disallowed_methods::Trait>::provided_method",
]
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@ use regex::Regex;
fn local_fn() {}

struct Struct;
struct StructBad;

impl Struct {
fn method(&self) {}
@@ -27,6 +28,10 @@ impl Trait for Struct {
fn implemented_method(&self) {}
}

impl Trait for StructBad {
fn implemented_method(&self) {}
}

mod local_mod {
pub fn f() {}
}
@@ -58,4 +63,8 @@ fn main() {
s.method();
s.provided_method();
s.implemented_method();

_ = String::default();
let s_bad = StructBad;
s_bad.provided_method();
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: use of a disallowed method `regex::Regex::new`
--> $DIR/conf_disallowed_methods.rs:35:14
--> $DIR/conf_disallowed_methods.rs:40:14
|
LL | let re = Regex::new(r"ab.*c").unwrap();
| ^^^^^^^^^^^^^^^^^^^^
@@ -8,84 +8,96 @@ LL | let re = Regex::new(r"ab.*c").unwrap();
= help: to override `-D warnings` add `#[allow(clippy::disallowed_methods)]`

error: use of a disallowed method `regex::Regex::is_match`
--> $DIR/conf_disallowed_methods.rs:36:5
--> $DIR/conf_disallowed_methods.rs:41:5
|
LL | re.is_match("abc");
| ^^^^^^^^^^^^^^^^^^
|
= note: no matching allowed (from clippy.toml)

error: use of a disallowed method `std::iter::Iterator::sum`
--> $DIR/conf_disallowed_methods.rs:39:5
--> $DIR/conf_disallowed_methods.rs:44:5
|
LL | a.iter().sum::<i32>();
| ^^^^^^^^^^^^^^^^^^^^^

error: use of a disallowed method `slice::sort_unstable`
--> $DIR/conf_disallowed_methods.rs:41:5
--> $DIR/conf_disallowed_methods.rs:46:5
|
LL | a.sort_unstable();
| ^^^^^^^^^^^^^^^^^

error: use of a disallowed method `f32::clamp`
--> $DIR/conf_disallowed_methods.rs:43:13
--> $DIR/conf_disallowed_methods.rs:48:13
|
LL | let _ = 2.0f32.clamp(3.0f32, 4.0f32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: use of a disallowed method `regex::Regex::new`
--> $DIR/conf_disallowed_methods.rs:46:61
--> $DIR/conf_disallowed_methods.rs:51:61
|
LL | let indirect: fn(&str) -> Result<Regex, regex::Error> = Regex::new;
| ^^^^^^^^^^

error: use of a disallowed method `f32::clamp`
--> $DIR/conf_disallowed_methods.rs:49:28
--> $DIR/conf_disallowed_methods.rs:54:28
|
LL | let in_call = Box::new(f32::clamp);
| ^^^^^^^^^^

error: use of a disallowed method `regex::Regex::new`
--> $DIR/conf_disallowed_methods.rs:50:53
--> $DIR/conf_disallowed_methods.rs:55:53
|
LL | let in_method_call = ["^", "$"].into_iter().map(Regex::new);
| ^^^^^^^^^^

error: use of a disallowed method `futures::stream::select_all`
--> $DIR/conf_disallowed_methods.rs:53:31
--> $DIR/conf_disallowed_methods.rs:58:31
|
LL | let same_name_as_module = select_all(vec![empty::<()>()]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: use of a disallowed method `conf_disallowed_methods::local_fn`
--> $DIR/conf_disallowed_methods.rs:55:5
--> $DIR/conf_disallowed_methods.rs:60:5
|
LL | local_fn();
| ^^^^^^^^^^

error: use of a disallowed method `conf_disallowed_methods::local_mod::f`
--> $DIR/conf_disallowed_methods.rs:56:5
--> $DIR/conf_disallowed_methods.rs:61:5
|
LL | local_mod::f();
| ^^^^^^^^^^^^^^

error: use of a disallowed method `conf_disallowed_methods::Struct::method`
--> $DIR/conf_disallowed_methods.rs:58:5
--> $DIR/conf_disallowed_methods.rs:63:5
|
LL | s.method();
| ^^^^^^^^^^

error: use of a disallowed method `conf_disallowed_methods::Trait::provided_method`
--> $DIR/conf_disallowed_methods.rs:59:5
--> $DIR/conf_disallowed_methods.rs:64:5
|
LL | s.provided_method();
| ^^^^^^^^^^^^^^^^^^^

error: use of a disallowed method `conf_disallowed_methods::Trait::implemented_method`
--> $DIR/conf_disallowed_methods.rs:60:5
--> $DIR/conf_disallowed_methods.rs:65:5
|
LL | s.implemented_method();
| ^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 14 previous errors
error: use of a disallowed method `<std::string::String as std::default::Default>::default`
--> $DIR/conf_disallowed_methods.rs:67:9
|
LL | _ = String::default();
| ^^^^^^^^^^^^^^^^^

error: use of a disallowed method `conf_disallowed_methods::Trait::provided_method`
--> $DIR/conf_disallowed_methods.rs:69:5
|
LL | s_bad.provided_method();
| ^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 16 previous errors