diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_all.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_complexity.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/matches/manual_filter.rs | 153 | ||||
| -rw-r--r-- | clippy_lints/src/matches/manual_map.rs | 318 | ||||
| -rw-r--r-- | clippy_lints/src/matches/manual_utils.rs | 278 | ||||
| -rw-r--r-- | clippy_lints/src/matches/mod.rs | 40 | ||||
| -rw-r--r-- | clippy_utils/src/sugg.rs | 6 | ||||
| -rw-r--r-- | src/docs.rs | 1 | ||||
| -rw-r--r-- | src/docs/manual_filter.txt | 21 | ||||
| -rw-r--r-- | tests/ui/manual_filter.fixed | 119 | ||||
| -rw-r--r-- | tests/ui/manual_filter.rs | 243 | ||||
| -rw-r--r-- | tests/ui/manual_filter.stderr | 191 |
14 files changed, 1118 insertions, 256 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 42615179f70..da29804fad0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3988,6 +3988,7 @@ Released 2018-09-13 [`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn [`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits [`manual_clamp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp +[`manual_filter`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter [`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map [`manual_find`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find [`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 5d26e4b3360..758f9936064 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -137,6 +137,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(match_result_ok::MATCH_RESULT_OK), LintId::of(matches::COLLAPSIBLE_MATCH), LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH), + LintId::of(matches::MANUAL_FILTER), LintId::of(matches::MANUAL_MAP), LintId::of(matches::MANUAL_UNWRAP_OR), LintId::of(matches::MATCH_AS_REF), diff --git a/clippy_lints/src/lib.register_complexity.rs b/clippy_lints/src/lib.register_complexity.rs index a58d066fa6b..e3849e5a626 100644 --- a/clippy_lints/src/lib.register_complexity.rs +++ b/clippy_lints/src/lib.register_complexity.rs @@ -27,6 +27,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec! LintId::of(manual_strip::MANUAL_STRIP), LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN), + LintId::of(matches::MANUAL_FILTER), LintId::of(matches::MANUAL_UNWRAP_OR), LintId::of(matches::MATCH_AS_REF), LintId::of(matches::MATCH_SINGLE_BINDING), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 05d927dbea7..72715eddbb9 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -259,6 +259,7 @@ store.register_lints(&[ match_result_ok::MATCH_RESULT_OK, matches::COLLAPSIBLE_MATCH, matches::INFALLIBLE_DESTRUCTURING_MATCH, + matches::MANUAL_FILTER, matches::MANUAL_MAP, matches::MANUAL_UNWRAP_OR, matches::MATCH_AS_REF, diff --git a/clippy_lints/src/matches/manual_filter.rs b/clippy_lints/src/matches/manual_filter.rs new file mode 100644 index 00000000000..66ba1f6f9c5 --- /dev/null +++ b/clippy_lints/src/matches/manual_filter.rs @@ -0,0 +1,153 @@ +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_res_lang_ctor, path_res, path_to_local_id}; + +use rustc_hir::LangItem::OptionSome; +use rustc_hir::{Arm, Expr, ExprKind, HirId, Pat, PatKind}; +use rustc_lint::LateContext; +use rustc_span::{sym, SyntaxContext}; + +use super::manual_utils::{check_with, SomeExpr}; +use super::MANUAL_FILTER; + +// Function called on the <expr> of `[&+]Some((ref | ref mut) x) => <expr>` +// Need to check if it's of the form `<expr>=if <cond> {<then_expr>} else {<else_expr>}` +// AND that only one `then/else_expr` resolves to `Some(x)` while the other resolves to `None` +// return the `cond` expression if so. +fn get_cond_expr<'tcx>( + cx: &LateContext<'tcx>, + pat: &Pat<'_>, + expr: &'tcx Expr<'_>, + ctxt: SyntaxContext, +) -> Option<SomeExpr<'tcx>> { + if_chain! { + if let Some(block_expr) = peels_blocks_incl_unsafe_opt(expr); + 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) + = (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 { + return Some(SomeExpr { + expr: peels_blocks_incl_unsafe(cond.peel_drop_temps()), + needs_unsafe_block: contains_unsafe_block(cx, expr), + needs_negated: !then_visitor // if the `then_expr` resolves to `None`, need to negate the cond + }) + } + }; + None +} + +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 `contains_unsafe_block` + if let ExprKind::Block(block, None) = expr.kind { + if block.stmts.is_empty() { + return block.expr; + } + }; + None +} + +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 <expr> expression: +// Some(x) => if <cond> { +// <expr> +// } else { +// <expr> +// } +// Returns true if <expr> 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(callee, [arg]) = inner_expr.kind { + return ctxt == expr.span.ctxt() + && is_res_lang_ctor(cx, path_res(cx, callee), OptionSome) + && path_to_local_id(arg, target); + } + }; + false +} + +// given the closure: `|<pattern>| <expr>` +// returns `|&<pattern>| <expr>` +fn add_ampersand_if_copy(body_str: String, has_copy_trait: bool) -> String { + if has_copy_trait { + let mut with_ampersand = body_str; + with_ampersand.insert(1, '&'); + with_ampersand + } else { + body_str + } +} + +pub(super) fn check_match<'tcx>( + cx: &LateContext<'tcx>, + scrutinee: &'tcx Expr<'_>, + arms: &'tcx [Arm<'_>], + expr: &'tcx Expr<'_>, +) { + let ty = cx.typeck_results().expr_ty(expr); + 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>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + let_pat: &'tcx Pat<'_>, + let_expr: &'tcx Expr<'_>, + then_expr: &'tcx Expr<'_>, + else_expr: &'tcx Expr<'_>, +) { + check(cx, expr, let_expr, let_pat, then_expr, None, else_expr); +} + +fn check<'tcx>( + 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<'_>, +) { + if let Some(sugg_info) = check_with( + cx, + expr, + scrutinee, + then_pat, + then_body, + else_pat, + else_body, + get_cond_expr, + ) { + let body_str = add_ampersand_if_copy(sugg_info.body_str, sugg_info.scrutinee_impl_copy); + span_lint_and_sugg( + cx, + MANUAL_FILTER, + expr.span, + "manual implementation of `Option::filter`", + "try this", + if sugg_info.needs_brackets { + format!( + "{{ {}{}.filter({body_str}) }}", + sugg_info.scrutinee_str, sugg_info.as_ref_str + ) + } else { + 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 76f5e1c941c..aaba239677f 100644 --- a/clippy_lints/src/matches/manual_map.rs +++ b/clippy_lints/src/matches/manual_map.rs @@ -1,22 +1,13 @@ -use crate::{map_unit_fn::OPTION_MAP_UNIT_FN, matches::MATCH_AS_REF}; +use super::manual_utils::{check_with, SomeExpr}; +use super::MANUAL_MAP; use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; -use clippy_utils::ty::{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_lint_allowed, is_res_lang_ctor, path_res, path_to_local_id, - peel_blocks, peel_hir_expr_refs, peel_hir_expr_while, 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 rustc_lint::LateContext; -use rustc_span::{sym, SyntaxContext}; -use super::MANUAL_MAP; +use clippy_utils::{is_res_lang_ctor, path_res}; + +use rustc_hir::LangItem::OptionSome; +use rustc_hir::{Arm, Block, BlockCheckMode, Expr, ExprKind, Pat, UnsafeSource}; +use rustc_lint::LateContext; +use rustc_span::SyntaxContext; pub(super) fn check_match<'tcx>( cx: &LateContext<'tcx>, @@ -43,7 +34,6 @@ pub(super) fn check_if_let<'tcx>( check(cx, expr, let_expr, let_pat, then_expr, None, else_expr); } -#[expect(clippy::too_many_lines)] fn check<'tcx>( cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, @@ -53,254 +43,74 @@ fn check<'tcx>( else_pat: Option<&'tcx Pat<'_>>, else_body: &'tcx 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) - && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Option)) - { - 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), - else_pat.map_or(Some(OptionPat::Wild), |p| try_parse_pattern(cx, p, expr_ctxt)), + if let Some(sugg_info) = check_with( + cx, + expr, + scrutinee, + then_pat, + then_body, + else_pat, + else_body, + get_some_expr, ) { - (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, - }; - - // Top level or patterns aren't allowed in closures. - if matches!(some_pat.kind, PatKind::Or(_)) { - return; - } - - let some_expr = match get_some_expr(cx, some_expr, false, expr_ctxt) { - Some(expr) => expr, - None => return, - }; - - // 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; - } - - // `map` won't perform any adjustments. - if !cx.typeck_results().expr_adjustments(some_expr.expr).is_empty() { - 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_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 `<local>[.<field>]*` 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, - Some(CaptureKind::Ref(Mutability::Not)) if binding_ref_mutability == Mutability::Mut => { - return; - }, - Some(CaptureKind::Ref(Mutability::Not)) | None => (), - } - } - } - }, - None => return, - }; - - 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 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() + span_lint_and_sugg( + cx, + MANUAL_MAP, + expr.span, + "manual implementation of `Option::map`", + "try this", + if sugg_info.needs_brackets { + format!( + "{{ {}{}.map({}) }}", + sugg_info.scrutinee_str, sugg_info.as_ref_str, sugg_info.body_str + ) } 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; - } - - // `ref` and `ref mut` annotations were handled earlier. - let annotation = if matches!(annotation, BindingAnnotation::MUT) { - "mut " - } else { - "" - }; - let expr_snip = snippet_with_context(cx, some_expr.expr.span, expr_ctxt, "..", &mut app).0; - if some_expr.needs_unsafe_block { - format!("|{annotation}{some_binding}| unsafe {{ {expr_snip} }}") - } else { - format!("|{annotation}{some_binding}| {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; - let expr_snip = snippet_with_context(cx, some_expr.expr.span, expr_ctxt, "..", &mut app).0; - if some_expr.needs_unsafe_block { - format!("|{pat_snip}| unsafe {{ {expr_snip} }}") - } else { - format!("|{pat_snip}| {expr_snip}") - } - } 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 else_pat.is_none() && is_else_clause(cx.tcx, expr) { - format!("{{ {scrutinee_str}{as_ref_str}.map({body_str}) }}") - } else { - format!("{scrutinee_str}{as_ref_str}.map({body_str})") - }, - app, - ); -} - -// 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, - } -} - -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, - }, -} - -struct SomeExpr<'tcx> { - expr: &'tcx Expr<'tcx>, - needs_unsafe_block: bool, -} - -// Try to parse into a recognized `Option` pattern. -// i.e. `_`, `None`, `Some(..)`, or a reference to any of those. -fn try_parse_pattern<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, ctxt: SyntaxContext) -> Option<OptionPat<'tcx>> { - fn f<'tcx>( - cx: &LateContext<'tcx>, - pat: &'tcx Pat<'_>, - ref_count: usize, - ctxt: SyntaxContext, - ) -> Option<OptionPat<'tcx>> { - match pat.kind { - PatKind::Wild => Some(OptionPat::Wild), - PatKind::Ref(pat, _) => f(cx, pat, ref_count + 1, ctxt), - PatKind::Path(ref qpath) if is_res_lang_ctor(cx, cx.qpath_res(qpath, pat.hir_id), OptionNone) => { - Some(OptionPat::None) - }, - PatKind::TupleStruct(ref qpath, [pattern], _) - if is_res_lang_ctor(cx, cx.qpath_res(qpath, pat.hir_id), OptionSome) && pat.span.ctxt() == ctxt => - { - Some(OptionPat::Some { pattern, ref_count }) + format!( + "{}{}.map({})", + sugg_info.scrutinee_str, sugg_info.as_ref_str, sugg_info.body_str + ) }, - _ => None, - } + sugg_info.app, + ); } - 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>, + _: &'tcx Pat<'_>, expr: &'tcx Expr<'_>, - needs_unsafe_block: bool, ctxt: SyntaxContext, ) -> Option<SomeExpr<'tcx>> { - // TODO: Allow more complex expressions. - match expr.kind { - ExprKind::Call(callee, [arg]) - if ctxt == expr.span.ctxt() && is_res_lang_ctor(cx, path_res(cx, callee), OptionSome) => - { - Some(SomeExpr { - expr: arg, - needs_unsafe_block, - }) - }, - ExprKind::Block( - Block { - stmts: [], - expr: Some(expr), - rules, - .. + fn get_some_expr_internal<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + needs_unsafe_block: bool, + ctxt: SyntaxContext, + ) -> Option<SomeExpr<'tcx>> { + // TODO: Allow more complex expressions. + match expr.kind { + ExprKind::Call(callee, [arg]) + if ctxt == expr.span.ctxt() && is_res_lang_ctor(cx, path_res(cx, callee), OptionSome) => + { + Some(SomeExpr::new_no_negated(arg, needs_unsafe_block)) }, - _, - ) => get_some_expr( - cx, - expr, - needs_unsafe_block || *rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided), - ctxt, - ), - _ => None, + ExprKind::Block( + Block { + stmts: [], + expr: Some(expr), + rules, + .. + }, + _, + ) => get_some_expr_internal( + cx, + expr, + needs_unsafe_block || *rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided), + ctxt, + ), + _ => None, + } } -} - -// Checks for the `None` value. -fn is_none_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - is_res_lang_ctor(cx, path_res(cx, peel_blocks(expr)), OptionNone) + get_some_expr_internal(cx, expr, false, ctxt) } diff --git a/clippy_lints/src/matches/manual_utils.rs b/clippy_lints/src/matches/manual_utils.rs new file mode 100644 index 00000000000..792908aa7df --- /dev/null +++ b/clippy_lints/src/matches/manual_utils.rs @@ -0,0 +1,278 @@ +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_lint_allowed, is_res_lang_ctor, path_res, 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<SuggInfo<'tcx>> +where + F: Fn(&LateContext<'tcx>, &'tcx Pat<'_>, &'tcx Expr<'_>, SyntaxContext) -> Option<SomeExpr<'tcx>>, +{ + 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 `<local>[.<field>]*` 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<T>: 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<OptionPat<'tcx>> { + fn f<'tcx>( + cx: &LateContext<'tcx>, + pat: &'tcx Pat<'_>, + ref_count: usize, + ctxt: SyntaxContext, + ) -> Option<OptionPat<'tcx>> { + match pat.kind { + PatKind::Wild => Some(OptionPat::Wild), + PatKind::Ref(pat, _) => f(cx, pat, ref_count + 1, ctxt), + PatKind::Path(ref qpath) if is_res_lang_ctor(cx, cx.qpath_res(qpath, pat.hir_id), OptionNone) => { + Some(OptionPat::None) + }, + PatKind::TupleStruct(ref qpath, [pattern], _) + if is_res_lang_ctor(cx, cx.qpath_res(qpath, pat.hir_id), 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 { + is_res_lang_ctor(cx, path_res(cx, peel_blocks(expr)), OptionNone) +} diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index e6b183fc05f..7d8171ead89 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -1,7 +1,9 @@ mod collapsible_match; 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; @@ -898,6 +900,34 @@ declare_clippy_lint! { "reimplementation of `map`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for usages of `match` which could be implemented using `filter` + /// + /// ### Why is this bad? + /// Using the `filter` method is clearer and more concise. + /// + /// ### Example + /// ```rust + /// match Some(0) { + /// Some(x) => if x % 2 == 0 { + /// Some(x) + /// } else { + /// None + /// }, + /// None => None, + /// }; + /// ``` + /// Use instead: + /// ```rust + /// Some(0).filter(|&x| x % 2 == 0); + /// ``` + #[clippy::version = "1.66.0"] + pub MANUAL_FILTER, + complexity, + "reimplentation of `filter`" +} + #[derive(Default)] pub struct Matches { msrv: Option<RustcVersion>, @@ -939,6 +969,7 @@ impl_lint_pass!(Matches => [ SIGNIFICANT_DROP_IN_SCRUTINEE, TRY_ERR, MANUAL_MAP, + MANUAL_FILTER, ]); impl<'tcx> LateLintPass<'tcx> for Matches { @@ -988,6 +1019,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { if !in_constant(cx, expr.hir_id) { manual_unwrap_or::check(cx, expr, ex, arms); manual_map::check_match(cx, expr, ex, arms); + manual_filter::check_match(cx, ex, arms, expr); } if self.infallible_destructuring_match_linted { @@ -1014,6 +1046,14 @@ impl<'tcx> LateLintPass<'tcx> for Matches { } if !in_constant(cx, expr.hir_id) { manual_map::check_if_let(cx, expr, if_let.let_pat, if_let.let_expr, if_let.if_then, else_expr); + manual_filter::check_if_let( + cx, + expr, + if_let.let_pat, + if_let.let_expr, + if_let.if_then, + else_expr, + ); } } redundant_pattern_match::check_if_let( diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs index ef836e84829..f25bced0c2b 100644 --- a/clippy_utils/src/sugg.rs +++ b/clippy_utils/src/sugg.rs @@ -1,7 +1,9 @@ //! Contains utility functions to generate suggestions. #![deny(clippy::missing_docs_in_private_items)] -use crate::source::{snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite}; +use crate::source::{ + snippet, snippet_opt, snippet_with_applicability, snippet_with_context, snippet_with_macro_callsite, +}; use crate::ty::expr_sig; use crate::{get_parent_expr_for_hir, higher}; use rustc_ast::util::parser::AssocOp; @@ -110,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_applicability(cx, expr.span, default, applicability); + let (snip, _) = snippet_with_context(cx, expr.span, ctxt, default, applicability); Sugg::NonParen(snip) } } diff --git a/src/docs.rs b/src/docs.rs index 3bf488ab477..c7534186004 100644 --- a/src/docs.rs +++ b/src/docs.rs @@ -258,6 +258,7 @@ docs! { "manual_async_fn", "manual_bits", "manual_clamp", + "manual_filter", "manual_filter_map", "manual_find", "manual_find_map", diff --git a/src/docs/manual_filter.txt b/src/docs/manual_filter.txt new file mode 100644 index 00000000000..19a4d9319d9 --- /dev/null +++ b/src/docs/manual_filter.txt @@ -0,0 +1,21 @@ +### What it does +Checks for usages of `match` which could be implemented using `filter` + +### Why is this bad? +Using the `filter` method is clearer and more concise. + +### Example +``` +match Some(0) { + Some(x) => if x % 2 == 0 { + Some(x) + } else { + None + }, + None => None, +}; +``` +Use instead: +``` +Some(0).filter(|&x| x % 2 == 0); +``` \ No newline at end of file diff --git a/tests/ui/manual_filter.fixed b/tests/ui/manual_filter.fixed new file mode 100644 index 00000000000..3553291b87d --- /dev/null +++ b/tests/ui/manual_filter.fixed @@ -0,0 +1,119 @@ +// run-rustfix + +#![warn(clippy::manual_filter)] +#![allow(unused_variables, dead_code)] + +fn main() { + Some(0).filter(|&x| x <= 0); + + Some(1).filter(|&x| x <= 0); + + Some(2).filter(|&x| x <= 0); + + Some(3).filter(|&x| x > 0); + + let y = Some(4); + y.filter(|&x| x <= 0); + + Some(5).filter(|&x| x > 0); + + Some(6).as_ref().filter(|&x| x > &0); + + let external_cond = true; + Some(String::new()).filter(|x| external_cond); + + Some(7).filter(|&x| external_cond); + + Some(8).filter(|&x| x != 0); + + Some(9).filter(|&x| x > 10 && x < 100); + + const fn f1() { + // Don't lint, `.filter` is not const + match Some(10) { + Some(x) => { + if x > 10 && x < 100 { + Some(x) + } else { + None + } + }, + None => None, + }; + } + + #[allow(clippy::blocks_in_if_conditions)] + Some(11).filter(|&x| { + println!("foo"); + x > 10 && x < 100 + }); + + match Some(12) { + // Don't Lint, statement is lost by `.filter` + Some(x) => { + if x > 10 && x < 100 { + println!("foo"); + Some(x) + } else { + None + } + }, + None => None, + }; + + match Some(13) { + // Don't Lint, because of `None => Some(1)` + Some(x) => { + if x > 10 && x < 100 { + println!("foo"); + Some(x) + } else { + None + } + }, + None => Some(1), + }; + + unsafe fn f(x: u32) -> bool { + true + } + let _ = Some(14).filter(|&x| unsafe { f(x) }); + let _ = Some(15).filter(|&x| unsafe { f(x) }); + + #[allow(clippy::redundant_pattern_matching)] + if let Some(_) = Some(16) { + Some(16) + } else { Some(16).filter(|&x| x % 2 == 0) }; + + match Some((17, 17)) { + // Not linted for now could be + Some((x, y)) => { + if y != x { + Some((x, y)) + } else { + None + } + }, + None => None, + }; + + struct NamedTuple { + pub x: u8, + pub y: (i32, u32), + } + + match Some(NamedTuple { + // Not linted for now could be + x: 17, + y: (18, 19), + }) { + Some(NamedTuple { x, y }) => { + if y.1 != x as u32 { + Some(NamedTuple { x, y }) + } else { + None + } + }, + None => None, + }; +} diff --git a/tests/ui/manual_filter.rs b/tests/ui/manual_filter.rs new file mode 100644 index 00000000000..aa9f90f752b --- /dev/null +++ b/tests/ui/manual_filter.rs @@ -0,0 +1,243 @@ +// run-rustfix + +#![warn(clippy::manual_filter)] +#![allow(unused_variables, dead_code)] + +fn main() { + match Some(0) { + None => None, + Some(x) => { + if x > 0 { + None + } else { + Some(x) + } + }, + }; + + match Some(1) { + Some(x) => { + if x > 0 { + None + } else { + Some(x) + } + }, + None => None, + }; + + match Some(2) { + Some(x) => { + if x > 0 { + None + } else { + Some(x) + } + }, + _ => None, + }; + + match Some(3) { + Some(x) => { + if x > 0 { + Some(x) + } else { + None + } + }, + None => None, + }; + + let y = Some(4); + match y { + // Some(4) + None => None, + Some(x) => { + if x > 0 { + None + } else { + Some(x) + } + }, + }; + + match Some(5) { + Some(x) => { + if x > 0 { + Some(x) + } else { + None + } + }, + _ => None, + }; + + match Some(6) { + Some(ref x) => { + if x > &0 { + Some(x) + } else { + None + } + }, + _ => None, + }; + + let external_cond = true; + match Some(String::new()) { + Some(x) => { + if external_cond { + Some(x) + } else { + None + } + }, + _ => None, + }; + + if let Some(x) = Some(7) { + if external_cond { Some(x) } else { None } + } else { + None + }; + + match &Some(8) { + &Some(x) => { + if x != 0 { + Some(x) + } else { + None + } + }, + _ => None, + }; + + match Some(9) { + Some(x) => { + if x > 10 && x < 100 { + Some(x) + } else { + None + } + }, + None => None, + }; + + const fn f1() { + // Don't lint, `.filter` is not const + match Some(10) { + Some(x) => { + if x > 10 && x < 100 { + Some(x) + } else { + None + } + }, + None => None, + }; + } + + #[allow(clippy::blocks_in_if_conditions)] + match Some(11) { + // Lint, statement is preserved by `.filter` + Some(x) => { + if { + println!("foo"); + x > 10 && x < 100 + } { + Some(x) + } else { + None + } + }, + None => None, + }; + + match Some(12) { + // Don't Lint, statement is lost by `.filter` + Some(x) => { + if x > 10 && x < 100 { + println!("foo"); + Some(x) + } else { + None + } + }, + None => None, + }; + + match Some(13) { + // Don't Lint, because of `None => Some(1)` + Some(x) => { + if x > 10 && x < 100 { + println!("foo"); + Some(x) + } else { + None + } + }, + None => Some(1), + }; + + unsafe fn f(x: u32) -> bool { + true + } + let _ = match Some(14) { + Some(x) => { + if unsafe { f(x) } { + Some(x) + } else { + None + } + }, + None => None, + }; + let _ = match Some(15) { + Some(x) => unsafe { + if f(x) { Some(x) } else { None } + }, + None => None, + }; + + #[allow(clippy::redundant_pattern_matching)] + if let Some(_) = Some(16) { + Some(16) + } else if let Some(x) = Some(16) { + // Lint starting from here + if x % 2 == 0 { Some(x) } else { None } + } else { + None + }; + + match Some((17, 17)) { + // Not linted for now could be + Some((x, y)) => { + if y != x { + Some((x, y)) + } else { + None + } + }, + None => None, + }; + + struct NamedTuple { + pub x: u8, + pub y: (i32, u32), + } + + match Some(NamedTuple { + // Not linted for now could be + x: 17, + y: (18, 19), + }) { + Some(NamedTuple { x, y }) => { + if y.1 != x as u32 { + Some(NamedTuple { x, y }) + } else { + None + } + }, + None => None, + }; +} diff --git a/tests/ui/manual_filter.stderr b/tests/ui/manual_filter.stderr new file mode 100644 index 00000000000..53dea922930 --- /dev/null +++ b/tests/ui/manual_filter.stderr @@ -0,0 +1,191 @@ +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:7:5 + | +LL | / match Some(0) { +LL | | None => None, +LL | | Some(x) => { +LL | | if x > 0 { +... | +LL | | }, +LL | | }; + | |_____^ help: try this: `Some(0).filter(|&x| x <= 0)` + | + = note: `-D clippy::manual-filter` implied by `-D warnings` + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:18:5 + | +LL | / match Some(1) { +LL | | Some(x) => { +LL | | if x > 0 { +LL | | None +... | +LL | | None => None, +LL | | }; + | |_____^ help: try this: `Some(1).filter(|&x| x <= 0)` + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:29:5 + | +LL | / match Some(2) { +LL | | Some(x) => { +LL | | if x > 0 { +LL | | None +... | +LL | | _ => None, +LL | | }; + | |_____^ help: try this: `Some(2).filter(|&x| x <= 0)` + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:40:5 + | +LL | / match Some(3) { +LL | | Some(x) => { +LL | | if x > 0 { +LL | | Some(x) +... | +LL | | None => None, +LL | | }; + | |_____^ help: try this: `Some(3).filter(|&x| x > 0)` + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:52:5 + | +LL | / match y { +LL | | // Some(4) +LL | | None => None, +LL | | Some(x) => { +... | +LL | | }, +LL | | }; + | |_____^ help: try this: `y.filter(|&x| x <= 0)` + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:64:5 + | +LL | / match Some(5) { +LL | | Some(x) => { +LL | | if x > 0 { +LL | | Some(x) +... | +LL | | _ => None, +LL | | }; + | |_____^ help: try this: `Some(5).filter(|&x| x > 0)` + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:75:5 + | +LL | / match Some(6) { +LL | | Some(ref x) => { +LL | | if x > &0 { +LL | | Some(x) +... | +LL | | _ => None, +LL | | }; + | |_____^ help: try this: `Some(6).as_ref().filter(|&x| x > &0)` + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:87:5 + | +LL | / match Some(String::new()) { +LL | | Some(x) => { +LL | | if external_cond { +LL | | Some(x) +... | +LL | | _ => None, +LL | | }; + | |_____^ help: try this: `Some(String::new()).filter(|x| external_cond)` + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:98:5 + | +LL | / if let Some(x) = Some(7) { +LL | | if external_cond { Some(x) } else { None } +LL | | } else { +LL | | None +LL | | }; + | |_____^ help: try this: `Some(7).filter(|&x| external_cond)` + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:104:5 + | +LL | / match &Some(8) { +LL | | &Some(x) => { +LL | | if x != 0 { +LL | | Some(x) +... | +LL | | _ => None, +LL | | }; + | |_____^ help: try this: `Some(8).filter(|&x| x != 0)` + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:115:5 + | +LL | / match Some(9) { +LL | | Some(x) => { +LL | | if x > 10 && x < 100 { +LL | | Some(x) +... | +LL | | None => None, +LL | | }; + | |_____^ help: try this: `Some(9).filter(|&x| x > 10 && x < 100)` + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:141:5 + | +LL | / match Some(11) { +LL | | // Lint, statement is preserved by `.filter` +LL | | Some(x) => { +LL | | if { +... | +LL | | None => None, +LL | | }; + | |_____^ + | +help: try this + | +LL ~ Some(11).filter(|&x| { +LL + println!("foo"); +LL + x > 10 && x < 100 +LL ~ }); + | + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:185:13 + | +LL | let _ = match Some(14) { + | _____________^ +LL | | Some(x) => { +LL | | if unsafe { f(x) } { +LL | | Some(x) +... | +LL | | None => None, +LL | | }; + | |_____^ help: try this: `Some(14).filter(|&x| unsafe { f(x) })` + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:195:13 + | +LL | let _ = match Some(15) { + | _____________^ +LL | | Some(x) => unsafe { +LL | | if f(x) { Some(x) } else { None } +LL | | }, +LL | | None => None, +LL | | }; + | |_____^ help: try this: `Some(15).filter(|&x| unsafe { f(x) })` + +error: manual implementation of `Option::filter` + --> $DIR/manual_filter.rs:205:12 + | +LL | } else if let Some(x) = Some(16) { + | ____________^ +LL | | // Lint starting from here +LL | | if x % 2 == 0 { Some(x) } else { None } +LL | | } else { +LL | | None +LL | | }; + | |_____^ help: try this: `{ Some(16).filter(|&x| x % 2 == 0) }` + +error: aborting due to 15 previous errors + |