Skip to content

Commit

Permalink
Add variant_name function to LangItem
Browse files Browse the repository at this point in the history
Clippy has an internal lint that checks for the usage of hardcoded def
paths and suggests to replace them with a lang or diagnostic item, if
possible. This was implemented with a hack, by getting all the variants
of the `LangItem` enum and then index into it with the position of the
`LangItem` in the `items` list. This is no longer possible, because the
`items` list can't be accessed anymore.
  • Loading branch information
flip1995 committed Nov 17, 2022
1 parent b6097f2 commit 4e65f5e
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 36 deletions.
8 changes: 8 additions & 0 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,14 @@ macro_rules! language_item_table {
}
}

/// Returns the name of the `LangItem` enum variant.
// This method is used by Clippy for internal lints.
pub fn variant_name(self) -> &'static str {
match self {
$( LangItem::$variant => stringify!($variant), )*
}
}

pub fn target(self) -> Target {
match self {
$( LangItem::$variant => $target, )*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,22 +79,22 @@ pub fn check_path(cx: &LateContext<'_>, path: &[&str]) -> bool {
SimplifiedTypeGen::StrSimplifiedType,
]
.iter()
.flat_map(|&ty| cx.tcx.incoherent_impls(ty));
for item_def_id in lang_items.items().iter().flatten().chain(incoherent_impls) {
let lang_item_path = cx.get_def_path(*item_def_id);
.flat_map(|&ty| cx.tcx.incoherent_impls(ty).iter().copied());
for item_def_id in lang_items.iter().map(|(_, def_id)| def_id).chain(incoherent_impls) {
let lang_item_path = cx.get_def_path(item_def_id);
if path_syms.starts_with(&lang_item_path) {
if let [item] = &path_syms[lang_item_path.len()..] {
if matches!(
cx.tcx.def_kind(*item_def_id),
cx.tcx.def_kind(item_def_id),
DefKind::Mod | DefKind::Enum | DefKind::Trait
) {
for child in cx.tcx.module_children(*item_def_id) {
for child in cx.tcx.module_children(item_def_id) {
if child.ident.name == *item {
return true;
}
}
} else {
for child in cx.tcx.associated_item_def_ids(*item_def_id) {
for child in cx.tcx.associated_item_def_ids(item_def_id) {
if cx.tcx.item_name(*child) == *item {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ 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};
Expand Down Expand Up @@ -91,7 +91,7 @@ impl UnnecessaryDefPath {
#[allow(clippy::too_many_lines)]
fn check_call(&mut self, cx: &LateContext<'_>, func: &Expr<'_>, args: &[Expr<'_>], span: Span) {
enum Item {
LangItem(Symbol),
LangItem(&'static str),
DiagnosticItem(Symbol),
}
static PATHS: &[&[&str]] = &[
Expand Down Expand Up @@ -325,18 +325,9 @@ fn inherent_def_path_res(cx: &LateContext<'_>, segments: &[&str]) -> Option<DefI
})
}

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 item_name = cx
.tcx
.adt_def(lang_items)
.variants()
.iter()
.nth(lang_item)
.unwrap()
.name;
Some(item_name)
fn get_lang_item_name(cx: &LateContext<'_>, def_id: DefId) -> Option<&'static str> {
if let Some((lang_item, _)) = cx.tcx.lang_items().iter().find(|(_, id)| *id == def_id) {
Some(lang_item.variant_name())
} else {
None
}
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/tests/ui-internal/unnecessary_def_path.fixed
Original file line number Diff line number Diff line change
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 _ = cx.tcx.lang_items().require(LangItem::OwnedBox).ok() == Some(did);
let _ = cx.tcx.lang_items().get(LangItem::OwnedBox) == Some(did);
let _ = cx.tcx.is_diagnostic_item(sym::Option, did);
let _ = cx.tcx.lang_items().require(LangItem::OptionSome).ok() == Some(did);
let _ = cx.tcx.lang_items().get(LangItem::OptionSome) == Some(did);

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ 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: `cx.tcx.lang_items().require(LangItem::OwnedBox).ok() == Some(did)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cx.tcx.lang_items().get(LangItem::OwnedBox) == Some(did)`

error: use of a def path to a diagnostic item
--> $DIR/unnecessary_def_path.rs:52:13
Expand All @@ -69,7 +69,7 @@ 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: `cx.tcx.lang_items().require(LangItem::OptionSome).ok() == Some(did)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cx.tcx.lang_items().get(LangItem::OptionSome) == Some(did)`
|
= help: if this `DefId` came from a constructor expression or pattern then the parent `DefId` should be used instead

Expand All @@ -89,7 +89,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| cx.tcx.lang_items().require(LangItem::IteratorNext).ok() == Some(id))`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `path_res(cx, expr).opt_def_id().map_or(false, |id| cx.tcx.lang_items().get(LangItem::IteratorNext) == Some(id))`

error: use of a def path to a `LangItem`
--> $DIR/unnecessary_def_path.rs:59:13
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error: hardcoded path to a language item
--> $DIR/unnecessary_def_path_hardcoded_path.rs:11:40
error: hardcoded path to a diagnostic item
--> $DIR/unnecessary_def_path_hardcoded_path.rs:10:36
|
LL | const DEREF_MUT_TRAIT: [&str; 4] = ["core", "ops", "deref", "DerefMut"];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | const DEREF_TRAIT: [&str; 4] = ["core", "ops", "deref", "Deref"];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: convert all references to use `LangItem::DerefMut`
= help: convert all references to use `sym::Deref`
= note: `-D clippy::unnecessary-def-path` implied by `-D warnings`

error: hardcoded path to a diagnostic item
Expand All @@ -15,13 +15,13 @@ LL | const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref",
|
= help: convert all references to use `sym::deref_method`

error: hardcoded path to a diagnostic item
--> $DIR/unnecessary_def_path_hardcoded_path.rs:10:36
error: hardcoded path to a language item
--> $DIR/unnecessary_def_path_hardcoded_path.rs:11:40
|
LL | const DEREF_TRAIT: [&str; 4] = ["core", "ops", "deref", "Deref"];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | const DEREF_MUT_TRAIT: [&str; 4] = ["core", "ops", "deref", "DerefMut"];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: convert all references to use `sym::Deref`
= help: convert all references to use `LangItem::DerefMut`

error: aborting due to 3 previous errors

0 comments on commit 4e65f5e

Please sign in to comment.