Skip to content

Commit

Permalink
Provide different suggestions for constructors.
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarcho committed May 27, 2022
1 parent c0a48b9 commit 5e63922
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 34 deletions.
15 changes: 11 additions & 4 deletions clippy_lints/src/infinite_iter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::higher;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
use clippy_utils::{higher, path_def_id};
use rustc_hir::{BorrowKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -167,9 +167,16 @@ fn is_infinite(cx: &LateContext<'_>, expr: &Expr<'_>) -> Finiteness {
},
ExprKind::Block(block, _) => block.expr.as_ref().map_or(Finite, |e| is_infinite(cx, e)),
ExprKind::Box(e) | ExprKind::AddrOf(BorrowKind::Ref, _, e) => is_infinite(cx, e),
ExprKind::Call(path, _) => path_def_id(cx, path)
.map_or(false, |id| cx.tcx.is_diagnostic_item(sym::iter_repeat, id))
.into(),
ExprKind::Call(path, _) => {
if let ExprKind::Path(ref qpath) = path.kind {
cx.qpath_res(qpath, path.hir_id)
.opt_def_id()
.map_or(false, |id| cx.tcx.is_diagnostic_item(sym::iter_repeat, id))
.into()
} else {
Finite
}
},
ExprKind::Struct(..) => higher::Range::hir(expr).map_or(false, |r| r.end.is_none()).into(),
_ => Finite,
}
Expand Down
9 changes: 4 additions & 5 deletions clippy_lints/src/loops/while_let_on_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ use super::WHILE_LET_ON_ITERATOR;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::higher;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{get_enclosing_loop_or_closure, is_lang_item, is_refutable, is_trait_method, visitors::is_res_used};
use clippy_utils::{get_enclosing_loop_or_closure, is_lang_ctor, is_refutable, is_trait_method, visitors::is_res_used};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, Visitor};
use rustc_hir::{def::Res, Expr, ExprKind, HirId, LangItem, Local, Mutability, PatKind, QPath, UnOp};
use rustc_hir::{def::Res, Expr, ExprKind, HirId, LangItem, Local, Mutability, PatKind, UnOp};
use rustc_lint::LateContext;
use rustc_middle::ty::adjustment::Adjust;
use rustc_span::{symbol::sym, Symbol};
Expand All @@ -15,9 +15,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
let (scrutinee_expr, iter_expr_struct, iter_expr, some_pat, loop_expr) = if_chain! {
if let Some(higher::WhileLet { if_then, let_pat, let_expr }) = higher::WhileLet::hir(expr);
// check for `Some(..)` pattern
if let PatKind::TupleStruct(QPath::Resolved(None, pat_path), some_pat, _) = let_pat.kind;
if let Res::Def(_, pat_did) = pat_path.res;
if is_lang_item(cx, pat_did, LangItem::OptionSome);
if let PatKind::TupleStruct(ref pat_path, some_pat, _) = let_pat.kind;
if is_lang_ctor(cx, pat_path, LangItem::OptionSome);
// check for call to `Iterator::next`
if let ExprKind::MethodCall(method_name, [iter_expr], _) = let_expr.kind;
if method_name.ident.name == sym::next;
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/methods/inefficient_to_string.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::{is_type_diagnostic_item, walk_ptrs_ty_depth};
use clippy_utils::{is_diagnostic_item, match_def_path, paths};
use clippy_utils::{match_def_path, paths};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
Expand Down Expand Up @@ -60,7 +60,7 @@ fn specializes_tostring(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
}

if let ty::Adt(adt, substs) = ty.kind() {
is_diagnostic_item(cx, adt.did(), sym::Cow) && substs.type_at(1).is_str()
cx.tcx.is_diagnostic_item(sym::Cow, adt.did()) && substs.type_at(1).is_str()
} else {
false
}
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/useless_conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
use clippy_utils::source::{snippet, snippet_with_macro_callsite};
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{is_type_diagnostic_item, same_type_and_consts};
use clippy_utils::{get_parent_expr, is_lang_item, is_trait_method, match_def_path, paths};
use clippy_utils::{get_parent_expr, is_trait_method, match_def_path, paths};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, HirId, LangItem, MatchSource};
Expand Down Expand Up @@ -154,7 +154,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
}

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

then {
Expand Down
38 changes: 34 additions & 4 deletions clippy_lints/src/utils/internal_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ use rustc_hir::{
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
use rustc_middle::hir::nested_filter;
use rustc_middle::mir::interpret::{Allocation, ConstValue, GlobalAlloc};
use rustc_middle::ty::{self, fast_reject::SimplifiedTypeGen, subst::GenericArgKind, AssocKind, FloatTy, Ty};
use rustc_middle::ty::{
self, fast_reject::SimplifiedTypeGen, subst::GenericArgKind, AssocKind, DefIdTree, FloatTy, Ty,
};
use rustc_semver::RustcVersion;
use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::Spanned;
Expand Down Expand Up @@ -914,15 +916,31 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
return;
};

let has_ctor = match cx.tcx.def_kind(def_id) {
DefKind::Struct => {
let variant = cx.tcx.adt_def(def_id).non_enum_variant();
variant.ctor_def_id.is_some() && variant.fields.iter().all(|f| f.vis.is_public())
}
DefKind::Variant => {
let variant = cx.tcx.adt_def(cx.tcx.parent(def_id)).variant_with_id(def_id);
variant.ctor_def_id.is_some() && variant.fields.iter().all(|f| f.vis.is_public())
}
_ => false,
};

let mut app = Applicability::MachineApplicable;
let cx_snip = snippet_with_applicability(cx, cx_arg.span, "..", &mut app);
let def_snip = snippet_with_applicability(cx, def_arg.span, "..", &mut app);
let sugg = match (which_path, item) {
// match_def_path
(0, Item::DiagnosticItem(item)) if has_ctor =>
format!("is_diagnostic_item_or_ctor({}, {}, sym::{})", cx_snip, def_snip, item),
(0, Item::LangItem(item)) if has_ctor =>
format!("is_lang_item_or_ctor({}, {}, LangItem::{})", cx_snip, def_snip, item),
(0, Item::DiagnosticItem(item)) =>
format!("is_diagnostic_item({}, {}, sym::{})", cx_snip, def_snip, item),
format!("{}.tcx.is_diagnostic_item(sym::{}, {})", cx_snip, item, def_snip),
(0, Item::LangItem(item)) =>
format!("is_lang_item({}, {}, LangItem::{})", cx_snip, def_snip, item),
format!("{}.tcx.lang_items().require(LangItem::{}).ok() == Some({})", cx_snip, item, def_snip),
// match_trait_method
(1, Item::DiagnosticItem(item)) =>
format!("is_trait_method({}, {}, sym::{})", cx_snip, def_snip, item),
Expand All @@ -932,12 +950,24 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
(2, Item::LangItem(item)) =>
format!("is_type_lang_item({}, {}, LangItem::{})", cx_snip, def_snip, item),
// is_expr_path_def_path
(3, Item::DiagnosticItem(item)) if has_ctor =>
format!(
"path_res({}, {}).opt_def_id()\
.map_or(false, |id| is_diagnostic_item_or_ctor({}, id, sym::{})",
cx_snip, def_snip, cx_snip, item,
),
(3, Item::LangItem(item)) if has_ctor =>
format!(
"path_res({}, {}).opt_def_id()\
.map_or(false, |id| is_lang_item_or_ctor({}, id, LangItem::{})",
cx_snip, def_snip, cx_snip, item,
),
(3, Item::DiagnosticItem(item)) =>
format!("is_expr_diagnostic_item({}, {}, sym::{})", cx_snip, def_snip, item),
(3, Item::LangItem(item)) =>
format!(
"path_res({}, {}).opt_def_id()\
.map_or(false, |id| is_lang_item({}, id, LangItem::{}))",
.map_or(false, |id| {}.tcx.lang_items().require(LangItem::{}).ok() == Some(id))",
cx_snip, def_snip, cx_snip, item,
),
_ => return,
Expand Down
20 changes: 15 additions & 5 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,18 @@ pub fn is_lang_ctor(cx: &LateContext<'_>, qpath: &QPath<'_>, lang_item: LangItem
false
}

/// Checks if the `DefId` matches the given diagnostic item.
pub fn is_diagnostic_item(cx: &LateContext<'_>, did: DefId, item: Symbol) -> bool {
/// Checks if a `QPath` resolves to a constructor of a diagnostic item.
pub fn is_diagnostic_ctor(cx: &LateContext<'_>, qpath: &QPath<'_>, diagnostic_item: Symbol) -> bool {
if let QPath::Resolved(_, path) = qpath {
if let Res::Def(DefKind::Ctor(..), ctor_id) = path.res {
return cx.tcx.is_diagnostic_item(diagnostic_item, cx.tcx.parent(ctor_id));
}
}
false
}

/// Checks if the `DefId` matches the given diagnostic item or it's constructor.
pub fn is_diagnostic_item_or_ctor(cx: &LateContext<'_>, did: DefId, item: Symbol) -> bool {
let did = match cx.tcx.def_kind(did) {
DefKind::Ctor(..) => cx.tcx.parent(did),
// Constructors for types in external crates seem to have `DefKind::Variant`
Expand All @@ -257,8 +267,8 @@ pub fn is_diagnostic_item(cx: &LateContext<'_>, did: DefId, item: Symbol) -> boo
cx.tcx.is_diagnostic_item(item, did)
}

/// Checks if the `DefId` matches the given `LangItem`.
pub fn is_lang_item(cx: &LateContext<'_>, did: DefId, item: LangItem) -> bool {
/// Checks if the `DefId` matches the given `LangItem` or it's constructor.
pub fn is_lang_item_or_ctor(cx: &LateContext<'_>, did: DefId, item: LangItem) -> bool {
let did = match cx.tcx.def_kind(did) {
DefKind::Ctor(..) => cx.tcx.parent(did),
// Constructors for types in external crates seem to have `DefKind::Variant`
Expand Down Expand Up @@ -403,7 +413,7 @@ pub fn is_expr_path_def_path(cx: &LateContext<'_>, expr: &Expr<'_>, segments: &[
/// If the expression is a path, resolves it to a `DefId` and checks if it matches the given
/// diagnostic item.
pub fn is_expr_diagnostic_item(cx: &LateContext<'_>, expr: &Expr<'_>, diag_item: Symbol) -> bool {
path_def_id(cx, expr).map_or(false, |id| cx.tcx.is_diagnostic_item(diag_item, id))
path_def_id(cx, expr).map_or(false, |id| is_diagnostic_item_or_ctor(cx, id, diag_item))
}

/// THIS METHOD IS DEPRECATED and will eventually be removed since it does not match against the
Expand Down
12 changes: 6 additions & 6 deletions tests/ui-internal/unnecessary_def_path.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ extern crate rustc_span;
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item, match_type};
#[allow(unused)]
use clippy_utils::{
is_diagnostic_item, is_expr_diagnostic_item, is_expr_path_def_path, is_lang_ctor, is_lang_item, is_trait_method,
match_def_path, match_trait_method, path_res,
is_diagnostic_ctor, is_diagnostic_item_or_ctor, is_expr_diagnostic_item, is_expr_path_def_path, is_lang_ctor,
is_lang_item_or_ctor, is_trait_method, match_def_path, match_trait_method, path_res,
};

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

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

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

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

fn main() {}
4 changes: 2 additions & 2 deletions tests/ui-internal/unnecessary_def_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ extern crate rustc_span;
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item, match_type};
#[allow(unused)]
use clippy_utils::{
is_diagnostic_item, is_expr_diagnostic_item, is_expr_path_def_path, is_lang_ctor, is_lang_item, is_trait_method,
match_def_path, match_trait_method, path_res,
is_diagnostic_ctor, is_diagnostic_item_or_ctor, is_expr_diagnostic_item, is_expr_path_def_path, is_lang_ctor,
is_lang_item_or_ctor, is_trait_method, match_def_path, match_trait_method, path_res,
};

#[allow(unused)]
Expand Down
8 changes: 4 additions & 4 deletions tests/ui-internal/unnecessary_def_path.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,19 @@ error: use of a def path to a `LangItem`
--> $DIR/unnecessary_def_path.rs:51:13
|
LL | let _ = match_def_path(cx, did, &["alloc", "boxed", "Box"]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_lang_item(cx, did, LangItem::OwnedBox)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cx.tcx.lang_items().require(LangItem::OwnedBox).ok() == Some(did)`

error: use of a def path to a diagnostic item
--> $DIR/unnecessary_def_path.rs:52:13
|
LL | let _ = match_def_path(cx, did, &["core", "option", "Option"]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_diagnostic_item(cx, did, sym::Option)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cx.tcx.is_diagnostic_item(sym::Option, did)`

error: use of a def path to a `LangItem`
--> $DIR/unnecessary_def_path.rs:53:13
|
LL | let _ = match_def_path(cx, did, &["core", "option", "Option", "Some"]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_lang_item(cx, did, LangItem::OptionSome)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_lang_item_or_ctor(cx, did, LangItem::OptionSome)`

error: use of a def path to a diagnostic item
--> $DIR/unnecessary_def_path.rs:55:13
Expand All @@ -87,7 +87,7 @@ error: use of a def path to a `LangItem`
--> $DIR/unnecessary_def_path.rs:58:13
|
LL | let _ = is_expr_path_def_path(cx, expr, &["core", "iter", "traits", "Iterator", "next"]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `path_res(cx, expr).opt_def_id().map_or(false, |id| is_lang_item(cx, id, LangItem::IteratorNext))`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `path_res(cx, expr).opt_def_id().map_or(false, |id| cx.tcx.lang_items().require(LangItem::IteratorNext).ok() == Some(id))`

error: aborting due to 14 previous errors

0 comments on commit 5e63922

Please sign in to comment.