Skip to content

Commit d588291

Browse files
JarchoJarcho
committed
Provide different suggestions for constructors.
1 parent 2b45008 commit d588291

15 files changed

+116
-85
lines changed

‎clippy_lints/src/drop_forget_ref.rs

+22-36
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use clippy_utils::diagnostics::span_lint_and_note;
2-
use clippy_utils::is_diagnostic_item;
32
use clippy_utils::ty::is_copy;
43
use if_chain::if_chain;
54
use rustc_hir::{Expr, ExprKind};
@@ -118,49 +117,36 @@ declare_lint_pass!(DropForgetRef => [DROP_REF, FORGET_REF, DROP_COPY, FORGET_COP
118117
impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
119118
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
120119
if_chain! {
121-
if let ExprKind::Call(path, args) = expr.kind;
120+
if let ExprKind::Call(path, [arg]) = expr.kind;
122121
if let ExprKind::Path(ref qpath) = path.kind;
123-
if args.len() == 1;
124122
if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id();
125123
then {
126-
let lint;
127-
let msg;
128-
let arg = &args[0];
129124
let arg_ty = cx.typeck_results().expr_ty(arg);
130125

131-
if let ty::Ref(..) = arg_ty.kind() {
132-
if is_diagnostic_item(cx, def_id, sym::mem_drop) {
133-
lint = DROP_REF;
134-
msg = DROP_REF_SUMMARY.to_string();
135-
} else if is_diagnostic_item(cx, def_id, sym::mem_forget) {
136-
lint = FORGET_REF;
137-
msg = FORGET_REF_SUMMARY.to_string();
138-
} else {
139-
return;
126+
let (lint, msg) = if let ty::Ref(..) = arg_ty.kind() {
127+
match cx.tcx.get_diagnostic_name(def_id) {
128+
Some(sym::mem_drop) => (DROP_REF, DROP_REF_SUMMARY),
129+
Some(sym::mem_forget) => (FORGET_REF, FORGET_REF_SUMMARY),
130+
_ => return,
140131
}
141-
span_lint_and_note(cx,
142-
lint,
143-
expr.span,
144-
&msg,
145-
Some(arg.span),
146-
&format!("argument has type `{}`", arg_ty));
147132
} else if is_copy(cx, arg_ty) {
148-
if is_diagnostic_item(cx, def_id, sym::mem_drop) {
149-
lint = DROP_COPY;
150-
msg = DROP_COPY_SUMMARY.to_string();
151-
} else if is_diagnostic_item(cx, def_id, sym::mem_forget) {
152-
lint = FORGET_COPY;
153-
msg = FORGET_COPY_SUMMARY.to_string();
154-
} else {
155-
return;
133+
match cx.tcx.get_diagnostic_name(def_id) {
134+
Some(sym::mem_drop) => (DROP_COPY, DROP_COPY_SUMMARY),
135+
Some(sym::mem_forget) => (FORGET_COPY, FORGET_COPY_SUMMARY),
136+
_ => return,
156137
}
157-
span_lint_and_note(cx,
158-
lint,
159-
expr.span,
160-
&msg,
161-
Some(arg.span),
162-
&format!("argument has type {}", arg_ty));
163-
}
138+
} else {
139+
return;
140+
};
141+
142+
span_lint_and_note(
143+
cx,
144+
lint,
145+
expr.span,
146+
msg,
147+
Some(arg.span),
148+
&format!("argument has type `{}`", arg_ty)
149+
);
164150
}
165151
}
166152
}

‎clippy_lints/src/infinite_iter.rs

+11-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint;
2+
use clippy_utils::higher;
23
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
3-
use clippy_utils::{higher, path_def_id};
44
use rustc_hir::{BorrowKind, Expr, ExprKind};
55
use rustc_lint::{LateContext, LateLintPass};
66
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -167,9 +167,16 @@ fn is_infinite(cx: &LateContext<'_>, expr: &Expr<'_>) -> Finiteness {
167167
},
168168
ExprKind::Block(block, _) => block.expr.as_ref().map_or(Finite, |e| is_infinite(cx, e)),
169169
ExprKind::Box(e) | ExprKind::AddrOf(BorrowKind::Ref, _, e) => is_infinite(cx, e),
170-
ExprKind::Call(path, _) => path_def_id(cx, path)
171-
.map_or(false, |id| cx.tcx.is_diagnostic_item(sym::iter_repeat, id))
172-
.into(),
170+
ExprKind::Call(path, _) => {
171+
if let ExprKind::Path(ref qpath) = path.kind {
172+
cx.qpath_res(qpath, path.hir_id)
173+
.opt_def_id()
174+
.map_or(false, |id| cx.tcx.is_diagnostic_item(sym::iter_repeat, id))
175+
.into()
176+
} else {
177+
Finite
178+
}
179+
},
173180
ExprKind::Struct(..) => higher::Range::hir(expr).map_or(false, |r| r.end.is_none()).into(),
174181
_ => Finite,
175182
}

‎clippy_lints/src/loops/while_let_on_iterator.rs

+4-5
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ use super::WHILE_LET_ON_ITERATOR;
22
use clippy_utils::diagnostics::span_lint_and_sugg;
33
use clippy_utils::higher;
44
use clippy_utils::source::snippet_with_applicability;
5-
use clippy_utils::{get_enclosing_loop_or_closure, is_lang_item, is_refutable, is_trait_method, visitors::is_res_used};
5+
use clippy_utils::{get_enclosing_loop_or_closure, is_lang_ctor, is_refutable, is_trait_method, visitors::is_res_used};
66
use if_chain::if_chain;
77
use rustc_errors::Applicability;
88
use rustc_hir::intravisit::{walk_expr, Visitor};
9-
use rustc_hir::{def::Res, Expr, ExprKind, HirId, LangItem, Local, Mutability, PatKind, QPath, UnOp};
9+
use rustc_hir::{def::Res, Expr, ExprKind, HirId, LangItem, Local, Mutability, PatKind, UnOp};
1010
use rustc_lint::LateContext;
1111
use rustc_middle::ty::adjustment::Adjust;
1212
use rustc_span::{symbol::sym, Symbol};
@@ -15,9 +15,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
1515
let (scrutinee_expr, iter_expr_struct, iter_expr, some_pat, loop_expr) = if_chain! {
1616
if let Some(higher::WhileLet { if_then, let_pat, let_expr }) = higher::WhileLet::hir(expr);
1717
// check for `Some(..)` pattern
18-
if let PatKind::TupleStruct(QPath::Resolved(None, pat_path), some_pat, _) = let_pat.kind;
19-
if let Res::Def(_, pat_did) = pat_path.res;
20-
if is_lang_item(cx, pat_did, LangItem::OptionSome);
18+
if let PatKind::TupleStruct(ref pat_path, some_pat, _) = let_pat.kind;
19+
if is_lang_ctor(cx, pat_path, LangItem::OptionSome);
2120
// check for call to `Iterator::next`
2221
if let ExprKind::MethodCall(method_name, [iter_expr], _) = let_expr.kind;
2322
if method_name.ident.name == sym::next;

‎clippy_lints/src/mem_forget.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use clippy_utils::diagnostics::span_lint;
2-
use clippy_utils::is_diagnostic_item;
32
use rustc_hir::{Expr, ExprKind};
43
use rustc_lint::{LateContext, LateLintPass};
54
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -33,7 +32,7 @@ impl<'tcx> LateLintPass<'tcx> for MemForget {
3332
if let ExprKind::Call(path_expr, [ref first_arg, ..]) = e.kind {
3433
if let ExprKind::Path(ref qpath) = path_expr.kind {
3534
if let Some(def_id) = cx.qpath_res(qpath, path_expr.hir_id).opt_def_id() {
36-
if is_diagnostic_item(cx, def_id, sym::mem_forget) {
35+
if cx.tcx.is_diagnostic_item(sym::mem_forget, def_id) {
3736
let forgot_ty = cx.typeck_results().expr_ty(first_arg);
3837

3938
if forgot_ty.ty_adt_def().map_or(false, |def| def.has_dtor(cx.tcx)) {

‎clippy_lints/src/mem_replace.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
22
use clippy_utils::source::{snippet, snippet_with_applicability};
33
use clippy_utils::ty::is_non_aggregate_primitive_type;
4-
use clippy_utils::{is_default_equivalent, is_diagnostic_item, is_lang_ctor, meets_msrv, msrvs};
4+
use clippy_utils::{is_default_equivalent, is_lang_ctor, meets_msrv, msrvs};
55
use if_chain::if_chain;
66
use rustc_errors::Applicability;
77
use rustc_hir::LangItem::OptionNone;
@@ -249,7 +249,7 @@ impl<'tcx> LateLintPass<'tcx> for MemReplace {
249249
if let ExprKind::Call(func, func_args) = expr.kind;
250250
if let ExprKind::Path(ref func_qpath) = func.kind;
251251
if let Some(def_id) = cx.qpath_res(func_qpath, func.hir_id).opt_def_id();
252-
if is_diagnostic_item(cx, def_id, sym::mem_replace);
252+
if cx.tcx.is_diagnostic_item(sym::mem_replace, def_id);
253253
if let [dest, src] = func_args;
254254
then {
255255
check_replace_option_with_none(cx, src, dest, expr.span);

‎clippy_lints/src/methods/inefficient_to_string.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::source::snippet_with_applicability;
33
use clippy_utils::ty::{is_type_diagnostic_item, walk_ptrs_ty_depth};
4-
use clippy_utils::{is_diagnostic_item, match_def_path, paths};
4+
use clippy_utils::{match_def_path, paths};
55
use if_chain::if_chain;
66
use rustc_errors::Applicability;
77
use rustc_hir as hir;
@@ -60,7 +60,7 @@ fn specializes_tostring(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
6060
}
6161

6262
if let ty::Adt(adt, substs) = ty.kind() {
63-
is_diagnostic_item(cx, adt.did, sym::Cow) && substs.type_at(1).is_str()
63+
cx.tcx.is_diagnostic_item(sym::Cow, adt.did) && substs.type_at(1).is_str()
6464
} else {
6565
false
6666
}

‎clippy_lints/src/redundant_clone.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then};
22
use clippy_utils::source::snippet_opt;
33
use clippy_utils::ty::{has_drop, is_copy, is_type_diagnostic_item, walk_ptrs_ty_depth};
4-
use clippy_utils::{fn_has_unsatisfiable_preds, is_lang_item, match_def_path, paths};
4+
use clippy_utils::{fn_has_unsatisfiable_preds, match_def_path, paths};
55
use if_chain::if_chain;
66
use rustc_data_structures::{fx::FxHashMap, transitive_relation::TransitiveRelation};
77
use rustc_errors::Applicability;
@@ -135,7 +135,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone {
135135
}
136136

137137
if let ty::Adt(def, _) = arg_ty.kind() {
138-
if is_lang_item(cx, def.did, LangItem::ManuallyDrop) {
138+
if cx.tcx.lang_items().require(LangItem::ManuallyDrop).ok() == Some(def.did) {
139139
continue;
140140
}
141141
}

‎clippy_lints/src/size_of_in_element_count.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
//! expecting a count of T
33
44
use clippy_utils::diagnostics::span_lint_and_help;
5-
use clippy_utils::{is_diagnostic_item, match_def_path, paths};
5+
use clippy_utils::{match_def_path, paths};
66
use if_chain::if_chain;
77
use rustc_hir::BinOpKind;
88
use rustc_hir::{Expr, ExprKind};
@@ -45,8 +45,7 @@ fn get_size_of_ty<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, inverted:
4545
if !inverted;
4646
if let ExprKind::Path(ref count_func_qpath) = count_func.kind;
4747
if let Some(def_id) = cx.qpath_res(count_func_qpath, count_func.hir_id).opt_def_id();
48-
if is_diagnostic_item(cx, def_id, sym::mem_size_of)
49-
|| is_diagnostic_item(cx, def_id, sym::mem_size_of_val);
48+
if matches!(cx.tcx.get_diagnostic_name(def_id), Some(sym::mem_size_of | sym::mem_size_of_val));
5049
then {
5150
cx.typeck_results().node_substs(count_func.hir_id).types().next()
5251
} else {

‎clippy_lints/src/useless_conversion.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
22
use clippy_utils::source::{snippet, snippet_with_macro_callsite};
33
use clippy_utils::sugg::Sugg;
44
use clippy_utils::ty::{is_type_diagnostic_item, same_type_and_consts};
5-
use clippy_utils::{get_parent_expr, is_lang_item, is_trait_method, match_def_path, paths};
5+
use clippy_utils::{get_parent_expr, is_trait_method, match_def_path, paths};
66
use if_chain::if_chain;
77
use rustc_errors::Applicability;
88
use rustc_hir::{Expr, ExprKind, HirId, LangItem, MatchSource};
@@ -154,7 +154,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
154154
}
155155

156156
if_chain! {
157-
if is_lang_item(cx, def_id, LangItem::FromFrom);
157+
if cx.tcx.lang_items().require(LangItem::FromFrom).ok() == Some(def_id);
158158
if same_type_and_consts(a, b);
159159

160160
then {

‎clippy_lints/src/utils/internal_lints.rs

+33-4
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use rustc_hir::{
2424
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
2525
use rustc_middle::hir::nested_filter;
2626
use rustc_middle::mir::interpret::{Allocation, ConstValue, GlobalAlloc};
27-
use rustc_middle::ty::{self, AssocKind, Ty};
27+
use rustc_middle::ty::{self, AssocKind, DefIdTree, Ty};
2828
use rustc_semver::RustcVersion;
2929
use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
3030
use rustc_span::source_map::Spanned;
@@ -905,15 +905,32 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
905905
return;
906906
};
907907

908+
let has_ctor = match cx.tcx.def_kind(def_id) {
909+
DefKind::Struct => {
910+
let variant = cx.tcx.adt_def(def_id).non_enum_variant();
911+
variant.ctor_def_id.is_some() && variant.fields.iter().all(|f| f.vis.is_public())
912+
}
913+
DefKind::Variant =>
914+
cx.tcx.parent(def_id).map_or(false, |adt_id| {
915+
let variant = cx.tcx.adt_def(adt_id).variant_with_id(def_id);
916+
variant.ctor_def_id.is_some() && variant.fields.iter().all(|f| f.vis.is_public())
917+
}),
918+
_ => false,
919+
};
920+
908921
let mut app = Applicability::MachineApplicable;
909922
let cx_snip = snippet_with_applicability(cx, cx_arg.span, "..", &mut app);
910923
let def_snip = snippet_with_applicability(cx, def_arg.span, "..", &mut app);
911924
let sugg = match (which_path, item) {
912925
// match_def_path
926+
(0, Item::DiagnosticItem(item)) if has_ctor =>
927+
format!("is_diagnostic_item_or_ctor({}, {}, sym::{})", cx_snip, def_snip, item),
928+
(0, Item::LangItem(item)) if has_ctor =>
929+
format!("is_lang_item_or_ctor({}, {}, LangItem::{})", cx_snip, def_snip, item),
913930
(0, Item::DiagnosticItem(item)) =>
914-
format!("is_diagnostic_item({}, {}, sym::{})", cx_snip, def_snip, item),
931+
format!("{}.tcx.is_diagnostic_item(sym::{}, {})", cx_snip, item, def_snip),
915932
(0, Item::LangItem(item)) =>
916-
format!("is_lang_item({}, {}, LangItem::{})", cx_snip, def_snip, item),
933+
format!("{}.tcx.lang_items().require(LangItem::{}).ok() == Some({})", cx_snip, item, def_snip),
917934
// match_trait_method
918935
(1, Item::DiagnosticItem(item)) =>
919936
format!("is_trait_method({}, {}, sym::{})", cx_snip, def_snip, item),
@@ -923,12 +940,24 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
923940
(2, Item::LangItem(item)) =>
924941
format!("is_type_lang_item({}, {}, LangItem::{})", cx_snip, def_snip, item),
925942
// is_expr_path_def_path
943+
(3, Item::DiagnosticItem(item)) if has_ctor =>
944+
format!(
945+
"path_res({}, {}).opt_def_id()\
946+
.map_or(false, |id| is_diagnostic_item_or_ctor({}, id, sym::{})",
947+
cx_snip, def_snip, cx_snip, item,
948+
),
949+
(3, Item::LangItem(item)) if has_ctor =>
950+
format!(
951+
"path_res({}, {}).opt_def_id()\
952+
.map_or(false, |id| is_lang_item_or_ctor({}, id, LangItem::{})",
953+
cx_snip, def_snip, cx_snip, item,
954+
),
926955
(3, Item::DiagnosticItem(item)) =>
927956
format!("is_expr_diagnostic_item({}, {}, sym::{})", cx_snip, def_snip, item),
928957
(3, Item::LangItem(item)) =>
929958
format!(
930959
"path_res({}, {}).opt_def_id()\
931-
.map_or(false, |id| is_lang_item({}, id, LangItem::{}))",
960+
.map_or(false, |id| {}.tcx.lang_items().require(LangItem::{}).ok() == Some(id))",
932961
cx_snip, def_snip, cx_snip, item,
933962
),
934963
_ => return,

‎clippy_utils/src/lib.rs

+17-5
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,20 @@ pub fn is_lang_ctor(cx: &LateContext<'_>, qpath: &QPath<'_>, lang_item: LangItem
237237
false
238238
}
239239

240-
/// Checks if the `DefId` matches the given diagnostic item.
241-
pub fn is_diagnostic_item(cx: &LateContext<'_>, did: DefId, item: Symbol) -> bool {
240+
/// Checks if a `QPath` resolves to a constructor of a diagnostic item.
241+
pub fn is_diagnostic_ctor(cx: &LateContext<'_>, qpath: &QPath<'_>, diagnostic_item: Symbol) -> bool {
242+
if let QPath::Resolved(_, path) = qpath {
243+
if let Res::Def(DefKind::Ctor(..), ctor_id) = path.res {
244+
if let Some(variant_id) = cx.tcx.parent(ctor_id) {
245+
return cx.tcx.is_diagnostic_item(diagnostic_item, variant_id);
246+
}
247+
}
248+
}
249+
false
250+
}
251+
252+
/// Checks if the `DefId` matches the given diagnostic item or it's constructor.
253+
pub fn is_diagnostic_item_or_ctor(cx: &LateContext<'_>, did: DefId, item: Symbol) -> bool {
242254
let did = match cx.tcx.def_kind(did) {
243255
DefKind::Ctor(..) => match cx.tcx.parent(did) {
244256
Some(did) => did,
@@ -255,8 +267,8 @@ pub fn is_diagnostic_item(cx: &LateContext<'_>, did: DefId, item: Symbol) -> boo
255267
cx.tcx.is_diagnostic_item(item, did)
256268
}
257269

258-
/// Checks if the `DefId` matches the given `LangItem`.
259-
pub fn is_lang_item(cx: &LateContext<'_>, did: DefId, item: LangItem) -> bool {
270+
/// Checks if the `DefId` matches the given `LangItem` or it's constructor.
271+
pub fn is_lang_item_or_ctor(cx: &LateContext<'_>, did: DefId, item: LangItem) -> bool {
260272
let did = match cx.tcx.def_kind(did) {
261273
DefKind::Ctor(..) => match cx.tcx.parent(did) {
262274
Some(did) => did,
@@ -404,7 +416,7 @@ pub fn is_expr_path_def_path(cx: &LateContext<'_>, expr: &Expr<'_>, segments: &[
404416
/// If the expression is a path, resolves it to a `DefId` and checks if it matches the given
405417
/// diagnostic item.
406418
pub fn is_expr_diagnostic_item(cx: &LateContext<'_>, expr: &Expr<'_>, diag_item: Symbol) -> bool {
407-
path_def_id(cx, expr).map_or(false, |id| cx.tcx.is_diagnostic_item(diag_item, id))
419+
path_def_id(cx, expr).map_or(false, |id| is_diagnostic_item_or_ctor(cx, id, diag_item))
408420
}
409421

410422
/// THIS METHOD IS DEPRECATED and will eventually be removed since it does not match against the

‎tests/ui-internal/unnecessary_def_path.fixed

+6-6
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ extern crate rustc_span;
1414
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item, match_type};
1515
#[allow(unused)]
1616
use clippy_utils::{
17-
is_diagnostic_item, is_expr_diagnostic_item, is_expr_path_def_path, is_lang_ctor, is_lang_item, is_trait_method,
18-
match_def_path, match_trait_method, path_res,
17+
is_diagnostic_ctor, is_diagnostic_item_or_ctor, is_expr_diagnostic_item, is_expr_path_def_path, is_lang_ctor,
18+
is_lang_item_or_ctor, is_trait_method, match_def_path, match_trait_method, path_res,
1919
};
2020

2121
#[allow(unused)]
@@ -48,14 +48,14 @@ fn _f<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, did: DefId, expr: &Expr<'_>) {
4848
let _ = is_type_lang_item(cx, ty, LangItem::OwnedBox);
4949
let _ = is_type_diagnostic_item(cx, ty, sym::maybe_uninit_uninit);
5050

51-
let _ = is_lang_item(cx, did, LangItem::OwnedBox);
52-
let _ = is_diagnostic_item(cx, did, sym::Option);
53-
let _ = is_lang_item(cx, did, LangItem::OptionSome);
51+
let _ = cx.tcx.lang_items().require(LangItem::OwnedBox).ok() == Some(did);
52+
let _ = cx.tcx.is_diagnostic_item(sym::Option, did);
53+
let _ = is_lang_item_or_ctor(cx, did, LangItem::OptionSome);
5454

5555
let _ = is_trait_method(cx, expr, sym::AsRef);
5656

5757
let _ = is_expr_diagnostic_item(cx, expr, sym::Option);
58-
let _ = path_res(cx, expr).opt_def_id().map_or(false, |id| is_lang_item(cx, id, LangItem::IteratorNext));
58+
let _ = path_res(cx, expr).opt_def_id().map_or(false, |id| cx.tcx.lang_items().require(LangItem::IteratorNext).ok() == Some(id));
5959
}
6060

6161
fn main() {}

0 commit comments

Comments
 (0)