Auto merge of rust-lang#6013 - ebroto:diagnostic_item_restriction, r=flip1995

Internal lint: suggest `is_type_diagnostic_item` over `match_type` where applicable

changelog: none

Author: bors

clippy_lints/src/lib.rs

@@ -867,6 +867,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &utils::internal_lints::COMPILER_LINT_FUNCTIONS,
         &utils::internal_lints::DEFAULT_LINT,
         &utils::internal_lints::LINT_WITHOUT_LINT_PASS,
+        &utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM,
         &utils::internal_lints::OUTER_EXPN_EXPN_DATA,
         &utils::internal_lints::PRODUCE_ICE,
         &vec::USELESS_VEC,
@@ -1112,6 +1113,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box self_assignment::SelfAssignment);
     store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs);
     store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync);
+    store.register_late_pass(|| box utils::internal_lints::MatchTypeOnDiagItem);
 
     store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1240,6 +1242,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS),
         LintId::of(&utils::internal_lints::DEFAULT_LINT),
         LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS),
+        LintId::of(&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM),
         LintId::of(&utils::internal_lints::OUTER_EXPN_EXPN_DATA),
         LintId::of(&utils::internal_lints::PRODUCE_ICE),
     ]);

clippy_lints/src/loops.rs

@@ -1114,7 +1114,7 @@ fn get_vec_push<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) -> Option<(&
             if let Some(self_expr) = args.get(0);
             if let Some(pushed_item) = args.get(1);
             // Check that the method being called is push() on a Vec
-            if match_type(cx, cx.typeck_results().expr_ty(self_expr), &paths::VEC);
+            if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr), sym!(vec_type));
             if path.ident.name.as_str() == "push";
             then {
                 return Some((self_expr, pushed_item))

clippy_lints/src/methods/mod.rs

@@ -1808,7 +1808,7 @@ fn lint_or_fun_call<'tcx>(
                     _ => (),
                 }
 
-                if match_type(cx, ty, &paths::VEC) {
+                if is_type_diagnostic_item(cx, ty, sym!(vec_type)) {
                     return;
                 }
             }

clippy_lints/src/option_if_let_else.rs

@@ -1,6 +1,6 @@
 use crate::utils;
 use crate::utils::sugg::Sugg;
-use crate::utils::{match_type, paths, span_lint_and_sugg};
+use crate::utils::{is_type_diagnostic_item, paths, span_lint_and_sugg};
 use if_chain::if_chain;
 
 use rustc_errors::Applicability;
@@ -73,7 +73,7 @@ declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]);
 fn is_result_ok(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool {
     if let ExprKind::MethodCall(ref path, _, &[ref receiver], _) = &expr.kind {
         path.ident.name.to_ident_string() == "ok"
-            && match_type(cx, &cx.typeck_results().expr_ty(&receiver), &paths::RESULT)
+            && is_type_diagnostic_item(cx, &cx.typeck_results().expr_ty(&receiver), sym!(result_type))
     } else {
         false
     }

clippy_lints/src/utils/diagnostics.rs

@@ -51,6 +51,8 @@ pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: impl Into<Mult
 ///
 /// The `help` message can be optionally attached to a `Span`.
 ///
+/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
+///
 /// # Example
 ///
 /// ```ignore
@@ -87,6 +89,8 @@ pub fn span_lint_and_help<'a, T: LintContext>(
 /// The `note` message is presented separately from the main lint message
 /// and is attached to a specific span:
 ///
+/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
+///
 /// # Example
 ///
 /// ```ignore
@@ -126,6 +130,7 @@ pub fn span_lint_and_note<'a, T: LintContext>(
 /// Like `span_lint` but allows to add notes, help and suggestions using a closure.
 ///
 /// If you need to customize your lint output a lot, use this function.
+/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
 pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str, f: F)
 where
     F: for<'b> FnOnce(&mut DiagnosticBuilder<'b>),
@@ -168,6 +173,10 @@ pub fn span_lint_hir_and_then(
 /// In the example below, `help` is `"try"` and `sugg` is the suggested replacement `".any(|x| x >
 /// 2)"`.
 ///
+/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
+///
+/// # Example
+///
 /// ```ignore
 /// error: This `.fold` can be more succinctly expressed as `.any`
 /// --> $DIR/methods.rs:390:13

clippy_lints/src/utils/internal_lints.rs

@@ -1,6 +1,6 @@
 use crate::utils::{
-    is_expn_of, match_def_path, match_qpath, match_type, method_calls, paths, run_lints, snippet, span_lint,
-    span_lint_and_help, span_lint_and_sugg, walk_ptrs_ty, SpanlessEq,
+    is_expn_of, match_def_path, match_qpath, match_type, method_calls, path_to_res, paths, qpath_res, run_lints,
+    snippet, span_lint, span_lint_and_help, span_lint_and_sugg, walk_ptrs_ty, SpanlessEq,
 };
 use if_chain::if_chain;
 use rustc_ast::ast::{Crate as AstCrate, ItemKind, LitKind, NodeId};
@@ -11,7 +11,7 @@ use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::hir_id::CRATE_HIR_ID;
 use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
-use rustc_hir::{Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Path, StmtKind, Ty, TyKind};
+use rustc_hir::{Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Node, Path, StmtKind, Ty, TyKind};
 use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
 use rustc_middle::hir::map::Map;
 use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
@@ -206,6 +206,29 @@ declare_clippy_lint! {
     "found collapsible `span_lint_and_then` calls"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for calls to `utils::match_type()` on a type diagnostic item
+    /// and suggests to use `utils::is_type_diagnostic_item()` instead.
+    ///
+    /// **Why is this bad?** `utils::is_type_diagnostic_item()` does not require hardcoded paths.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// Bad:
+    /// ```rust,ignore
+    /// utils::match_type(cx, ty, &paths::VEC)
+    /// ```
+    ///
+    /// Good:
+    /// ```rust,ignore
+    /// utils::is_type_diagnostic_item(cx, ty, sym!(vec_type))
+    /// ```
+    pub MATCH_TYPE_ON_DIAGNOSTIC_ITEM,
+    internal,
+    "using `utils::match_type()` instead of `utils::is_type_diagnostic_item()`"
+}
+
 declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]);
 
 impl EarlyLintPass for ClippyLintsInternal {
@@ -652,3 +675,89 @@ fn suggest_note(
         Applicability::MachineApplicable,
     );
 }
+
+declare_lint_pass!(MatchTypeOnDiagItem => [MATCH_TYPE_ON_DIAGNOSTIC_ITEM]);
+
+impl<'tcx> LateLintPass<'tcx> for MatchTypeOnDiagItem {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
+        if !run_lints(cx, &[MATCH_TYPE_ON_DIAGNOSTIC_ITEM], expr.hir_id) {
+            return;
+        }
+
+        if_chain! {
+            // Check if this is a call to utils::match_type()
+            if let ExprKind::Call(fn_path, [context, ty, ty_path]) = expr.kind;
+            if let ExprKind::Path(fn_qpath) = &fn_path.kind;
+            if match_qpath(&fn_qpath, &["utils", "match_type"]);
+            // Extract the path to the matched type
+            if let Some(segments) = path_to_matched_type(cx, ty_path);
+            let segments: Vec<&str> = segments.iter().map(|sym| &**sym).collect();
+            if let Some(ty_did) = path_to_res(cx, &segments[..]).and_then(|res| res.opt_def_id());
+            // Check if the matched type is a diagnostic item
+            let diag_items = cx.tcx.diagnostic_items(ty_did.krate);
+            if let Some(item_name) = diag_items.iter().find_map(|(k, v)| if *v == ty_did { Some(k) } else { None });
+            then {
+                let cx_snippet = snippet(cx, context.span, "_");
+                let ty_snippet = snippet(cx, ty.span, "_");
+
+                span_lint_and_sugg(
+                    cx,
+                    MATCH_TYPE_ON_DIAGNOSTIC_ITEM,
+                    expr.span,
+                    "usage of `utils::match_type()` on a type diagnostic item",
+                    "try",
+                    format!("utils::is_type_diagnostic_item({}, {}, sym!({}))", cx_snippet, ty_snippet, item_name),
+                    Applicability::MaybeIncorrect,
+                );
+            }
+        }
+    }
+}
+
+fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Vec<SymbolStr>> {
+    use rustc_hir::ItemKind;
+
+    match &expr.kind {
+        ExprKind::AddrOf(.., expr) => return path_to_matched_type(cx, expr),
+        ExprKind::Path(qpath) => match qpath_res(cx, qpath, expr.hir_id) {
+            Res::Local(hir_id) => {
+                let parent_id = cx.tcx.hir().get_parent_node(hir_id);
+                if let Some(Node::Local(local)) = cx.tcx.hir().find(parent_id) {
+                    if let Some(init) = local.init {
+                        return path_to_matched_type(cx, init);
+                    }
+                }
+            },
+            Res::Def(DefKind::Const | DefKind::Static, def_id) => {
+                if let Some(Node::Item(item)) = cx.tcx.hir().get_if_local(def_id) {
+                    if let ItemKind::Const(.., body_id) | ItemKind::Static(.., body_id) = item.kind {
+                        let body = cx.tcx.hir().body(body_id);
+                        return path_to_matched_type(cx, &body.value);
+                    }
+                }
+            },
+            _ => {},
+        },
+        ExprKind::Array(exprs) => {
+            let segments: Vec<SymbolStr> = exprs
+                .iter()
+                .filter_map(|expr| {
+                    if let ExprKind::Lit(lit) = &expr.kind {
+                        if let LitKind::Str(sym, _) = lit.node {
+                            return Some(sym.as_str());
+                        }
+                    }
+
+                    None
+                })
+                .collect();
+
+            if segments.len() == exprs.len() {
+                return Some(segments);
+            }
+        },
+        _ => {},
+    }
+
+    None
+}

clippy_lints/src/utils/mod.rs

@@ -130,6 +130,9 @@ pub fn is_wild<'tcx>(pat: &impl std::ops::Deref<Target = Pat<'tcx>>) -> bool {
 }
 
 /// Checks if type is struct, enum or union type with the given def path.
+///
+/// If the type is a diagnostic item, use `is_type_diagnostic_item` instead.
+/// If you change the signature, remember to update the internal lint `MatchTypeOnDiagItem`
 pub fn match_type(cx: &LateContext<'_>, ty: Ty<'_>, path: &[&str]) -> bool {
     match ty.kind() {
         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 {
 }
 
 /// Checks if the type is equal to a diagnostic item
+///
+/// If you change the signature, remember to update the internal lint `MatchTypeOnDiagItem`
 pub fn is_type_diagnostic_item(cx: &LateContext<'_>, ty: Ty<'_>, diag_item: Symbol) -> bool {
     match ty.kind() {
         ty::Adt(adt, _) => cx.tcx.is_diagnostic_item(diag_item, adt.did),

doc/common_tools_writing_lints.md

@@ -60,7 +60,7 @@ impl LateLintPass<'_> for MyStructLint {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
         if_chain! {
             // Check our expr is calling a method
-            if let hir::ExprKind::MethodCall(path, _, _args) = &expr.kind;
+            if let hir::ExprKind::MethodCall(path, _, _args, _) = &expr.kind;
             // Check the name of this method is `some_method`
             if path.ident.name == sym!(some_method);
             then {

tests/ui/match_type_on_diag_item.rs

@@ -0,0 +1,50 @@
+#![deny(clippy::internal)]
+#![feature(rustc_private)]
+
+extern crate rustc_hir;
+extern crate rustc_lint;
+extern crate rustc_middle;
+#[macro_use]
+extern crate rustc_session;
+use rustc_hir::Expr;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::Ty;
+
+mod paths {
+    pub const VEC: [&str; 3] = ["alloc", "vec", "Vec"];
+}
+
+mod utils {
+    use super::*;
+
+    pub fn match_type(_cx: &LateContext<'_>, _ty: Ty<'_>, _path: &[&str]) -> bool {
+        false
+    }
+}
+
+use utils::match_type;
+
+declare_lint! {
+    pub TEST_LINT,
+    Warn,
+    ""
+}
+
+declare_lint_pass!(Pass => [TEST_LINT]);
+
+static OPTION: [&str; 3] = ["core", "option", "Option"];
+
+impl<'tcx> LateLintPass<'tcx> for Pass {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr) {
+        let ty = cx.typeck_results().expr_ty(expr);
+
+        let _ = match_type(cx, ty, &paths::VEC);
+        let _ = match_type(cx, ty, &OPTION);
+        let _ = match_type(cx, ty, &["core", "result", "Result"]);
+
+        let rc_path = &["alloc", "rc", "Rc"];
+        let _ = utils::match_type(cx, ty, rc_path);
+    }
+}
+
+fn main() {}

tests/ui/match_type_on_diag_item.stderr

@@ -0,0 +1,33 @@
+error: usage of `utils::match_type()` on a type diagnostic item
+  --> $DIR/match_type_on_diag_item.rs:41:17
+   |
+LL |         let _ = match_type(cx, ty, &paths::VEC);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `utils::is_type_diagnostic_item(cx, ty, sym!(vec_type))`
+   |
+note: the lint level is defined here
+  --> $DIR/match_type_on_diag_item.rs:1:9
+   |
+LL | #![deny(clippy::internal)]
+   |         ^^^^^^^^^^^^^^^^
+   = note: `#[deny(clippy::match_type_on_diagnostic_item)]` implied by `#[deny(clippy::internal)]`
+
+error: usage of `utils::match_type()` on a type diagnostic item
+  --> $DIR/match_type_on_diag_item.rs:42:17
+   |
+LL |         let _ = match_type(cx, ty, &OPTION);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `utils::is_type_diagnostic_item(cx, ty, sym!(option_type))`
+
+error: usage of `utils::match_type()` on a type diagnostic item
+  --> $DIR/match_type_on_diag_item.rs:43:17
+   |
+LL |         let _ = match_type(cx, ty, &["core", "result", "Result"]);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `utils::is_type_diagnostic_item(cx, ty, sym!(result_type))`
+
+error: usage of `utils::match_type()` on a type diagnostic item
+  --> $DIR/match_type_on_diag_item.rs:46:17
+   |
+LL |         let _ = utils::match_type(cx, ty, rc_path);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `utils::is_type_diagnostic_item(cx, ty, sym!(Rc))`
+
+error: aborting due to 4 previous errors
+