From 711c5cb6d2bd88dffff5c4da378d5796095c0cd2 Mon Sep 17 00:00:00 2001 From: dswij Date: Sat, 14 Aug 2021 16:59:08 +0800 Subject: [PATCH 1/2] Add false positive test for `manual_flatten` Add a scenario where `manual_flatten` is triggered when match expression will still be used after the match in `if let`. --- tests/ui/manual_flatten.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/ui/manual_flatten.rs b/tests/ui/manual_flatten.rs index b5bd35a68785a..7db6b730963c9 100644 --- a/tests/ui/manual_flatten.rs +++ b/tests/ui/manual_flatten.rs @@ -91,6 +91,19 @@ fn main() { } } + struct Test { + a: usize, + } + + let mut vec_of_struct = [Some(Test { a: 1 }), None]; + + // Usage of `if let` expression should not trigger lint + for n in vec_of_struct.iter_mut() { + if let Some(z) = n { + *n = None; + } + } + // Using manual flatten should not trigger the lint for n in vec![Some(1), Some(2), Some(3)].iter().flatten() { println!("{}", n); From a09bc1b6fe97839b23122ae7e386f75a1bf9c823 Mon Sep 17 00:00:00 2001 From: dswij Date: Sat, 14 Aug 2021 17:43:28 +0800 Subject: [PATCH 2/2] Check `if let` expr usage in `manual_flatten` `manual_flatten` should not trigger when match expression in `if let` is going to be used. --- clippy_lints/src/loops/manual_flatten.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/loops/manual_flatten.rs b/clippy_lints/src/loops/manual_flatten.rs index 64ff7574f86b7..d8153abc9653b 100644 --- a/clippy_lints/src/loops/manual_flatten.rs +++ b/clippy_lints/src/loops/manual_flatten.rs @@ -1,6 +1,7 @@ use super::utils::make_iterator_snippet; use super::MANUAL_FLATTEN; use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::visitors::LocalUsedVisitor; use clippy_utils::{is_lang_ctor, path_to_local_id}; use if_chain::if_chain; use rustc_errors::Applicability; @@ -37,16 +38,18 @@ pub(super) fn check<'tcx>( if_chain! { if let Some(inner_expr) = inner_expr; if let ExprKind::Match( - match_expr, match_arms, MatchSource::IfLetDesugar{ contains_else_clause: false } + match_expr, [true_arm, _else_arm], MatchSource::IfLetDesugar{ contains_else_clause: false } ) = inner_expr.kind; // Ensure match_expr in `if let` statement is the same as the pat from the for-loop if let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind; if path_to_local_id(match_expr, pat_hir_id); // Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result` - if let PatKind::TupleStruct(ref qpath, _, _) = match_arms[0].pat.kind; + if let PatKind::TupleStruct(ref qpath, _, _) = true_arm.pat.kind; let some_ctor = is_lang_ctor(cx, qpath, OptionSome); let ok_ctor = is_lang_ctor(cx, qpath, ResultOk); if some_ctor || ok_ctor; + // Ensure epxr in `if let` is not used afterwards + if !LocalUsedVisitor::new(cx, pat_hir_id).check_arm(true_arm); then { let if_let_type = if some_ctor { "Some" } else { "Ok" }; // Prepare the error message