Skip to content
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

Allow primitive types in disallowed_methods #8112

Merged
merged 2 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
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
29 changes: 12 additions & 17 deletions clippy_lints/src/disallowed_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ declare_clippy_lint! {
#[derive(Clone, Debug)]
pub struct DisallowedMethods {
conf_disallowed: Vec<conf::DisallowedMethod>,
disallowed: DefIdMap<Option<String>>,
disallowed: DefIdMap<usize>,
}

impl DisallowedMethods {
Expand All @@ -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);
}
}
}
Expand All @@ -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));
}
});
}
Expand Down
8 changes: 8 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ pub enum DisallowedMethod {
WithReason { path: String, reason: Option<String> },
}

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)]
Expand Down
36 changes: 26 additions & 10 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<DefId> {
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<DefId> {
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()
Expand Down
2 changes: 2 additions & 0 deletions tests/ui-toml/toml_disallowed_methods/clippy.toml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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::<i32>();

a.sort_unstable();

let _ = 2.0f32.clamp(3.0f32, 4.0f32);
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
let _ = 2.0f64.clamp(3.0f64, 4.0f64);
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,17 @@ error: use of a disallowed method `std::iter::Iterator::sum`
LL | a.iter().sum::<i32>();
| ^^^^^^^^^^^^^^^^^^^^^

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