]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/redundant_pattern_matching.rs
Auto merge of #4809 - iankronquist:patch-1, r=flip1995
[rust.git] / clippy_lints / src / redundant_pattern_matching.rs
index f8c5b29bad16a9f6afcb844fb1cbd4d2f5a1024d..f0acd993480b600c8393cfbcecb3ca5dd4c8f22e 100644 (file)
@@ -1,74 +1,56 @@
-// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
-// file at the top-level directory of this distribution.
-//
-// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
-// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
-// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
-// option. This file may not be copied, modified, or distributed
-// except according to those terms.
-
-
-use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
-use crate::rustc::{declare_tool_lint, lint_array};
-use crate::rustc::hir::*;
-use crate::syntax::ptr::P;
-use crate::syntax::ast::LitKind;
 use crate::utils::{match_qpath, paths, snippet, span_lint_and_then};
-use crate::rustc_errors::Applicability;
+use rustc_errors::Applicability;
+use rustc_hir::*;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use syntax::ast::LitKind;
 
-/// **What it does:** Lint for redundant pattern matching over `Result` or
-/// `Option`
-///
-/// **Why is this bad?** It's more concise and clear to just use the proper
-/// utility function
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-///
-/// ```rust
-/// if let Ok(_) = Ok::<i32, i32>(42) {}
-/// if let Err(_) = Err::<i32, i32>(42) {}
-/// if let None = None::<()> {}
-/// if let Some(_) = Some(42) {}
-/// match Ok::<i32, i32>(42) {
-///     Ok(_) => true,
-///     Err(_) => false,
-/// };
-/// ```
-///
-/// The more idiomatic use would be:
-///
-/// ```rust
-/// if Ok::<i32, i32>(42).is_ok() {}
-/// if Err::<i32, i32>(42).is_err() {}
-/// if None::<()>.is_none() {}
-/// if Some(42).is_some() {}
-/// Ok::<i32, i32>(42).is_ok();
-/// ```
-///
 declare_clippy_lint! {
+    /// **What it does:** Lint for redundant pattern matching over `Result` or
+    /// `Option`
+    ///
+    /// **Why is this bad?** It's more concise and clear to just use the proper
+    /// utility function
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// if let Ok(_) = Ok::<i32, i32>(42) {}
+    /// if let Err(_) = Err::<i32, i32>(42) {}
+    /// if let None = None::<()> {}
+    /// if let Some(_) = Some(42) {}
+    /// match Ok::<i32, i32>(42) {
+    ///     Ok(_) => true,
+    ///     Err(_) => false,
+    /// };
+    /// ```
+    ///
+    /// The more idiomatic use would be:
+    ///
+    /// ```rust
+    /// if Ok::<i32, i32>(42).is_ok() {}
+    /// if Err::<i32, i32>(42).is_err() {}
+    /// if None::<()>.is_none() {}
+    /// if Some(42).is_some() {}
+    /// Ok::<i32, i32>(42).is_ok();
+    /// ```
     pub REDUNDANT_PATTERN_MATCHING,
     style,
     "use the proper utility function avoiding an `if let`"
 }
 
-#[derive(Copy, Clone)]
-pub struct Pass;
-
-impl LintPass for Pass {
-    fn get_lints(&self) -> LintArray {
-        lint_array!(REDUNDANT_PATTERN_MATCHING)
-    }
-}
+declare_lint_pass!(RedundantPatternMatching => [REDUNDANT_PATTERN_MATCHING]);
 
-impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
-    #[allow(clippy::similar_names)]
-    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
-        if let ExprKind::Match(ref op, ref arms, ref match_source) = expr.node {
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPatternMatching {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
+        if let ExprKind::Match(op, arms, ref match_source) = &expr.kind {
             match match_source {
                 MatchSource::Normal => find_sugg_for_match(cx, expr, op, arms),
-                MatchSource::IfLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms),
+                MatchSource::IfLetDesugar { contains_else_clause } => {
+                    find_sugg_for_if_let(cx, expr, op, arms, *contains_else_clause)
+                },
                 _ => return,
             }
         }
@@ -77,68 +59,62 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
 
 fn find_sugg_for_if_let<'a, 'tcx>(
     cx: &LateContext<'a, 'tcx>,
-    expr: &'tcx Expr,
-    op: &P<Expr>,
-    arms: &HirVec<Arm>
+    expr: &'tcx Expr<'_>,
+    op: &Expr<'_>,
+    arms: &[Arm<'_>],
+    has_else: bool,
 ) {
-    if arms[0].pats.len() == 1 {
-        let good_method = match arms[0].pats[0].node {
-            PatKind::TupleStruct(ref path, ref pats, _) if pats.len() == 1 => {
-                if let PatKind::Wild = pats[0].node {
-                    if match_qpath(path, &paths::RESULT_OK) {
-                        "is_ok()"
-                    } else if match_qpath(path, &paths::RESULT_ERR) {
-                        "is_err()"
-                    } else if match_qpath(path, &paths::OPTION_SOME) {
-                        "is_some()"
-                    } else {
-                        return;
-                    }
+    let good_method = match arms[0].pat.kind {
+        PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => {
+            if let PatKind::Wild = patterns[0].kind {
+                if match_qpath(path, &paths::RESULT_OK) {
+                    "is_ok()"
+                } else if match_qpath(path, &paths::RESULT_ERR) {
+                    "is_err()"
+                } else if match_qpath(path, &paths::OPTION_SOME) {
+                    "is_some()"
                 } else {
                     return;
                 }
-            },
+            } else {
+                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()",
 
-            _ => return,
-        };
+        _ => return,
+    };
 
-        span_lint_and_then(
-            cx,
-            REDUNDANT_PATTERN_MATCHING,
-            arms[0].pats[0].span,
-            &format!("redundant pattern matching, consider using `{}`", good_method),
-            |db| {
-                let span = expr.span.to(op.span);
-                db.span_suggestion_with_applicability(
-                    span,
-                    "try this",
-                    format!("if {}.{}", snippet(cx, op.span, "_"), good_method),
-                    Applicability::MachineApplicable, // snippet
-                );
-            },
-        );
-    } else {
-        return;
-    }
+    let maybe_semi = if has_else { "" } else { ";" };
+
+    span_lint_and_then(
+        cx,
+        REDUNDANT_PATTERN_MATCHING,
+        arms[0].pat.span,
+        &format!("redundant pattern matching, consider using `{}`", good_method),
+        |db| {
+            let span = expr.span.to(op.span);
+            db.span_suggestion(
+                span,
+                "try this",
+                format!("{}.{}{}", snippet(cx, op.span, "_"), good_method, maybe_semi),
+                Applicability::MaybeIncorrect, // snippet
+            );
+        },
+    );
 }
 
-fn find_sugg_for_match<'a, 'tcx>(
-    cx: &LateContext<'a, 'tcx>,
-    expr: &'tcx Expr,
-    op: &P<Expr>,
-    arms: &HirVec<Arm>
-) {
+fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>, op: &Expr<'_>, arms: &[Arm<'_>]) {
     if arms.len() == 2 {
-        let node_pair = (&arms[0].pats[0].node, &arms[1].pats[0].node);
+        let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);
 
         let found_good_method = match node_pair {
             (
-                PatKind::TupleStruct(ref path_left, ref pats_left, _),
-                PatKind::TupleStruct(ref path_right, ref pats_right, _)
-            ) if pats_left.len() == 1 && pats_right.len() == 1 => {
-                if let (PatKind::Wild, PatKind::Wild) = (&pats_left[0].node, &pats_right[0].node) {
+                PatKind::TupleStruct(ref path_left, ref patterns_left, _),
+                PatKind::TupleStruct(ref path_right, ref patterns_right, _),
+            ) if patterns_left.len() == 1 && patterns_right.len() == 1 => {
+                if let (PatKind::Wild, PatKind::Wild) = (&patterns_left[0].kind, &patterns_right[0].kind) {
                     find_good_method_for_match(
                         arms,
                         path_left,
@@ -146,20 +122,17 @@ fn find_sugg_for_match<'a, 'tcx>(
                         &paths::RESULT_OK,
                         &paths::RESULT_ERR,
                         "is_ok()",
-                        "is_err()"
+                        "is_err()",
                     )
                 } else {
                     None
                 }
             },
-            (
-                PatKind::TupleStruct(ref path_left, ref pats, _),
-                PatKind::Path(ref path_right)
-            ) | (
-                PatKind::Path(ref path_left),
-                PatKind::TupleStruct(ref path_right, ref pats, _)
-            ) if pats.len() == 1 => {
-                if let PatKind::Wild = pats[0].node {
+            (PatKind::TupleStruct(ref path_left, ref patterns, _), PatKind::Path(ref path_right))
+            | (PatKind::Path(ref path_left), PatKind::TupleStruct(ref path_right, ref patterns, _))
+                if patterns.len() == 1 =>
+            {
+                if let PatKind::Wild = patterns[0].kind {
                     find_good_method_for_match(
                         arms,
                         path_left,
@@ -167,7 +140,7 @@ fn find_sugg_for_match<'a, 'tcx>(
                         &paths::OPTION_SOME,
                         &paths::OPTION_NONE,
                         "is_some()",
-                        "is_none()"
+                        "is_none()",
                     )
                 } else {
                     None
@@ -184,44 +157,40 @@ fn find_sugg_for_match<'a, 'tcx>(
                 &format!("redundant pattern matching, consider using `{}`", good_method),
                 |db| {
                     let span = expr.span.to(op.span);
-                    db.span_suggestion_with_applicability(
+                    db.span_suggestion(
                         span,
                         "try this",
                         format!("{}.{}", snippet(cx, op.span, "_"), good_method),
-                        Applicability::MachineApplicable, // snippet
+                        Applicability::MaybeIncorrect, // snippet
                     );
                 },
             );
         }
-    } else {
-        return;
     }
 }
 
 fn find_good_method_for_match<'a>(
-    arms: &HirVec<Arm>,
-    path_left: &QPath,
-    path_right: &QPath,
+    arms: &[Arm<'_>],
+    path_left: &QPath<'_>,
+    path_right: &QPath<'_>,
     expected_left: &[&str],
     expected_right: &[&str],
     should_be_left: &'a str,
-    should_be_right: &'a str
+    should_be_right: &'a str,
 ) -> Option<&'a str> {
     let body_node_pair = if match_qpath(path_left, expected_left) && match_qpath(path_right, expected_right) {
-        (&(*arms[0].body).node, &(*arms[1].body).node)
+        (&(*arms[0].body).kind, &(*arms[1].body).kind)
     } else if match_qpath(path_right, expected_left) && match_qpath(path_left, expected_right) {
-        (&(*arms[1].body).node, &(*arms[0].body).node)
+        (&(*arms[1].body).kind, &(*arms[0].body).kind)
     } else {
         return None;
     };
 
     match body_node_pair {
-        (ExprKind::Lit(ref lit_left), ExprKind::Lit(ref lit_right)) => {
-            match (&lit_left.node, &lit_right.node) {
-                (LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
-                (LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
-                _ => None,
-            }
+        (ExprKind::Lit(ref lit_left), ExprKind::Lit(ref lit_right)) => match (&lit_left.node, &lit_right.node) {
+            (LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
+            (LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
+            _ => None,
         },
         _ => None,
     }