diff --git a/clippy_lints/src/matches/manual_filter.rs b/clippy_lints/src/matches/manual_filter.rs index a36ffb61752b..cc60ae89a6e6 100644 --- a/clippy_lints/src/matches/manual_filter.rs +++ b/clippy_lints/src/matches/manual_filter.rs @@ -1,39 +1,19 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::visitors::contains_unsafe_block; use clippy_utils::{is_lang_ctor, path_to_local_id}; -use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_hir::LangItem::OptionSome; -use rustc_hir::{Arm, Block, BlockCheckMode, Expr, ExprKind, HirId, Pat, PatKind, UnsafeSource}; +use rustc_hir::{Arm, Expr, ExprKind, HirId, Pat, PatKind}; use rustc_lint::LateContext; use rustc_span::{sym, SyntaxContext}; -use super::manual_map::{check_with, SomeExpr}; +use super::manual_utils::{check_with, SomeExpr}; use super::MANUAL_FILTER; -#[derive(Default)] -struct NeedsUnsafeBlock(pub bool); - -impl<'tcx> Visitor<'tcx> for NeedsUnsafeBlock { - fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { - match expr.kind { - ExprKind::Block( - Block { - rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided), - .. - }, - _, - ) => { - self.0 = true; - }, - _ => walk_expr(self, expr), - } - } -} - -// Function called on the `expr` of `[&+]Some((ref | ref mut) x) => ` -// Need to check if it's of the `if {} else {}` +// Function called on the of `[&+]Some((ref | ref mut) x) => ` +// Need to check if it's of the form `=if {} else {}` // AND that only one `then/else_expr` resolves to `Some(x)` while the other resolves to `None` -// return `cond` if +// return the `cond` expression if so. fn get_cond_expr<'tcx>( cx: &LateContext<'tcx>, pat: &Pat<'_>, @@ -45,15 +25,13 @@ fn get_cond_expr<'tcx>( if let ExprKind::If(cond, then_expr, Some(else_expr)) = block_expr.kind; if let PatKind::Binding(_,target, ..) = pat.kind; if let (then_visitor, else_visitor) - = (handle_if_or_else_expr(cx, target, ctxt, then_expr), - handle_if_or_else_expr(cx, target, ctxt, else_expr)); + = (is_some_expr(cx, target, ctxt, then_expr), + is_some_expr(cx, target, ctxt, else_expr)); if then_visitor != else_visitor; // check that one expr resolves to `Some(x)`, the other to `None` then { - let mut needs_unsafe_block = NeedsUnsafeBlock::default(); - needs_unsafe_block.visit_expr(expr); return Some(SomeExpr { expr: peels_blocks_incl_unsafe(cond.peel_drop_temps()), - needs_unsafe_block: needs_unsafe_block.0, + needs_unsafe_block: contains_unsafe_block(cx, expr), needs_negated: !then_visitor // if the `then_expr` resolves to `None`, need to negate the cond }) } @@ -63,7 +41,7 @@ fn get_cond_expr<'tcx>( fn peels_blocks_incl_unsafe_opt<'a>(expr: &'a Expr<'a>) -> Option<&'a Expr<'a>> { // we don't want to use `peel_blocks` here because we don't care if the block is unsafe, it's - // checked by `NeedsUnsafeBlock` + // checked by `contains_unsafe_block` if let ExprKind::Block(block, None) = expr.kind { if block.stmts.is_empty() { return block.expr; @@ -76,20 +54,15 @@ fn peels_blocks_incl_unsafe<'a>(expr: &'a Expr<'a>) -> &'a Expr<'a> { peels_blocks_incl_unsafe_opt(expr).unwrap_or(expr) } -// function called for each expression: +// function called for each expression: // Some(x) => if { -// +// // } else { -// +// // } -// Returns true if resolves to `Some(x)`, `false` otherwise -fn handle_if_or_else_expr<'tcx>( - cx: &LateContext<'_>, - target: HirId, - ctxt: SyntaxContext, - if_or_else_expr: &'tcx Expr<'_>, -) -> bool { - if let Some(inner_expr) = peels_blocks_incl_unsafe_opt(if_or_else_expr) { +// Returns true if resolves to `Some(x)`, `false` otherwise +fn is_some_expr<'tcx>(cx: &LateContext<'_>, target: HirId, ctxt: SyntaxContext, expr: &'tcx Expr<'_>) -> bool { + if let Some(inner_expr) = peels_blocks_incl_unsafe_opt(expr) { // there can be not statements in the block as they would be removed when switching to `.filter` if let ExprKind::Call( Expr { @@ -99,9 +72,7 @@ fn handle_if_or_else_expr<'tcx>( [arg], ) = inner_expr.kind { - return ctxt == if_or_else_expr.span.ctxt() - && is_lang_ctor(cx, qpath, OptionSome) - && path_to_local_id(arg, target); + return ctxt == expr.span.ctxt() && is_lang_ctor(cx, qpath, OptionSome) && path_to_local_id(arg, target); } }; false @@ -126,15 +97,13 @@ pub(super) fn check_match<'tcx>( expr: &'tcx Expr<'_>, ) { let ty = cx.typeck_results().expr_ty(expr); - if_chain! { - if is_type_diagnostic_item(cx, ty, sym::Option); - if arms.len() == 2; - if arms[0].guard.is_none(); - if arms[1].guard.is_none(); - then { - check(cx, expr, scrutinee, arms[0].pat, arms[0].body, Some(arms[1].pat), arms[1].body) + if is_type_diagnostic_item(cx, ty, sym::Option) + && let [first_arm, second_arm] = arms + && first_arm.guard.is_none() + && second_arm.guard.is_none() + { + check(cx, expr, scrutinee, first_arm.pat, first_arm.body, Some(second_arm.pat), second_arm.body) } - } } pub(super) fn check_if_let<'tcx>( @@ -176,14 +145,11 @@ fn check<'tcx>( "try this", if sugg_info.needs_brackets { format!( - "{{ {}{}.filter({}) }}", - sugg_info.scrutinee_str, sugg_info.as_ref_str, body_str + "{{ {}{}.filter({body_str}) }}", + sugg_info.scrutinee_str, sugg_info.as_ref_str ) } else { - format!( - "{}{}.filter({})", - sugg_info.scrutinee_str, sugg_info.as_ref_str, body_str - ) + format!("{}{}.filter({body_str})", sugg_info.scrutinee_str, sugg_info.as_ref_str) }, sugg_info.app, ); diff --git a/clippy_lints/src/matches/manual_map.rs b/clippy_lints/src/matches/manual_map.rs index b73122409fb6..d3222eff1d1a 100644 --- a/clippy_lints/src/matches/manual_map.rs +++ b/clippy_lints/src/matches/manual_map.rs @@ -1,21 +1,11 @@ +use super::manual_utils::{check_with, SomeExpr}; use super::MANUAL_MAP; -use crate::{map_unit_fn::OPTION_MAP_UNIT_FN, matches::MATCH_AS_REF}; use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; -use clippy_utils::ty::{is_copy, is_type_diagnostic_item, peel_mid_ty_refs_is_mutable, type_is_unsafe_function}; -use clippy_utils::{ - can_move_expr_to_closure, is_else_clause, is_lang_ctor, is_lint_allowed, path_to_local_id, peel_blocks, - peel_hir_expr_refs, peel_hir_expr_while, sugg::Sugg, CaptureKind, -}; -use rustc_ast::util::parser::PREC_POSTFIX; -use rustc_errors::Applicability; -use rustc_hir::LangItem::{OptionNone, OptionSome}; -use rustc_hir::{ - def::Res, Arm, BindingAnnotation, Block, BlockCheckMode, Expr, ExprKind, HirId, Mutability, Pat, PatKind, Path, - QPath, UnsafeSource, -}; +use clippy_utils::is_lang_ctor; +use rustc_hir::LangItem::OptionSome; +use rustc_hir::{Arm, Block, BlockCheckMode, Expr, ExprKind, Pat, UnsafeSource}; use rustc_lint::LateContext; -use rustc_span::{sym, SyntaxContext}; +use rustc_span::SyntaxContext; pub(super) fn check_match<'tcx>( cx: &LateContext<'tcx>, @@ -83,264 +73,6 @@ fn check<'tcx>( } } -#[expect(clippy::too_many_arguments)] -#[expect(clippy::too_many_lines)] -pub(super) fn check_with<'tcx, F>( - cx: &LateContext<'tcx>, - expr: &'tcx Expr<'_>, - scrutinee: &'tcx Expr<'_>, - then_pat: &'tcx Pat<'_>, - then_body: &'tcx Expr<'_>, - else_pat: Option<&'tcx Pat<'_>>, - else_body: &'tcx Expr<'_>, - get_some_expr_fn: F, -) -> Option> -where - F: Fn(&LateContext<'tcx>, &'tcx Pat<'_>, &'tcx Expr<'_>, SyntaxContext) -> Option>, -{ - 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) - && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Option)) - { - return None; - } - - 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), - else_pat.map_or(Some(OptionPat::Wild), |p| try_parse_pattern(cx, p, expr_ctxt)), - ) { - (Some(OptionPat::Wild), Some(OptionPat::Some { pattern, ref_count })) if is_none_expr(cx, then_body) => { - (else_body, pattern, ref_count, true) - }, - (Some(OptionPat::None), Some(OptionPat::Some { pattern, ref_count })) if is_none_expr(cx, then_body) => { - (else_body, pattern, ref_count, false) - }, - (Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::Wild)) if is_none_expr(cx, else_body) => { - (then_body, pattern, ref_count, true) - }, - (Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::None)) if is_none_expr(cx, else_body) => { - (then_body, pattern, ref_count, false) - }, - _ => return None, - }; - - // Top level or patterns aren't allowed in closures. - if matches!(some_pat.kind, PatKind::Or(_)) { - return None; - } - - let some_expr = match get_some_expr_fn(cx, some_pat, some_expr, expr_ctxt) { - Some(expr) => expr, - None => return None, - }; - - // These two lints will go back and forth with each other. - if cx.typeck_results().expr_ty(some_expr.expr) == cx.tcx.types.unit - && !is_lint_allowed(cx, OPTION_MAP_UNIT_FN, expr.hir_id) - { - return None; - } - - // `map` won't perform any adjustments. - if !cx.typeck_results().expr_adjustments(some_expr.expr).is_empty() { - return None; - } - - // 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_some(ty_mutability)); - - let as_ref_str = match binding_ref { - Some(Mutability::Mut) => ".as_mut()", - Some(Mutability::Not) => ".as_ref()", - None => "", - }; - - match can_move_expr_to_closure(cx, some_expr.expr) { - Some(captures) => { - // Check if captures the closure will need conflict with borrows made in the scrutinee. - // TODO: check all the references made in the scrutinee expression. This will require interacting - // with the borrow checker. Currently only `[.]*` is checked for. - if let Some(binding_ref_mutability) = binding_ref { - let e = peel_hir_expr_while(scrutinee, |e| match e.kind { - ExprKind::Field(e, _) | ExprKind::AddrOf(_, _, e) => Some(e), - _ => None, - }); - if let ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(l), .. })) = e.kind { - match captures.get(l) { - Some(CaptureKind::Value | CaptureKind::Ref(Mutability::Mut)) => return None, - Some(CaptureKind::Ref(Mutability::Not)) if binding_ref_mutability == Mutability::Mut => { - return None; - }, - Some(CaptureKind::Ref(Mutability::Not)) | None => (), - } - } - } - }, - None => return None, - }; - - let mut app = Applicability::MachineApplicable; - - // 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() - }; - - let closure_expr_snip = some_expr.to_snippet_with_context(cx, expr_ctxt, &mut app); - let body_str = if let PatKind::Binding(annotation, id, some_binding, None) = some_pat.kind { - if_chain! { - if !some_expr.needs_unsafe_block; - if let Some(func) = can_pass_as_func(cx, id, some_expr.expr); - if func.span.ctxt() == some_expr.expr.span.ctxt(); - then { - snippet_with_applicability(cx, func.span, "..", &mut app).into_owned() - } else { - if path_to_local_id(some_expr.expr, id) - && !is_lint_allowed(cx, MATCH_AS_REF, expr.hir_id) - && binding_ref.is_some() - { - return None; - } - - // `ref` and `ref mut` annotations were handled earlier. - let annotation = if matches!(annotation, BindingAnnotation::MUT) { - "mut " - } else { - "" - }; - - if some_expr.needs_unsafe_block { - format!("|{annotation}{some_binding}| unsafe {{ {closure_expr_snip} }}") - } else { - format!("|{annotation}{some_binding}| {closure_expr_snip}") - } - } - } - } else if !is_wild_none && explicit_ref.is_none() { - // TODO: handle explicit reference annotations. - let pat_snip = snippet_with_context(cx, some_pat.span, expr_ctxt, "..", &mut app).0; - if some_expr.needs_unsafe_block { - format!("|{pat_snip}| unsafe {{ {closure_expr_snip} }}") - } else { - format!("|{pat_snip}| {closure_expr_snip}") - } - } else { - // Refutable bindings and mixed reference annotations can't be handled by `map`. - return None; - }; - - // relies on the fact that Option: Copy where T: copy - let scrutinee_impl_copy = is_copy(cx, scrutinee_ty); - - Some(SuggInfo { - needs_brackets: else_pat.is_none() && is_else_clause(cx.tcx, expr), - scrutinee_impl_copy, - scrutinee_str, - as_ref_str, - body_str, - app, - }) -} - -pub struct SuggInfo<'a> { - pub needs_brackets: bool, - pub scrutinee_impl_copy: bool, - pub scrutinee_str: String, - pub as_ref_str: &'a str, - pub body_str: String, - pub app: Applicability, -} - -// Checks whether the expression could be passed as a function, or whether a closure is needed. -// Returns the function to be passed to `map` if it exists. -fn can_pass_as_func<'tcx>(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() - && !type_is_unsafe_function(cx, cx.typeck_results().expr_ty(func).peel_refs()) => - { - Some(func) - }, - _ => None, - } -} - -#[derive(Debug)] -pub(super) enum OptionPat<'a> { - Wild, - None, - Some { - // The pattern contained in the `Some` tuple. - pattern: &'a Pat<'a>, - // The number of references before the `Some` tuple. - // e.g. `&&Some(_)` has a ref count of 2. - ref_count: usize, - }, -} - -pub(super) struct SomeExpr<'tcx> { - pub expr: &'tcx Expr<'tcx>, - pub needs_unsafe_block: bool, - pub needs_negated: bool, // for `manual_filter` lint -} - -impl<'tcx> SomeExpr<'tcx> { - pub fn new_no_negated(expr: &'tcx Expr<'tcx>, needs_unsafe_block: bool) -> Self { - Self { - expr, - needs_unsafe_block, - needs_negated: false, - } - } - - pub fn to_snippet_with_context( - &self, - cx: &LateContext<'tcx>, - ctxt: SyntaxContext, - app: &mut Applicability, - ) -> Sugg<'tcx> { - let sugg = Sugg::hir_with_context(cx, self.expr, ctxt, "..", app); - if self.needs_negated { !sugg } else { sugg } - } -} - -// Try to parse into a recognized `Option` pattern. -// i.e. `_`, `None`, `Some(..)`, or a reference to any of those. -pub(super) fn try_parse_pattern<'tcx>( - cx: &LateContext<'tcx>, - pat: &'tcx Pat<'_>, - ctxt: SyntaxContext, -) -> Option> { - fn f<'tcx>( - cx: &LateContext<'tcx>, - pat: &'tcx Pat<'_>, - 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::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 => - { - Some(OptionPat::Some { pattern, ref_count }) - }, - _ => None, - } - } - f(cx, pat, 0, ctxt) -} - // Checks for an expression wrapped by the `Some` constructor. Returns the contained expression. fn get_some_expr<'tcx>( cx: &LateContext<'tcx>, @@ -384,8 +116,3 @@ fn get_some_expr<'tcx>( } get_some_expr_internal(cx, expr, false, ctxt) } - -// Checks for the `None` value. -fn is_none_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - matches!(peel_blocks(expr).kind, ExprKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone)) -} diff --git a/clippy_lints/src/matches/manual_utils.rs b/clippy_lints/src/matches/manual_utils.rs new file mode 100644 index 000000000000..b32e9b8a774e --- /dev/null +++ b/clippy_lints/src/matches/manual_utils.rs @@ -0,0 +1,276 @@ +use crate::{map_unit_fn::OPTION_MAP_UNIT_FN, matches::MATCH_AS_REF}; +use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; +use clippy_utils::ty::{is_copy, is_type_diagnostic_item, peel_mid_ty_refs_is_mutable, type_is_unsafe_function}; +use clippy_utils::{ + can_move_expr_to_closure, is_else_clause, is_lang_ctor, is_lint_allowed, path_to_local_id, peel_blocks, + peel_hir_expr_refs, peel_hir_expr_while, sugg::Sugg, CaptureKind, +}; +use rustc_ast::util::parser::PREC_POSTFIX; +use rustc_errors::Applicability; +use rustc_hir::LangItem::{OptionNone, OptionSome}; +use rustc_hir::{def::Res, BindingAnnotation, Expr, ExprKind, HirId, Mutability, Pat, PatKind, Path, QPath}; +use rustc_lint::LateContext; +use rustc_span::{sym, SyntaxContext}; + +#[expect(clippy::too_many_arguments)] +#[expect(clippy::too_many_lines)] +pub(super) fn check_with<'tcx, F>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + scrutinee: &'tcx Expr<'_>, + then_pat: &'tcx Pat<'_>, + then_body: &'tcx Expr<'_>, + else_pat: Option<&'tcx Pat<'_>>, + else_body: &'tcx Expr<'_>, + get_some_expr_fn: F, +) -> Option> +where + F: Fn(&LateContext<'tcx>, &'tcx Pat<'_>, &'tcx Expr<'_>, SyntaxContext) -> Option>, +{ + 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) + && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Option)) + { + return None; + } + + 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), + else_pat.map_or(Some(OptionPat::Wild), |p| try_parse_pattern(cx, p, expr_ctxt)), + ) { + (Some(OptionPat::Wild), Some(OptionPat::Some { pattern, ref_count })) if is_none_expr(cx, then_body) => { + (else_body, pattern, ref_count, true) + }, + (Some(OptionPat::None), Some(OptionPat::Some { pattern, ref_count })) if is_none_expr(cx, then_body) => { + (else_body, pattern, ref_count, false) + }, + (Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::Wild)) if is_none_expr(cx, else_body) => { + (then_body, pattern, ref_count, true) + }, + (Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::None)) if is_none_expr(cx, else_body) => { + (then_body, pattern, ref_count, false) + }, + _ => return None, + }; + + // Top level or patterns aren't allowed in closures. + if matches!(some_pat.kind, PatKind::Or(_)) { + return None; + } + + let some_expr = match get_some_expr_fn(cx, some_pat, some_expr, expr_ctxt) { + Some(expr) => expr, + None => return None, + }; + + // These two lints will go back and forth with each other. + if cx.typeck_results().expr_ty(some_expr.expr) == cx.tcx.types.unit + && !is_lint_allowed(cx, OPTION_MAP_UNIT_FN, expr.hir_id) + { + return None; + } + + // `map` won't perform any adjustments. + if !cx.typeck_results().expr_adjustments(some_expr.expr).is_empty() { + return None; + } + + // 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_some(ty_mutability)); + + let as_ref_str = match binding_ref { + Some(Mutability::Mut) => ".as_mut()", + Some(Mutability::Not) => ".as_ref()", + None => "", + }; + + match can_move_expr_to_closure(cx, some_expr.expr) { + Some(captures) => { + // Check if captures the closure will need conflict with borrows made in the scrutinee. + // TODO: check all the references made in the scrutinee expression. This will require interacting + // with the borrow checker. Currently only `[.]*` is checked for. + if let Some(binding_ref_mutability) = binding_ref { + let e = peel_hir_expr_while(scrutinee, |e| match e.kind { + ExprKind::Field(e, _) | ExprKind::AddrOf(_, _, e) => Some(e), + _ => None, + }); + if let ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(l), .. })) = e.kind { + match captures.get(l) { + Some(CaptureKind::Value | CaptureKind::Ref(Mutability::Mut)) => return None, + Some(CaptureKind::Ref(Mutability::Not)) if binding_ref_mutability == Mutability::Mut => { + return None; + }, + Some(CaptureKind::Ref(Mutability::Not)) | None => (), + } + } + } + }, + None => return None, + }; + + let mut app = Applicability::MachineApplicable; + + // 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() + }; + + let closure_expr_snip = some_expr.to_snippet_with_context(cx, expr_ctxt, &mut app); + let body_str = if let PatKind::Binding(annotation, id, some_binding, None) = some_pat.kind { + if_chain! { + if !some_expr.needs_unsafe_block; + if let Some(func) = can_pass_as_func(cx, id, some_expr.expr); + if func.span.ctxt() == some_expr.expr.span.ctxt(); + then { + snippet_with_applicability(cx, func.span, "..", &mut app).into_owned() + } else { + if path_to_local_id(some_expr.expr, id) + && !is_lint_allowed(cx, MATCH_AS_REF, expr.hir_id) + && binding_ref.is_some() + { + return None; + } + + // `ref` and `ref mut` annotations were handled earlier. + let annotation = if matches!(annotation, BindingAnnotation::MUT) { + "mut " + } else { + "" + }; + + if some_expr.needs_unsafe_block { + format!("|{annotation}{some_binding}| unsafe {{ {closure_expr_snip} }}") + } else { + format!("|{annotation}{some_binding}| {closure_expr_snip}") + } + } + } + } else if !is_wild_none && explicit_ref.is_none() { + // TODO: handle explicit reference annotations. + let pat_snip = snippet_with_context(cx, some_pat.span, expr_ctxt, "..", &mut app).0; + if some_expr.needs_unsafe_block { + format!("|{pat_snip}| unsafe {{ {closure_expr_snip} }}") + } else { + format!("|{pat_snip}| {closure_expr_snip}") + } + } else { + // Refutable bindings and mixed reference annotations can't be handled by `map`. + return None; + }; + + // relies on the fact that Option: Copy where T: copy + let scrutinee_impl_copy = is_copy(cx, scrutinee_ty); + + Some(SuggInfo { + needs_brackets: else_pat.is_none() && is_else_clause(cx.tcx, expr), + scrutinee_impl_copy, + scrutinee_str, + as_ref_str, + body_str, + app, + }) +} + +pub struct SuggInfo<'a> { + pub needs_brackets: bool, + pub scrutinee_impl_copy: bool, + pub scrutinee_str: String, + pub as_ref_str: &'a str, + pub body_str: String, + pub app: Applicability, +} + +// Checks whether the expression could be passed as a function, or whether a closure is needed. +// Returns the function to be passed to `map` if it exists. +fn can_pass_as_func<'tcx>(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() + && !type_is_unsafe_function(cx, cx.typeck_results().expr_ty(func).peel_refs()) => + { + Some(func) + }, + _ => None, + } +} + +#[derive(Debug)] +pub(super) enum OptionPat<'a> { + Wild, + None, + Some { + // The pattern contained in the `Some` tuple. + pattern: &'a Pat<'a>, + // The number of references before the `Some` tuple. + // e.g. `&&Some(_)` has a ref count of 2. + ref_count: usize, + }, +} + +pub(super) struct SomeExpr<'tcx> { + pub expr: &'tcx Expr<'tcx>, + pub needs_unsafe_block: bool, + pub needs_negated: bool, // for `manual_filter` lint +} + +impl<'tcx> SomeExpr<'tcx> { + pub fn new_no_negated(expr: &'tcx Expr<'tcx>, needs_unsafe_block: bool) -> Self { + Self { + expr, + needs_unsafe_block, + needs_negated: false, + } + } + + pub fn to_snippet_with_context( + &self, + cx: &LateContext<'tcx>, + ctxt: SyntaxContext, + app: &mut Applicability, + ) -> Sugg<'tcx> { + let sugg = Sugg::hir_with_context(cx, self.expr, ctxt, "..", app); + if self.needs_negated { !sugg } else { sugg } + } +} + +// Try to parse into a recognized `Option` pattern. +// i.e. `_`, `None`, `Some(..)`, or a reference to any of those. +pub(super) fn try_parse_pattern<'tcx>( + cx: &LateContext<'tcx>, + pat: &'tcx Pat<'_>, + ctxt: SyntaxContext, +) -> Option> { + fn f<'tcx>( + cx: &LateContext<'tcx>, + pat: &'tcx Pat<'_>, + 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::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 => + { + Some(OptionPat::Some { pattern, ref_count }) + }, + _ => None, + } + } + f(cx, pat, 0, ctxt) +} + +// Checks for the `None` value. +fn is_none_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + matches!(peel_blocks(expr).kind, ExprKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone)) +} diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index cf1bd7a12ade..c472d6280985 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -3,6 +3,7 @@ mod infallible_destructuring_match; mod manual_filter; mod manual_map; mod manual_unwrap_or; +mod manual_utils; mod match_as_ref; mod match_bool; mod match_like_matches; diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs index e88542b77a67..f25bced0c2b3 100644 --- a/clippy_utils/src/sugg.rs +++ b/clippy_utils/src/sugg.rs @@ -112,7 +112,7 @@ impl<'a> Sugg<'a> { if expr.span.ctxt() == ctxt { Self::hir_from_snippet(expr, |span| snippet(cx, span, default)) } else { - let snip = snippet_with_context(cx, expr.span, ctxt, default, applicability).0; + let (snip, _) = snippet_with_context(cx, expr.span, ctxt, default, applicability); Sugg::NonParen(snip) } }