From 91581f6d5e0cc442397067363052f1e60f0d0941 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Thu, 9 Dec 2021 19:11:40 +0000 Subject: [PATCH 1/2] Resolve primitive impls in clippy_utils::path_to_res --- clippy_utils/src/lib.rs | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 9f2018385f3..44f634b6519 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -70,16 +70,16 @@ use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::unhash::UnhashMap; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::def_id::DefId; +use rustc_hir::def_id::{CrateNum, DefId}; use rustc_hir::hir_id::{HirIdMap, HirIdSet}; use rustc_hir::intravisit::{walk_expr, ErasedMap, FnKind, NestedVisitorMap, Visitor}; use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_hir::LangItem::{OptionNone, ResultErr, ResultOk}; use rustc_hir::{ - def, Arm, BindingAnnotation, Block, BlockCheckMode, Body, Constness, Destination, Expr, ExprKind, FnDecl, - ForeignItem, GenericArgs, HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, - MatchSource, Mutability, Node, Param, Pat, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, - TraitItemKind, TraitRef, TyKind, UnOp, + def, lang_items, Arm, BindingAnnotation, Block, BlockCheckMode, Body, Constness, Destination, Expr, ExprKind, + FnDecl, ForeignItem, GenericArgs, HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, + MatchSource, Mutability, Node, Param, Pat, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, Target, + TraitItem, TraitItemKind, TraitRef, TyKind, UnOp, }; use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_middle::hir::exports::Export; @@ -525,18 +525,34 @@ pub fn path_to_res(cx: &LateContext<'_>, path: &[&str]) -> Res { .iter() .find(|item| item.ident.name.as_str() == name) } + fn find_primitive(tcx: TyCtxt<'_>, name: &str) -> Option { + if let Some(&(index, Target::Impl)) = lang_items::ITEM_REFS.get(&Symbol::intern(name)) { + tcx.lang_items().items()[index] + } else { + None + } + } + fn find_crate(tcx: TyCtxt<'_>, name: &str) -> Option { + tcx.crates(()) + .iter() + .find(|&&num| tcx.crate_name(num).as_str() == name) + .map(CrateNum::as_def_id) + } - let (krate, first, path) = match *path { - [krate, first, ref path @ ..] => (krate, first, path), + let (base, first, path) = match *path { + [base, first, ref path @ ..] => (base, first, path), [primitive] => { return PrimTy::from_name(Symbol::intern(primitive)).map_or(Res::Err, Res::PrimTy); }, _ => return Res::Err, }; let tcx = cx.tcx; - let crates = tcx.crates(()); - let krate = try_res!(crates.iter().find(|&&num| tcx.crate_name(num).as_str() == krate)); - let first = try_res!(item_child_by_name(tcx, krate.as_def_id(), first)); + let first = try_res!( + find_primitive(tcx, base) + .or_else(|| find_crate(tcx, base)) + .and_then(|id| item_child_by_name(tcx, id, first)) + ); + let last = path .iter() .copied() From 04eb27aeaf283415011d7786fa4091f0ceaf8a11 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Thu, 9 Dec 2021 20:42:44 +0000 Subject: [PATCH 2/2] Use method name from conf::DisallowedMethod Since def_path_str returns e.g. "core::f32::::clamp" for "f32::clamp" --- clippy_lints/src/disallowed_methods.rs | 29 ++++++++----------- clippy_lints/src/utils/conf.rs | 8 +++++ .../toml_disallowed_methods/clippy.toml | 2 ++ .../conf_disallowed_methods.rs | 7 ++++- .../conf_disallowed_methods.stderr | 14 ++++++++- 5 files changed, 41 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/disallowed_methods.rs b/clippy_lints/src/disallowed_methods.rs index 6d5b1efe270..73c00d97020 100644 --- a/clippy_lints/src/disallowed_methods.rs +++ b/clippy_lints/src/disallowed_methods.rs @@ -59,7 +59,7 @@ declare_clippy_lint! { #[derive(Clone, Debug)] pub struct DisallowedMethods { conf_disallowed: Vec, - disallowed: DefIdMap>, + disallowed: DefIdMap, } impl DisallowedMethods { @@ -75,17 +75,10 @@ impl_lint_pass!(DisallowedMethods => [DISALLOWED_METHODS]); impl<'tcx> LateLintPass<'tcx> for DisallowedMethods { fn check_crate(&mut self, cx: &LateContext<'_>) { - 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(); + for (index, conf) in self.conf_disallowed.iter().enumerate() { + let segs: Vec<_> = conf.path().split("::").collect(); if let Res::Def(_, id) = clippy_utils::path_to_res(cx, &segs) { - self.disallowed.insert(id, reason); + self.disallowed.insert(id, index); } } } @@ -95,15 +88,17 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedMethods { Some(def_id) => def_id, None => return, }; - let reason = match self.disallowed.get(&def_id) { - Some(reason) => reason, + let conf = match self.disallowed.get(&def_id) { + Some(&index) => &self.conf_disallowed[index], None => return, }; - let func_path = cx.tcx.def_path_str(def_id); - let msg = format!("use of a disallowed method `{}`", func_path); + let msg = format!("use of a disallowed method `{}`", conf.path()); span_lint_and_then(cx, DISALLOWED_METHODS, expr.span, &msg, |diag| { - if let Some(reason) = reason { - diag.note(reason); + if let conf::DisallowedMethod::WithReason { + reason: Some(reason), .. + } = conf + { + diag.note(&format!("{} (from clippy.toml)", reason)); } }); } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 4e1c538c1c2..d6deb50cc90 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -23,6 +23,14 @@ pub enum DisallowedMethod { WithReason { path: String, reason: Option }, } +impl DisallowedMethod { + pub fn path(&self) -> &str { + let (Self::Simple(path) | Self::WithReason { path, .. }) = self; + + path + } +} + /// A single disallowed type, used by the `DISALLOWED_TYPES` lint. #[derive(Clone, Debug, Deserialize)] #[serde(untagged)] diff --git a/tests/ui-toml/toml_disallowed_methods/clippy.toml b/tests/ui-toml/toml_disallowed_methods/clippy.toml index f1d4a4619c5..c902d21123d 100644 --- a/tests/ui-toml/toml_disallowed_methods/clippy.toml +++ b/tests/ui-toml/toml_disallowed_methods/clippy.toml @@ -1,6 +1,8 @@ disallowed-methods = [ # just a string is shorthand for path only "std::iter::Iterator::sum", + "f32::clamp", + "slice::sort_unstable", # 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 diff --git a/tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs b/tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs index cb449b45bde..338b3b5b28f 100644 --- a/tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs +++ b/tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs @@ -7,6 +7,11 @@ fn main() { let re = Regex::new(r"ab.*c").unwrap(); re.is_match("abc"); - let a = vec![1, 2, 3, 4]; + let mut a = vec![1, 2, 3, 4]; a.iter().sum::(); + + a.sort_unstable(); + + let _ = 2.0f32.clamp(3.0f32, 4.0f32); + let _ = 2.0f64.clamp(3.0f64, 4.0f64); } diff --git a/tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.stderr b/tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.stderr index 999ead10d51..5533676aea2 100644 --- a/tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.stderr +++ b/tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.stderr @@ -20,5 +20,17 @@ error: use of a disallowed method `std::iter::Iterator::sum` LL | a.iter().sum::(); | ^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 3 previous errors +error: use of a disallowed method `slice::sort_unstable` + --> $DIR/conf_disallowed_methods.rs:13:5 + | +LL | a.sort_unstable(); + | ^^^^^^^^^^^^^^^^^ + +error: use of a disallowed method `f32::clamp` + --> $DIR/conf_disallowed_methods.rs:15:13 + | +LL | let _ = 2.0f32.clamp(3.0f32, 4.0f32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 5 previous errors