Skip to content

Commit

Permalink
Auto merge of #9800 - Alexendoo:def_path_res_multiple, r=dswij
Browse files Browse the repository at this point in the history
Return multiple resolutions from `def_path_res`

Changes `def_path_res` to return all the resolutions matching the path rather than the first one (with a namespace hint that covered some cases).  This would fix any issues that come up with multiple versions of the same crate being present as they all have the same crate name

It also adds resolution of `impl _ {}` items for local items, and removes struct field resolution as it didn't seem to be used anywhere

I tested it on a local crate and it worked for the multiple crate issue, but I couldn't come up with a test that worked well with `// aux-build`, maybe `// aux-crate` after rust-lang/rust#103266 could work but I'm not sure on that either

changelog: [`disallowed_methods`], [`disallowed_types`], [`disallowed_macros`]: fix path resolution with multiple versions of the same crate
changelog: [`disallowed_methods`]: Resolve methods in `impl`s in the current crate
  • Loading branch information
bors committed Nov 19, 2022
2 parents e144c7d + 1e1ac2b commit f60186f
Show file tree
Hide file tree
Showing 15 changed files with 281 additions and 236 deletions.
3 changes: 1 addition & 2 deletions clippy_lints/src/await_holding_invalid.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::{match_def_path, paths};
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def::{Namespace, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::{AsyncGeneratorKind, Body, BodyId, GeneratorKind};
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -189,7 +188,7 @@ impl LateLintPass<'_> for AwaitHolding {
fn check_crate(&mut self, cx: &LateContext<'_>) {
for conf in &self.conf_invalid_types {
let segs: Vec<_> = conf.path().split("::").collect();
if let Res::Def(_, id) = clippy_utils::def_path_res(cx, &segs, Some(Namespace::TypeNS)) {
for id in clippy_utils::def_path_def_ids(cx, &segs) {
self.def_ids.insert(id, conf.clone());
}
}
Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/disallowed_macros.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::macro_backtrace;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::def::{Namespace, Res};
use rustc_hir::def_id::DefIdMap;
use rustc_hir::{Expr, ForeignItem, HirId, ImplItem, Item, Pat, Path, Stmt, TraitItem, Ty};
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -89,7 +88,7 @@ impl DisallowedMacros {
&format!("use of a disallowed macro `{}`", conf.path()),
|diag| {
if let Some(reason) = conf.reason() {
diag.note(&format!("{reason} (from clippy.toml)"));
diag.note(reason);
}
},
);
Expand All @@ -104,7 +103,7 @@ impl LateLintPass<'_> for DisallowedMacros {
fn check_crate(&mut self, cx: &LateContext<'_>) {
for (index, conf) in self.conf_disallowed.iter().enumerate() {
let segs: Vec<_> = conf.path().split("::").collect();
if let Res::Def(_, id) = clippy_utils::def_path_res(cx, &segs, Some(Namespace::MacroNS)) {
for id in clippy_utils::def_path_def_ids(cx, &segs) {
self.disallowed.insert(id, index);
}
}
Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/disallowed_methods.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::{fn_def_id, get_parent_expr, path_def_id};

use rustc_hir::def::{Namespace, Res};
use rustc_hir::def_id::DefIdMap;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -79,7 +78,7 @@ 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();
if let Res::Def(_, id) = clippy_utils::def_path_res(cx, &segs, Some(Namespace::ValueNS)) {
for id in clippy_utils::def_path_def_ids(cx, &segs) {
self.disallowed.insert(id, index);
}
}
Expand All @@ -104,7 +103,7 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedMethods {
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) = conf.reason() {
diag.note(&format!("{reason} (from clippy.toml)"));
diag.note(reason);
}
});
}
Expand Down
40 changes: 21 additions & 19 deletions clippy_lints/src/disallowed_types.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_then;

use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def::{Namespace, Res};
use rustc_hir::def::Res;
use rustc_hir::def_id::DefId;
use rustc_hir::{Item, ItemKind, PolyTraitRef, PrimTy, Ty, TyKind, UseKind};
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -53,8 +53,8 @@ declare_clippy_lint! {
#[derive(Clone, Debug)]
pub struct DisallowedTypes {
conf_disallowed: Vec<conf::DisallowedPath>,
def_ids: FxHashMap<DefId, Option<String>>,
prim_tys: FxHashMap<PrimTy, Option<String>>,
def_ids: FxHashMap<DefId, usize>,
prim_tys: FxHashMap<PrimTy, usize>,
}

impl DisallowedTypes {
Expand All @@ -69,13 +69,13 @@ impl DisallowedTypes {
fn check_res_emit(&self, cx: &LateContext<'_>, res: &Res, span: Span) {
match res {
Res::Def(_, did) => {
if let Some(reason) = self.def_ids.get(did) {
emit(cx, &cx.tcx.def_path_str(*did), span, reason.as_deref());
if let Some(&index) = self.def_ids.get(did) {
emit(cx, &cx.tcx.def_path_str(*did), span, &self.conf_disallowed[index]);
}
},
Res::PrimTy(prim) => {
if let Some(reason) = self.prim_tys.get(prim) {
emit(cx, prim.name_str(), span, reason.as_deref());
if let Some(&index) = self.prim_tys.get(prim) {
emit(cx, prim.name_str(), span, &self.conf_disallowed[index]);
}
},
_ => {},
Expand All @@ -87,17 +87,19 @@ impl_lint_pass!(DisallowedTypes => [DISALLOWED_TYPES]);

impl<'tcx> LateLintPass<'tcx> for DisallowedTypes {
fn check_crate(&mut self, cx: &LateContext<'_>) {
for conf in &self.conf_disallowed {
for (index, conf) in self.conf_disallowed.iter().enumerate() {
let segs: Vec<_> = conf.path().split("::").collect();
let reason = conf.reason().map(|reason| format!("{reason} (from clippy.toml)"));
match clippy_utils::def_path_res(cx, &segs, Some(Namespace::TypeNS)) {
Res::Def(_, id) => {
self.def_ids.insert(id, reason);
},
Res::PrimTy(ty) => {
self.prim_tys.insert(ty, reason);
},
_ => {},

for res in clippy_utils::def_path_res(cx, &segs) {
match res {
Res::Def(_, id) => {
self.def_ids.insert(id, index);
},
Res::PrimTy(ty) => {
self.prim_tys.insert(ty, index);
},
_ => {},
}
}
}
}
Expand All @@ -119,14 +121,14 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedTypes {
}
}

fn emit(cx: &LateContext<'_>, name: &str, span: Span, reason: Option<&str>) {
fn emit(cx: &LateContext<'_>, name: &str, span: Span, conf: &conf::DisallowedPath) {
span_lint_and_then(
cx,
DISALLOWED_TYPES,
span,
&format!("`{name}` is not allowed according to config"),
|diag| {
if let Some(reason) = reason {
if let Some(reason) = conf.reason() {
diag.note(reason);
}
},
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/missing_enforced_import_rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl LateLintPass<'_> for ImportRename {
fn check_crate(&mut self, cx: &LateContext<'_>) {
for Rename { path, rename } in &self.conf_renames {
let segs = path.split("::").collect::<Vec<_>>();
if let Res::Def(_, id) = clippy_utils::def_path_res(cx, &segs, None) {
for id in clippy_utils::def_path_def_ids(cx, &segs) {
self.renames.insert(id, Symbol::intern(rename));
}
}
Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/mut_key.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::{def_path_res, trait_ref_of_method};
use clippy_utils::{def_path_def_ids, trait_ref_of_method};
use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_hir::def::Namespace;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::TypeVisitable;
use rustc_middle::ty::{Adt, Array, Ref, Slice, Tuple, Ty};
Expand Down Expand Up @@ -94,7 +93,7 @@ impl<'tcx> LateLintPass<'tcx> for MutableKeyType {
let mut path = Vec::new();
for ty in &self.ignore_interior_mutability {
path.extend(ty.split("::"));
if let Some(id) = def_path_res(cx, &path[..], Some(Namespace::TypeNS)).opt_def_id() {
for id in def_path_def_ids(cx, &path[..]) {
self.ignore_mut_def_ids.insert(id);
}
path.clear();
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ impl DisallowedPath {
path
}

pub fn reason(&self) -> Option<&str> {
pub fn reason(&self) -> Option<String> {
match self {
Self::WithReason {
reason: Some(reason), ..
} => Some(reason),
} => Some(format!("{reason} (from clippy.toml)")),
_ => None,
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use clippy_utils::consts::{constant_simple, Constant};
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
use clippy_utils::ty::match_type;
use clippy_utils::{def_path_res, is_expn_of, match_def_path, paths};
use clippy_utils::{def_path_def_ids, is_expn_of, match_def_path, paths};
use if_chain::if_chain;
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::Applicability;
Expand Down Expand Up @@ -74,7 +74,7 @@ impl<'tcx> LateLintPass<'tcx> for InterningDefinedSymbol {
}

for &module in &[&paths::KW_MODULE, &paths::SYM_MODULE] {
if let Some(def_id) = def_path_res(cx, module, None).opt_def_id() {
for def_id in def_path_def_ids(cx, module) {
for item in cx.tcx.module_children(def_id).iter() {
if_chain! {
if let Res::Def(DefKind::Const, item_def_id) = item.res;
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/utils/internal_lints/invalid_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use clippy_utils::def_path_res;
use clippy_utils::diagnostics::span_lint;
use if_chain::if_chain;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def::DefKind;
use rustc_hir::Item;
use rustc_hir_analysis::hir_ty_to_ty;
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -63,7 +63,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidPaths {
// This is not a complete resolver for paths. It works on all the paths currently used in the paths
// module. That's all it does and all it needs to do.
pub fn check_path(cx: &LateContext<'_>, path: &[&str]) -> bool {
if def_path_res(cx, path, None) != Res::Err {
if !def_path_res(cx, path).is_empty() {
return true;
}

Expand Down
48 changes: 9 additions & 39 deletions clippy_lints/src/utils/internal_lints/unnecessary_def_path.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then};
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{def_path_res, is_lint_allowed, match_any_def_paths, peel_hir_expr_refs};
use clippy_utils::{def_path_def_ids, is_lint_allowed, match_any_def_paths, peel_hir_expr_refs};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Namespace, Res};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::{Expr, ExprKind, Local, Mutability, Node};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::mir::interpret::{Allocation, ConstValue, GlobalAlloc};
use rustc_middle::ty::{self, AssocKind, DefIdTree, Ty};
use rustc_middle::ty::{self, DefIdTree, Ty};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::{Ident, Symbol};
use rustc_span::symbol::Symbol;
use rustc_span::Span;

use std::str;
Expand Down Expand Up @@ -110,7 +110,7 @@ impl UnnecessaryDefPath {
// Extract the path to the matched type
if let Some(segments) = path_to_matched_type(cx, item_arg);
let segments: Vec<&str> = segments.iter().map(|sym| &**sym).collect();
if let Some(def_id) = inherent_def_path_res(cx, &segments[..]);
if let Some(def_id) = def_path_def_ids(cx, &segments[..]).next();
then {
// Check if the target item is a diagnostic item or LangItem.
#[rustfmt::skip]
Expand Down Expand Up @@ -209,7 +209,7 @@ impl UnnecessaryDefPath {
fn check_array(&mut self, cx: &LateContext<'_>, elements: &[Expr<'_>], span: Span) {
let Some(path) = path_from_array(elements) else { return };

if let Some(def_id) = inherent_def_path_res(cx, &path.iter().map(AsRef::as_ref).collect::<Vec<_>>()) {
for def_id in def_path_def_ids(cx, &path.iter().map(AsRef::as_ref).collect::<Vec<_>>()) {
self.array_def_ids.insert((def_id, span));
}
}
Expand Down Expand Up @@ -293,41 +293,11 @@ fn path_from_array(exprs: &[Expr<'_>]) -> Option<Vec<String>> {
.collect()
}

// def_path_res will match field names before anything else, but for this we want to match
// inherent functions first.
fn inherent_def_path_res(cx: &LateContext<'_>, segments: &[&str]) -> Option<DefId> {
def_path_res(cx, segments, None).opt_def_id().map(|def_id| {
if cx.tcx.def_kind(def_id) == DefKind::Field {
let method_name = *segments.last().unwrap();
cx.tcx
.def_key(def_id)
.parent
.and_then(|parent_idx| {
cx.tcx
.inherent_impls(DefId {
index: parent_idx,
krate: def_id.krate,
})
.iter()
.find_map(|impl_id| {
cx.tcx.associated_items(*impl_id).find_by_name_and_kind(
cx.tcx,
Ident::from_str(method_name),
AssocKind::Fn,
*impl_id,
)
})
})
.map_or(def_id, |item| item.def_id)
} else {
def_id
}
})
}

fn get_lang_item_name(cx: &LateContext<'_>, def_id: DefId) -> Option<Symbol> {
if let Some(lang_item) = cx.tcx.lang_items().items().iter().position(|id| *id == Some(def_id)) {
let lang_items = def_path_res(cx, &["rustc_hir", "lang_items", "LangItem"], Some(Namespace::TypeNS)).def_id();
let lang_items = def_path_def_ids(cx, &["rustc_hir", "lang_items", "LangItem"])
.next()
.unwrap();
let item_name = cx
.tcx
.adt_def(lang_items)
Expand Down
Loading

0 comments on commit f60186f

Please sign in to comment.