diff --git a/clippy_lints/src/manual_pop_if.rs b/clippy_lints/src/manual_pop_if.rs index 6662a34bc332..78176a9ad5db 100644 --- a/clippy_lints/src/manual_pop_if.rs +++ b/clippy_lints/src/manual_pop_if.rs @@ -1,10 +1,10 @@ use clippy_config::Conf; -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::res::MaybeDef; use clippy_utils::source::snippet_with_context; -use clippy_utils::visitors::is_local_used; -use clippy_utils::{eq_expr_value, is_else_clause, is_lang_item_or_ctor, peel_blocks_with_stmt, sym}; +use clippy_utils::visitors::{for_each_expr_without_closures, is_local_used}; +use clippy_utils::{eq_expr_value, is_else_clause, is_lang_item_or_ctor, sym}; use rustc_ast::LitKind; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, LangItem, PatKind, RustcVersion, StmtKind}; @@ -12,6 +12,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; use rustc_span::{Span, Symbol}; use std::fmt; +use std::ops::ControlFlow; declare_clippy_lint! { /// ### What it does @@ -21,11 +22,9 @@ declare_clippy_lint! { /// Using `pop_if` is more concise and idiomatic. /// /// ### Known issues - /// Currently, the lint does not handle the case where the - /// `if` condition is part of an `else if` branch. - /// - /// The lint also does not handle the case where - /// the popped value is assigned and used. + /// When the popped value is assigned or used in an expression, + /// or when the `if` condition is part of an `else if` branch, the + /// lint will trigger but will not provide an automatic suggestion. /// /// ### Examples /// ```no_run @@ -78,7 +77,7 @@ enum ManualPopIfKind { } impl ManualPopIfKind { - fn check_method(self) -> Symbol { + fn peek_method(self) -> Symbol { match self { ManualPopIfKind::Vec => sym::last, ManualPopIfKind::VecDequeBack => sym::back, @@ -143,6 +142,9 @@ struct ManualPopIfPattern<'tcx> { /// Span of the if expression (including the `if` keyword) if_span: Span, + + /// Whether we are able to provide a suggestion + suggestable: bool, } impl ManualPopIfPattern<'_> { @@ -154,36 +156,23 @@ impl ManualPopIfPattern<'_> { let param_name = self.param_name; let pop_if_method = self.kind.pop_if_method(); - let suggestion = format!("{collection_snippet}.{pop_if_method}(|{param_name}| {predicate_snippet});"); - - span_lint_and_sugg( + span_lint_and_then( cx, MANUAL_POP_IF, self.if_span, format!("manual implementation of {}", self.kind), - "try", - suggestion, - app, + |diag| { + let sugg = format!("{collection_snippet}.{pop_if_method}(|{param_name}| {predicate_snippet});"); + if self.suggestable { + diag.span_suggestion(self.if_span, "try", sugg, app); + } else { + diag.help(format!("try using `{sugg}`")); + } + }, ); } } -fn pop_value_is_used(then_block: &Expr<'_>) -> bool { - let ExprKind::Block(block, _) = then_block.kind else { - return true; - }; - - if block.expr.is_some() { - return true; - } - - match block.stmts { - [stmt] => !matches!(stmt.kind, StmtKind::Semi(_) | StmtKind::Item(_)), - [.., last] => matches!(last.kind, StmtKind::Expr(_)), - [] => false, - } -} - /// Checks for the pattern: /// ```ignore /// if vec.last().is_some_and(|x| *x > 5) { @@ -197,21 +186,17 @@ fn check_is_some_and_pattern<'tcx>( if_expr_span: Span, kind: ManualPopIfKind, ) -> Option> { - if pop_value_is_used(then_block) { - return None; - } - - let check_method = kind.check_method(); + let peek_method = kind.peek_method(); let pop_method = kind.pop_method(); if let ExprKind::MethodCall(path, receiver, [closure_arg], _) = cond.kind && path.ident.name == sym::is_some_and && let ExprKind::MethodCall(check_path, collection_expr, [], _) = receiver.kind - && check_path.ident.name == check_method + && check_path.ident.name == peek_method && kind.is_diag_item(cx, collection_expr) && let ExprKind::Closure(closure) = closure_arg.kind && let body = cx.tcx.hir_body(closure.body) - && let Some((pop_collection, _pop_span)) = check_pop_unwrap(then_block, pop_method) + && let Some((pop_collection, suggestable)) = check_pop_unwrap(then_block, pop_method) && eq_expr_value(cx, collection_expr, pop_collection) && let Some(param) = body.params.first() && let Some(ident) = param.pat.simple_ident() @@ -222,6 +207,7 @@ fn check_is_some_and_pattern<'tcx>( predicate: body.value, param_name: ident.name, if_span: if_expr_span, + suggestable, }); } @@ -243,7 +229,7 @@ fn check_if_let_pattern<'tcx>( if_expr_span: Span, kind: ManualPopIfKind, ) -> Option> { - let check_method = kind.check_method(); + let peek_method = kind.peek_method(); let pop_method = kind.pop_method(); if let ExprKind::Let(let_expr) = cond.kind @@ -255,7 +241,7 @@ fn check_if_let_pattern<'tcx>( && is_lang_item_or_ctor(cx, def_id, LangItem::OptionSome) && let PatKind::Binding(_, binding_id, binding_name, _) = binding_pat.kind && let ExprKind::MethodCall(path, collection_expr, [], _) = let_expr.init.kind - && path.ident.name == check_method + && path.ident.name == peek_method && kind.is_diag_item(cx, collection_expr) && let ExprKind::Block(block, _) = then_block.kind { @@ -271,8 +257,7 @@ fn check_if_let_pattern<'tcx>( if let ExprKind::If(inner_cond, inner_then, None) = inner_if.kind && is_local_used(cx, inner_cond, binding_id) - && !pop_value_is_used(inner_then) - && let Some((pop_collection, _pop_span)) = check_pop_unwrap(inner_then, pop_method) + && let Some((pop_collection, suggestable)) = check_pop_unwrap(inner_then, pop_method) && eq_expr_value(cx, collection_expr, pop_collection) { return Some(ManualPopIfPattern { @@ -281,6 +266,7 @@ fn check_if_let_pattern<'tcx>( predicate: inner_cond, param_name: binding_name.name, if_span: if_expr_span, + suggestable, }); } } @@ -302,11 +288,7 @@ fn check_let_chain_pattern<'tcx>( if_expr_span: Span, kind: ManualPopIfKind, ) -> Option> { - if pop_value_is_used(then_block) { - return None; - } - - let check_method = kind.check_method(); + let peek_method = kind.peek_method(); let pop_method = kind.pop_method(); if let ExprKind::Binary(op, left, right) = cond.kind @@ -320,11 +302,10 @@ fn check_let_chain_pattern<'tcx>( && is_lang_item_or_ctor(cx, def_id, LangItem::OptionSome) && let PatKind::Binding(_, binding_id, binding_name, _) = binding_pat.kind && let ExprKind::MethodCall(path, collection_expr, [], _) = let_expr.init.kind - && path.ident.name == check_method + && path.ident.name == peek_method && kind.is_diag_item(cx, collection_expr) && is_local_used(cx, right, binding_id) - && !pop_value_is_used(then_block) - && let Some((pop_collection, _pop_span)) = check_pop_unwrap(then_block, pop_method) + && let Some((pop_collection, suggestable)) = check_pop_unwrap(then_block, pop_method) && eq_expr_value(cx, collection_expr, pop_collection) { return Some(ManualPopIfPattern { @@ -333,6 +314,7 @@ fn check_let_chain_pattern<'tcx>( predicate: right, param_name: binding_name.name, if_span: if_expr_span, + suggestable, }); } } @@ -353,11 +335,7 @@ fn check_map_unwrap_or_pattern<'tcx>( if_expr_span: Span, kind: ManualPopIfKind, ) -> Option> { - if pop_value_is_used(then_block) { - return None; - } - - let check_method = kind.check_method(); + let peek_method = kind.peek_method(); let pop_method = kind.pop_method(); if let ExprKind::MethodCall(unwrap_path, receiver, [default_arg], _) = cond.kind @@ -366,12 +344,12 @@ fn check_map_unwrap_or_pattern<'tcx>( && let ExprKind::MethodCall(map_path, map_receiver, [closure_arg], _) = receiver.kind && map_path.ident.name == sym::map && let ExprKind::MethodCall(check_path, collection_expr, [], _) = map_receiver.kind - && check_path.ident.name == check_method + && check_path.ident.name == peek_method && kind.is_diag_item(cx, collection_expr) && let ExprKind::Closure(closure) = closure_arg.kind && let body = cx.tcx.hir_body(closure.body) && cx.typeck_results().expr_ty(body.value).is_bool() - && let Some((pop_collection, _pop_span)) = check_pop_unwrap(then_block, pop_method) + && let Some((pop_collection, suggestable)) = check_pop_unwrap(then_block, pop_method) && eq_expr_value(cx, collection_expr, pop_collection) && let Some(param) = body.params.first() && let Some(ident) = param.pat.simple_ident() @@ -382,6 +360,7 @@ fn check_map_unwrap_or_pattern<'tcx>( predicate: body.value, param_name: ident.name, if_span: if_expr_span, + suggestable, }); } @@ -389,19 +368,38 @@ fn check_map_unwrap_or_pattern<'tcx>( } /// Checks for `collection.().unwrap()` or `collection.().expect(..)` -/// and returns the collection and the span of the peeled expr -fn check_pop_unwrap<'tcx>(expr: &'tcx Expr<'_>, pop_method: Symbol) -> Option<(&'tcx Expr<'tcx>, Span)> { - let inner_expr = peel_blocks_with_stmt(expr); +/// and returns the expression. Also returns a boolean to indicate whether we are able +// to provide an automatic suggestion on how to use `pop_if`. +fn check_pop_unwrap<'tcx>(expr: &'tcx Expr<'_>, pop_method: Symbol) -> Option<(&'tcx Expr<'tcx>, bool)> { + let ExprKind::Block(block, _) = expr.kind else { + return None; + }; - if let ExprKind::MethodCall(unwrap_path, receiver, _, _) = inner_expr.kind + // Check for single statement with the pop unwrap (not in a macro or other expression) + if let [stmt] = block.stmts + && block.expr.is_none() + && let StmtKind::Semi(stmt_expr) | StmtKind::Expr(stmt_expr) = &stmt.kind + && let ExprKind::MethodCall(unwrap_path, receiver, _, _) = stmt_expr.kind && matches!(unwrap_path.ident.name, sym::unwrap | sym::expect) && let ExprKind::MethodCall(pop_path, collection_expr, [], _) = receiver.kind && pop_path.ident.name == pop_method + && !stmt_expr.span.from_expansion() { - return Some((collection_expr, inner_expr.span)); + return Some((collection_expr, true)); } - None + // Check if the pop unwrap is present at all + for_each_expr_without_closures(block, |expr| { + if let ExprKind::MethodCall(unwrap_path, receiver, _, _) = expr.kind + && matches!(unwrap_path.ident.name, sym::unwrap | sym::expect) + && let ExprKind::MethodCall(pop_path, collection_expr, [], _) = receiver.kind + && pop_path.ident.name == pop_method + { + ControlFlow::Break((collection_expr, false)) + } else { + ControlFlow::Continue(()) + } + }) } impl<'tcx> LateLintPass<'tcx> for ManualPopIf { @@ -410,22 +408,23 @@ impl<'tcx> LateLintPass<'tcx> for ManualPopIf { return; }; - // Do not lint if we are in an else-if branch. - if is_else_clause(cx.tcx, expr) { - return; - } + let in_else_clause = is_else_clause(cx.tcx, expr); for kind in [ ManualPopIfKind::Vec, ManualPopIfKind::VecDequeBack, ManualPopIfKind::VecDequeFront, ] { - if let Some(pattern) = check_is_some_and_pattern(cx, cond, then_block, expr.span, kind) + if let Some(mut pattern) = check_is_some_and_pattern(cx, cond, then_block, expr.span, kind) .or_else(|| check_if_let_pattern(cx, cond, then_block, expr.span, kind)) .or_else(|| check_let_chain_pattern(cx, cond, then_block, expr.span, kind)) .or_else(|| check_map_unwrap_or_pattern(cx, cond, then_block, expr.span, kind)) && self.msrv.meets(cx, kind.msrv()) { + if in_else_clause { + pattern.suggestable = false; + } + pattern.emit_lint(cx); return; } diff --git a/tests/ui/manual_pop_if.fixed b/tests/ui/manual_pop_if.fixed index 6e30786c8097..168fd1cdb50c 100644 --- a/tests/ui/manual_pop_if.fixed +++ b/tests/ui/manual_pop_if.fixed @@ -40,24 +40,6 @@ fn is_some_and_pattern_negative(mut vec: Vec, mut deque: VecDeque) { fake_vec.pop().unwrap(); } - // Do not lint, else-if branch - if false { - // something - } else if vec.last().is_some_and(|x| *x > 2) { - vec.pop().unwrap(); - } - - // Do not lint, value used in let binding - if vec.last().is_some_and(|x| *x > 2) { - let _value = vec.pop().unwrap(); - println!("Popped: {}", _value); - } - - // Do not lint, value used in expression - if vec.last().is_some_and(|x| *x > 2) { - println!("Popped: {}", vec.pop().unwrap()); - } - // Do not lint, else block let _result = if vec.last().is_some_and(|x| *x > 2) { vec.pop().unwrap() @@ -100,13 +82,6 @@ fn if_let_pattern_negative(mut vec: Vec) { } } - // Do not lint, value used in let binding - if let Some(x) = vec.last() { - if *x > 2 { - let _val = vec.pop().unwrap(); - } - } - // Do not lint, else block let _result = if let Some(x) = vec.last() { if *x > 2 { vec.pop().unwrap() } else { 0 } @@ -141,20 +116,6 @@ fn let_chain_pattern_negative(mut vec: Vec) { vec.pop().unwrap(); } - // Do not lint, value used in let binding - if let Some(x) = vec.last() - && *x > 2 - { - let _val = vec.pop().unwrap(); - } - - // Do not lint, value used in expression - if let Some(x) = vec.last() - && *x > 2 - { - println!("Popped: {}", vec.pop().unwrap()); - } - // Do not lint, else block let _result = if let Some(x) = vec.last() && *x > 2 @@ -198,11 +159,6 @@ fn map_unwrap_or_pattern_negative(mut vec: Vec) { vec.pop().unwrap(); } - // Do not lint, value used in let binding - if vec.last().map(|x| *x > 2).unwrap_or(false) { - let _val = vec.pop().unwrap(); - } - // Do not lint, else block let _result = if vec.last().map(|x| *x > 2).unwrap_or(false) { vec.pop().unwrap() diff --git a/tests/ui/manual_pop_if.rs b/tests/ui/manual_pop_if.rs index a2f679dfc6b3..2f403dcf89cb 100644 --- a/tests/ui/manual_pop_if.rs +++ b/tests/ui/manual_pop_if.rs @@ -52,24 +52,6 @@ fn is_some_and_pattern_negative(mut vec: Vec, mut deque: VecDeque) { fake_vec.pop().unwrap(); } - // Do not lint, else-if branch - if false { - // something - } else if vec.last().is_some_and(|x| *x > 2) { - vec.pop().unwrap(); - } - - // Do not lint, value used in let binding - if vec.last().is_some_and(|x| *x > 2) { - let _value = vec.pop().unwrap(); - println!("Popped: {}", _value); - } - - // Do not lint, value used in expression - if vec.last().is_some_and(|x| *x > 2) { - println!("Popped: {}", vec.pop().unwrap()); - } - // Do not lint, else block let _result = if vec.last().is_some_and(|x| *x > 2) { vec.pop().unwrap() @@ -132,13 +114,6 @@ fn if_let_pattern_negative(mut vec: Vec) { } } - // Do not lint, value used in let binding - if let Some(x) = vec.last() { - if *x > 2 { - let _val = vec.pop().unwrap(); - } - } - // Do not lint, else block let _result = if let Some(x) = vec.last() { if *x > 2 { vec.pop().unwrap() } else { 0 } @@ -189,20 +164,6 @@ fn let_chain_pattern_negative(mut vec: Vec) { vec.pop().unwrap(); } - // Do not lint, value used in let binding - if let Some(x) = vec.last() - && *x > 2 - { - let _val = vec.pop().unwrap(); - } - - // Do not lint, value used in expression - if let Some(x) = vec.last() - && *x > 2 - { - println!("Popped: {}", vec.pop().unwrap()); - } - // Do not lint, else block let _result = if let Some(x) = vec.last() && *x > 2 @@ -258,11 +219,6 @@ fn map_unwrap_or_pattern_negative(mut vec: Vec) { vec.pop().unwrap(); } - // Do not lint, value used in let binding - if vec.last().map(|x| *x > 2).unwrap_or(false) { - let _val = vec.pop().unwrap(); - } - // Do not lint, else block let _result = if vec.last().map(|x| *x > 2).unwrap_or(false) { vec.pop().unwrap() diff --git a/tests/ui/manual_pop_if.stderr b/tests/ui/manual_pop_if.stderr index d57e20dc6205..632027b39f67 100644 --- a/tests/ui/manual_pop_if.stderr +++ b/tests/ui/manual_pop_if.stderr @@ -38,7 +38,7 @@ LL | | } | |_____^ help: try: `deque.pop_front_if(|x| *x > 2);` error: manual implementation of `Vec::pop_if` - --> tests/ui/manual_pop_if.rs:82:5 + --> tests/ui/manual_pop_if.rs:64:5 | LL | / if let Some(x) = vec.last() { LL | | @@ -49,7 +49,7 @@ LL | | } | |_____^ help: try: `vec.pop_if(|x| *x > 2);` error: manual implementation of `Vec::pop_if` - --> tests/ui/manual_pop_if.rs:89:5 + --> tests/ui/manual_pop_if.rs:71:5 | LL | / if let Some(x) = vec.last() { LL | | @@ -60,7 +60,7 @@ LL | | } | |_____^ help: try: `vec.pop_if(|x| *x > 2);` error: manual implementation of `VecDeque::pop_back_if` - --> tests/ui/manual_pop_if.rs:96:5 + --> tests/ui/manual_pop_if.rs:78:5 | LL | / if let Some(x) = deque.back() { LL | | @@ -71,7 +71,7 @@ LL | | } | |_____^ help: try: `deque.pop_back_if(|x| *x > 2);` error: manual implementation of `VecDeque::pop_front_if` - --> tests/ui/manual_pop_if.rs:103:5 + --> tests/ui/manual_pop_if.rs:85:5 | LL | / if let Some(x) = deque.front() { LL | | @@ -82,7 +82,7 @@ LL | | } | |_____^ help: try: `deque.pop_front_if(|x| *x > 2);` error: manual implementation of `Vec::pop_if` - --> tests/ui/manual_pop_if.rs:151:5 + --> tests/ui/manual_pop_if.rs:126:5 | LL | / if let Some(x) = vec.last() LL | | && *x > 2 @@ -92,7 +92,7 @@ LL | | } | |_____^ help: try: `vec.pop_if(|x| *x > 2);` error: manual implementation of `Vec::pop_if` - --> tests/ui/manual_pop_if.rs:157:5 + --> tests/ui/manual_pop_if.rs:132:5 | LL | / if let Some(x) = vec.last() LL | | && *x > 2 @@ -102,7 +102,7 @@ LL | | } | |_____^ help: try: `vec.pop_if(|x| *x > 2);` error: manual implementation of `VecDeque::pop_back_if` - --> tests/ui/manual_pop_if.rs:163:5 + --> tests/ui/manual_pop_if.rs:138:5 | LL | / if let Some(x) = deque.back() LL | | && *x > 2 @@ -112,7 +112,7 @@ LL | | } | |_____^ help: try: `deque.pop_back_if(|x| *x > 2);` error: manual implementation of `VecDeque::pop_front_if` - --> tests/ui/manual_pop_if.rs:169:5 + --> tests/ui/manual_pop_if.rs:144:5 | LL | / if let Some(x) = deque.front() LL | | && *x > 2 @@ -122,7 +122,7 @@ LL | | } | |_____^ help: try: `deque.pop_front_if(|x| *x > 2);` error: manual implementation of `Vec::pop_if` - --> tests/ui/manual_pop_if.rs:217:5 + --> tests/ui/manual_pop_if.rs:178:5 | LL | / if vec.last().map(|x| *x > 2).unwrap_or(false) { LL | | @@ -131,7 +131,7 @@ LL | | } | |_____^ help: try: `vec.pop_if(|x| *x > 2);` error: manual implementation of `Vec::pop_if` - --> tests/ui/manual_pop_if.rs:222:5 + --> tests/ui/manual_pop_if.rs:183:5 | LL | / if vec.last().map(|x| *x > 2).unwrap_or(false) { LL | | @@ -140,7 +140,7 @@ LL | | } | |_____^ help: try: `vec.pop_if(|x| *x > 2);` error: manual implementation of `VecDeque::pop_back_if` - --> tests/ui/manual_pop_if.rs:227:5 + --> tests/ui/manual_pop_if.rs:188:5 | LL | / if deque.back().map(|x| *x > 2).unwrap_or(false) { LL | | @@ -149,7 +149,7 @@ LL | | } | |_____^ help: try: `deque.pop_back_if(|x| *x > 2);` error: manual implementation of `VecDeque::pop_front_if` - --> tests/ui/manual_pop_if.rs:232:5 + --> tests/ui/manual_pop_if.rs:193:5 | LL | / if deque.front().map(|x| *x > 2).unwrap_or(false) { LL | | @@ -158,7 +158,7 @@ LL | | } | |_____^ help: try: `deque.pop_front_if(|x| *x > 2);` error: manual implementation of `Vec::pop_if` - --> tests/ui/manual_pop_if.rs:276:5 + --> tests/ui/manual_pop_if.rs:232:5 | LL | / if vec.last().is_some_and(|e| *e == vec![1]) { LL | | @@ -167,7 +167,7 @@ LL | | } | |_____^ help: try: `vec.pop_if(|e| *e == vec![1]);` error: manual implementation of `Vec::pop_if` - --> tests/ui/manual_pop_if.rs:302:5 + --> tests/ui/manual_pop_if.rs:258:5 | LL | / if vec.last().is_some_and(|x| *x > 2) { LL | | @@ -176,7 +176,7 @@ LL | | } | |_____^ help: try: `vec.pop_if(|x| *x > 2);` error: manual implementation of `VecDeque::pop_back_if` - --> tests/ui/manual_pop_if.rs:310:5 + --> tests/ui/manual_pop_if.rs:266:5 | LL | / if deque.back().is_some_and(|x| *x > 2) { LL | | @@ -185,7 +185,7 @@ LL | | } | |_____^ help: try: `deque.pop_back_if(|x| *x > 2);` error: manual implementation of `VecDeque::pop_front_if` - --> tests/ui/manual_pop_if.rs:315:5 + --> tests/ui/manual_pop_if.rs:271:5 | LL | / if deque.front().is_some_and(|x| *x > 2) { LL | | diff --git a/tests/ui/manual_pop_if_unfixable.rs b/tests/ui/manual_pop_if_unfixable.rs new file mode 100644 index 000000000000..1150a68c2193 --- /dev/null +++ b/tests/ui/manual_pop_if_unfixable.rs @@ -0,0 +1,70 @@ +#![warn(clippy::manual_pop_if)] +#![allow(clippy::collapsible_if, clippy::redundant_closure)] +//@no-rustfix + +fn is_some_and_pattern(mut vec: Vec) { + if false { + // something + } else if vec.last().is_some_and(|x| *x > 2) { + //~^ manual_pop_if + vec.pop().unwrap(); + } + + if vec.last().is_some_and(|x| *x > 2) { + //~^ manual_pop_if + let val = vec.pop().unwrap(); + println!("Popped: {}", val); + } + + if vec.last().is_some_and(|x| *x > 2) { + //~^ manual_pop_if + println!("Popped: {}", vec.pop().unwrap()); + } +} + +fn if_let_pattern(mut vec: Vec) { + if let Some(x) = vec.last() { + //~^ manual_pop_if + if *x > 2 { + let val = vec.pop().unwrap(); + println!("Popped: {}", val); + } + } + + if let Some(x) = vec.last() { + //~^ manual_pop_if + if *x > 2 { + println!("Popped: {}", vec.pop().unwrap()); + } + } +} + +fn let_chain_pattern(mut vec: Vec) { + if let Some(x) = vec.last() + //~^ manual_pop_if + && *x > 2 + { + let val = vec.pop().unwrap(); + println!("Popped: {}", val); + } + + if let Some(x) = vec.last() + //~^ manual_pop_if + && *x > 2 + { + println!("Popped: {}", vec.pop().unwrap()); + } +} + +fn map_unwrap_or_pattern(mut vec: Vec) { + if vec.last().map(|x| *x > 2).unwrap_or(false) { + //~^ manual_pop_if + let val = vec.pop().unwrap(); + println!("Popped: {}", val); + } + + if vec.last().map(|x| *x > 2).unwrap_or(false) { + //~^ manual_pop_if + println!("Popped: {}", vec.pop().unwrap()); + } +} diff --git a/tests/ui/manual_pop_if_unfixable.stderr b/tests/ui/manual_pop_if_unfixable.stderr new file mode 100644 index 000000000000..1333ac22dc8b --- /dev/null +++ b/tests/ui/manual_pop_if_unfixable.stderr @@ -0,0 +1,113 @@ +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if_unfixable.rs:8:12 + | +LL | } else if vec.last().is_some_and(|x| *x > 2) { + | ____________^ +LL | | +LL | | vec.pop().unwrap(); +LL | | } + | |_____^ + | + = help: try using `vec.pop_if(|x| *x > 2);` + = note: `-D clippy::manual-pop-if` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::manual_pop_if)]` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if_unfixable.rs:13:5 + | +LL | / if vec.last().is_some_and(|x| *x > 2) { +LL | | +LL | | let val = vec.pop().unwrap(); +LL | | println!("Popped: {}", val); +LL | | } + | |_____^ + | + = help: try using `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if_unfixable.rs:19:5 + | +LL | / if vec.last().is_some_and(|x| *x > 2) { +LL | | +LL | | println!("Popped: {}", vec.pop().unwrap()); +LL | | } + | |_____^ + | + = help: try using `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if_unfixable.rs:26:5 + | +LL | / if let Some(x) = vec.last() { +LL | | +LL | | if *x > 2 { +LL | | let val = vec.pop().unwrap(); +... | +LL | | } + | |_____^ + | + = help: try using `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if_unfixable.rs:34:5 + | +LL | / if let Some(x) = vec.last() { +LL | | +LL | | if *x > 2 { +LL | | println!("Popped: {}", vec.pop().unwrap()); +LL | | } +LL | | } + | |_____^ + | + = help: try using `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if_unfixable.rs:43:5 + | +LL | / if let Some(x) = vec.last() +LL | | +LL | | && *x > 2 +... | +LL | | println!("Popped: {}", val); +LL | | } + | |_____^ + | + = help: try using `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if_unfixable.rs:51:5 + | +LL | / if let Some(x) = vec.last() +LL | | +LL | | && *x > 2 +... | +LL | | } + | |_____^ + | + = help: try using `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if_unfixable.rs:60:5 + | +LL | / if vec.last().map(|x| *x > 2).unwrap_or(false) { +LL | | +LL | | let val = vec.pop().unwrap(); +LL | | println!("Popped: {}", val); +LL | | } + | |_____^ + | + = help: try using `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if_unfixable.rs:66:5 + | +LL | / if vec.last().map(|x| *x > 2).unwrap_or(false) { +LL | | +LL | | println!("Popped: {}", vec.pop().unwrap()); +LL | | } + | |_____^ + | + = help: try using `vec.pop_if(|x| *x > 2);` + +error: aborting due to 9 previous errors +