Skip to content

Commit 332b5b3

Browse files
borsbors
committed
Auto merge of #9566 - smoelius:diagnostic-item-path, r=dswij
Expand internal lint `unnecessary_def_path` This PR does essentially two things: * Separates the internal lints into modules by pass. (`internal_lints.rs` was over 1400 lines, which is a little unruly IMHO.) * ~Adds a new~ Expands the `unnecessary_def_path` internal lint to flag hardcoded paths to diagnostic and language items. My understanding is that the latter is currently done by reviewers. Automating this process should make things easier for both reviewers and contributors. I could make the first bullet a separate PR, or remove it entirely, if desired. changelog: Add internal lint `diagnostic_item_path`
2 parents eba5ff9 + 5dc54c6 commit 332b5b3

38 files changed

+1996
-1738
lines changed

‎clippy_lints/src/booleans.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
2+
use clippy_utils::eq_expr_value;
23
use clippy_utils::source::snippet_opt;
34
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
4-
use clippy_utils::{eq_expr_value, get_trait_def_id, paths};
55
use if_chain::if_chain;
66
use rustc_ast::ast::LitKind;
77
use rustc_errors::Applicability;
@@ -483,7 +483,9 @@ impl<'a, 'tcx> Visitor<'tcx> for NonminimalBoolVisitor<'a, 'tcx> {
483483

484484
fn implements_ord<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> bool {
485485
let ty = cx.typeck_results().expr_ty(expr);
486-
get_trait_def_id(cx, &paths::ORD).map_or(false, |id| implements_trait(cx, ty, id, &[]))
486+
cx.tcx
487+
.get_diagnostic_item(sym::Ord)
488+
.map_or(false, |id| implements_trait(cx, ty, id, &[]))
487489
}
488490

489491
struct NotSimplificationVisitor<'a, 'tcx> {

‎clippy_lints/src/comparison_chain.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
22
use clippy_utils::ty::implements_trait;
3-
use clippy_utils::{get_trait_def_id, if_sequence, in_constant, is_else_clause, paths, SpanlessEq};
3+
use clippy_utils::{if_sequence, in_constant, is_else_clause, SpanlessEq};
44
use rustc_hir::{BinOpKind, Expr, ExprKind};
55
use rustc_lint::{LateContext, LateLintPass};
66
use rustc_session::{declare_lint_pass, declare_tool_lint};
7+
use rustc_span::sym;
78

89
declare_clippy_lint! {
910
/// ### What it does
@@ -106,7 +107,10 @@ impl<'tcx> LateLintPass<'tcx> for ComparisonChain {
106107

107108
// Check that the type being compared implements `core::cmp::Ord`
108109
let ty = cx.typeck_results().expr_ty(lhs1);
109-
let is_ord = get_trait_def_id(cx, &paths::ORD).map_or(false, |id| implements_trait(cx, ty, id, &[]));
110+
let is_ord = cx
111+
.tcx
112+
.get_diagnostic_item(sym::Ord)
113+
.map_or(false, |id| implements_trait(cx, ty, id, &[]));
110114

111115
if !is_ord {
112116
return;

‎clippy_lints/src/functions/must_use.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ use rustc_middle::{
77
lint::in_external_macro,
88
ty::{self, Ty},
99
};
10-
use rustc_span::{sym, Span};
10+
use rustc_span::{sym, Span, Symbol};
1111

1212
use clippy_utils::attrs::is_proc_macro;
1313
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then};
1414
use clippy_utils::source::snippet_opt;
1515
use clippy_utils::ty::is_must_use_ty;
1616
use clippy_utils::visitors::for_each_expr;
17-
use clippy_utils::{match_def_path, return_ty, trait_ref_of_method};
17+
use clippy_utils::{return_ty, trait_ref_of_method};
1818

1919
use core::ops::ControlFlow;
2020

@@ -181,15 +181,17 @@ fn is_mutable_pat(cx: &LateContext<'_>, pat: &hir::Pat<'_>, tys: &mut DefIdSet)
181181
}
182182
}
183183

184-
static KNOWN_WRAPPER_TYS: &[&[&str]] = &[&["alloc", "rc", "Rc"], &["std", "sync", "Arc"]];
184+
static KNOWN_WRAPPER_TYS: &[Symbol] = &[sym::Rc, sym::Arc];
185185

186186
fn is_mutable_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span, tys: &mut DefIdSet) -> bool {
187187
match *ty.kind() {
188188
// primitive types are never mutable
189189
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Str => false,
190190
ty::Adt(adt, substs) => {
191191
tys.insert(adt.did()) && !ty.is_freeze(cx.tcx.at(span), cx.param_env)
192-
|| KNOWN_WRAPPER_TYS.iter().any(|path| match_def_path(cx, adt.did(), path))
192+
|| KNOWN_WRAPPER_TYS
193+
.iter()
194+
.any(|&sym| cx.tcx.is_diagnostic_item(sym, adt.did()))
193195
&& substs.types().any(|ty| is_mutable_ty(cx, ty, span, tys))
194196
},
195197
ty::Tuple(substs) => substs.iter().any(|ty| is_mutable_ty(cx, ty, span, tys)),

‎clippy_lints/src/let_underscore.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
2-
use clippy_utils::ty::{is_must_use_ty, match_type};
2+
use clippy_utils::ty::{is_must_use_ty, is_type_diagnostic_item, match_type};
33
use clippy_utils::{is_must_use_func_call, paths};
44
use if_chain::if_chain;
55
use rustc_hir::{Local, PatKind};
66
use rustc_lint::{LateContext, LateLintPass};
77
use rustc_middle::lint::in_external_macro;
88
use rustc_middle::ty::subst::GenericArgKind;
99
use rustc_session::{declare_lint_pass, declare_tool_lint};
10+
use rustc_span::{sym, Symbol};
1011

1112
declare_clippy_lint! {
1213
/// ### What it does
@@ -99,10 +100,9 @@ declare_clippy_lint! {
99100

100101
declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_MUST_USE, LET_UNDERSCORE_LOCK, LET_UNDERSCORE_DROP]);
101102

102-
const SYNC_GUARD_PATHS: [&[&str]; 6] = [
103-
&paths::MUTEX_GUARD,
104-
&paths::RWLOCK_READ_GUARD,
105-
&paths::RWLOCK_WRITE_GUARD,
103+
const SYNC_GUARD_SYMS: [Symbol; 3] = [sym::MutexGuard, sym::RwLockReadGuard, sym::RwLockWriteGuard];
104+
105+
const SYNC_GUARD_PATHS: [&[&str]; 3] = [
106106
&paths::PARKING_LOT_MUTEX_GUARD,
107107
&paths::PARKING_LOT_RWLOCK_READ_GUARD,
108108
&paths::PARKING_LOT_RWLOCK_WRITE_GUARD,
@@ -121,7 +121,10 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
121121
let init_ty = cx.typeck_results().expr_ty(init);
122122
let contains_sync_guard = init_ty.walk().any(|inner| match inner.unpack() {
123123
GenericArgKind::Type(inner_ty) => {
124-
SYNC_GUARD_PATHS.iter().any(|path| match_type(cx, inner_ty, path))
124+
SYNC_GUARD_SYMS
125+
.iter()
126+
.any(|&sym| is_type_diagnostic_item(cx, inner_ty, sym))
127+
|| SYNC_GUARD_PATHS.iter().any(|path| match_type(cx, inner_ty, path))
125128
},
126129

127130
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
@@ -134,7 +137,7 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
134137
"non-binding let on a synchronization lock",
135138
None,
136139
"consider using an underscore-prefixed named \
137-
binding or dropping explicitly with `std::mem::drop`"
140+
binding or dropping explicitly with `std::mem::drop`",
138141
);
139142
} else if init_ty.needs_drop(cx.tcx, cx.param_env) {
140143
span_lint_and_help(
@@ -144,7 +147,7 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
144147
"non-binding `let` on a type that implements `Drop`",
145148
None,
146149
"consider using an underscore-prefixed named \
147-
binding or dropping explicitly with `std::mem::drop`"
150+
binding or dropping explicitly with `std::mem::drop`",
148151
);
149152
} else if is_must_use_ty(cx, cx.typeck_results().expr_ty(init)) {
150153
span_lint_and_help(
@@ -153,7 +156,7 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
153156
local.span,
154157
"non-binding let on an expression with `#[must_use]` type",
155158
None,
156-
"consider explicitly using expression value"
159+
"consider explicitly using expression value",
157160
);
158161
} else if is_must_use_func_call(cx, init) {
159162
span_lint_and_help(
@@ -162,7 +165,7 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
162165
local.span,
163166
"non-binding let on a result of a `#[must_use]` function",
164167
None,
165-
"consider explicitly using function result"
168+
"consider explicitly using function result",
166169
);
167170
}
168171
}

‎clippy_lints/src/lib.register_internal.rs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,20 @@
33
// Manual edits will be overwritten.
44

55
store.register_group(true, "clippy::internal", Some("clippy_internal"), vec![
6-
LintId::of(utils::internal_lints::CLIPPY_LINTS_INTERNAL),
7-
LintId::of(utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS),
8-
LintId::of(utils::internal_lints::COMPILER_LINT_FUNCTIONS),
9-
LintId::of(utils::internal_lints::DEFAULT_DEPRECATION_REASON),
10-
LintId::of(utils::internal_lints::DEFAULT_LINT),
11-
LintId::of(utils::internal_lints::IF_CHAIN_STYLE),
12-
LintId::of(utils::internal_lints::INTERNING_DEFINED_SYMBOL),
13-
LintId::of(utils::internal_lints::INVALID_CLIPPY_VERSION_ATTRIBUTE),
14-
LintId::of(utils::internal_lints::INVALID_PATHS),
15-
LintId::of(utils::internal_lints::LINT_WITHOUT_LINT_PASS),
16-
LintId::of(utils::internal_lints::MISSING_CLIPPY_VERSION_ATTRIBUTE),
17-
LintId::of(utils::internal_lints::MISSING_MSRV_ATTR_IMPL),
18-
LintId::of(utils::internal_lints::OUTER_EXPN_EXPN_DATA),
19-
LintId::of(utils::internal_lints::PRODUCE_ICE),
20-
LintId::of(utils::internal_lints::UNNECESSARY_DEF_PATH),
21-
LintId::of(utils::internal_lints::UNNECESSARY_SYMBOL_STR),
6+
LintId::of(utils::internal_lints::clippy_lints_internal::CLIPPY_LINTS_INTERNAL),
7+
LintId::of(utils::internal_lints::collapsible_calls::COLLAPSIBLE_SPAN_LINT_CALLS),
8+
LintId::of(utils::internal_lints::compiler_lint_functions::COMPILER_LINT_FUNCTIONS),
9+
LintId::of(utils::internal_lints::if_chain_style::IF_CHAIN_STYLE),
10+
LintId::of(utils::internal_lints::interning_defined_symbol::INTERNING_DEFINED_SYMBOL),
11+
LintId::of(utils::internal_lints::interning_defined_symbol::UNNECESSARY_SYMBOL_STR),
12+
LintId::of(utils::internal_lints::invalid_paths::INVALID_PATHS),
13+
LintId::of(utils::internal_lints::lint_without_lint_pass::DEFAULT_DEPRECATION_REASON),
14+
LintId::of(utils::internal_lints::lint_without_lint_pass::DEFAULT_LINT),
15+
LintId::of(utils::internal_lints::lint_without_lint_pass::INVALID_CLIPPY_VERSION_ATTRIBUTE),
16+
LintId::of(utils::internal_lints::lint_without_lint_pass::LINT_WITHOUT_LINT_PASS),
17+
LintId::of(utils::internal_lints::lint_without_lint_pass::MISSING_CLIPPY_VERSION_ATTRIBUTE),
18+
LintId::of(utils::internal_lints::msrv_attr_impl::MISSING_MSRV_ATTR_IMPL),
19+
LintId::of(utils::internal_lints::outer_expn_data_pass::OUTER_EXPN_EXPN_DATA),
20+
LintId::of(utils::internal_lints::produce_ice::PRODUCE_ICE),
21+
LintId::of(utils::internal_lints::unnecessary_def_path::UNNECESSARY_DEF_PATH),
2222
])

‎clippy_lints/src/lib.register_lints.rs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,37 +4,37 @@
44

55
store.register_lints(&[
66
#[cfg(feature = "internal")]
7-
utils::internal_lints::CLIPPY_LINTS_INTERNAL,
7+
utils::internal_lints::clippy_lints_internal::CLIPPY_LINTS_INTERNAL,
88
#[cfg(feature = "internal")]
9-
utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS,
9+
utils::internal_lints::collapsible_calls::COLLAPSIBLE_SPAN_LINT_CALLS,
1010
#[cfg(feature = "internal")]
11-
utils::internal_lints::COMPILER_LINT_FUNCTIONS,
11+
utils::internal_lints::compiler_lint_functions::COMPILER_LINT_FUNCTIONS,
1212
#[cfg(feature = "internal")]
13-
utils::internal_lints::DEFAULT_DEPRECATION_REASON,
13+
utils::internal_lints::if_chain_style::IF_CHAIN_STYLE,
1414
#[cfg(feature = "internal")]
15-
utils::internal_lints::DEFAULT_LINT,
15+
utils::internal_lints::interning_defined_symbol::INTERNING_DEFINED_SYMBOL,
1616
#[cfg(feature = "internal")]
17-
utils::internal_lints::IF_CHAIN_STYLE,
17+
utils::internal_lints::interning_defined_symbol::UNNECESSARY_SYMBOL_STR,
1818
#[cfg(feature = "internal")]
19-
utils::internal_lints::INTERNING_DEFINED_SYMBOL,
19+
utils::internal_lints::invalid_paths::INVALID_PATHS,
2020
#[cfg(feature = "internal")]
21-
utils::internal_lints::INVALID_CLIPPY_VERSION_ATTRIBUTE,
21+
utils::internal_lints::lint_without_lint_pass::DEFAULT_DEPRECATION_REASON,
2222
#[cfg(feature = "internal")]
23-
utils::internal_lints::INVALID_PATHS,
23+
utils::internal_lints::lint_without_lint_pass::DEFAULT_LINT,
2424
#[cfg(feature = "internal")]
25-
utils::internal_lints::LINT_WITHOUT_LINT_PASS,
25+
utils::internal_lints::lint_without_lint_pass::INVALID_CLIPPY_VERSION_ATTRIBUTE,
2626
#[cfg(feature = "internal")]
27-
utils::internal_lints::MISSING_CLIPPY_VERSION_ATTRIBUTE,
27+
utils::internal_lints::lint_without_lint_pass::LINT_WITHOUT_LINT_PASS,
2828
#[cfg(feature = "internal")]
29-
utils::internal_lints::MISSING_MSRV_ATTR_IMPL,
29+
utils::internal_lints::lint_without_lint_pass::MISSING_CLIPPY_VERSION_ATTRIBUTE,
3030
#[cfg(feature = "internal")]
31-
utils::internal_lints::OUTER_EXPN_EXPN_DATA,
31+
utils::internal_lints::msrv_attr_impl::MISSING_MSRV_ATTR_IMPL,
3232
#[cfg(feature = "internal")]
33-
utils::internal_lints::PRODUCE_ICE,
33+
utils::internal_lints::outer_expn_data_pass::OUTER_EXPN_EXPN_DATA,
3434
#[cfg(feature = "internal")]
35-
utils::internal_lints::UNNECESSARY_DEF_PATH,
35+
utils::internal_lints::produce_ice::PRODUCE_ICE,
3636
#[cfg(feature = "internal")]
37-
utils::internal_lints::UNNECESSARY_SYMBOL_STR,
37+
utils::internal_lints::unnecessary_def_path::UNNECESSARY_DEF_PATH,
3838
almost_complete_letter_range::ALMOST_COMPLETE_LETTER_RANGE,
3939
approx_const::APPROX_CONSTANT,
4040
as_conversions::AS_CONVERSIONS,

‎clippy_lints/src/lib.rs

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -528,17 +528,23 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
528528
// all the internal lints
529529
#[cfg(feature = "internal")]
530530
{
531-
store.register_early_pass(|| Box::new(utils::internal_lints::ClippyLintsInternal));
532-
store.register_early_pass(|| Box::new(utils::internal_lints::ProduceIce));
533-
store.register_late_pass(|_| Box::new(utils::internal_lints::CollapsibleCalls));
534-
store.register_late_pass(|_| Box::new(utils::internal_lints::CompilerLintFunctions::new()));
535-
store.register_late_pass(|_| Box::new(utils::internal_lints::IfChainStyle));
536-
store.register_late_pass(|_| Box::new(utils::internal_lints::InvalidPaths));
537-
store.register_late_pass(|_| Box::<utils::internal_lints::InterningDefinedSymbol>::default());
538-
store.register_late_pass(|_| Box::<utils::internal_lints::LintWithoutLintPass>::default());
539-
store.register_late_pass(|_| Box::new(utils::internal_lints::UnnecessaryDefPath));
540-
store.register_late_pass(|_| Box::new(utils::internal_lints::OuterExpnDataPass));
541-
store.register_late_pass(|_| Box::new(utils::internal_lints::MsrvAttrImpl));
531+
store.register_early_pass(|| Box::new(utils::internal_lints::clippy_lints_internal::ClippyLintsInternal));
532+
store.register_early_pass(|| Box::new(utils::internal_lints::produce_ice::ProduceIce));
533+
store.register_late_pass(|_| Box::new(utils::internal_lints::collapsible_calls::CollapsibleCalls));
534+
store.register_late_pass(|_| {
535+
Box::new(utils::internal_lints::compiler_lint_functions::CompilerLintFunctions::new())
536+
});
537+
store.register_late_pass(|_| Box::new(utils::internal_lints::if_chain_style::IfChainStyle));
538+
store.register_late_pass(|_| Box::new(utils::internal_lints::invalid_paths::InvalidPaths));
539+
store.register_late_pass(|_| {
540+
Box::<utils::internal_lints::interning_defined_symbol::InterningDefinedSymbol>::default()
541+
});
542+
store.register_late_pass(|_| {
543+
Box::<utils::internal_lints::lint_without_lint_pass::LintWithoutLintPass>::default()
544+
});
545+
store.register_late_pass(|_| Box::<utils::internal_lints::unnecessary_def_path::UnnecessaryDefPath>::default());
546+
store.register_late_pass(|_| Box::new(utils::internal_lints::outer_expn_data_pass::OuterExpnDataPass));
547+
store.register_late_pass(|_| Box::new(utils::internal_lints::msrv_attr_impl::MsrvAttrImpl));
542548
}
543549

544550
let arithmetic_side_effects_allowed = conf.arithmetic_side_effects_allowed.clone();

‎clippy_lints/src/loops/needless_range_loop.rs

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then};
33
use clippy_utils::source::snippet;
44
use clippy_utils::ty::has_iter_method;
55
use clippy_utils::visitors::is_local_used;
6-
use clippy_utils::{contains_name, higher, is_integer_const, match_trait_method, paths, sugg, SpanlessEq};
6+
use clippy_utils::{contains_name, higher, is_integer_const, sugg, SpanlessEq};
77
use if_chain::if_chain;
88
use rustc_ast::ast;
99
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
@@ -263,7 +263,8 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
263263
match res {
264264
Res::Local(hir_id) => {
265265
let parent_def_id = self.cx.tcx.hir().get_parent_item(expr.hir_id);
266-
let extent = self.cx
266+
let extent = self
267+
.cx
267268
.tcx
268269
.region_scope_tree(parent_def_id)
269270
.var_scope(hir_id.local_id)
@@ -274,11 +275,12 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
274275
(Some(extent), self.cx.typeck_results().node_type(seqexpr.hir_id)),
275276
);
276277
} else {
277-
self.indexed_indirectly.insert(seqvar.segments[0].ident.name, Some(extent));
278+
self.indexed_indirectly
279+
.insert(seqvar.segments[0].ident.name, Some(extent));
278280
}
279-
return false; // no need to walk further *on the variable*
280-
}
281-
Res::Def(DefKind::Static (_)| DefKind::Const, ..) => {
281+
return false; // no need to walk further *on the variable*
282+
},
283+
Res::Def(DefKind::Static(_) | DefKind::Const, ..) => {
282284
if index_used_directly {
283285
self.indexed_directly.insert(
284286
seqvar.segments[0].ident.name,
@@ -287,8 +289,8 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
287289
} else {
288290
self.indexed_indirectly.insert(seqvar.segments[0].ident.name, None);
289291
}
290-
return false; // no need to walk further *on the variable*
291-
}
292+
return false; // no need to walk further *on the variable*
293+
},
292294
_ => (),
293295
}
294296
}
@@ -302,17 +304,26 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
302304
if_chain! {
303305
// a range index op
304306
if let ExprKind::MethodCall(meth, args_0, [args_1, ..], _) = &expr.kind;
305-
if (meth.ident.name == sym::index && match_trait_method(self.cx, expr, &paths::INDEX))
306-
|| (meth.ident.name == sym::index_mut && match_trait_method(self.cx, expr, &paths::INDEX_MUT));
307+
if let Some(trait_id) = self
308+
.cx
309+
.typeck_results()
310+
.type_dependent_def_id(expr.hir_id)
311+
.and_then(|def_id| self.cx.tcx.trait_of_item(def_id));
312+
if (meth.ident.name == sym::index && self.cx.tcx.lang_items().index_trait() == Some(trait_id))
313+
|| (meth.ident.name == sym::index_mut && self.cx.tcx.lang_items().index_mut_trait() == Some(trait_id));
307314
if !self.check(args_1, args_0, expr);
308-
then { return }
315+
then {
316+
return;
317+
}
309318
}
310319

311320
if_chain! {
312321
// an index op
313322
if let ExprKind::Index(seqexpr, idx) = expr.kind;
314323
if !self.check(idx, seqexpr, expr);
315-
then { return }
324+
then {
325+
return;
326+
}
316327
}
317328

318329
if_chain! {

0 commit comments

Comments
 (0)