]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/matches/mod.rs
Split out `infalliable_detructuring_match`
[rust.git] / clippy_lints / src / matches / mod.rs
index e2a103c58d1ef48d91a00a97cb65a3fb5a55d33d..0adfa424ee146165a79b29f8837604b474390fea 100644 (file)
@@ -1,29 +1,20 @@
-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::sugg::Sugg;
-use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::{
-    get_parent_expr, is_lang_ctor, is_refutable, is_wild, meets_msrv, msrvs, path_to_local_id, peel_blocks,
-    peel_hir_pat_refs, recurse_or_patterns, strip_pat_refs,
-};
-use core::iter::once;
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::{is_wild, meets_msrv, msrvs};
 use if_chain::if_chain;
-use rustc_errors::Applicability;
-use rustc_hir::def::{CtorKind, DefKind, Res};
-use rustc_hir::LangItem::{OptionNone, OptionSome};
-use rustc_hir::{
-    self as hir, Arm, BindingAnnotation, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, Node, Pat,
-    PatKind, PathSegment, QPath, TyKind,
-};
+use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat, PatKind, QPath};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::{self, VariantDef};
+use rustc_middle::ty;
 use rustc_semver::RustcVersion;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
-use rustc_span::sym;
 
+mod infalliable_detructuring_match;
+mod match_as_ref;
 mod match_bool;
 mod match_like_matches;
+mod match_ref_pats;
 mod match_same_arms;
+mod match_single_binding;
+mod match_wild_enum;
 mod match_wild_err_arm;
 mod overlapping_arms;
 mod redundant_pattern_match;
@@ -629,54 +620,23 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
             match_bool::check(cx, ex, arms, expr);
             overlapping_arms::check(cx, ex, arms);
             match_wild_err_arm::check(cx, ex, arms);
-            check_wild_enum_match(cx, ex, arms);
-            check_match_as_ref(cx, ex, arms, expr);
+            match_wild_enum::check(cx, ex, arms);
+            match_as_ref::check(cx, ex, arms, expr);
             check_wild_in_or_pats(cx, arms);
 
             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 {
-            check_match_ref_pats(cx, ex, arms.iter().map(|el| el.pat), expr);
+            match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr);
         }
     }
 
     fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'_>) {
-        if_chain! {
-            if !local.span.from_expansion();
-            if let Some(expr) = local.init;
-            if let ExprKind::Match(target, arms, MatchSource::Normal) = expr.kind;
-            if arms.len() == 1 && arms[0].guard.is_none();
-            if let PatKind::TupleStruct(
-                QPath::Resolved(None, variant_name), args, _) = arms[0].pat.kind;
-            if args.len() == 1;
-            if let PatKind::Binding(_, arg, ..) = strip_pat_refs(&args[0]).kind;
-            let body = peel_blocks(arms[0].body);
-            if path_to_local_id(body, arg);
-
-            then {
-                let mut applicability = Applicability::MachineApplicable;
-                self.infallible_destructuring_match_linted = true;
-                span_lint_and_sugg(
-                    cx,
-                    INFALLIBLE_DESTRUCTURING_MATCH,
-                    local.span,
-                    "you seem to be trying to use `match` to destructure a single infallible pattern. \
-                    Consider using `let`",
-                    "try this",
-                    format!(
-                        "let {}({}) = {};",
-                        snippet_with_applicability(cx, variant_name.span, "..", &mut applicability),
-                        snippet_with_applicability(cx, local.pat.span, "..", &mut applicability),
-                        snippet_with_applicability(cx, target.span, "..", &mut applicability),
-                    ),
-                    applicability,
-                );
-            }
-        }
+        self.infallible_destructuring_match_linted |= infalliable_detructuring_match::check(cx, local);
     }
 
     fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
@@ -705,281 +665,6 @@ fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
     extract_msrv_attr!(LateContext);
 }
 
-enum CommonPrefixSearcher<'a> {
-    None,
-    Path(&'a [PathSegment<'a>]),
-    Mixed,
-}
-impl<'a> CommonPrefixSearcher<'a> {
-    fn with_path(&mut self, path: &'a [PathSegment<'a>]) {
-        match path {
-            [path @ .., _] => self.with_prefix(path),
-            [] => (),
-        }
-    }
-
-    fn with_prefix(&mut self, path: &'a [PathSegment<'a>]) {
-        match self {
-            Self::None => *self = Self::Path(path),
-            Self::Path(self_path)
-                if path
-                    .iter()
-                    .map(|p| p.ident.name)
-                    .eq(self_path.iter().map(|p| p.ident.name)) => {},
-            Self::Path(_) => *self = Self::Mixed,
-            Self::Mixed => (),
-        }
-    }
-}
-
-fn is_hidden(cx: &LateContext<'_>, variant_def: &VariantDef) -> bool {
-    let attrs = cx.tcx.get_attrs(variant_def.def_id);
-    clippy_utils::attrs::is_doc_hidden(attrs) || clippy_utils::attrs::is_unstable(attrs)
-}
-
-#[allow(clippy::too_many_lines)]
-fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
-    let ty = cx.typeck_results().expr_ty(ex).peel_refs();
-    let adt_def = match ty.kind() {
-        ty::Adt(adt_def, _)
-            if adt_def.is_enum()
-                && !(is_type_diagnostic_item(cx, ty, sym::Option) || is_type_diagnostic_item(cx, ty, sym::Result)) =>
-        {
-            adt_def
-        },
-        _ => return,
-    };
-
-    // First pass - check for violation, but don't do much book-keeping because this is hopefully
-    // the uncommon case, and the book-keeping is slightly expensive.
-    let mut wildcard_span = None;
-    let mut wildcard_ident = None;
-    let mut has_non_wild = false;
-    for arm in arms {
-        match peel_hir_pat_refs(arm.pat).0.kind {
-            PatKind::Wild => wildcard_span = Some(arm.pat.span),
-            PatKind::Binding(_, _, ident, None) => {
-                wildcard_span = Some(arm.pat.span);
-                wildcard_ident = Some(ident);
-            },
-            _ => has_non_wild = true,
-        }
-    }
-    let wildcard_span = match wildcard_span {
-        Some(x) if has_non_wild => x,
-        _ => return,
-    };
-
-    // Accumulate the variants which should be put in place of the wildcard because they're not
-    // already covered.
-    let has_hidden = adt_def.variants.iter().any(|x| is_hidden(cx, x));
-    let mut missing_variants: Vec<_> = adt_def.variants.iter().filter(|x| !is_hidden(cx, x)).collect();
-
-    let mut path_prefix = CommonPrefixSearcher::None;
-    for arm in arms {
-        // Guards mean that this case probably isn't exhaustively covered. Technically
-        // this is incorrect, as we should really check whether each variant is exhaustively
-        // covered by the set of guards that cover it, but that's really hard to do.
-        recurse_or_patterns(arm.pat, |pat| {
-            let path = match &peel_hir_pat_refs(pat).0.kind {
-                PatKind::Path(path) => {
-                    #[allow(clippy::match_same_arms)]
-                    let id = match cx.qpath_res(path, pat.hir_id) {
-                        Res::Def(
-                            DefKind::Const | DefKind::ConstParam | DefKind::AnonConst | DefKind::InlineConst,
-                            _,
-                        ) => return,
-                        Res::Def(_, id) => id,
-                        _ => return,
-                    };
-                    if arm.guard.is_none() {
-                        missing_variants.retain(|e| e.ctor_def_id != Some(id));
-                    }
-                    path
-                },
-                PatKind::TupleStruct(path, patterns, ..) => {
-                    if let Some(id) = cx.qpath_res(path, pat.hir_id).opt_def_id() {
-                        if arm.guard.is_none() && patterns.iter().all(|p| !is_refutable(cx, p)) {
-                            missing_variants.retain(|e| e.ctor_def_id != Some(id));
-                        }
-                    }
-                    path
-                },
-                PatKind::Struct(path, patterns, ..) => {
-                    if let Some(id) = cx.qpath_res(path, pat.hir_id).opt_def_id() {
-                        if arm.guard.is_none() && patterns.iter().all(|p| !is_refutable(cx, p.pat)) {
-                            missing_variants.retain(|e| e.def_id != id);
-                        }
-                    }
-                    path
-                },
-                _ => return,
-            };
-            match path {
-                QPath::Resolved(_, path) => path_prefix.with_path(path.segments),
-                QPath::TypeRelative(
-                    hir::Ty {
-                        kind: TyKind::Path(QPath::Resolved(_, path)),
-                        ..
-                    },
-                    _,
-                ) => path_prefix.with_prefix(path.segments),
-                _ => (),
-            }
-        });
-    }
-
-    let format_suggestion = |variant: &VariantDef| {
-        format!(
-            "{}{}{}{}",
-            if let Some(ident) = wildcard_ident {
-                format!("{} @ ", ident.name)
-            } else {
-                String::new()
-            },
-            if let CommonPrefixSearcher::Path(path_prefix) = path_prefix {
-                let mut s = String::new();
-                for seg in path_prefix {
-                    s.push_str(seg.ident.as_str());
-                    s.push_str("::");
-                }
-                s
-            } else {
-                let mut s = cx.tcx.def_path_str(adt_def.did);
-                s.push_str("::");
-                s
-            },
-            variant.name,
-            match variant.ctor_kind {
-                CtorKind::Fn if variant.fields.len() == 1 => "(_)",
-                CtorKind::Fn => "(..)",
-                CtorKind::Const => "",
-                CtorKind::Fictive => "{ .. }",
-            }
-        )
-    };
-
-    match missing_variants.as_slice() {
-        [] => (),
-        [x] if !adt_def.is_variant_list_non_exhaustive() && !has_hidden => span_lint_and_sugg(
-            cx,
-            MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
-            wildcard_span,
-            "wildcard matches only a single variant and will also match any future added variants",
-            "try this",
-            format_suggestion(x),
-            Applicability::MaybeIncorrect,
-        ),
-        variants => {
-            let mut suggestions: Vec<_> = variants.iter().copied().map(format_suggestion).collect();
-            let message = if adt_def.is_variant_list_non_exhaustive() || has_hidden {
-                suggestions.push("_".into());
-                "wildcard matches known variants and will also match future added variants"
-            } else {
-                "wildcard match will also match any future added variants"
-            };
-
-            span_lint_and_sugg(
-                cx,
-                WILDCARD_ENUM_MATCH_ARM,
-                wildcard_span,
-                message,
-                "try this",
-                suggestions.join(" | "),
-                Applicability::MaybeIncorrect,
-            );
-        },
-    };
-}
-
-fn check_match_ref_pats<'a, 'b, I>(cx: &LateContext<'_>, ex: &Expr<'_>, pats: I, expr: &Expr<'_>)
-where
-    'b: 'a,
-    I: Clone + Iterator<Item = &'a Pat<'b>>,
-{
-    if !has_multiple_ref_pats(pats.clone()) {
-        return;
-    }
-
-    let (first_sugg, msg, title);
-    let span = ex.span.source_callsite();
-    if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner) = ex.kind {
-        first_sugg = once((span, Sugg::hir_with_macro_callsite(cx, inner, "..").to_string()));
-        msg = "try";
-        title = "you don't need to add `&` to both the expression and the patterns";
-    } else {
-        first_sugg = once((span, Sugg::hir_with_macro_callsite(cx, ex, "..").deref().to_string()));
-        msg = "instead of prefixing all patterns with `&`, you can dereference the expression";
-        title = "you don't need to add `&` to all patterns";
-    }
-
-    let remaining_suggs = pats.filter_map(|pat| {
-        if let PatKind::Ref(refp, _) = pat.kind {
-            Some((pat.span, snippet(cx, refp.span, "..").to_string()))
-        } else {
-            None
-        }
-    });
-
-    span_lint_and_then(cx, MATCH_REF_PATS, expr.span, title, |diag| {
-        if !expr.span.from_expansion() {
-            multispan_sugg(diag, msg, first_sugg.chain(remaining_suggs));
-        }
-    });
-}
-
-fn check_match_as_ref(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) {
-    if arms.len() == 2 && arms[0].guard.is_none() && arms[1].guard.is_none() {
-        let arm_ref: Option<BindingAnnotation> = if is_none_arm(cx, &arms[0]) {
-            is_ref_some_arm(cx, &arms[1])
-        } else if is_none_arm(cx, &arms[1]) {
-            is_ref_some_arm(cx, &arms[0])
-        } else {
-            None
-        };
-        if let Some(rb) = arm_ref {
-            let suggestion = if rb == BindingAnnotation::Ref {
-                "as_ref"
-            } else {
-                "as_mut"
-            };
-
-            let output_ty = cx.typeck_results().expr_ty(expr);
-            let input_ty = cx.typeck_results().expr_ty(ex);
-
-            let cast = if_chain! {
-                if let ty::Adt(_, substs) = input_ty.kind();
-                let input_ty = substs.type_at(0);
-                if let ty::Adt(_, substs) = output_ty.kind();
-                let output_ty = substs.type_at(0);
-                if let ty::Ref(_, output_ty, _) = *output_ty.kind();
-                if input_ty != output_ty;
-                then {
-                    ".map(|x| x as _)"
-                } else {
-                    ""
-                }
-            };
-
-            let mut applicability = Applicability::MachineApplicable;
-            span_lint_and_sugg(
-                cx,
-                MATCH_AS_REF,
-                expr.span,
-                &format!("use `{}()` instead", suggestion),
-                "try this",
-                format!(
-                    "{}.{}(){}",
-                    snippet_with_applicability(cx, ex.span, "_", &mut applicability),
-                    suggestion,
-                    cast,
-                ),
-                applicability,
-            );
-        }
-    }
-}
-
 fn check_wild_in_or_pats(cx: &LateContext<'_>, arms: &[Arm<'_>]) {
     for arm in arms {
         if let PatKind::Or(fields) = arm.pat.kind {
@@ -997,206 +682,3 @@ 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
-}
-
-// Checks if arm has the form `None => None`
-fn is_none_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {
-    matches!(arm.pat.kind, PatKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone))
-}
-
-// Checks if arm has the form `Some(ref v) => Some(v)` (checks for `ref` and `ref mut`)
-fn is_ref_some_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> Option<BindingAnnotation> {
-    if_chain! {
-        if let PatKind::TupleStruct(ref qpath, [first_pat, ..], _) = arm.pat.kind;
-        if is_lang_ctor(cx, qpath, OptionSome);
-        if let PatKind::Binding(rb, .., ident, _) = first_pat.kind;
-        if rb == BindingAnnotation::Ref || rb == BindingAnnotation::RefMut;
-        if let ExprKind::Call(e, args) = peel_blocks(arm.body).kind;
-        if let ExprKind::Path(ref some_path) = e.kind;
-        if is_lang_ctor(cx, some_path, OptionSome) && args.len() == 1;
-        if let ExprKind::Path(QPath::Resolved(_, path2)) = args[0].kind;
-        if path2.segments.len() == 1 && ident.name == path2.segments[0].ident.name;
-        then {
-            return Some(rb)
-        }
-    }
-    None
-}
-
-fn has_multiple_ref_pats<'a, 'b, I>(pats: I) -> bool
-where
-    'b: 'a,
-    I: Iterator<Item = &'a Pat<'b>>,
-{
-    let mut ref_count = 0;
-    for opt in pats.map(|pat| match pat.kind {
-        PatKind::Ref(..) => Some(true), // &-patterns
-        PatKind::Wild => Some(false),   // an "anything" wildcard is also fine
-        _ => None,                      // any other pattern is not fine
-    }) {
-        if let Some(inner) = opt {
-            if inner {
-                ref_count += 1;
-            }
-        } else {
-            return false;
-        }
-    }
-    ref_count > 1
-}