Skip to content

Commit

Permalink
Expand unnecessary_def_path lint
Browse files Browse the repository at this point in the history
  • Loading branch information
@smoelius
smoelius committed Oct 9, 2022
1 parent 65762bd commit e9919a6
Show file tree
Hide file tree
Showing 6 changed files with 182 additions and 56 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| {
Box::<utils::internal_lints::lint_without_lint_pass::LintWithoutLintPass>::default()
});
store.register_late_pass(|_| Box::new(utils::internal_lints::unnecessary_def_path::UnnecessaryDefPath));
store.register_late_pass(|_| Box::<utils::internal_lints::unnecessary_def_path::UnnecessaryDefPath>::default());
store.register_late_pass(|_| Box::new(utils::internal_lints::outer_expn_data_pass::OuterExpnDataPass));
store.register_late_pass(|_| Box::new(utils::internal_lints::msrv_attr_impl::MsrvAttrImpl));
}
Expand Down
181 changes: 132 additions & 49 deletions clippy_lints/src/utils/internal_lints/unnecessary_def_path.rs
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then};
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{def_path_res, is_lint_allowed, match_any_def_paths, peel_hir_expr_refs};
use if_chain::if_chain;
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_id::DefId;
use rustc_hir::{ExprKind, Local, Mutability, Node};
use rustc_hir::{Expr, ExprKind, Local, Mutability, Node};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::mir::interpret::{Allocation, ConstValue, GlobalAlloc};
use rustc_middle::ty::{self, AssocKind, DefIdTree, Ty};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::{Ident, Symbol};
use rustc_span::Span;

use std::str;

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

declare_lint_pass!(UnnecessaryDefPath => [UNNECESSARY_DEF_PATH]);
impl_lint_pass!(UnnecessaryDefPath => [UNNECESSARY_DEF_PATH]);

#[derive(Default)]
pub struct UnnecessaryDefPath {
array_def_ids: FxHashSet<(DefId, Span)>,
linted_def_ids: FxHashSet<DefId>,
}

#[allow(clippy::too_many_lines)]
impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
if is_lint_allowed(cx, UNNECESSARY_DEF_PATH, expr.hir_id) {
return;
}

match expr.kind {
ExprKind::Call(func, args) => self.check_call(cx, func, args, expr.span),
ExprKind::Array(elements) => self.check_array(cx, elements, expr.span),
_ => {},
}
}

fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
for &(def_id, span) in &self.array_def_ids {
if self.linted_def_ids.contains(&def_id) {
continue;
}

let (msg, sugg) = if let Some(sym) = cx.tcx.get_diagnostic_name(def_id) {
("diagnostic item", format!("sym::{sym}"))
} else if let Some(sym) = get_lang_item_name(cx, def_id) {
("language item", format!("LangItem::{sym}"))
} else {
continue;
};

span_lint_and_help(
cx,
UNNECESSARY_DEF_PATH,
span,
&format!("hardcoded path to a {msg}"),
None,
&format!("convert all references to use `{sugg}`"),
);
}
}
}

impl UnnecessaryDefPath {
#[allow(clippy::too_many_lines)]
fn check_call(&mut self, cx: &LateContext<'_>, func: &Expr<'_>, args: &[Expr<'_>], span: Span) {
enum Item {
LangItem(Symbol),
DiagnosticItem(Symbol),
Expand All @@ -54,42 +101,17 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
&["clippy_utils", "is_expr_path_def_path"],
];

if is_lint_allowed(cx, UNNECESSARY_DEF_PATH, expr.hir_id) {
return;
}

if_chain! {
if let ExprKind::Call(func, [cx_arg, def_arg, args@..]) = expr.kind;
if let [cx_arg, def_arg, args@..] = args;
if let ExprKind::Path(path) = &func.kind;
if let Some(id) = cx.qpath_res(path, func.hir_id).opt_def_id();
if let Some(which_path) = match_any_def_paths(cx, id, PATHS);
let item_arg = if which_path == 4 { &args[1] } else { &args[0] };
// Extract the path to the matched type
if let Some(segments) = path_to_matched_type(cx, item_arg);
let segments: Vec<&str> = segments.iter().map(|sym| &**sym).collect();
if let Some(def_id) = def_path_res(cx, &segments[..], None).opt_def_id();
if let Some(def_id) = inherent_def_path_res(cx, &segments[..]);
then {
// def_path_res will match field names before anything else, but for this we want to match
// inherent functions first.
let def_id = if cx.tcx.def_kind(def_id) == DefKind::Field {
let method_name = *segments.last().unwrap();
cx.tcx.def_key(def_id).parent
.and_then(|parent_idx|
cx.tcx.inherent_impls(DefId { index: parent_idx, krate: def_id.krate }).iter()
.find_map(|impl_id| cx.tcx.associated_items(*impl_id)
.find_by_name_and_kind(
cx.tcx,
Ident::from_str(method_name),
AssocKind::Fn,
*impl_id,
)
)
)
.map_or(def_id, |item| item.def_id)
} else {
def_id
};

// Check if the target item is a diagnostic item or LangItem.
let (msg, item) = if let Some(item_name)
= cx.tcx.diagnostic_items(def_id.krate).id_to_name.get(&def_id)
Expand All @@ -98,9 +120,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
"use of a def path to a diagnostic item",
Item::DiagnosticItem(*item_name),
)
} else 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;
} else if let Some(item_name) = get_lang_item_name(cx, def_id) {
(
"use of a def path to a `LangItem`",
Item::LangItem(item_name),
Expand Down Expand Up @@ -168,10 +188,10 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
span_lint_and_then(
cx,
UNNECESSARY_DEF_PATH,
expr.span,
span,
msg,
|diag| {
diag.span_suggestion(expr.span, "try", sugg, app);
diag.span_suggestion(span, "try", sugg, app);
if with_note {
diag.help(
"if this `DefId` came from a constructor expression or pattern then the \
Expand All @@ -180,9 +200,19 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath {
}
},
);

self.linted_def_ids.insert(def_id);
}
}
}

fn check_array(&mut self, cx: &LateContext<'_>, elements: &[Expr<'_>], span: Span) {
let Some(path) = path_from_array(elements) else { return };

if let Some(def_id) = inherent_def_path_res(cx, &path.iter().map(AsRef::as_ref).collect::<Vec<_>>()) {
self.array_def_ids.insert((def_id, span));
}
}
}

fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Vec<String>> {
Expand All @@ -209,18 +239,7 @@ fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Ve
},
_ => None,
},
ExprKind::Array(exprs) => exprs
.iter()
.map(|expr| {
if let ExprKind::Lit(lit) = &expr.kind {
if let LitKind::Str(sym, _) = lit.node {
return Some((*sym.as_str()).to_owned());
}
}

None
})
.collect(),
ExprKind::Array(exprs) => path_from_array(exprs),
_ => None,
}
}
Expand Down Expand Up @@ -258,3 +277,67 @@ fn read_mir_alloc_def_path<'tcx>(cx: &LateContext<'tcx>, alloc: &'tcx Allocation
None
}
}

fn path_from_array(exprs: &[Expr<'_>]) -> Option<Vec<String>> {
exprs
.iter()
.map(|expr| {
if let ExprKind::Lit(lit) = &expr.kind {
if let LitKind::Str(sym, _) = lit.node {
return Some((*sym.as_str()).to_owned());
}
}

None
})
.collect()
}

// def_path_res will match field names before anything else, but for this we want to match
// inherent functions first.
fn inherent_def_path_res(cx: &LateContext<'_>, segments: &[&str]) -> Option<DefId> {
def_path_res(cx, segments, None).opt_def_id().map(|def_id| {
if cx.tcx.def_kind(def_id) == DefKind::Field {
let method_name = *segments.last().unwrap();
cx.tcx
.def_key(def_id)
.parent
.and_then(|parent_idx| {
cx.tcx
.inherent_impls(DefId {
index: parent_idx,
krate: def_id.krate,
})
.iter()
.find_map(|impl_id| {
cx.tcx.associated_items(*impl_id).find_by_name_and_kind(
cx.tcx,
Ident::from_str(method_name),
AssocKind::Fn,
*impl_id,
)
})
})
.map_or(def_id, |item| item.def_id)
} else {
def_id
}
})
}

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)
} else {
None
}
}
6 changes: 3 additions & 3 deletions tests/ui-internal/unnecessary_def_path.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,17 @@ use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_middle::ty::Ty;

#[allow(unused)]
#[allow(unused, clippy::unnecessary_def_path)]
static OPTION: [&str; 3] = ["core", "option", "Option"];
#[allow(unused)]
#[allow(unused, clippy::unnecessary_def_path)]
const RESULT: &[&str] = &["core", "result", "Result"];

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

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

Expand Down
6 changes: 3 additions & 3 deletions tests/ui-internal/unnecessary_def_path.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,17 @@ use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_middle::ty::Ty;

#[allow(unused)]
#[allow(unused, clippy::unnecessary_def_path)]
static OPTION: [&str; 3] = ["core", "option", "Option"];
#[allow(unused)]
#[allow(unused, clippy::unnecessary_def_path)]
const RESULT: &[&str] = &["core", "result", "Result"];

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

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

Expand Down
16 changes: 16 additions & 0 deletions tests/ui-internal/unnecessary_def_path_hardcoded_path.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#![feature(rustc_private)]
#![allow(unused)]
#![warn(clippy::unnecessary_def_path)]

extern crate rustc_hir;

use rustc_hir::LangItem;

fn main() {
const DEREF_TRAIT: [&str; 4] = ["core", "ops", "deref", "Deref"];
const DEREF_MUT_TRAIT: [&str; 4] = ["core", "ops", "deref", "DerefMut"];
const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"];

// Don't lint, not yet a diagnostic or language item
const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"];
}
27 changes: 27 additions & 0 deletions tests/ui-internal/unnecessary_def_path_hardcoded_path.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
error: hardcoded path to a language item
--> $DIR/unnecessary_def_path_hardcoded_path.rs:11:40
|
LL | const DEREF_MUT_TRAIT: [&str; 4] = ["core", "ops", "deref", "DerefMut"];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: convert all references to use `LangItem::DerefMut`
= note: `-D clippy::unnecessary-def-path` implied by `-D warnings`

error: hardcoded path to a diagnostic item
--> $DIR/unnecessary_def_path_hardcoded_path.rs:12:43
|
LL | const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "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
|
LL | const DEREF_TRAIT: [&str; 4] = ["core", "ops", "deref", "Deref"];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: convert all references to use `sym::Deref`

error: aborting due to 3 previous errors

0 comments on commit e9919a6

Please sign in to comment.