add manual_pop_if lint

Add a lint to detect when the recently added Vec::pop_if,
VecDeque::pop_front_if, and VecDeque::pop_back_if are manually
implemented.

changelog: add [`manual_pop_if`] lint
This commit is contained in:
Paolo Borelli
2026-02-13 14:55:18 +01:00
parent e645f93552
commit 25171f23ae
9 changed files with 1211 additions and 0 deletions
+1
View File
@@ -6705,6 +6705,7 @@ Released 2018-09-13
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
[`manual_option_as_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_option_as_slice
[`manual_pattern_char_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison
[`manual_pop_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_pop_if
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
[`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns
[`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid
+1
View File
@@ -312,6 +312,7 @@
crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,
crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,
crate::manual_option_as_slice::MANUAL_OPTION_AS_SLICE_INFO,
crate::manual_pop_if::MANUAL_POP_IF_INFO,
crate::manual_range_patterns::MANUAL_RANGE_PATTERNS_INFO,
crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO,
crate::manual_retain::MANUAL_RETAIN_INFO,
+2
View File
@@ -211,6 +211,7 @@
mod manual_main_separator_str;
mod manual_non_exhaustive;
mod manual_option_as_slice;
mod manual_pop_if;
mod manual_range_patterns;
mod manual_rem_euclid;
mod manual_retain;
@@ -864,6 +865,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
Box::new(move |tcx| Box::new(duration_suboptimal_units::DurationSuboptimalUnits::new(tcx, conf))),
Box::new(move |_| Box::new(manual_take::ManualTake::new(conf))),
Box::new(|_| Box::new(manual_checked_ops::ManualCheckedOps)),
Box::new(move |_| Box::new(manual_pop_if::ManualPopIf::new(conf))),
// add late passes here, used by `cargo dev new_lint`
];
store.late_passes.extend(late_lints);
+434
View File
@@ -0,0 +1,434 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_sugg;
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 rustc_ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, LangItem, PatKind, RustcVersion, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::impl_lint_pass;
use rustc_span::{Span, Symbol};
use std::fmt;
declare_clippy_lint! {
/// ### What it does
/// Checks for code to be replaced by `pop_if` methods.
///
/// ### Why is this bad?
/// 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.
///
/// ### Examples
/// ```no_run
/// # use std::collections::VecDeque;
/// # let mut vec = vec![1, 2, 3, 4, 5];
/// # let mut deque: VecDeque<i32> = VecDeque::from([1, 2, 3, 4, 5]);
/// if vec.last().is_some_and(|x| *x > 5) {
/// vec.pop().unwrap();
/// }
/// if deque.back().is_some_and(|x| *x > 5) {
/// deque.pop_back().unwrap();
/// }
/// if deque.front().is_some_and(|x| *x > 5) {
/// deque.pop_front().unwrap();
/// }
/// ```
/// Use instead:
/// ```no_run
/// # use std::collections::VecDeque;
/// # let mut vec = vec![1, 2, 3, 4, 5];
/// # let mut deque: VecDeque<i32> = VecDeque::from([1, 2, 3, 4, 5]);
/// vec.pop_if(|x| *x > 5);
/// deque.pop_back_if(|x| *x > 5);
/// deque.pop_front_if(|x| *x > 5);
/// ```
#[clippy::version = "1.95.0"]
pub MANUAL_POP_IF,
complexity,
"manual implementation of `pop_if` methods"
}
impl_lint_pass!(ManualPopIf => [MANUAL_POP_IF]);
pub struct ManualPopIf {
msrv: Msrv,
}
impl ManualPopIf {
pub fn new(conf: &'static Conf) -> Self {
Self { msrv: conf.msrv }
}
}
#[allow(clippy::enum_variant_names)]
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum ManualPopIfKind {
Vec,
VecDequeBack,
VecDequeFront,
}
impl ManualPopIfKind {
fn check_method(self) -> Symbol {
match self {
ManualPopIfKind::Vec => sym::last,
ManualPopIfKind::VecDequeBack => sym::back,
ManualPopIfKind::VecDequeFront => sym::front,
}
}
fn pop_method(self) -> Symbol {
match self {
ManualPopIfKind::Vec => sym::pop,
ManualPopIfKind::VecDequeBack => sym::pop_back,
ManualPopIfKind::VecDequeFront => sym::pop_front,
}
}
fn pop_if_method(self) -> Symbol {
match self {
ManualPopIfKind::Vec => sym::pop_if,
ManualPopIfKind::VecDequeBack => sym::pop_back_if,
ManualPopIfKind::VecDequeFront => sym::pop_front_if,
}
}
fn is_diag_item(self, cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
let ty = cx.typeck_results().expr_ty(expr).peel_refs();
match self {
ManualPopIfKind::Vec => ty.is_diag_item(cx, sym::Vec),
ManualPopIfKind::VecDequeBack | ManualPopIfKind::VecDequeFront => ty.is_diag_item(cx, sym::VecDeque),
}
}
fn msrv(self) -> RustcVersion {
match self {
ManualPopIfKind::Vec => msrvs::VEC_POP_IF,
ManualPopIfKind::VecDequeBack => msrvs::VEC_DEQUE_POP_BACK_IF,
ManualPopIfKind::VecDequeFront => msrvs::VEC_DEQUE_POP_FRONT_IF,
}
}
}
impl fmt::Display for ManualPopIfKind {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ManualPopIfKind::Vec => write!(f, "`Vec::pop_if`"),
ManualPopIfKind::VecDequeBack => write!(f, "`VecDeque::pop_back_if`"),
ManualPopIfKind::VecDequeFront => write!(f, "`VecDeque::pop_front_if`"),
}
}
}
struct ManualPopIfPattern<'tcx> {
kind: ManualPopIfKind,
/// The collection (`vec` in `vec.last()`)
collection_expr: &'tcx Expr<'tcx>,
/// The closure (`*x > 5` in `|x| *x > 5`)
predicate: &'tcx Expr<'tcx>,
/// Parameter name for the closure (`x` in `|x| *x > 5`)
param_name: Symbol,
/// Span of the if expression (including the `if` keyword)
if_span: Span,
}
impl ManualPopIfPattern<'_> {
fn emit_lint(&self, cx: &LateContext<'_>) {
let mut app = Applicability::MachineApplicable;
let ctxt = self.if_span.ctxt();
let collection_snippet = snippet_with_context(cx, self.collection_expr.span, ctxt, "..", &mut app).0;
let predicate_snippet = snippet_with_context(cx, self.predicate.span, ctxt, "..", &mut app).0;
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(
cx,
MANUAL_POP_IF,
self.if_span,
format!("manual implementation of {}", self.kind),
"try",
suggestion,
app,
);
}
}
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) {
/// vec.pop().unwrap();
/// }
/// ```
fn check_is_some_and_pattern<'tcx>(
cx: &LateContext<'tcx>,
cond: &'tcx Expr<'_>,
then_block: &'tcx Expr<'_>,
if_expr_span: Span,
kind: ManualPopIfKind,
) -> Option<ManualPopIfPattern<'tcx>> {
if pop_value_is_used(then_block) {
return None;
}
let check_method = kind.check_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
&& 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)
&& eq_expr_value(cx, collection_expr, pop_collection)
&& let Some(param) = body.params.first()
&& let Some(ident) = param.pat.simple_ident()
{
return Some(ManualPopIfPattern {
kind,
collection_expr,
predicate: body.value,
param_name: ident.name,
if_span: if_expr_span,
});
}
None
}
/// Checks for the pattern:
/// ```ignore
/// if let Some(x) = vec.last() {
/// if *x > 5 {
/// vec.pop().unwrap();
/// }
/// }
/// ```
fn check_if_let_pattern<'tcx>(
cx: &LateContext<'tcx>,
cond: &'tcx Expr<'_>,
then_block: &'tcx Expr<'_>,
if_expr_span: Span,
kind: ManualPopIfKind,
) -> Option<ManualPopIfPattern<'tcx>> {
let check_method = kind.check_method();
let pop_method = kind.pop_method();
if let ExprKind::Let(let_expr) = cond.kind
&& let PatKind::TupleStruct(qpath, [binding_pat], _) = let_expr.pat.kind
{
let res = cx.qpath_res(&qpath, let_expr.pat.hir_id);
if let Some(def_id) = res.opt_def_id()
&& 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
&& kind.is_diag_item(cx, collection_expr)
&& let ExprKind::Block(block, _) = then_block.kind
{
// The inner if can be either a statement or a block expression
let inner_if = match (block.stmts, block.expr) {
([stmt], _) => match stmt.kind {
StmtKind::Expr(expr) | StmtKind::Semi(expr) => expr,
_ => return None,
},
([], Some(expr)) => expr,
_ => return None,
};
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)
&& eq_expr_value(cx, collection_expr, pop_collection)
{
return Some(ManualPopIfPattern {
kind,
collection_expr,
predicate: inner_cond,
param_name: binding_name.name,
if_span: if_expr_span,
});
}
}
}
None
}
/// Checks for the pattern:
/// ```ignore
/// if let Some(x) = vec.last() && *x > 5 {
/// vec.pop().unwrap();
/// }
/// ```
fn check_let_chain_pattern<'tcx>(
cx: &LateContext<'tcx>,
cond: &'tcx Expr<'_>,
then_block: &'tcx Expr<'_>,
if_expr_span: Span,
kind: ManualPopIfKind,
) -> Option<ManualPopIfPattern<'tcx>> {
if pop_value_is_used(then_block) {
return None;
}
let check_method = kind.check_method();
let pop_method = kind.pop_method();
if let ExprKind::Binary(op, left, right) = cond.kind
&& op.node == rustc_ast::BinOpKind::And
&& let ExprKind::Let(let_expr) = left.kind
&& let PatKind::TupleStruct(qpath, [binding_pat], _) = let_expr.pat.kind
{
let res = cx.qpath_res(&qpath, let_expr.pat.hir_id);
if let Some(def_id) = res.opt_def_id()
&& 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
&& 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)
&& eq_expr_value(cx, collection_expr, pop_collection)
{
return Some(ManualPopIfPattern {
kind,
collection_expr,
predicate: right,
param_name: binding_name.name,
if_span: if_expr_span,
});
}
}
None
}
/// Checks for the pattern:
/// ```ignore
/// if vec.last().map(|x| *x > 5).unwrap_or(false) {
/// vec.pop().unwrap();
/// }
/// ```
fn check_map_unwrap_or_pattern<'tcx>(
cx: &LateContext<'tcx>,
cond: &'tcx Expr<'_>,
then_block: &'tcx Expr<'_>,
if_expr_span: Span,
kind: ManualPopIfKind,
) -> Option<ManualPopIfPattern<'tcx>> {
if pop_value_is_used(then_block) {
return None;
}
let check_method = kind.check_method();
let pop_method = kind.pop_method();
if let ExprKind::MethodCall(unwrap_path, receiver, [default_arg], _) = cond.kind
&& unwrap_path.ident.name == sym::unwrap_or
&& matches!(default_arg.kind, ExprKind::Lit(lit) if matches!(lit.node, LitKind::Bool(false)))
&& 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
&& 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)
&& eq_expr_value(cx, collection_expr, pop_collection)
&& let Some(param) = body.params.first()
&& let Some(ident) = param.pat.simple_ident()
{
return Some(ManualPopIfPattern {
kind,
collection_expr,
predicate: body.value,
param_name: ident.name,
if_span: if_expr_span,
});
}
None
}
/// Checks for `collection.<pop_method>().unwrap()` or `collection.<pop_method>().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);
if let ExprKind::MethodCall(unwrap_path, receiver, _, _) = inner_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
{
return Some((collection_expr, inner_expr.span));
}
None
}
impl<'tcx> LateLintPass<'tcx> for ManualPopIf {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
let ExprKind::If(cond, then_block, None) = expr.kind else {
return;
};
// Do not lint if we are in an else-if branch.
if is_else_clause(cx.tcx, expr) {
return;
}
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)
.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())
{
pattern.emit_lint(cx);
return;
}
}
}
}
+2
View File
@@ -23,9 +23,11 @@ macro_rules! msrv_aliases {
// names may refer to stabilized feature flags or library items
msrv_aliases! {
1,93,0 { VEC_DEQUE_POP_BACK_IF, VEC_DEQUE_POP_FRONT_IF }
1,91,0 { DURATION_FROM_MINUTES_HOURS }
1,88,0 { LET_CHAINS }
1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT, CONST_CHAR_IS_DIGIT, UNSIGNED_IS_MULTIPLE_OF, INTEGER_SIGN_CAST }
1,86,0 { VEC_POP_IF }
1,85,0 { UINT_FLOAT_MIDPOINT, CONST_SIZE_OF_VAL }
1,84,0 { CONST_OPTION_AS_SLICE, MANUAL_DANGLING_PTR }
1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY, CONST_MUT_REFS, CONST_UNWRAP }
+8
View File
@@ -140,6 +140,7 @@ macro_rules! generate {
as_str,
assert_failed,
author,
back,
binaryheap_iter,
bool_then,
borrow,
@@ -295,6 +296,7 @@ macro_rules! generate {
from_str_method,
from_str_radix,
from_weeks,
front,
fs,
fs_create_dir,
fuse,
@@ -464,6 +466,12 @@ macro_rules! generate {
peekable,
permissions_from_mode,
pin_macro,
pop,
pop_back,
pop_back_if,
pop_front,
pop_front_if,
pop_if,
position,
pow,
powf,
+247
View File
@@ -0,0 +1,247 @@
#![warn(clippy::manual_pop_if)]
#![allow(clippy::collapsible_if, clippy::redundant_closure)]
use std::collections::VecDeque;
use std::marker::PhantomData;
// FakeVec has the same methods as Vec but isn't actually a Vec
struct FakeVec<T>(PhantomData<T>);
impl<T> FakeVec<T> {
fn last(&self) -> Option<&T> {
None
}
fn pop(&mut self) -> Option<T> {
None
}
}
fn is_some_and_pattern_positive(mut vec: Vec<i32>, mut deque: VecDeque<i32>) {
vec.pop_if(|x| *x > 2);
vec.pop_if(|x| *x > 2);
deque.pop_back_if(|x| *x > 2);
deque.pop_front_if(|x| *x > 2);
}
fn is_some_and_pattern_negative(mut vec: Vec<i32>, mut deque: VecDeque<i32>) {
// Do not lint, different vectors
let mut vec2 = vec![0];
if vec.last().is_some_and(|x| *x > 2) {
vec2.pop().unwrap();
}
// Do not lint, non-Vec type
let mut fake_vec: FakeVec<i32> = FakeVec(PhantomData);
if fake_vec.last().is_some_and(|x| *x > 2) {
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()
} else {
0
};
}
fn if_let_pattern_positive(mut vec: Vec<i32>, mut deque: VecDeque<i32>) {
vec.pop_if(|x| *x > 2);
vec.pop_if(|x| *x > 2);
deque.pop_back_if(|x| *x > 2);
deque.pop_front_if(|x| *x > 2);
}
fn if_let_pattern_negative(mut vec: Vec<i32>) {
// Do not lint, different vectors
let mut vec2 = vec![0];
if let Some(x) = vec.last() {
if *x > 2 {
vec2.pop().unwrap();
}
}
// Do not lint, intervening statements
if let Some(x) = vec.last() {
println!("Checking {}", x);
if *x > 2 {
vec.pop().unwrap();
}
}
// Do not lint, bound variable not used in condition
if let Some(_x) = vec.last() {
if vec.len() > 2 {
vec.pop().unwrap();
}
}
// 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 }
} else {
0
};
}
fn let_chain_pattern_positive(mut vec: Vec<i32>, mut deque: VecDeque<i32>) {
vec.pop_if(|x| *x > 2);
vec.pop_if(|x| *x > 2);
deque.pop_back_if(|x| *x > 2);
deque.pop_front_if(|x| *x > 2);
}
fn let_chain_pattern_negative(mut vec: Vec<i32>) {
// Do not lint, different vectors
let mut vec2 = vec![0];
if let Some(x) = vec.last()
&& *x > 2
{
vec2.pop().unwrap();
}
// Do not lint, bound variable not used in condition
if let Some(_x) = vec.last()
&& vec.len() > 2
{
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
{
vec.pop().unwrap()
} else {
0
};
}
fn map_unwrap_or_pattern_positive(mut vec: Vec<i32>, mut deque: VecDeque<i32>) {
vec.pop_if(|x| *x > 2);
vec.pop_if(|x| *x > 2);
deque.pop_back_if(|x| *x > 2);
deque.pop_front_if(|x| *x > 2);
}
fn map_unwrap_or_pattern_negative(mut vec: Vec<i32>) {
// Do not lint, unwrap_or(true) instead of false
if vec.last().map(|x| *x > 2).unwrap_or(true) {
vec.pop().unwrap();
}
// Do not lint, different vectors
let mut vec2 = vec![0];
if vec.last().map(|x| *x > 2).unwrap_or(false) {
vec2.pop().unwrap();
}
// Do not lint, non-Vec type
let mut fake_vec: FakeVec<i32> = FakeVec(PhantomData);
if fake_vec.last().map(|x| *x > 2).unwrap_or(false) {
fake_vec.pop().unwrap();
}
// Do not lint, map returns non-boolean
if vec.last().map(|x| x + 1).unwrap_or(0) > 2 {
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()
} else {
0
};
}
// this makes sure we do not expand vec![] in the suggestion
fn handle_macro_in_closure(mut vec: Vec<Vec<i32>>) {
vec.pop_if(|e| *e == vec![1]);
}
#[clippy::msrv = "1.85.0"]
fn msrv_too_low_vec(mut vec: Vec<i32>) {
if vec.last().is_some_and(|x| *x > 2) {
vec.pop().unwrap();
}
}
#[clippy::msrv = "1.92.0"]
fn msrv_too_low_vecdeque(mut deque: VecDeque<i32>) {
if deque.back().is_some_and(|x| *x > 2) {
deque.pop_back().unwrap();
}
if deque.front().is_some_and(|x| *x > 2) {
deque.pop_front().unwrap();
}
}
#[clippy::msrv = "1.86.0"]
fn msrv_high_enough_vec(mut vec: Vec<i32>) {
vec.pop_if(|x| *x > 2);
}
#[clippy::msrv = "1.93.0"]
fn msrv_high_enough_vecdeque(mut deque: VecDeque<i32>) {
deque.pop_back_if(|x| *x > 2);
deque.pop_front_if(|x| *x > 2);
}
+319
View File
@@ -0,0 +1,319 @@
#![warn(clippy::manual_pop_if)]
#![allow(clippy::collapsible_if, clippy::redundant_closure)]
use std::collections::VecDeque;
use std::marker::PhantomData;
// FakeVec has the same methods as Vec but isn't actually a Vec
struct FakeVec<T>(PhantomData<T>);
impl<T> FakeVec<T> {
fn last(&self) -> Option<&T> {
None
}
fn pop(&mut self) -> Option<T> {
None
}
}
fn is_some_and_pattern_positive(mut vec: Vec<i32>, mut deque: VecDeque<i32>) {
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
vec.pop().expect("element");
}
if deque.back().is_some_and(|x| *x > 2) {
//~^ manual_pop_if
deque.pop_back().unwrap();
}
if deque.front().is_some_and(|x| *x > 2) {
//~^ manual_pop_if
deque.pop_front().unwrap();
}
}
fn is_some_and_pattern_negative(mut vec: Vec<i32>, mut deque: VecDeque<i32>) {
// Do not lint, different vectors
let mut vec2 = vec![0];
if vec.last().is_some_and(|x| *x > 2) {
vec2.pop().unwrap();
}
// Do not lint, non-Vec type
let mut fake_vec: FakeVec<i32> = FakeVec(PhantomData);
if fake_vec.last().is_some_and(|x| *x > 2) {
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()
} else {
0
};
}
fn if_let_pattern_positive(mut vec: Vec<i32>, mut deque: VecDeque<i32>) {
if let Some(x) = vec.last() {
//~^ manual_pop_if
if *x > 2 {
vec.pop().unwrap();
}
}
if let Some(x) = vec.last() {
//~^ manual_pop_if
if *x > 2 {
vec.pop().expect("element");
}
}
if let Some(x) = deque.back() {
//~^ manual_pop_if
if *x > 2 {
deque.pop_back().unwrap();
}
}
if let Some(x) = deque.front() {
//~^ manual_pop_if
if *x > 2 {
deque.pop_front().unwrap();
}
}
}
fn if_let_pattern_negative(mut vec: Vec<i32>) {
// Do not lint, different vectors
let mut vec2 = vec![0];
if let Some(x) = vec.last() {
if *x > 2 {
vec2.pop().unwrap();
}
}
// Do not lint, intervening statements
if let Some(x) = vec.last() {
println!("Checking {}", x);
if *x > 2 {
vec.pop().unwrap();
}
}
// Do not lint, bound variable not used in condition
if let Some(_x) = vec.last() {
if vec.len() > 2 {
vec.pop().unwrap();
}
}
// 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 }
} else {
0
};
}
fn let_chain_pattern_positive(mut vec: Vec<i32>, mut deque: VecDeque<i32>) {
if let Some(x) = vec.last() //~ manual_pop_if
&& *x > 2
{
vec.pop().unwrap();
}
if let Some(x) = vec.last() //~ manual_pop_if
&& *x > 2
{
vec.pop().expect("element");
}
if let Some(x) = deque.back() //~ manual_pop_if
&& *x > 2
{
deque.pop_back().unwrap();
}
if let Some(x) = deque.front() //~ manual_pop_if
&& *x > 2
{
deque.pop_front().unwrap();
}
}
fn let_chain_pattern_negative(mut vec: Vec<i32>) {
// Do not lint, different vectors
let mut vec2 = vec![0];
if let Some(x) = vec.last()
&& *x > 2
{
vec2.pop().unwrap();
}
// Do not lint, bound variable not used in condition
if let Some(_x) = vec.last()
&& vec.len() > 2
{
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
{
vec.pop().unwrap()
} else {
0
};
}
fn map_unwrap_or_pattern_positive(mut vec: Vec<i32>, mut deque: VecDeque<i32>) {
if vec.last().map(|x| *x > 2).unwrap_or(false) {
//~^ manual_pop_if
vec.pop().unwrap();
}
if vec.last().map(|x| *x > 2).unwrap_or(false) {
//~^ manual_pop_if
vec.pop().expect("element");
}
if deque.back().map(|x| *x > 2).unwrap_or(false) {
//~^ manual_pop_if
deque.pop_back().unwrap();
}
if deque.front().map(|x| *x > 2).unwrap_or(false) {
//~^ manual_pop_if
deque.pop_front().unwrap();
}
}
fn map_unwrap_or_pattern_negative(mut vec: Vec<i32>) {
// Do not lint, unwrap_or(true) instead of false
if vec.last().map(|x| *x > 2).unwrap_or(true) {
vec.pop().unwrap();
}
// Do not lint, different vectors
let mut vec2 = vec![0];
if vec.last().map(|x| *x > 2).unwrap_or(false) {
vec2.pop().unwrap();
}
// Do not lint, non-Vec type
let mut fake_vec: FakeVec<i32> = FakeVec(PhantomData);
if fake_vec.last().map(|x| *x > 2).unwrap_or(false) {
fake_vec.pop().unwrap();
}
// Do not lint, map returns non-boolean
if vec.last().map(|x| x + 1).unwrap_or(0) > 2 {
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()
} else {
0
};
}
// this makes sure we do not expand vec![] in the suggestion
fn handle_macro_in_closure(mut vec: Vec<Vec<i32>>) {
if vec.last().is_some_and(|e| *e == vec![1]) {
//~^ manual_pop_if
vec.pop().unwrap();
}
}
#[clippy::msrv = "1.85.0"]
fn msrv_too_low_vec(mut vec: Vec<i32>) {
if vec.last().is_some_and(|x| *x > 2) {
vec.pop().unwrap();
}
}
#[clippy::msrv = "1.92.0"]
fn msrv_too_low_vecdeque(mut deque: VecDeque<i32>) {
if deque.back().is_some_and(|x| *x > 2) {
deque.pop_back().unwrap();
}
if deque.front().is_some_and(|x| *x > 2) {
deque.pop_front().unwrap();
}
}
#[clippy::msrv = "1.86.0"]
fn msrv_high_enough_vec(mut vec: Vec<i32>) {
if vec.last().is_some_and(|x| *x > 2) {
//~^ manual_pop_if
vec.pop().unwrap();
}
}
#[clippy::msrv = "1.93.0"]
fn msrv_high_enough_vecdeque(mut deque: VecDeque<i32>) {
if deque.back().is_some_and(|x| *x > 2) {
//~^ manual_pop_if
deque.pop_back().unwrap();
}
if deque.front().is_some_and(|x| *x > 2) {
//~^ manual_pop_if
deque.pop_front().unwrap();
}
}
+197
View File
@@ -0,0 +1,197 @@
error: manual implementation of `Vec::pop_if`
--> tests/ui/manual_pop_if.rs:21:5
|
LL | / if vec.last().is_some_and(|x| *x > 2) {
LL | |
LL | | vec.pop().unwrap();
LL | | }
| |_____^ help: try: `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.rs:26:5
|
LL | / if vec.last().is_some_and(|x| *x > 2) {
LL | |
LL | | vec.pop().expect("element");
LL | | }
| |_____^ help: try: `vec.pop_if(|x| *x > 2);`
error: manual implementation of `VecDeque::pop_back_if`
--> tests/ui/manual_pop_if.rs:31:5
|
LL | / if deque.back().is_some_and(|x| *x > 2) {
LL | |
LL | | deque.pop_back().unwrap();
LL | | }
| |_____^ help: try: `deque.pop_back_if(|x| *x > 2);`
error: manual implementation of `VecDeque::pop_front_if`
--> tests/ui/manual_pop_if.rs:36:5
|
LL | / if deque.front().is_some_and(|x| *x > 2) {
LL | |
LL | | deque.pop_front().unwrap();
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
|
LL | / if let Some(x) = vec.last() {
LL | |
LL | | if *x > 2 {
LL | | vec.pop().unwrap();
LL | | }
LL | | }
| |_____^ help: try: `vec.pop_if(|x| *x > 2);`
error: manual implementation of `Vec::pop_if`
--> tests/ui/manual_pop_if.rs:89:5
|
LL | / if let Some(x) = vec.last() {
LL | |
LL | | if *x > 2 {
LL | | vec.pop().expect("element");
LL | | }
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
|
LL | / if let Some(x) = deque.back() {
LL | |
LL | | if *x > 2 {
LL | | deque.pop_back().unwrap();
LL | | }
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
|
LL | / if let Some(x) = deque.front() {
LL | |
LL | | if *x > 2 {
LL | | deque.pop_front().unwrap();
LL | | }
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
|
LL | / if let Some(x) = vec.last()
LL | | && *x > 2
LL | | {
LL | | vec.pop().unwrap();
LL | | }
| |_____^ help: try: `vec.pop_if(|x| *x > 2);`
error: manual implementation of `Vec::pop_if`
--> tests/ui/manual_pop_if.rs:157:5
|
LL | / if let Some(x) = vec.last()
LL | | && *x > 2
LL | | {
LL | | vec.pop().expect("element");
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
|
LL | / if let Some(x) = deque.back()
LL | | && *x > 2
LL | | {
LL | | deque.pop_back().unwrap();
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
|
LL | / if let Some(x) = deque.front()
LL | | && *x > 2
LL | | {
LL | | deque.pop_front().unwrap();
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
|
LL | / if vec.last().map(|x| *x > 2).unwrap_or(false) {
LL | |
LL | | vec.pop().unwrap();
LL | | }
| |_____^ help: try: `vec.pop_if(|x| *x > 2);`
error: manual implementation of `Vec::pop_if`
--> tests/ui/manual_pop_if.rs:222:5
|
LL | / if vec.last().map(|x| *x > 2).unwrap_or(false) {
LL | |
LL | | vec.pop().expect("element");
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
|
LL | / if deque.back().map(|x| *x > 2).unwrap_or(false) {
LL | |
LL | | deque.pop_back().unwrap();
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
|
LL | / if deque.front().map(|x| *x > 2).unwrap_or(false) {
LL | |
LL | | deque.pop_front().unwrap();
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
|
LL | / if vec.last().is_some_and(|e| *e == vec![1]) {
LL | |
LL | | vec.pop().unwrap();
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
|
LL | / if vec.last().is_some_and(|x| *x > 2) {
LL | |
LL | | vec.pop().unwrap();
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
|
LL | / if deque.back().is_some_and(|x| *x > 2) {
LL | |
LL | | deque.pop_back().unwrap();
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
|
LL | / if deque.front().is_some_and(|x| *x > 2) {
LL | |
LL | | deque.pop_front().unwrap();
LL | | }
| |_____^ help: try: `deque.pop_front_if(|x| *x > 2);`
error: aborting due to 20 previous errors