X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=src%2Ftools%2Fclippy%2Fclippy_lints%2Fsrc%2Fmanual_map.rs;h=161d884149075012d3cb7a300056910de428096e;hb=5ca596f486707ac1362edad717ad0e9f5b71d0a3;hp=7dec1595e0d108d270b0223661ed187c6143e101;hpb=ae90dcf0207c57c3034f00b07048d63f8b2363c8;p=rust.git diff --git a/src/tools/clippy/clippy_lints/src/manual_map.rs b/src/tools/clippy/clippy_lints/src/manual_map.rs index 7dec1595e0d..161d8841490 100644 --- a/src/tools/clippy/clippy_lints/src/manual_map.rs +++ b/src/tools/clippy/clippy_lints/src/manual_map.rs @@ -1,5 +1,6 @@ use crate::{map_unit_fn::OPTION_MAP_UNIT_FN, matches::MATCH_AS_REF}; use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::higher; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable}; use clippy_utils::{ @@ -9,7 +10,7 @@ use rustc_ast::util::parser::PREC_POSTFIX; use rustc_errors::Applicability; use rustc_hir::LangItem::{OptionNone, OptionSome}; -use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, HirId, MatchSource, Mutability, Pat, PatKind}; +use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, HirId, Mutability, Pat, PatKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -43,155 +44,168 @@ impl LateLintPass<'_> for ManualMap { #[allow(clippy::too_many_lines)] fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::Match( - scrutinee, - [arm1 @ Arm { guard: None, .. }, arm2 @ Arm { guard: None, .. }], - match_kind, - ) = expr.kind + if let Some(higher::IfLet { + let_pat, + let_expr, + if_then, + if_else: Some(if_else), + }) = higher::IfLet::hir(cx, expr) { - if in_external_macro(cx.sess(), expr.span) || in_constant(cx, expr.hir_id) { - return; - } + manage_lint(cx, expr, (&let_pat.kind, if_then), (&PatKind::Wild, if_else), let_expr); + } - let (scrutinee_ty, ty_ref_count, ty_mutability) = - peel_mid_ty_refs_is_mutable(cx.typeck_results().expr_ty(scrutinee)); - if !(is_type_diagnostic_item(cx, scrutinee_ty, sym::option_type) - && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::option_type)) - { - return; - } + if let ExprKind::Match(scrutinee, [then @ Arm { guard: None, .. }, r#else @ Arm { guard: None, .. }], _) = + expr.kind + { + manage_lint( + cx, + expr, + (&then.pat.kind, then.body), + (&r#else.pat.kind, r#else.body), + scrutinee, + ); + } + } +} - let expr_ctxt = expr.span.ctxt(); - let (some_expr, some_pat, pat_ref_count, is_wild_none) = match ( - try_parse_pattern(cx, arm1.pat, expr_ctxt), - try_parse_pattern(cx, arm2.pat, expr_ctxt), - ) { - (Some(OptionPat::Wild), Some(OptionPat::Some { pattern, ref_count })) - if is_none_expr(cx, arm1.body) => - { - (arm2.body, pattern, ref_count, true) - }, - (Some(OptionPat::None), Some(OptionPat::Some { pattern, ref_count })) - if is_none_expr(cx, arm1.body) => - { - (arm2.body, pattern, ref_count, false) - }, - (Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::Wild)) - if is_none_expr(cx, arm2.body) => - { - (arm1.body, pattern, ref_count, true) - }, - (Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::None)) - if is_none_expr(cx, arm2.body) => - { - (arm1.body, pattern, ref_count, false) - }, - _ => return, - }; +fn manage_lint<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + then: (&'tcx PatKind<'_>, &'tcx Expr<'_>), + r#else: (&'tcx PatKind<'_>, &'tcx Expr<'_>), + scrut: &'tcx Expr<'_>, +) { + if in_external_macro(cx.sess(), expr.span) || in_constant(cx, expr.hir_id) { + return; + } - // Top level or patterns aren't allowed in closures. - if matches!(some_pat.kind, PatKind::Or(_)) { - return; - } + let (scrutinee_ty, ty_ref_count, ty_mutability) = peel_mid_ty_refs_is_mutable(cx.typeck_results().expr_ty(scrut)); + if !(is_type_diagnostic_item(cx, scrutinee_ty, sym::option_type) + && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::option_type)) + { + return; + } - let some_expr = match get_some_expr(cx, some_expr, expr_ctxt) { - Some(expr) => expr, - None => return, - }; + let (then_pat, then_expr) = then; + let (else_pat, else_expr) = r#else; - // These two lints will go back and forth with each other. - if cx.typeck_results().expr_ty(some_expr) == cx.tcx.types.unit - && !is_lint_allowed(cx, OPTION_MAP_UNIT_FN, expr.hir_id) - { - return; - } + let expr_ctxt = expr.span.ctxt(); + let (some_expr, some_pat, pat_ref_count, is_wild_none) = match ( + try_parse_pattern(cx, then_pat, expr_ctxt), + try_parse_pattern(cx, else_pat, expr_ctxt), + ) { + (Some(OptionPat::Wild), Some(OptionPat::Some { pattern, ref_count })) if is_none_expr(cx, then_expr) => { + (else_expr, pattern, ref_count, true) + }, + (Some(OptionPat::None), Some(OptionPat::Some { pattern, ref_count })) if is_none_expr(cx, then_expr) => { + (else_expr, pattern, ref_count, false) + }, + (Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::Wild)) if is_none_expr(cx, else_expr) => { + (then_expr, pattern, ref_count, true) + }, + (Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::None)) if is_none_expr(cx, else_expr) => { + (then_expr, pattern, ref_count, false) + }, + _ => return, + }; - // `map` won't perform any adjustments. - if !cx.typeck_results().expr_adjustments(some_expr).is_empty() { - return; - } + // Top level or patterns aren't allowed in closures. + if matches!(some_pat.kind, PatKind::Or(_)) { + return; + } - if !can_move_expr_to_closure(cx, some_expr) { - return; - } + let some_expr = match get_some_expr(cx, some_expr, expr_ctxt) { + Some(expr) => expr, + None => return, + }; - // Determine which binding mode to use. - let explicit_ref = some_pat.contains_explicit_ref_binding(); - let binding_ref = explicit_ref.or_else(|| (ty_ref_count != pat_ref_count).then(|| ty_mutability)); + if cx.typeck_results().expr_ty(some_expr) == cx.tcx.types.unit && !is_lint_allowed(cx, OPTION_MAP_UNIT_FN, expr.hir_id) { + return; + } - let as_ref_str = match binding_ref { - Some(Mutability::Mut) => ".as_mut()", - Some(Mutability::Not) => ".as_ref()", - None => "", - }; + // `map` won't perform any adjustments. + if !cx.typeck_results().expr_adjustments(some_expr).is_empty() { + return; + } - let mut app = Applicability::MachineApplicable; + if !can_move_expr_to_closure(cx, some_expr) { + return; + } - // Remove address-of expressions from the scrutinee. Either `as_ref` will be called, or - // it's being passed by value. - let scrutinee = peel_hir_expr_refs(scrutinee).0; - let (scrutinee_str, _) = snippet_with_context(cx, scrutinee.span, expr_ctxt, "..", &mut app); - let scrutinee_str = - if scrutinee.span.ctxt() == expr.span.ctxt() && scrutinee.precedence().order() < PREC_POSTFIX { - format!("({})", scrutinee_str) - } else { - scrutinee_str.into() - }; + // Determine which binding mode to use. + let explicit_ref = some_pat.contains_explicit_ref_binding(); + let binding_ref = explicit_ref.or_else(|| (ty_ref_count != pat_ref_count).then(|| ty_mutability)); + + let as_ref_str = match binding_ref { + Some(Mutability::Mut) => ".as_mut()", + Some(Mutability::Not) => ".as_ref()", + None => "", + }; + + let mut app = Applicability::MachineApplicable; - let body_str = if let PatKind::Binding(annotation, id, some_binding, None) = some_pat.kind { - match can_pass_as_func(cx, id, some_expr) { - Some(func) if func.span.ctxt() == some_expr.span.ctxt() => { - snippet_with_applicability(cx, func.span, "..", &mut app).into_owned() - }, - _ => { - if path_to_local_id(some_expr, id) - && !is_lint_allowed(cx, MATCH_AS_REF, expr.hir_id) - && binding_ref.is_some() - { - return; - } + // Remove address-of expressions from the scrutinee. Either `as_ref` will be called, or + // it's being passed by value. + let scrutinee = peel_hir_expr_refs(scrut).0; + let (scrutinee_str, _) = snippet_with_context(cx, scrutinee.span, expr_ctxt, "..", &mut app); + let scrutinee_str = if scrutinee.span.ctxt() == expr.span.ctxt() && scrutinee.precedence().order() < PREC_POSTFIX { + format!("({})", scrutinee_str) + } else { + scrutinee_str.into() + }; - // `ref` and `ref mut` annotations were handled earlier. - let annotation = if matches!(annotation, BindingAnnotation::Mutable) { - "mut " - } else { - "" - }; - format!( - "|{}{}| {}", - annotation, - some_binding, - snippet_with_context(cx, some_expr.span, expr_ctxt, "..", &mut app).0 - ) - }, + let body_str = if let PatKind::Binding(annotation, id, some_binding, None) = some_pat.kind { + match can_pass_as_func(cx, id, some_expr) { + Some(func) if func.span.ctxt() == some_expr.span.ctxt() => { + snippet_with_applicability(cx, func.span, "..", &mut app).into_owned() + }, + _ => { + if path_to_local_id(some_expr, id) + && !is_lint_allowed(cx, MATCH_AS_REF, expr.hir_id) + && binding_ref.is_some() + { + return; } - } else if !is_wild_none && explicit_ref.is_none() { - // TODO: handle explicit reference annotations. + + // `ref` and `ref mut` annotations were handled earlier. + let annotation = if matches!(annotation, BindingAnnotation::Mutable) { + "mut " + } else { + "" + }; format!( - "|{}| {}", - snippet_with_context(cx, some_pat.span, expr_ctxt, "..", &mut app).0, + "|{}{}| {}", + annotation, + some_binding, snippet_with_context(cx, some_expr.span, expr_ctxt, "..", &mut app).0 ) - } else { - // Refutable bindings and mixed reference annotations can't be handled by `map`. - return; - }; - - span_lint_and_sugg( - cx, - MANUAL_MAP, - expr.span, - "manual implementation of `Option::map`", - "try this", - if matches!(match_kind, MatchSource::IfLetDesugar { .. }) && is_else_clause(cx.tcx, expr) { - format!("{{ {}{}.map({}) }}", scrutinee_str, as_ref_str, body_str) - } else { - format!("{}{}.map({})", scrutinee_str, as_ref_str, body_str) - }, - app, - ); + }, } - } + } else if !is_wild_none && explicit_ref.is_none() { + // TODO: handle explicit reference annotations. + format!( + "|{}| {}", + snippet_with_context(cx, some_pat.span, expr_ctxt, "..", &mut app).0, + snippet_with_context(cx, some_expr.span, expr_ctxt, "..", &mut app).0 + ) + } else { + // Refutable bindings and mixed reference annotations can't be handled by `map`. + return; + }; + + span_lint_and_sugg( + cx, + MANUAL_MAP, + expr.span, + "manual implementation of `Option::map`", + "try this", + if is_else_clause(cx.tcx, expr) { + format!("{{ {}{}.map({}) }}", scrutinee_str, as_ref_str, body_str) + } else { + format!("{}{}.map({})", scrutinee_str, as_ref_str, body_str) + }, + app, + ); } // Checks whether the expression could be passed as a function, or whether a closure is needed. @@ -199,7 +213,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { fn can_pass_as_func(cx: &LateContext<'tcx>, binding: HirId, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> { match expr.kind { ExprKind::Call(func, [arg]) - if path_to_local_id(arg, binding) && cx.typeck_results().expr_adjustments(arg).is_empty() => + if path_to_local_id (arg, binding) && cx.typeck_results().expr_adjustments(arg).is_empty() => { Some(func) }, @@ -221,21 +235,28 @@ enum OptionPat<'a> { // Try to parse into a recognized `Option` pattern. // i.e. `_`, `None`, `Some(..)`, or a reference to any of those. -fn try_parse_pattern(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, ctxt: SyntaxContext) -> Option> { - fn f(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, ref_count: usize, ctxt: SyntaxContext) -> Option> { - match pat.kind { +fn try_parse_pattern( + cx: &LateContext<'tcx>, + pat_kind: &'tcx PatKind<'_>, + ctxt: SyntaxContext, +) -> Option> { + fn f( + cx: &LateContext<'tcx>, + pat_kind: &'tcx PatKind<'_>, + ref_count: usize, + ctxt: SyntaxContext, + ) -> Option> { + match pat_kind { PatKind::Wild => Some(OptionPat::Wild), - PatKind::Ref(pat, _) => f(cx, pat, ref_count + 1, ctxt), + PatKind::Ref(ref_pat, _) => f(cx, &ref_pat.kind, ref_count + 1, ctxt), PatKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone) => Some(OptionPat::None), - PatKind::TupleStruct(ref qpath, [pattern], _) - if is_lang_ctor(cx, qpath, OptionSome) && pat.span.ctxt() == ctxt => - { + PatKind::TupleStruct(ref qpath, [pattern], _) if is_lang_ctor(cx, qpath, OptionSome) => { Some(OptionPat::Some { pattern, ref_count }) }, _ => None, } } - f(cx, pat, 0, ctxt) + f(cx, pat_kind, 0, ctxt) } // Checks for an expression wrapped by the `Some` constructor. Returns the contained expression.