Skip to content

Commit 8611a0b

Browse files
smoeliussmoelius
committed
Expand unnecessary_def_path lint
1 parent c84ac4c commit 8611a0b

File tree

6 files changed

+182
-56
lines changed

6 files changed

+182
-56
lines changed

‎clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
542542
store.register_late_pass(|_| {
543543
Box::<utils::internal_lints::lint_without_lint_pass::LintWithoutLintPass>::default()
544544
});
545-
store.register_late_pass(|_| Box::new(utils::internal_lints::unnecessary_def_path::UnnecessaryDefPath));
545+
store.register_late_pass(|_| Box::<utils::internal_lints::unnecessary_def_path::UnnecessaryDefPath>::default());
546546
store.register_late_pass(|_| Box::new(utils::internal_lints::outer_expn_data_pass::OuterExpnDataPass));
547547
store.register_late_pass(|_| Box::new(utils::internal_lints::msrv_attr_impl::MsrvAttrImpl));
548548
}

‎clippy_lints/src/utils/internal_lints/unnecessary_def_path.rs

Lines changed: 132 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,20 @@
1-
use clippy_utils::diagnostics::span_lint_and_then;
1+
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then};
22
use clippy_utils::source::snippet_with_applicability;
33
use clippy_utils::{def_path_res, is_lint_allowed, match_any_def_paths, peel_hir_expr_refs};
44
use if_chain::if_chain;
55
use rustc_ast::ast::LitKind;
6+
use rustc_data_structures::fx::FxHashSet;
67
use rustc_errors::Applicability;
78
use rustc_hir as hir;
89
use rustc_hir::def::{DefKind, Namespace, Res};
910
use rustc_hir::def_id::DefId;
10-
use rustc_hir::{ExprKind, Local, Mutability, Node};
11+
use rustc_hir::{Expr, ExprKind, Local, Mutability, Node};
1112
use rustc_lint::{LateContext, LateLintPass};
1213
use rustc_middle::mir::interpret::{Allocation, ConstValue, GlobalAlloc};
1314
use rustc_middle::ty::{self, AssocKind, DefIdTree, Ty};
14-
use rustc_session::{declare_lint_pass, declare_tool_lint};
15+
use rustc_session::{declare_tool_lint, impl_lint_pass};
1516
use rustc_span::symbol::{Ident, Symbol};
17+
use rustc_span::Span;
1618

1719
use std::str;
1820

@@ -38,11 +40,56 @@ declare_clippy_lint! {
3840
"using a def path when a diagnostic item or a `LangItem` is available"
3941
}
4042

41-
declare_lint_pass!(UnnecessaryDefPath => [UNNECESSARY_DEF_PATH]);
43+
impl_lint_pass!(UnnecessaryDefPath => [UNNECESSARY_DEF_PATH]);
44+
45+
#[derive(Default)]
46+
pub struct UnnecessaryDefPath {
47+
array_def_ids: FxHashSet<(DefId, Span)>,
48+
linted_def_ids: FxHashSet<DefId>,
49+
}
4250

43-
#[allow(clippy::too_many_lines)]
4451
impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
4552
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
53+
if is_lint_allowed(cx, UNNECESSARY_DEF_PATH, expr.hir_id) {
54+
return;
55+
}
56+
57+
match expr.kind {
58+
ExprKind::Call(func, args) => self.check_call(cx, func, args, expr.span),
59+
ExprKind::Array(elements) => self.check_array(cx, elements, expr.span),
60+
_ => {},
61+
}
62+
}
63+
64+
fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
65+
for &(def_id, span) in &self.array_def_ids {
66+
if self.linted_def_ids.contains(&def_id) {
67+
continue;
68+
}
69+
70+
let (msg, sugg) = if let Some(sym) = cx.tcx.get_diagnostic_name(def_id) {
71+
("diagnostic item", format!("sym::{sym}"))
72+
} else if let Some(sym) = get_lang_item_name(cx, def_id) {
73+
("language item", format!("LangItem::{sym}"))
74+
} else {
75+
continue;
76+
};
77+
78+
span_lint_and_help(
79+
cx,
80+
UNNECESSARY_DEF_PATH,
81+
span,
82+
&format!("hardcoded path to a {msg}"),
83+
None,
84+
&format!("convert all references to use `{sugg}`"),
85+
);
86+
}
87+
}
88+
}
89+
90+
impl UnnecessaryDefPath {
91+
#[allow(clippy::too_many_lines)]
92+
fn check_call(&mut self, cx: &LateContext<'_>, func: &Expr<'_>, args: &[Expr<'_>], span: Span) {
4693
enum Item {
4794
LangItem(Symbol),
4895
DiagnosticItem(Symbol),
@@ -54,42 +101,17 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
54101
&["clippy_utils", "is_expr_path_def_path"],
55102
];
56103

57-
if is_lint_allowed(cx, UNNECESSARY_DEF_PATH, expr.hir_id) {
58-
return;
59-
}
60-
61104
if_chain! {
62-
if let ExprKind::Call(func, [cx_arg, def_arg, args@..]) = expr.kind;
105+
if let [cx_arg, def_arg, args@..] = args;
63106
if let ExprKind::Path(path) = &func.kind;
64107
if let Some(id) = cx.qpath_res(path, func.hir_id).opt_def_id();
65108
if let Some(which_path) = match_any_def_paths(cx, id, PATHS);
66109
let item_arg = if which_path == 4 { &args[1] } else { &args[0] };
67110
// Extract the path to the matched type
68111
if let Some(segments) = path_to_matched_type(cx, item_arg);
69112
let segments: Vec<&str> = segments.iter().map(|sym| &**sym).collect();
70-
if let Some(def_id) = def_path_res(cx, &segments[..], None).opt_def_id();
113+
if let Some(def_id) = inherent_def_path_res(cx, &segments[..]);
71114
then {
72-
// def_path_res will match field names before anything else, but for this we want to match
73-
// inherent functions first.
74-
let def_id = if cx.tcx.def_kind(def_id) == DefKind::Field {
75-
let method_name = *segments.last().unwrap();
76-
cx.tcx.def_key(def_id).parent
77-
.and_then(|parent_idx|
78-
cx.tcx.inherent_impls(DefId { index: parent_idx, krate: def_id.krate }).iter()
79-
.find_map(|impl_id| cx.tcx.associated_items(*impl_id)
80-
.find_by_name_and_kind(
81-
cx.tcx,
82-
Ident::from_str(method_name),
83-
AssocKind::Fn,
84-
*impl_id,
85-
)
86-
)
87-
)
88-
.map_or(def_id, |item| item.def_id)
89-
} else {
90-
def_id
91-
};
92-
93115
// Check if the target item is a diagnostic item or LangItem.
94116
let (msg, item) = if let Some(item_name)
95117
= cx.tcx.diagnostic_items(def_id.krate).id_to_name.get(&def_id)
@@ -98,9 +120,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
98120
"use of a def path to a diagnostic item",
99121
Item::DiagnosticItem(*item_name),
100122
)
101-
} else if let Some(lang_item) = cx.tcx.lang_items().items().iter().position(|id| *id == Some(def_id)) {
102-
let lang_items = def_path_res(cx, &["rustc_hir", "lang_items", "LangItem"], Some(Namespace::TypeNS)).def_id();
103-
let item_name = cx.tcx.adt_def(lang_items).variants().iter().nth(lang_item).unwrap().name;
123+
} else if let Some(item_name) = get_lang_item_name(cx, def_id) {
104124
(
105125
"use of a def path to a `LangItem`",
106126
Item::LangItem(item_name),
@@ -168,10 +188,10 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
168188
span_lint_and_then(
169189
cx,
170190
UNNECESSARY_DEF_PATH,
171-
expr.span,
191+
span,
172192
msg,
173193
|diag| {
174-
diag.span_suggestion(expr.span, "try", sugg, app);
194+
diag.span_suggestion(span, "try", sugg, app);
175195
if with_note {
176196
diag.help(
177197
"if this `DefId` came from a constructor expression or pattern then the \
@@ -180,9 +200,19 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
180200
}
181201
},
182202
);
203+
204+
self.linted_def_ids.insert(def_id);
183205
}
184206
}
185207
}
208+
209+
fn check_array(&mut self, cx: &LateContext<'_>, elements: &[Expr<'_>], span: Span) {
210+
let Some(path) = path_from_array(elements) else { return };
211+
212+
if let Some(def_id) = inherent_def_path_res(cx, &path.iter().map(AsRef::as_ref).collect::<Vec<_>>()) {
213+
self.array_def_ids.insert((def_id, span));
214+
}
215+
}
186216
}
187217

188218
fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Vec<String>> {
@@ -209,18 +239,7 @@ fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Ve
209239
},
210240
_ => None,
211241
},
212-
ExprKind::Array(exprs) => exprs
213-
.iter()
214-
.map(|expr| {
215-
if let ExprKind::Lit(lit) = &expr.kind {
216-
if let LitKind::Str(sym, _) = lit.node {
217-
return Some((*sym.as_str()).to_owned());
218-
}
219-
}
220-
221-
None
222-
})
223-
.collect(),
242+
ExprKind::Array(exprs) => path_from_array(exprs),
224243
_ => None,
225244
}
226245
}
@@ -258,3 +277,67 @@ fn read_mir_alloc_def_path<'tcx>(cx: &LateContext<'tcx>, alloc: &'tcx Allocation
258277
None
259278
}
260279
}
280+
281+
fn path_from_array(exprs: &[Expr<'_>]) -> Option<Vec<String>> {
282+
exprs
283+
.iter()
284+
.map(|expr| {
285+
if let ExprKind::Lit(lit) = &expr.kind {
286+
if let LitKind::Str(sym, _) = lit.node {
287+
return Some((*sym.as_str()).to_owned());
288+
}
289+
}
290+
291+
None
292+
})
293+
.collect()
294+
}
295+
296+
// def_path_res will match field names before anything else, but for this we want to match
297+
// inherent functions first.
298+
fn inherent_def_path_res(cx: &LateContext<'_>, segments: &[&str]) -> Option<DefId> {
299+
def_path_res(cx, segments, None).opt_def_id().map(|def_id| {
300+
if cx.tcx.def_kind(def_id) == DefKind::Field {
301+
let method_name = *segments.last().unwrap();
302+
cx.tcx
303+
.def_key(def_id)
304+
.parent
305+
.and_then(|parent_idx| {
306+
cx.tcx
307+
.inherent_impls(DefId {
308+
index: parent_idx,
309+
krate: def_id.krate,
310+
})
311+
.iter()
312+
.find_map(|impl_id| {
313+
cx.tcx.associated_items(*impl_id).find_by_name_and_kind(
314+
cx.tcx,
315+
Ident::from_str(method_name),
316+
AssocKind::Fn,
317+
*impl_id,
318+
)
319+
})
320+
})
321+
.map_or(def_id, |item| item.def_id)
322+
} else {
323+
def_id
324+
}
325+
})
326+
}
327+
328+
fn get_lang_item_name(cx: &LateContext<'_>, def_id: DefId) -> Option<Symbol> {
329+
if let Some(lang_item) = cx.tcx.lang_items().items().iter().position(|id| *id == Some(def_id)) {
330+
let lang_items = def_path_res(cx, &["rustc_hir", "lang_items", "LangItem"], Some(Namespace::TypeNS)).def_id();
331+
let item_name = cx
332+
.tcx
333+
.adt_def(lang_items)
334+
.variants()
335+
.iter()
336+
.nth(lang_item)
337+
.unwrap()
338+
.name;
339+
Some(item_name)
340+
} else {
341+
None
342+
}
343+
}

‎tests/ui-internal/unnecessary_def_path.fixed

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,17 @@ use rustc_hir::Expr;
2828
use rustc_lint::LateContext;
2929
use rustc_middle::ty::Ty;
3030

31-
#[allow(unused)]
31+
#[allow(unused, clippy::unnecessary_def_path)]
3232
static OPTION: [&str; 3] = ["core", "option", "Option"];
33-
#[allow(unused)]
33+
#[allow(unused, clippy::unnecessary_def_path)]
3434
const RESULT: &[&str] = &["core", "result", "Result"];
3535

3636
fn _f<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, did: DefId, expr: &Expr<'_>) {
3737
let _ = is_type_diagnostic_item(cx, ty, sym::Option);
3838
let _ = is_type_diagnostic_item(cx, ty, sym::Result);
3939
let _ = is_type_diagnostic_item(cx, ty, sym::Result);
4040

41-
#[allow(unused)]
41+
#[allow(unused, clippy::unnecessary_def_path)]
4242
let rc_path = &["alloc", "rc", "Rc"];
4343
let _ = is_type_diagnostic_item(cx, ty, sym::Rc);
4444

‎tests/ui-internal/unnecessary_def_path.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,17 @@ use rustc_hir::Expr;
2828
use rustc_lint::LateContext;
2929
use rustc_middle::ty::Ty;
3030

31-
#[allow(unused)]
31+
#[allow(unused, clippy::unnecessary_def_path)]
3232
static OPTION: [&str; 3] = ["core", "option", "Option"];
33-
#[allow(unused)]
33+
#[allow(unused, clippy::unnecessary_def_path)]
3434
const RESULT: &[&str] = &["core", "result", "Result"];
3535

3636
fn _f<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, did: DefId, expr: &Expr<'_>) {
3737
let _ = match_type(cx, ty, &OPTION);
3838
let _ = match_type(cx, ty, RESULT);
3939
let _ = match_type(cx, ty, &["core", "result", "Result"]);
4040

41-
#[allow(unused)]
41+
#[allow(unused, clippy::unnecessary_def_path)]
4242
let rc_path = &["alloc", "rc", "Rc"];
4343
let _ = clippy_utils::ty::match_type(cx, ty, rc_path);
4444

‎tests/ui-internal/unnecessary_def_path_hardcoded_path.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#![feature(rustc_private)]
2+
#![allow(unused)]
3+
#![warn(clippy::unnecessary_def_path)]
4+
5+
extern crate rustc_hir;
6+
7+
use rustc_hir::LangItem;
8+
9+
fn main() {
10+
const DEREF_TRAIT: [&str; 4] = ["core", "ops", "deref", "Deref"];
11+
const DEREF_MUT_TRAIT: [&str; 4] = ["core", "ops", "deref", "DerefMut"];
12+
const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"];
13+
14+
// Don't lint, not yet a diagnostic or language item
15+
const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"];
16+
}

‎tests/ui-internal/unnecessary_def_path_hardcoded_path.stderr

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
error: hardcoded path to a language item
2+
--> $DIR/unnecessary_def_path_hardcoded_path.rs:11:40
3+
|
4+
LL | const DEREF_MUT_TRAIT: [&str; 4] = ["core", "ops", "deref", "DerefMut"];
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= help: convert all references to use `LangItem::DerefMut`
8+
= note: `-D clippy::unnecessary-def-path` implied by `-D warnings`
9+
10+
error: hardcoded path to a diagnostic item
11+
--> $DIR/unnecessary_def_path_hardcoded_path.rs:12:43
12+
|
13+
LL | const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"];
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15+
|
16+
= help: convert all references to use `sym::deref_method`
17+
18+
error: hardcoded path to a diagnostic item
19+
--> $DIR/unnecessary_def_path_hardcoded_path.rs:10:36
20+
|
21+
LL | const DEREF_TRAIT: [&str; 4] = ["core", "ops", "deref", "Deref"];
22+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
23+
|
24+
= help: convert all references to use `sym::Deref`
25+
26+
error: aborting due to 3 previous errors
27+

0 commit comments

Comments
 (0)