]> git.lizzy.rs Git - rust.git/blobdiff - src/tools/clippy/clippy_lints/src/matches.rs
Auto merge of #79780 - camelid:use-summary_opts, r=GuillaumeGomez
[rust.git] / src / tools / clippy / clippy_lints / src / matches.rs
index 4bdfca1a2921d8e16c8f0571a0f965714fdfe508..274d20cfa8005b772f3aabeb9866b524d67fed95 100644 (file)
@@ -3,8 +3,8 @@
 use crate::utils::usage::is_unused;
 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, multispan_sugg, remove_blocks, snippet,
-    snippet_block, snippet_with_applicability, span_lint_and_help, span_lint_and_note, span_lint_and_sugg,
+    is_type_diagnostic_item, is_wild, match_qpath, match_type, match_var, meets_msrv, multispan_sugg, remove_blocks,
+    snippet, snippet_block, snippet_with_applicability, span_lint_and_help, span_lint_and_note, span_lint_and_sugg,
     span_lint_and_then,
 };
 use crate::utils::{paths, search_same, SpanlessEq, SpanlessHash};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::{self, Ty, TyS};
+use rustc_semver::RustcVersion;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::{Span, Spanned};
-use rustc_span::Symbol;
+use rustc_span::{sym, Symbol};
 use std::cmp::Ordering;
 use std::collections::hash_map::Entry;
 use std::collections::Bound;
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Lint for redundant pattern matching over `Result` or
-    /// `Option`
+    /// **What it does:** Lint for redundant pattern matching over `Result`, `Option`,
+    /// `std::task::Poll` or `std::net::IpAddr`
     ///
     /// **Why is this bad?** It's more concise and clear to just use the proper
     /// utility function
     /// **Example:**
     ///
     /// ```rust
+    /// # use std::task::Poll;
+    /// # use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
     /// if let Ok(_) = Ok::<i32, i32>(42) {}
     /// if let Err(_) = Err::<i32, i32>(42) {}
     /// if let None = None::<()> {}
     /// if let Some(_) = Some(42) {}
+    /// if let Poll::Pending = Poll::Pending::<()> {}
+    /// if let Poll::Ready(_) = Poll::Ready(42) {}
+    /// if let IpAddr::V4(_) = IpAddr::V4(Ipv4Addr::LOCALHOST) {}
+    /// if let IpAddr::V6(_) = IpAddr::V6(Ipv6Addr::LOCALHOST) {}
     /// match Ok::<i32, i32>(42) {
     ///     Ok(_) => true,
     ///     Err(_) => false,
     /// The more idiomatic use would be:
     ///
     /// ```rust
+    /// # use std::task::Poll;
+    /// # use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
     /// if Ok::<i32, i32>(42).is_ok() {}
     /// if Err::<i32, i32>(42).is_err() {}
     /// if None::<()>.is_none() {}
     /// if Some(42).is_some() {}
+    /// if Poll::Pending::<()>.is_pending() {}
+    /// if Poll::Ready(42).is_ready() {}
+    /// if IpAddr::V4(Ipv4Addr::LOCALHOST).is_ipv4() {}
+    /// if IpAddr::V6(Ipv6Addr::LOCALHOST).is_ipv6() {}
     /// Ok::<i32, i32>(42).is_ok();
     /// ```
     pub REDUNDANT_PATTERN_MATCHING,
     ///
     /// **Why is this bad?** Readability and needless complexity.
     ///
-    /// **Known problems:** None
+    /// **Known problems:** This lint falsely triggers, if there are arms with
+    /// `cfg` attributes that remove an arm evaluating to `false`.
     ///
     /// **Example:**
     /// ```rust
 
 #[derive(Default)]
 pub struct Matches {
+    msrv: Option<RustcVersion>,
     infallible_destructuring_match_linted: bool,
 }
 
+impl Matches {
+    #[must_use]
+    pub fn new(msrv: Option<RustcVersion>) -> Self {
+        Self {
+            msrv,
+            ..Matches::default()
+        }
+    }
+}
+
 impl_lint_pass!(Matches => [
     SINGLE_MATCH,
     MATCH_REF_PATS,
@@ -543,6 +568,8 @@ pub struct Matches {
     MATCH_SAME_ARMS,
 ]);
 
+const MATCH_LIKE_MATCHES_MACRO_MSRV: RustcVersion = RustcVersion::new(1, 42, 0);
+
 impl<'tcx> LateLintPass<'tcx> for Matches {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         if in_external_macro(cx.sess(), expr.span) || in_macro(expr.span) {
@@ -550,7 +577,12 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         }
 
         redundant_pattern_match::check(cx, expr);
-        if !check_match_like_matches(cx, expr) {
+
+        if meets_msrv(self.msrv.as_ref(), &MATCH_LIKE_MATCHES_MACRO_MSRV) {
+            if !check_match_like_matches(cx, expr) {
+                lint_match_arms(cx, expr);
+            }
+        } else {
             lint_match_arms(cx, expr);
         }
 
@@ -614,8 +646,7 @@ fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
         if_chain! {
             if !in_external_macro(cx.sess(), pat.span);
             if !in_macro(pat.span);
-            if let PatKind::Struct(ref qpath, fields, true) = pat.kind;
-            if let QPath::Resolved(_, ref path) = qpath;
+            if let PatKind::Struct(QPath::Resolved(_, ref path), fields, true) = pat.kind;
             if let Some(def_id) = path.res.opt_def_id();
             let ty = cx.tcx.type_of(def_id);
             if let ty::Adt(def, _) = ty.kind();
@@ -634,6 +665,8 @@ fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
             }
         }
     }
+
+    extract_msrv_attr!(LateContext);
 }
 
 #[rustfmt::skip]
@@ -662,7 +695,7 @@ fn check_single_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], exp
             }
         } else {
             // not a block, don't lint
-            return; 
+            return;
         };
 
         let ty = cx.typeck_results().expr_ty(ex);
@@ -840,7 +873,7 @@ fn check_overlapping_arms<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'_>, arms
 
 fn check_wild_err_arm(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
     let ex_ty = cx.typeck_results().expr_ty(ex).peel_refs();
-    if is_type_diagnostic_item(cx, ex_ty, sym!(result_type)) {
+    if is_type_diagnostic_item(cx, ex_ty, sym::result_type) {
         for arm in arms {
             if let PatKind::TupleStruct(ref path, ref inner, _) = arm.pat.kind {
                 let path_str = rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_qpath(path, false));
@@ -922,16 +955,14 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>])
                 if let QPath::Resolved(_, p) = path {
                     missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
                 }
-            } else if let PatKind::TupleStruct(ref path, ref patterns, ..) = arm.pat.kind {
-                if let QPath::Resolved(_, p) = path {
-                    // Some simple checks for exhaustive patterns.
-                    // There is a room for improvements to detect more cases,
-                    // but it can be more expensive to do so.
-                    let is_pattern_exhaustive =
-                        |pat: &&Pat<'_>| matches!(pat.kind, PatKind::Wild | PatKind::Binding(.., None));
-                    if patterns.iter().all(is_pattern_exhaustive) {
-                        missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
-                    }
+            } else if let PatKind::TupleStruct(QPath::Resolved(_, p), ref patterns, ..) = arm.pat.kind {
+                // Some simple checks for exhaustive patterns.
+                // There is a room for improvements to detect more cases,
+                // but it can be more expensive to do so.
+                let is_pattern_exhaustive =
+                    |pat: &&Pat<'_>| matches!(pat.kind, PatKind::Wild | PatKind::Binding(.., None));
+                if patterns.iter().all(is_pattern_exhaustive) {
+                    missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
                 }
             }
         }
@@ -1134,13 +1165,16 @@ fn find_matches_sugg(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr
         if b0 != b1;
         let if_guard = &b0_arms[0].guard;
         if if_guard.is_none() || b0_arms.len() == 1;
+        if b0_arms[0].attrs.is_empty();
         if b0_arms[1..].iter()
             .all(|arm| {
                 find_bool_lit(&arm.body.kind, desugared).map_or(false, |b| b == b0) &&
-                arm.guard.is_none()
+                arm.guard.is_none() && arm.attrs.is_empty()
             });
         then {
-            let mut applicability = Applicability::MachineApplicable;
+            // The suggestion may be incorrect, because some arms can have `cfg` attributes
+            // evaluated into `false` and so such arms will be stripped before.
+            let mut applicability = Applicability::MaybeIncorrect;
             let pat = {
                 use itertools::Itertools as _;
                 b0_arms.iter()
@@ -1403,8 +1437,7 @@ fn is_ref_some_arm(arm: &Arm<'_>) -> Option<BindingAnnotation> {
         if let ExprKind::Call(ref e, ref args) = remove_blocks(&arm.body).kind;
         if let ExprKind::Path(ref some_path) = e.kind;
         if match_qpath(some_path, &paths::OPTION_SOME) && args.len() == 1;
-        if let ExprKind::Path(ref qpath) = args[0].kind;
-        if let &QPath::Resolved(_, ref path2) = qpath;
+        if let ExprKind::Path(QPath::Resolved(_, ref path2)) = args[0].kind;
         if path2.segments.len() == 1 && ident.name == path2.segments[0].ident.name;
         then {
             return Some(rb)
@@ -1509,6 +1542,7 @@ mod redundant_pattern_match {
     use rustc_errors::Applicability;
     use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind, QPath};
     use rustc_lint::LateContext;
+    use rustc_span::sym;
 
     pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         if let ExprKind::Match(op, arms, ref match_source) = &expr.kind {
@@ -1537,6 +1571,12 @@ fn find_sugg_for_if_let<'tcx>(
                         "is_err()"
                     } else if match_qpath(path, &paths::OPTION_SOME) {
                         "is_some()"
+                    } else if match_qpath(path, &paths::POLL_READY) {
+                        "is_ready()"
+                    } else if match_qpath(path, &paths::IPADDR_V4) {
+                        "is_ipv4()"
+                    } else if match_qpath(path, &paths::IPADDR_V6) {
+                        "is_ipv6()"
                     } else {
                         return;
                     }
@@ -1544,7 +1584,15 @@ fn find_sugg_for_if_let<'tcx>(
                     return;
                 }
             },
-            PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",
+            PatKind::Path(ref path) => {
+                if match_qpath(path, &paths::OPTION_NONE) {
+                    "is_none()"
+                } else if match_qpath(path, &paths::POLL_PENDING) {
+                    "is_pending()"
+                } else {
+                    return;
+                }
+            },
             _ => return,
         };
 
@@ -1552,7 +1600,7 @@ fn find_sugg_for_if_let<'tcx>(
         if_chain! {
             if keyword == "while";
             if let ExprKind::MethodCall(method_path, _, _, _) = op.kind;
-            if method_path.ident.name == sym!(next);
+            if method_path.ident.name == sym::next;
             if match_trait_method(cx, op, &paths::ITERATOR);
             then {
                 return;
@@ -1609,6 +1657,17 @@ fn find_sugg_for_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op: &
                             "is_ok()",
                             "is_err()",
                         )
+                        .or_else(|| {
+                            find_good_method_for_match(
+                                arms,
+                                path_left,
+                                path_right,
+                                &paths::IPADDR_V4,
+                                &paths::IPADDR_V6,
+                                "is_ipv4()",
+                                "is_ipv6()",
+                            )
+                        })
                     } else {
                         None
                     }
@@ -1627,6 +1686,17 @@ fn find_sugg_for_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op: &
                             "is_some()",
                             "is_none()",
                         )
+                        .or_else(|| {
+                            find_good_method_for_match(
+                                arms,
+                                path_left,
+                                path_right,
+                                &paths::POLL_READY,
+                                &paths::POLL_PENDING,
+                                "is_ready()",
+                                "is_pending()",
+                            )
+                        })
                     } else {
                         None
                     }