]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/methods/mod.rs
Account for trait alias when looking for defid
[rust.git] / clippy_lints / src / methods / mod.rs
index 43388a891532891d51c396165f3b7075169e683d..0eaec449dc4f020b2969c1dc79450a7ffad75193 100644 (file)
@@ -1075,56 +1075,57 @@ fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &'tcx hir::
             if let hir::ImplItemKind::Method(ref sig, id) = impl_item.node;
             if let Some(first_arg) = iter_input_pats(&sig.decl, cx.tcx.hir().body(id)).next();
             if let hir::ItemKind::Impl(_, _, _, _, None, _, _) = item.node;
+
+            let method_def_id = cx.tcx.hir().local_def_id(impl_item.hir_id);
+            let method_sig = cx.tcx.fn_sig(method_def_id);
+            let method_sig = cx.tcx.erase_late_bound_regions(&method_sig);
+
+            let first_arg_ty = &method_sig.inputs().iter().next();
+
+            // check conventions w.r.t. conversion method names and predicates
+            if let Some(first_arg_ty) = first_arg_ty;
+
             then {
-                let method_def_id = cx.tcx.hir().local_def_id(impl_item.hir_id);
-                let method_sig = cx.tcx.fn_sig(method_def_id);
-                let method_sig = cx.tcx.erase_late_bound_regions(&method_sig);
-
-                let first_arg_ty = &method_sig.inputs().iter().next();
-
-                // check conventions w.r.t. conversion method names and predicates
-                if let Some(first_arg_ty) = first_arg_ty {
-
-                    if cx.access_levels.is_exported(impl_item.hir_id) {
-                    // check missing trait implementations
-                        for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS {
-                            if name == method_name &&
-                            sig.decl.inputs.len() == n_args &&
-                            out_type.matches(cx, &sig.decl.output) &&
-                            self_kind.matches(cx, ty, first_arg_ty) {
-                                span_lint(cx, SHOULD_IMPLEMENT_TRAIT, impl_item.span, &format!(
-                                    "defining a method called `{}` on this type; consider implementing \
-                                    the `{}` trait or choosing a less ambiguous name", name, trait_name));
-                            }
+                if cx.access_levels.is_exported(impl_item.hir_id) {
+                // check missing trait implementations
+                    for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS {
+                        if name == method_name &&
+                        sig.decl.inputs.len() == n_args &&
+                        out_type.matches(cx, &sig.decl.output) &&
+                        self_kind.matches(cx, ty, first_arg_ty) {
+                            span_lint(cx, SHOULD_IMPLEMENT_TRAIT, impl_item.span, &format!(
+                                "defining a method called `{}` on this type; consider implementing \
+                                the `{}` trait or choosing a less ambiguous name", name, trait_name));
                         }
                     }
+                }
 
-                    for &(ref conv, self_kinds) in &CONVENTIONS {
-                        if conv.check(&name) {
-                            if !self_kinds
-                                    .iter()
-                                    .any(|k| k.matches(cx, ty, first_arg_ty)) {
-                                let lint = if item.vis.node.is_pub() {
-                                    WRONG_PUB_SELF_CONVENTION
-                                } else {
-                                    WRONG_SELF_CONVENTION
-                                };
-                                span_lint(cx,
-                                          lint,
-                                          first_arg.pat.span,
-                                          &format!("methods called `{}` usually take {}; consider choosing a less \
-                                                    ambiguous name",
-                                                   conv,
-                                                   &self_kinds.iter()
-                                                              .map(|k| k.description())
-                                                              .collect::<Vec<_>>()
-                                                              .join(" or ")));
-                            }
+                if let Some((ref conv, self_kinds)) = &CONVENTIONS
+                    .iter()
+                    .find(|(ref conv, _)| conv.check(&name))
+                {
+                    if !self_kinds.iter().any(|k| k.matches(cx, ty, first_arg_ty)) {
+                        let lint = if item.vis.node.is_pub() {
+                            WRONG_PUB_SELF_CONVENTION
+                        } else {
+                            WRONG_SELF_CONVENTION
+                        };
 
-                            // Only check the first convention to match (CONVENTIONS should be listed from most to least
-                            // specific)
-                            break;
-                        }
+                        span_lint(
+                            cx,
+                            lint,
+                            first_arg.pat.span,
+                            &format!(
+                               "methods called `{}` usually take {}; consider choosing a less \
+                                 ambiguous name",
+                                conv,
+                                &self_kinds
+                                    .iter()
+                                    .map(|k| k.description())
+                                    .collect::<Vec<_>>()
+                                    .join(" or ")
+                            ),
+                        );
                     }
                 }
             }
@@ -1134,10 +1135,8 @@ fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &'tcx hir::
             let ret_ty = return_ty(cx, impl_item.hir_id);
 
             // walk the return type and check for Self (this does not check associated types)
-            for inner_type in ret_ty.walk() {
-                if same_tys(cx, ty, inner_type) {
-                    return;
-                }
+            if ret_ty.walk().any(|inner_type| same_tys(cx, ty, inner_type)) {
+                return;
             }
 
             // if return type is impl trait, check the associated types
@@ -1230,43 +1229,36 @@ fn check_unwrap_or_default(
         or_has_args: bool,
         span: Span,
     ) -> bool {
-        if or_has_args {
-            return false;
-        }
-
-        if name == "unwrap_or" {
-            if let hir::ExprKind::Path(ref qpath) = fun.node {
-                let path = &*last_path_segment(qpath).ident.as_str();
+        if_chain! {
+            if !or_has_args;
+            if name == "unwrap_or";
+            if let hir::ExprKind::Path(ref qpath) = fun.node;
+            let path = &*last_path_segment(qpath).ident.as_str();
+            if ["default", "new"].contains(&path);
+            let arg_ty = cx.tables.expr_ty(arg);
+            if let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT);
+            if implements_trait(cx, arg_ty, default_trait_id, &[]);
 
-                if ["default", "new"].contains(&path) {
-                    let arg_ty = cx.tables.expr_ty(arg);
-                    let default_trait_id = if let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT) {
-                        default_trait_id
-                    } else {
-                        return false;
-                    };
+            then {
+                let mut applicability = Applicability::MachineApplicable;
+                span_lint_and_sugg(
+                    cx,
+                    OR_FUN_CALL,
+                    span,
+                    &format!("use of `{}` followed by a call to `{}`", name, path),
+                    "try this",
+                    format!(
+                        "{}.unwrap_or_default()",
+                        snippet_with_applicability(cx, self_expr.span, "_", &mut applicability)
+                    ),
+                    applicability,
+                );
 
-                    if implements_trait(cx, arg_ty, default_trait_id, &[]) {
-                        let mut applicability = Applicability::MachineApplicable;
-                        span_lint_and_sugg(
-                            cx,
-                            OR_FUN_CALL,
-                            span,
-                            &format!("use of `{}` followed by a call to `{}`", name, path),
-                            "try this",
-                            format!(
-                                "{}.unwrap_or_default()",
-                                snippet_with_applicability(cx, self_expr.span, "_", &mut applicability)
-                            ),
-                            applicability,
-                        );
-                        return true;
-                    }
-                }
+                true
+            } else {
+                false
             }
         }
-
-        false
     }
 
     /// Checks for `*or(foo())`.
@@ -1289,46 +1281,37 @@ fn check_general_case<'a, 'tcx>(
             (&paths::RESULT, true, &["or", "unwrap_or"], "else"),
         ];
 
-        // early check if the name is one we care about
-        if know_types.iter().all(|k| !k.2.contains(&name)) {
-            return;
-        }
+        if_chain! {
+            if know_types.iter().any(|k| k.2.contains(&name));
 
-        let mut finder = FunCallFinder { cx: &cx, found: false };
-        finder.visit_expr(&arg);
-        if !finder.found {
-            return;
-        }
+            let mut finder = FunCallFinder { cx: &cx, found: false };
+            if { finder.visit_expr(&arg); finder.found };
 
-        let self_ty = cx.tables.expr_ty(self_expr);
+            let self_ty = cx.tables.expr_ty(self_expr);
 
-        let (fn_has_arguments, poss, suffix) = if let Some(&(_, fn_has_arguments, poss, suffix)) =
-            know_types.iter().find(|&&i| match_type(cx, self_ty, i.0))
-        {
-            (fn_has_arguments, poss, suffix)
-        } else {
-            return;
-        };
+            if let Some(&(_, fn_has_arguments, poss, suffix)) =
+                   know_types.iter().find(|&&i| match_type(cx, self_ty, i.0));
 
-        if !poss.contains(&name) {
-            return;
-        }
+            if poss.contains(&name);
 
-        let sugg: Cow<'_, _> = match (fn_has_arguments, !or_has_args) {
-            (true, _) => format!("|_| {}", snippet_with_macro_callsite(cx, arg.span, "..")).into(),
-            (false, false) => format!("|| {}", snippet_with_macro_callsite(cx, arg.span, "..")).into(),
-            (false, true) => snippet_with_macro_callsite(cx, fun_span, ".."),
-        };
-        let span_replace_word = method_span.with_hi(span.hi());
-        span_lint_and_sugg(
-            cx,
-            OR_FUN_CALL,
-            span_replace_word,
-            &format!("use of `{}` followed by a function call", name),
-            "try this",
-            format!("{}_{}({})", name, suffix, sugg),
-            Applicability::HasPlaceholders,
-        );
+            then {
+                let sugg: Cow<'_, _> = match (fn_has_arguments, !or_has_args) {
+                    (true, _) => format!("|_| {}", snippet_with_macro_callsite(cx, arg.span, "..")).into(),
+                    (false, false) => format!("|| {}", snippet_with_macro_callsite(cx, arg.span, "..")).into(),
+                    (false, true) => snippet_with_macro_callsite(cx, fun_span, ".."),
+                };
+                let span_replace_word = method_span.with_hi(span.hi());
+                span_lint_and_sugg(
+                    cx,
+                    OR_FUN_CALL,
+                    span_replace_word,
+                    &format!("use of `{}` followed by a function call", name),
+                    "try this",
+                    format!("{}_{}({})", name, suffix, sugg),
+                    Applicability::HasPlaceholders,
+                );
+            }
+        }
     }
 
     if args.len() == 2 {
@@ -1413,18 +1396,20 @@ fn generate_format_arg_snippet(
         a: &hir::Expr,
         applicability: &mut Applicability,
     ) -> Vec<String> {
-        if let hir::ExprKind::AddrOf(_, ref format_arg) = a.node {
-            if let hir::ExprKind::Match(ref format_arg_expr, _, _) = format_arg.node {
-                if let hir::ExprKind::Tup(ref format_arg_expr_tup) = format_arg_expr.node {
-                    return format_arg_expr_tup
-                        .iter()
-                        .map(|a| snippet_with_applicability(cx, a.span, "..", applicability).into_owned())
-                        .collect();
-                }
-            }
-        };
+        if_chain! {
+            if let hir::ExprKind::AddrOf(_, ref format_arg) = a.node;
+            if let hir::ExprKind::Match(ref format_arg_expr, _, _) = format_arg.node;
+            if let hir::ExprKind::Tup(ref format_arg_expr_tup) = format_arg_expr.node;
 
-        unreachable!()
+            then {
+                format_arg_expr_tup
+                    .iter()
+                    .map(|a| snippet_with_applicability(cx, a.span, "..", applicability).into_owned())
+                    .collect()
+            } else {
+                unreachable!()
+            }
+        }
     }
 
     fn is_call(node: &hir::ExprKind) -> bool {
@@ -1688,20 +1673,22 @@ fn lint_cstring_as_ptr(cx: &LateContext<'_, '_>, expr: &hir::Expr, source: &hir:
 }
 
 fn lint_iter_cloned_collect<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Expr, iter_args: &'tcx [hir::Expr]) {
-    if match_type(cx, cx.tables.expr_ty(expr), &paths::VEC) {
-        if let Some(slice) = derefs_to_slice(cx, &iter_args[0], cx.tables.expr_ty(&iter_args[0])) {
-            if let Some(to_replace) = expr.span.trim_start(slice.span.source_callsite()) {
-                span_lint_and_sugg(
-                    cx,
-                    ITER_CLONED_COLLECT,
-                    to_replace,
-                    "called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and \
-                     more readable",
-                    "try",
-                    ".to_vec()".to_string(),
-                    Applicability::MachineApplicable,
-                );
-            }
+    if_chain! {
+        if match_type(cx, cx.tables.expr_ty(expr), &paths::VEC);
+        if let Some(slice) = derefs_to_slice(cx, &iter_args[0], cx.tables.expr_ty(&iter_args[0]));
+        if let Some(to_replace) = expr.span.trim_start(slice.span.source_callsite());
+
+        then {
+            span_lint_and_sugg(
+                cx,
+                ITER_CLONED_COLLECT,
+                to_replace,
+                "called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and \
+                 more readable",
+                "try",
+                ".to_vec()".to_string(),
+                Applicability::MachineApplicable,
+            );
         }
     }
 }
@@ -1961,18 +1948,20 @@ fn lint_unwrap(cx: &LateContext<'_, '_>, expr: &hir::Expr, unwrap_args: &[hir::E
 
 /// lint use of `ok().expect()` for `Result`s
 fn lint_ok_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr, ok_args: &[hir::Expr]) {
-    // lint if the caller of `ok()` is a `Result`
-    if match_type(cx, cx.tables.expr_ty(&ok_args[0]), &paths::RESULT) {
+    if_chain! {
+        // lint if the caller of `ok()` is a `Result`
+        if match_type(cx, cx.tables.expr_ty(&ok_args[0]), &paths::RESULT);
         let result_type = cx.tables.expr_ty(&ok_args[0]);
-        if let Some(error_type) = get_error_type(cx, result_type) {
-            if has_debug_impl(error_type, cx) {
-                span_lint(
-                    cx,
-                    OK_EXPECT,
-                    expr.span,
-                    "called `ok().expect()` on a Result value. You can call `expect` directly on the `Result`",
-                );
-            }
+        if let Some(error_type) = get_error_type(cx, result_type);
+        if has_debug_impl(error_type, cx);
+
+        then {
+            span_lint(
+                cx,
+                OK_EXPECT,
+                expr.span,
+                "called `ok().expect()` on a Result value. You can call `expect` directly on the `Result`",
+            );
         }
     }
 }
@@ -2524,11 +2513,11 @@ fn lint_chars_cmp_with_unwrap<'a, 'tcx>(
                 applicability,
             );
 
-            return true;
+            true
+        } else {
+            false
         }
     }
-
-    false
 }
 
 /// Checks for the `CHARS_NEXT_CMP` lint with `unwrap()`.
@@ -2616,7 +2605,7 @@ fn ty_has_iter_method(
     cx: &LateContext<'_, '_>,
     self_ref_ty: Ty<'_>,
 ) -> Option<(&'static Lint, &'static str, &'static str)> {
-    if let Some(ty_name) = has_iter_method(cx, self_ref_ty) {
+    has_iter_method(cx, self_ref_ty).map(|ty_name| {
         let lint = if ty_name == "array" || ty_name == "PathBuf" {
             INTO_ITER_ON_ARRAY
         } else {
@@ -2630,10 +2619,8 @@ fn ty_has_iter_method(
             hir::MutImmutable => "iter",
             hir::MutMutable => "iter_mut",
         };
-        Some((lint, ty_name, method_name))
-    } else {
-        None
-    }
+        (lint, ty_name, method_name)
+    })
 }
 
 fn lint_into_iter(cx: &LateContext<'_, '_>, expr: &hir::Expr, self_ref_ty: Ty<'_>, method_span: Span) {
@@ -2668,14 +2655,9 @@ fn lint_suspicious_map(cx: &LateContext<'_, '_>, expr: &hir::Expr) {
 
 /// Given a `Result<T, E>` type, return its error type (`E`).
 fn get_error_type<'a>(cx: &LateContext<'_, '_>, ty: Ty<'a>) -> Option<Ty<'a>> {
-    if let ty::Adt(_, substs) = ty.sty {
-        if match_type(cx, ty, &paths::RESULT) {
-            substs.types().nth(1)
-        } else {
-            None
-        }
-    } else {
-        None
+    match ty.sty {
+        ty::Adt(_, substs) if match_type(cx, ty, &paths::RESULT) => substs.types().nth(1),
+        _ => None,
     }
 }
 
@@ -2799,7 +2781,10 @@ fn matches_ref<'a>(
                 hir::Mutability::MutMutable => &paths::ASMUT_TRAIT,
             };
 
-            let trait_def_id = get_trait_def_id(cx, trait_path).expect("trait def id not found");
+            let trait_def_id = match get_trait_def_id(cx, trait_path) {
+                Some(did) => did,
+                None => return false,
+            };
             implements_trait(cx, ty, trait_def_id, &[parent_ty.into()])
         }