]> git.lizzy.rs Git - rust.git/commitdiff
Split out `match_single_binding`
authorJason Newcomb <jsnewcomb@pm.me>
Sun, 6 Feb 2022 21:38:34 +0000 (16:38 -0500)
committerJason Newcomb <jsnewcomb@pm.me>
Mon, 7 Feb 2022 17:22:27 +0000 (12:22 -0500)
clippy_lints/src/matches/match_single_binding.rs [new file with mode: 0644]
clippy_lints/src/matches/mod.rs

diff --git a/clippy_lints/src/matches/match_single_binding.rs b/clippy_lints/src/matches/match_single_binding.rs
new file mode 100644 (file)
index 0000000..8ae19e0
--- /dev/null
@@ -0,0 +1,166 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::{indent_of, snippet_block, snippet_opt, snippet_with_applicability};
+use clippy_utils::sugg::Sugg;
+use clippy_utils::{get_parent_expr, is_refutable, peel_blocks};
+use rustc_errors::Applicability;
+use rustc_hir::{Arm, Expr, ExprKind, Local, Node, PatKind};
+use rustc_lint::LateContext;
+
+use super::MATCH_SINGLE_BINDING;
+
+#[allow(clippy::too_many_lines)]
+pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], expr: &Expr<'_>) {
+    if expr.span.from_expansion() || arms.len() != 1 || is_refutable(cx, arms[0].pat) {
+        return;
+    }
+
+    // HACK:
+    // This is a hack to deal with arms that are excluded by macros like `#[cfg]`. It is only used here
+    // to prevent false positives as there is currently no better way to detect if code was excluded by
+    // a macro. See PR #6435
+    if_chain! {
+        if let Some(match_snippet) = snippet_opt(cx, expr.span);
+        if let Some(arm_snippet) = snippet_opt(cx, arms[0].span);
+        if let Some(ex_snippet) = snippet_opt(cx, ex.span);
+        let rest_snippet = match_snippet.replace(&arm_snippet, "").replace(&ex_snippet, "");
+        if rest_snippet.contains("=>");
+        then {
+            // The code it self contains another thick arrow "=>"
+            // -> Either another arm or a comment
+            return;
+        }
+    }
+
+    let matched_vars = ex.span;
+    let bind_names = arms[0].pat.span;
+    let match_body = peel_blocks(arms[0].body);
+    let mut snippet_body = if match_body.span.from_expansion() {
+        Sugg::hir_with_macro_callsite(cx, match_body, "..").to_string()
+    } else {
+        snippet_block(cx, match_body.span, "..", Some(expr.span)).to_string()
+    };
+
+    // Do we need to add ';' to suggestion ?
+    match match_body.kind {
+        ExprKind::Block(block, _) => {
+            // macro + expr_ty(body) == ()
+            if block.span.from_expansion() && cx.typeck_results().expr_ty(match_body).is_unit() {
+                snippet_body.push(';');
+            }
+        },
+        _ => {
+            // expr_ty(body) == ()
+            if cx.typeck_results().expr_ty(match_body).is_unit() {
+                snippet_body.push(';');
+            }
+        },
+    }
+
+    let mut applicability = Applicability::MaybeIncorrect;
+    match arms[0].pat.kind {
+        PatKind::Binding(..) | PatKind::Tuple(_, _) | PatKind::Struct(..) => {
+            // If this match is in a local (`let`) stmt
+            let (target_span, sugg) = if let Some(parent_let_node) = opt_parent_let(cx, ex) {
+                (
+                    parent_let_node.span,
+                    format!(
+                        "let {} = {};\n{}let {} = {};",
+                        snippet_with_applicability(cx, bind_names, "..", &mut applicability),
+                        snippet_with_applicability(cx, matched_vars, "..", &mut applicability),
+                        " ".repeat(indent_of(cx, expr.span).unwrap_or(0)),
+                        snippet_with_applicability(cx, parent_let_node.pat.span, "..", &mut applicability),
+                        snippet_body
+                    ),
+                )
+            } else {
+                // If we are in closure, we need curly braces around suggestion
+                let mut indent = " ".repeat(indent_of(cx, ex.span).unwrap_or(0));
+                let (mut cbrace_start, mut cbrace_end) = ("".to_string(), "".to_string());
+                if let Some(parent_expr) = get_parent_expr(cx, expr) {
+                    if let ExprKind::Closure(..) = parent_expr.kind {
+                        cbrace_end = format!("\n{}}}", indent);
+                        // Fix body indent due to the closure
+                        indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
+                        cbrace_start = format!("{{\n{}", indent);
+                    }
+                }
+                // If the parent is already an arm, and the body is another match statement,
+                // we need curly braces around suggestion
+                let parent_node_id = cx.tcx.hir().get_parent_node(expr.hir_id);
+                if let Node::Arm(arm) = &cx.tcx.hir().get(parent_node_id) {
+                    if let ExprKind::Match(..) = arm.body.kind {
+                        cbrace_end = format!("\n{}}}", indent);
+                        // Fix body indent due to the match
+                        indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
+                        cbrace_start = format!("{{\n{}", indent);
+                    }
+                }
+                (
+                    expr.span,
+                    format!(
+                        "{}let {} = {};\n{}{}{}",
+                        cbrace_start,
+                        snippet_with_applicability(cx, bind_names, "..", &mut applicability),
+                        snippet_with_applicability(cx, matched_vars, "..", &mut applicability),
+                        indent,
+                        snippet_body,
+                        cbrace_end
+                    ),
+                )
+            };
+            span_lint_and_sugg(
+                cx,
+                MATCH_SINGLE_BINDING,
+                target_span,
+                "this match could be written as a `let` statement",
+                "consider using `let` statement",
+                sugg,
+                applicability,
+            );
+        },
+        PatKind::Wild => {
+            if ex.can_have_side_effects() {
+                let indent = " ".repeat(indent_of(cx, expr.span).unwrap_or(0));
+                let sugg = format!(
+                    "{};\n{}{}",
+                    snippet_with_applicability(cx, ex.span, "..", &mut applicability),
+                    indent,
+                    snippet_body
+                );
+                span_lint_and_sugg(
+                    cx,
+                    MATCH_SINGLE_BINDING,
+                    expr.span,
+                    "this match could be replaced by its scrutinee and body",
+                    "consider using the scrutinee and body instead",
+                    sugg,
+                    applicability,
+                );
+            } else {
+                span_lint_and_sugg(
+                    cx,
+                    MATCH_SINGLE_BINDING,
+                    expr.span,
+                    "this match could be replaced by its body itself",
+                    "consider using the match body instead",
+                    snippet_body,
+                    Applicability::MachineApplicable,
+                );
+            }
+        },
+        _ => (),
+    }
+}
+
+/// Returns true if the `ex` match expression is in a local (`let`) statement
+fn opt_parent_let<'a>(cx: &LateContext<'a>, ex: &Expr<'a>) -> Option<&'a Local<'a>> {
+    let map = &cx.tcx.hir();
+    if_chain! {
+        if let Some(Node::Expr(parent_arm_expr)) = map.find(map.get_parent_node(ex.hir_id));
+        if let Some(Node::Local(parent_let_expr)) = map.find(map.get_parent_node(parent_arm_expr.hir_id));
+        then {
+            return Some(parent_let_expr);
+        }
+    }
+    None
+}
index b48cdcb7d7169a39268f62bd4c64910fee401df8..c2bf2aa6c9d1d888b6962a577bf75e74ea7d349c 100644 (file)
@@ -1,13 +1,11 @@
 use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
-use clippy_utils::source::{indent_of, snippet, snippet_block, snippet_opt, snippet_with_applicability};
+use clippy_utils::source::{snippet, snippet_with_applicability};
 use clippy_utils::sugg::Sugg;
-use clippy_utils::{
-    get_parent_expr, is_refutable, is_wild, meets_msrv, msrvs, path_to_local_id, peel_blocks, strip_pat_refs,
-};
+use clippy_utils::{is_wild, meets_msrv, msrvs, path_to_local_id, peel_blocks, strip_pat_refs};
 use core::iter::once;
 use if_chain::if_chain;
 use rustc_errors::Applicability;
-use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, Node, Pat, PatKind, QPath};
+use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, Pat, PatKind, QPath};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty;
 use rustc_semver::RustcVersion;
@@ -17,6 +15,7 @@
 mod match_bool;
 mod match_like_matches;
 mod match_same_arms;
+mod match_single_binding;
 mod match_wild_enum;
 mod match_wild_err_arm;
 mod overlapping_arms;
@@ -630,7 +629,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
             if self.infallible_destructuring_match_linted {
                 self.infallible_destructuring_match_linted = false;
             } else {
-                check_match_single_binding(cx, ex, arms, expr);
+                match_single_binding::check(cx, ex, arms, expr);
             }
         }
         if let ExprKind::Match(ex, arms, _) = expr.kind {
@@ -753,163 +752,6 @@ fn check_wild_in_or_pats(cx: &LateContext<'_>, arms: &[Arm<'_>]) {
     }
 }
 
-#[allow(clippy::too_many_lines)]
-fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], expr: &Expr<'_>) {
-    if expr.span.from_expansion() || arms.len() != 1 || is_refutable(cx, arms[0].pat) {
-        return;
-    }
-
-    // HACK:
-    // This is a hack to deal with arms that are excluded by macros like `#[cfg]`. It is only used here
-    // to prevent false positives as there is currently no better way to detect if code was excluded by
-    // a macro. See PR #6435
-    if_chain! {
-        if let Some(match_snippet) = snippet_opt(cx, expr.span);
-        if let Some(arm_snippet) = snippet_opt(cx, arms[0].span);
-        if let Some(ex_snippet) = snippet_opt(cx, ex.span);
-        let rest_snippet = match_snippet.replace(&arm_snippet, "").replace(&ex_snippet, "");
-        if rest_snippet.contains("=>");
-        then {
-            // The code it self contains another thick arrow "=>"
-            // -> Either another arm or a comment
-            return;
-        }
-    }
-
-    let matched_vars = ex.span;
-    let bind_names = arms[0].pat.span;
-    let match_body = peel_blocks(arms[0].body);
-    let mut snippet_body = if match_body.span.from_expansion() {
-        Sugg::hir_with_macro_callsite(cx, match_body, "..").to_string()
-    } else {
-        snippet_block(cx, match_body.span, "..", Some(expr.span)).to_string()
-    };
-
-    // Do we need to add ';' to suggestion ?
-    match match_body.kind {
-        ExprKind::Block(block, _) => {
-            // macro + expr_ty(body) == ()
-            if block.span.from_expansion() && cx.typeck_results().expr_ty(match_body).is_unit() {
-                snippet_body.push(';');
-            }
-        },
-        _ => {
-            // expr_ty(body) == ()
-            if cx.typeck_results().expr_ty(match_body).is_unit() {
-                snippet_body.push(';');
-            }
-        },
-    }
-
-    let mut applicability = Applicability::MaybeIncorrect;
-    match arms[0].pat.kind {
-        PatKind::Binding(..) | PatKind::Tuple(_, _) | PatKind::Struct(..) => {
-            // If this match is in a local (`let`) stmt
-            let (target_span, sugg) = if let Some(parent_let_node) = opt_parent_let(cx, ex) {
-                (
-                    parent_let_node.span,
-                    format!(
-                        "let {} = {};\n{}let {} = {};",
-                        snippet_with_applicability(cx, bind_names, "..", &mut applicability),
-                        snippet_with_applicability(cx, matched_vars, "..", &mut applicability),
-                        " ".repeat(indent_of(cx, expr.span).unwrap_or(0)),
-                        snippet_with_applicability(cx, parent_let_node.pat.span, "..", &mut applicability),
-                        snippet_body
-                    ),
-                )
-            } else {
-                // If we are in closure, we need curly braces around suggestion
-                let mut indent = " ".repeat(indent_of(cx, ex.span).unwrap_or(0));
-                let (mut cbrace_start, mut cbrace_end) = ("".to_string(), "".to_string());
-                if let Some(parent_expr) = get_parent_expr(cx, expr) {
-                    if let ExprKind::Closure(..) = parent_expr.kind {
-                        cbrace_end = format!("\n{}}}", indent);
-                        // Fix body indent due to the closure
-                        indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
-                        cbrace_start = format!("{{\n{}", indent);
-                    }
-                }
-                // If the parent is already an arm, and the body is another match statement,
-                // we need curly braces around suggestion
-                let parent_node_id = cx.tcx.hir().get_parent_node(expr.hir_id);
-                if let Node::Arm(arm) = &cx.tcx.hir().get(parent_node_id) {
-                    if let ExprKind::Match(..) = arm.body.kind {
-                        cbrace_end = format!("\n{}}}", indent);
-                        // Fix body indent due to the match
-                        indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
-                        cbrace_start = format!("{{\n{}", indent);
-                    }
-                }
-                (
-                    expr.span,
-                    format!(
-                        "{}let {} = {};\n{}{}{}",
-                        cbrace_start,
-                        snippet_with_applicability(cx, bind_names, "..", &mut applicability),
-                        snippet_with_applicability(cx, matched_vars, "..", &mut applicability),
-                        indent,
-                        snippet_body,
-                        cbrace_end
-                    ),
-                )
-            };
-            span_lint_and_sugg(
-                cx,
-                MATCH_SINGLE_BINDING,
-                target_span,
-                "this match could be written as a `let` statement",
-                "consider using `let` statement",
-                sugg,
-                applicability,
-            );
-        },
-        PatKind::Wild => {
-            if ex.can_have_side_effects() {
-                let indent = " ".repeat(indent_of(cx, expr.span).unwrap_or(0));
-                let sugg = format!(
-                    "{};\n{}{}",
-                    snippet_with_applicability(cx, ex.span, "..", &mut applicability),
-                    indent,
-                    snippet_body
-                );
-                span_lint_and_sugg(
-                    cx,
-                    MATCH_SINGLE_BINDING,
-                    expr.span,
-                    "this match could be replaced by its scrutinee and body",
-                    "consider using the scrutinee and body instead",
-                    sugg,
-                    applicability,
-                );
-            } else {
-                span_lint_and_sugg(
-                    cx,
-                    MATCH_SINGLE_BINDING,
-                    expr.span,
-                    "this match could be replaced by its body itself",
-                    "consider using the match body instead",
-                    snippet_body,
-                    Applicability::MachineApplicable,
-                );
-            }
-        },
-        _ => (),
-    }
-}
-
-/// Returns true if the `ex` match expression is in a local (`let`) statement
-fn opt_parent_let<'a>(cx: &LateContext<'a>, ex: &Expr<'a>) -> Option<&'a Local<'a>> {
-    let map = &cx.tcx.hir();
-    if_chain! {
-        if let Some(Node::Expr(parent_arm_expr)) = map.find(map.get_parent_node(ex.hir_id));
-        if let Some(Node::Local(parent_let_expr)) = map.find(map.get_parent_node(parent_arm_expr.hir_id));
-        then {
-            return Some(parent_let_expr);
-        }
-    }
-    None
-}
-
 fn has_multiple_ref_pats<'a, 'b, I>(pats: I) -> bool
 where
     'b: 'a,