]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/matches.rs
Merge remote-tracking branch 'upstream/master' into rustup
[rust.git] / clippy_lints / src / matches.rs
index 6372cb86616ec07c12cfb615f24df935b070fc0d..e33001b16bcde20b2a172738dcf71d98628d1f1f 100644 (file)
@@ -1,11 +1,12 @@
 use crate::consts::{constant, miri_to_const, Constant};
 use crate::utils::sugg::Sugg;
-use crate::utils::usage::is_unused;
+use crate::utils::visitors::LocalUsedVisitor;
 use crate::utils::{
-    expr_block, get_arg_name, get_parent_expr, in_macro, indent_of, is_allowed, is_expn_of, is_refutable,
-    is_type_diagnostic_item, is_wild, match_qpath, match_type, match_var, meets_msrv, multispan_sugg, remove_blocks,
-    snippet, snippet_block, snippet_opt, snippet_with_applicability, span_lint_and_help, span_lint_and_note,
-    span_lint_and_sugg, span_lint_and_then,
+    expr_block, get_parent_expr, implements_trait, in_macro, indent_of, is_allowed, is_expn_of, is_refutable,
+    is_type_diagnostic_item, is_wild, match_qpath, match_type, meets_msrv, multispan_sugg, path_to_local_id,
+    peel_hir_pat_refs, peel_mid_ty_refs, peel_n_hir_expr_refs, remove_blocks, snippet, snippet_block, snippet_opt,
+    snippet_with_applicability, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then,
+    strip_pat_refs,
 };
 use crate::utils::{paths, search_same, SpanlessEq, SpanlessHash};
 use if_chain::if_chain;
@@ -616,9 +617,9 @@ fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'_>) {
             if let PatKind::TupleStruct(
                 QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pat.kind;
             if args.len() == 1;
-            if let Some(arg) = get_arg_name(&args[0]);
+            if let PatKind::Binding(_, arg, ..) = strip_pat_refs(&args[0]).kind;
             let body = remove_blocks(&arms[0].body);
-            if match_var(body, arg);
+            if path_to_local_id(body, arg);
 
             then {
                 let mut applicability = Applicability::MachineApplicable;
@@ -728,20 +729,60 @@ fn report_single_match_single_pattern(
     let els_str = els.map_or(String::new(), |els| {
         format!(" else {}", expr_block(cx, els, None, "..", Some(expr.span)))
     });
+
+    let (msg, sugg) = if_chain! {
+        let (pat, pat_ref_count) = peel_hir_pat_refs(arms[0].pat);
+        if let PatKind::Path(_) | PatKind::Lit(_) = pat.kind;
+        let (ty, ty_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(ex));
+        if let Some(trait_id) = cx.tcx.lang_items().structural_peq_trait();
+        if ty.is_integral() || ty.is_char() || ty.is_str() || implements_trait(cx, ty, trait_id, &[]);
+        then {
+            // scrutinee derives PartialEq and the pattern is a constant.
+            let pat_ref_count = match pat.kind {
+                // string literals are already a reference.
+                PatKind::Lit(Expr { kind: ExprKind::Lit(lit), .. }) if lit.node.is_str() => pat_ref_count + 1,
+                _ => pat_ref_count,
+            };
+            // References are only implicitly added to the pattern, so no overflow here.
+            // e.g. will work: match &Some(_) { Some(_) => () }
+            // will not: match Some(_) { &Some(_) => () }
+            let ref_count_diff = ty_ref_count - pat_ref_count;
+
+            // Try to remove address of expressions first.
+            let (ex, removed) = peel_n_hir_expr_refs(ex, ref_count_diff);
+            let ref_count_diff = ref_count_diff - removed;
+
+            let msg = "you seem to be trying to use `match` for an equality check. Consider using `if`";
+            let sugg = format!(
+                "if {} == {}{} {}{}",
+                snippet(cx, ex.span, ".."),
+                // PartialEq for different reference counts may not exist.
+                "&".repeat(ref_count_diff),
+                snippet(cx, arms[0].pat.span, ".."),
+                expr_block(cx, &arms[0].body, None, "..", Some(expr.span)),
+                els_str,
+            );
+            (msg, sugg)
+        } else {
+            let msg = "you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`";
+            let sugg = format!(
+                "if let {} = {} {}{}",
+                snippet(cx, arms[0].pat.span, ".."),
+                snippet(cx, ex.span, ".."),
+                expr_block(cx, &arms[0].body, None, "..", Some(expr.span)),
+                els_str,
+            );
+            (msg, sugg)
+        }
+    };
+
     span_lint_and_sugg(
         cx,
         lint,
         expr.span,
-        "you seem to be trying to use match for destructuring a single pattern. Consider using `if \
-         let`",
+        msg,
         "try this",
-        format!(
-            "if let {} = {} {}{}",
-            snippet(cx, arms[0].pat.span, ".."),
-            snippet(cx, ex.span, ".."),
-            expr_block(cx, &arms[0].body, None, "..", Some(expr.span)),
-            els_str,
-        ),
+        sugg,
         Applicability::HasPlaceholders,
     );
 }
@@ -870,7 +911,7 @@ fn check_overlapping_arms<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'_>, arms
     }
 }
 
-fn check_wild_err_arm(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
+fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm<'tcx>]) {
     let ex_ty = cx.typeck_results().expr_ty(ex).peel_refs();
     if is_type_diagnostic_item(cx, ex_ty, sym::result_type) {
         for arm in arms {
@@ -882,8 +923,10 @@ fn check_wild_err_arm(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
                     if !matching_wild {
                         // Looking for unused bindings (i.e.: `_e`)
                         inner.iter().for_each(|pat| {
-                            if let PatKind::Binding(.., ident, None) = &pat.kind {
-                                if ident.as_str().starts_with('_') && is_unused(ident, arm.body) {
+                            if let PatKind::Binding(_, id, ident, None) = pat.kind {
+                                if ident.as_str().starts_with('_')
+                                    && !LocalUsedVisitor::new(cx, id).check_expr(arm.body)
+                                {
                                     ident_bind_name = (&ident.name.as_str()).to_string();
                                     matching_wild = true;
                                 }
@@ -1552,7 +1595,17 @@ fn cmp(&self, other: &Self) -> Ordering {
                 }
             },
             (&Kind::End(a, _), &Kind::Start(b, _)) if a != Bound::Included(b) => (),
-            _ => return Some((a.range(), b.range())),
+            _ => {
+                // skip if the range `a` is completely included into the range `b`
+                if let Ordering::Equal | Ordering::Less = a.cmp(&b) {
+                    let kind_a = Kind::End(a.range().node.1, a.range());
+                    let kind_b = Kind::End(b.range().node.1, b.range());
+                    if let Ordering::Equal | Ordering::Greater = kind_a.cmp(&kind_b) {
+                        return None;
+                    }
+                }
+                return Some((a.range(), b.range()));
+            },
         }
     }