Skip to content

Commit 8afa7ed

Browse files
ebrotoebroto
committed
Add internal lint MatchTypeOnDiagItem
1 parent f82e84c commit 8afa7ed

File tree

5 files changed

+203
-3
lines changed

5 files changed

+203
-3
lines changed

‎clippy_lints/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
867867
&utils::internal_lints::COMPILER_LINT_FUNCTIONS,
868868
&utils::internal_lints::DEFAULT_LINT,
869869
&utils::internal_lints::LINT_WITHOUT_LINT_PASS,
870+
&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM,
870871
&utils::internal_lints::OUTER_EXPN_EXPN_DATA,
871872
&utils::internal_lints::PRODUCE_ICE,
872873
&vec::USELESS_VEC,
@@ -1112,6 +1113,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
11121113
store.register_late_pass(|| box self_assignment::SelfAssignment);
11131114
store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs);
11141115
store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync);
1116+
store.register_late_pass(|| box utils::internal_lints::MatchTypeOnDiagItem);
11151117

11161118
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
11171119
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1240,6 +1242,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12401242
LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS),
12411243
LintId::of(&utils::internal_lints::DEFAULT_LINT),
12421244
LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS),
1245+
LintId::of(&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM),
12431246
LintId::of(&utils::internal_lints::OUTER_EXPN_EXPN_DATA),
12441247
LintId::of(&utils::internal_lints::PRODUCE_ICE),
12451248
]);

‎clippy_lints/src/utils/internal_lints.rs

Lines changed: 112 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::utils::{
2-
is_expn_of, match_def_path, match_qpath, match_type, method_calls, paths, run_lints, snippet, span_lint,
3-
span_lint_and_help, span_lint_and_sugg, walk_ptrs_ty, SpanlessEq,
2+
is_expn_of, match_def_path, match_qpath, match_type, method_calls, path_to_res, paths, qpath_res, run_lints,
3+
snippet, span_lint, span_lint_and_help, span_lint_and_sugg, walk_ptrs_ty, SpanlessEq,
44
};
55
use if_chain::if_chain;
66
use rustc_ast::ast::{Crate as AstCrate, ItemKind, LitKind, NodeId};
@@ -11,7 +11,7 @@ use rustc_hir as hir;
1111
use rustc_hir::def::{DefKind, Res};
1212
use rustc_hir::hir_id::CRATE_HIR_ID;
1313
use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
14-
use rustc_hir::{Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Path, StmtKind, Ty, TyKind};
14+
use rustc_hir::{Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Node, Path, StmtKind, Ty, TyKind};
1515
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
1616
use rustc_middle::hir::map::Map;
1717
use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
@@ -206,6 +206,29 @@ declare_clippy_lint! {
206206
"found collapsible `span_lint_and_then` calls"
207207
}
208208

209+
declare_clippy_lint! {
210+
/// **What it does:** Checks for calls to `utils::match_type()` on a type diagnostic item
211+
/// and suggests to use `utils::is_type_diagnostic_item()` instead.
212+
///
213+
/// **Why is this bad?** `utils::is_type_diagnostic_item()` does not require hardcoded paths.
214+
///
215+
/// **Known problems:** None.
216+
///
217+
/// **Example:**
218+
/// Bad:
219+
/// ```rust,ignore
220+
/// utils::match_type(cx, ty, &paths::VEC)
221+
/// ```
222+
///
223+
/// Good:
224+
/// ```rust,ignore
225+
/// utils::is_type_diagnostic_item(cx, ty, sym!(vec_type))
226+
/// ```
227+
pub MATCH_TYPE_ON_DIAGNOSTIC_ITEM,
228+
internal,
229+
"using `utils::match_type()` instead of `utils::is_type_diagnostic_item()`"
230+
}
231+
209232
declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]);
210233

211234
impl EarlyLintPass for ClippyLintsInternal {
@@ -652,3 +675,89 @@ fn suggest_note(
652675
Applicability::MachineApplicable,
653676
);
654677
}
678+
679+
declare_lint_pass!(MatchTypeOnDiagItem => [MATCH_TYPE_ON_DIAGNOSTIC_ITEM]);
680+
681+
impl<'tcx> LateLintPass<'tcx> for MatchTypeOnDiagItem {
682+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
683+
if !run_lints(cx, &[MATCH_TYPE_ON_DIAGNOSTIC_ITEM], expr.hir_id) {
684+
return;
685+
}
686+
687+
if_chain! {
688+
// Check if this is a call to utils::match_type()
689+
if let ExprKind::Call(fn_path, [context, ty, ty_path]) = expr.kind;
690+
if let ExprKind::Path(fn_qpath) = &fn_path.kind;
691+
if match_qpath(&fn_qpath, &["utils", "match_type"]);
692+
// Extract the path to the matched type
693+
if let Some(segments) = path_to_matched_type(cx, ty_path);
694+
let segments: Vec<&str> = segments.iter().map(|sym| &**sym).collect();
695+
if let Some(ty_did) = path_to_res(cx, &segments[..]).and_then(|res| res.opt_def_id());
696+
// Check if the matched type is a diagnostic item
697+
let diag_items = cx.tcx.diagnostic_items(ty_did.krate);
698+
if let Some(item_name) = diag_items.iter().find_map(|(k, v)| if *v == ty_did { Some(k) } else { None });
699+
then {
700+
let cx_snippet = snippet(cx, context.span, "_");
701+
let ty_snippet = snippet(cx, ty.span, "_");
702+
703+
span_lint_and_sugg(
704+
cx,
705+
MATCH_TYPE_ON_DIAGNOSTIC_ITEM,
706+
expr.span,
707+
"usage of `utils::match_type() on a type diagnostic item`",
708+
"try",
709+
format!("utils::is_type_diagnostic_item({}, {}, sym!({}))", cx_snippet, ty_snippet, item_name),
710+
Applicability::MaybeIncorrect,
711+
);
712+
}
713+
}
714+
}
715+
}
716+
717+
fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Vec<SymbolStr>> {
718+
use rustc_hir::ItemKind;
719+
720+
match &expr.kind {
721+
ExprKind::AddrOf(.., expr) => return path_to_matched_type(cx, expr),
722+
ExprKind::Path(qpath) => match qpath_res(cx, qpath, expr.hir_id) {
723+
Res::Local(hir_id) => {
724+
let parent_id = cx.tcx.hir().get_parent_node(hir_id);
725+
if let Some(Node::Local(local)) = cx.tcx.hir().find(parent_id) {
726+
if let Some(init) = local.init {
727+
return path_to_matched_type(cx, init);
728+
}
729+
}
730+
},
731+
Res::Def(DefKind::Const | DefKind::Static, def_id) => {
732+
if let Some(Node::Item(item)) = cx.tcx.hir().get_if_local(def_id) {
733+
if let ItemKind::Const(.., body_id) | ItemKind::Static(.., body_id) = item.kind {
734+
let body = cx.tcx.hir().body(body_id);
735+
return path_to_matched_type(cx, &body.value);
736+
}
737+
}
738+
},
739+
_ => {},
740+
},
741+
ExprKind::Array(exprs) => {
742+
let segments: Vec<SymbolStr> = exprs
743+
.iter()
744+
.filter_map(|expr| {
745+
if let ExprKind::Lit(lit) = &expr.kind {
746+
if let LitKind::Str(sym, _) = lit.node {
747+
return Some(sym.as_str());
748+
}
749+
}
750+
751+
None
752+
})
753+
.collect();
754+
755+
if segments.len() == exprs.len() {
756+
return Some(segments);
757+
}
758+
},
759+
_ => {},
760+
}
761+
762+
None
763+
}

‎clippy_lints/src/utils/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ pub fn is_wild<'tcx>(pat: &impl std::ops::Deref<Target = Pat<'tcx>>) -> bool {
130130
}
131131

132132
/// Checks if type is struct, enum or union type with the given def path.
133+
///
134+
/// If the type is a diagnostic item, use `is_type_diagnostic_item` instead.
135+
/// If you change the signature, remember to update the internal lint `MatchTypeOnDiagItem`
133136
pub fn match_type(cx: &LateContext<'_>, ty: Ty<'_>, path: &[&str]) -> bool {
134137
match ty.kind() {
135138
ty::Adt(adt, _) => match_def_path(cx, adt.did, path),
@@ -138,6 +141,8 @@ pub fn match_type(cx: &LateContext<'_>, ty: Ty<'_>, path: &[&str]) -> bool {
138141
}
139142

140143
/// Checks if the type is equal to a diagnostic item
144+
///
145+
/// If you change the signature, remember to update the internal lint `MatchTypeOnDiagItem`
141146
pub fn is_type_diagnostic_item(cx: &LateContext<'_>, ty: Ty<'_>, diag_item: Symbol) -> bool {
142147
match ty.kind() {
143148
ty::Adt(adt, _) => cx.tcx.is_diagnostic_item(diag_item, adt.did),

‎tests/ui/match_type_on_diag_item.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
#![deny(clippy::internal)]
2+
#![feature(rustc_private)]
3+
4+
extern crate rustc_hir;
5+
extern crate rustc_lint;
6+
extern crate rustc_middle;
7+
#[macro_use]
8+
extern crate rustc_session;
9+
use rustc_hir::Expr;
10+
use rustc_lint::{LateContext, LateLintPass};
11+
use rustc_middle::ty::Ty;
12+
13+
mod paths {
14+
pub const VEC: [&str; 3] = ["alloc", "vec", "Vec"];
15+
}
16+
17+
mod utils {
18+
use super::*;
19+
20+
pub fn match_type(_cx: &LateContext<'_>, _ty: Ty<'_>, _path: &[&str]) -> bool {
21+
false
22+
}
23+
}
24+
25+
use utils::match_type;
26+
27+
declare_lint! {
28+
pub TEST_LINT,
29+
Warn,
30+
""
31+
}
32+
33+
declare_lint_pass!(Pass => [TEST_LINT]);
34+
35+
static OPTION: [&str; 3] = ["core", "option", "Option"];
36+
37+
impl<'tcx> LateLintPass<'tcx> for Pass {
38+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr) {
39+
let ty = cx.typeck_results().expr_ty(expr);
40+
41+
let _ = match_type(cx, ty, &paths::VEC);
42+
let _ = match_type(cx, ty, &OPTION);
43+
let _ = match_type(cx, ty, &["core", "result", "Result"]);
44+
45+
let rc_path = &["alloc", "rc", "Rc"];
46+
let _ = utils::match_type(cx, ty, rc_path);
47+
}
48+
}
49+
50+
fn main() {}

‎tests/ui/match_type_on_diag_item.stderr

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
error: usage of `utils::match_type() on a type diagnostic item`
2+
--> $DIR/match_type_on_diag_item.rs:41:17
3+
|
4+
LL | let _ = match_type(cx, ty, &paths::VEC);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `utils::is_type_diagnostic_item(cx, ty, sym!(vec_type))`
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/match_type_on_diag_item.rs:1:9
9+
|
10+
LL | #![deny(clippy::internal)]
11+
| ^^^^^^^^^^^^^^^^
12+
= note: `#[deny(clippy::match_type_on_diagnostic_item)]` implied by `#[deny(clippy::internal)]`
13+
14+
error: usage of `utils::match_type() on a type diagnostic item`
15+
--> $DIR/match_type_on_diag_item.rs:42:17
16+
|
17+
LL | let _ = match_type(cx, ty, &OPTION);
18+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `utils::is_type_diagnostic_item(cx, ty, sym!(option_type))`
19+
20+
error: usage of `utils::match_type() on a type diagnostic item`
21+
--> $DIR/match_type_on_diag_item.rs:43:17
22+
|
23+
LL | let _ = match_type(cx, ty, &["core", "result", "Result"]);
24+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `utils::is_type_diagnostic_item(cx, ty, sym!(result_type))`
25+
26+
error: usage of `utils::match_type() on a type diagnostic item`
27+
--> $DIR/match_type_on_diag_item.rs:46:17
28+
|
29+
LL | let _ = utils::match_type(cx, ty, rc_path);
30+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `utils::is_type_diagnostic_item(cx, ty, sym!(Rc))`
31+
32+
error: aborting due to 4 previous errors
33+

0 commit comments

Comments
 (0)