From b87e189694eebb5b83d758528032cf4d4db81472 Mon Sep 17 00:00:00 2001
From: nahuakang <kangnahua@gmail.com>
Date: Fri, 29 Jan 2021 01:38:34 +0100
Subject: [PATCH] Implement manual flatten lint
---
CHANGELOG.md | 2 +-
clippy_lints/src/lib.rs | 6 +-
clippy_lints/src/loops.rs | 85 +++++++++++++------
clippy_lints/src/mut_mut.rs | 2 +-
clippy_lints/src/needless_question_mark.rs | 26 +-----
clippy_lints/src/ranges.rs | 2 +-
clippy_lints/src/utils/higher.rs | 9 +-
clippy_lints/src/utils/internal_lints.rs | 16 ++--
clippy_lints/src/utils/mod.rs | 28 +++++-
clippy_lints/src/vec.rs | 2 +-
tests/ui/for_loops_over_options_or_results.rs | 31 -------
tests/ui/manual_flatten.rs | 54 ++++++++++++
tests/ui/manual_flatten.stderr | 51 +++++++++++
13 files changed, 208 insertions(+), 106 deletions(-)
delete mode 100644 tests/ui/for_loops_over_options_or_results.rs
create mode 100644 tests/ui/manual_flatten.rs
create mode 100644 tests/ui/manual_flatten.stderr
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e40cd8174fc..aceabcbbdfc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1969,7 +1969,6 @@ Released 2018-09-13
[`fn_to_numeric_cast_with_truncation`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_with_truncation
[`for_kv_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_kv_map
[`for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles
-[`for_loops_over_options_or_results`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_options_or_results
[`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy
[`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref
[`from_iter_instead_of_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_iter_instead_of_collect
@@ -2040,6 +2039,7 @@ Released 2018-09-13
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
[`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map
[`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
+[`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index dae6c93c7cb..cd0a95a4585 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -685,8 +685,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&loops::EXPLICIT_ITER_LOOP,
&loops::FOR_KV_MAP,
&loops::FOR_LOOPS_OVER_FALLIBLES,
- &loops::FOR_LOOPS_OVER_OPTIONS_OR_RESULTS,
&loops::ITER_NEXT_LOOP,
+ &loops::MANUAL_FLATTEN,
&loops::MANUAL_MEMCPY,
&loops::MUT_RANGE_BOUND,
&loops::NEEDLESS_COLLECT,
@@ -1489,8 +1489,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::EXPLICIT_COUNTER_LOOP),
LintId::of(&loops::FOR_KV_MAP),
LintId::of(&loops::FOR_LOOPS_OVER_FALLIBLES),
- LintId::of(&loops::FOR_LOOPS_OVER_OPTIONS_OR_RESULTS),
LintId::of(&loops::ITER_NEXT_LOOP),
+ LintId::of(&loops::MANUAL_FLATTEN),
LintId::of(&loops::MANUAL_MEMCPY),
LintId::of(&loops::MUT_RANGE_BOUND),
LintId::of(&loops::NEEDLESS_COLLECT),
@@ -1822,7 +1822,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&lifetimes::EXTRA_UNUSED_LIFETIMES),
LintId::of(&lifetimes::NEEDLESS_LIFETIMES),
LintId::of(&loops::EXPLICIT_COUNTER_LOOP),
- LintId::of(&loops::FOR_LOOPS_OVER_OPTIONS_OR_RESULTS),
+ LintId::of(&loops::MANUAL_FLATTEN),
LintId::of(&loops::MUT_RANGE_BOUND),
LintId::of(&loops::SINGLE_ELEMENT_LOOP),
LintId::of(&loops::WHILE_LET_LOOP),
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index e9047cce15f..db5aec82e90 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -5,10 +5,10 @@ use crate::utils::usage::{is_unused, mutated_variables};
use crate::utils::visitors::LocalUsedVisitor;
use crate::utils::{
contains_name, get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
- indent_of, is_in_panic_handler, is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item,
- last_path_segment, match_trait_method, match_type, match_var, multispan_sugg, single_segment_path, snippet,
- snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg,
- span_lint_and_then, sugg, SpanlessEq,
+ indent_of, is_in_panic_handler, is_integer_const, is_no_std_crate, is_ok_ctor, is_refutable, is_some_ctor,
+ is_type_diagnostic_item, last_path_segment, match_trait_method, match_type, match_var, multispan_sugg,
+ single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint,
+ span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg, SpanlessEq,
};
use if_chain::if_chain;
use rustc_ast::ast;
@@ -495,8 +495,8 @@ declare_clippy_lint! {
}
declare_clippy_lint! {
- /// **What it does:** Checks for iteration of `Option`s with
- /// a single `if let Some()` expression inside.
+ /// **What it does:** Check for unnecessary `if let` usage in a for loop
+ /// where only the `Some` or `Ok` variant of the iterator element is used.
///
/// **Why is this bad?** It is verbose and can be simplified
/// by first calling the `flatten` method on the `Iterator`.
@@ -516,23 +516,23 @@ declare_clippy_lint! {
/// Use instead:
/// ```rust
/// let x = vec![Some(1), Some(2), Some(3)];
- /// for n in x.iter().flatten() {
+ /// for n in x.into_iter().flatten() {
/// println!("{}", n);
/// }
/// ```
- pub FOR_LOOPS_OVER_OPTIONS_OR_RESULTS,
+ pub MANUAL_FLATTEN,
complexity,
"for loops over `Option`s or `Result`s with a single expression can be simplified"
}
declare_lint_pass!(Loops => [
MANUAL_MEMCPY,
+ MANUAL_FLATTEN,
NEEDLESS_RANGE_LOOP,
EXPLICIT_ITER_LOOP,
EXPLICIT_INTO_ITER_LOOP,
ITER_NEXT_LOOP,
FOR_LOOPS_OVER_FALLIBLES,
- FOR_LOOPS_OVER_OPTIONS_OR_RESULTS,
WHILE_LET_LOOP,
NEEDLESS_COLLECT,
EXPLICIT_COUNTER_LOOP,
@@ -549,14 +549,14 @@ declare_lint_pass!(Loops => [
impl<'tcx> LateLintPass<'tcx> for Loops {
#[allow(clippy::too_many_lines)]
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
- if let Some((pat, arg, body)) = higher::for_loop(expr) {
+ if let Some((pat, arg, body, span)) = higher::for_loop(expr) {
// we don't want to check expanded macros
// this check is not at the top of the function
// since higher::for_loop expressions are marked as expansions
if body.span.from_expansion() {
return;
}
- check_for_loop(cx, pat, arg, body, expr);
+ check_for_loop(cx, pat, arg, body, expr, span);
}
// we don't want to check expanded macros
@@ -851,6 +851,7 @@ fn check_for_loop<'tcx>(
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
expr: &'tcx Expr<'_>,
+ span: Span,
) {
let is_manual_memcpy_triggered = detect_manual_memcpy(cx, pat, arg, body, expr);
if !is_manual_memcpy_triggered {
@@ -862,7 +863,7 @@ fn check_for_loop<'tcx>(
check_for_mut_range_bound(cx, arg, body);
check_for_single_element_loop(cx, pat, arg, body, expr);
detect_same_item_push(cx, pat, arg, body, expr);
- check_for_loop_over_options_or_results(cx, pat, arg, body, expr);
+ check_manual_flatten(cx, pat, arg, body, span);
}
// this function assumes the given expression is a `for` loop.
@@ -1986,33 +1987,61 @@ fn check_for_single_element_loop<'tcx>(
}
}
-/// Check if a for loop loops over `Option`s or `Result`s and contains only
-/// a `if let Some` or `if let Ok` expression.
-fn check_for_loop_over_options_or_results<'tcx>(
+/// Check for unnecessary `if let` usage in a for loop where only the `Some` or `Ok` variant of the
+/// iterator element is used.
+fn check_manual_flatten<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
- expr: &'tcx Expr<'_>,
+ span: Span,
) {
if_chain! {
+ // Ensure the `if let` statement is the only expression in the for-loop
if let ExprKind::Block(ref block, _) = body.kind;
if block.stmts.is_empty();
if let Some(inner_expr) = block.expr;
- if let ExprKind::Match(ref _match_expr, ref _match_arms, MatchSource::IfLetDesugar{ contains_else_clause }) = inner_expr.kind;
- if !contains_else_clause;
+ if let ExprKind::Match(
+ ref match_expr, ref match_arms, 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 let ExprKind::Path(QPath::Resolved(None, match_expr_path)) = match_expr.kind;
+ if let Res::Local(match_expr_path_id) = match_expr_path.res;
+ if pat_hir_id == match_expr_path_id;
+ // Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
+ if let PatKind::TupleStruct(QPath::Resolved(None, path), _, _) = match_arms[0].pat.kind;
+ if is_some_ctor(cx, path.res) || is_ok_ctor(cx, path.res);
+ let if_let_type = if is_some_ctor(cx, path.res) {
+ "Some"
+ } else {
+ "Ok"
+ };
+ // Determine if `arg` is `Iterator` or implicitly calls `into_iter`
+ let arg_ty = cx.typeck_results().expr_ty(arg);
+ if let Some(id) = get_trait_def_id(cx, &paths::ITERATOR);
+ if let is_iterator = implements_trait(cx, arg_ty, id, &[]);
+
then {
- // println!("if_let_expr:\n{:?}", snippet(cx, if_let_expr.span, ".."));
- // println!("pat is:\n {:?}", snippet(cx, pat.span, ".."));
- // println!("arg is:\n {:?}", snippet(cx, arg.span, ".."));
- // println!("body is:\n {:?}", snippet(cx, body.span, ".."));
- // println!("arg kind is: {:?}", arg.kind);
- // println!("expr is:\n {:?}", snippet(cx, expr.span, ".."));
- // todo!();
+ // Prepare the error message
+ let msg = format!("Unnecessary `if let` since only the `{}` variant of the iterator element is used.", if_let_type);
+
+ // Prepare the help message
let arg_snippet = snippet(cx, arg.span, "..");
- let msg = "looping over `Option`s or `Result`s with an `if let` expression.";
- let hint = format!("try turn {} into an `Iterator` and use `flatten`: `{}.iter().flatten()`", arg_snippet, arg_snippet);
- span_lint_and_help(cx, FOR_LOOPS_OVER_OPTIONS_OR_RESULTS, expr.span, msg, None, &hint);
+ let hint = if is_iterator {
+ format!("try: `{}.flatten()`", arg_snippet)
+ } else {
+ format!("try: `{}.into_iter().flatten()`", arg_snippet)
+ };
+
+ span_lint_and_help(
+ cx,
+ MANUAL_FLATTEN,
+ span,
+ &msg,
+ Some(arg.span),
+ &hint,
+ );
}
}
}
diff --git a/clippy_lints/src/mut_mut.rs b/clippy_lints/src/mut_mut.rs
index 2f3cdb894f0..d7239b328bb 100644
--- a/clippy_lints/src/mut_mut.rs
+++ b/clippy_lints/src/mut_mut.rs
@@ -52,7 +52,7 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for MutVisitor<'a, 'tcx> {
return;
}
- if let Some((_, arg, body)) = higher::for_loop(expr) {
+ if let Some((_, arg, body, _)) = higher::for_loop(expr) {
// A `for` loop lowers to:
// ```rust
// match ::std::iter::Iterator::next(&mut iter) {
diff --git a/clippy_lints/src/needless_question_mark.rs b/clippy_lints/src/needless_question_mark.rs
index 9e9b79ee1cf..fe8d4d07abc 100644
--- a/clippy_lints/src/needless_question_mark.rs
+++ b/clippy_lints/src/needless_question_mark.rs
@@ -1,8 +1,6 @@
use rustc_errors::Applicability;
-use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
use rustc_hir::{Body, Expr, ExprKind, LangItem, MatchSource, QPath};
use rustc_lint::{LateContext, LateLintPass, LintContext};
-use rustc_middle::ty::DefIdTree;
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::sym;
@@ -160,7 +158,7 @@ fn is_some_or_ok_call<'a>(
// Check outer expression matches CALL_IDENT(ARGUMENT) format
if let ExprKind::Call(path, args) = &expr.kind;
if let ExprKind::Path(QPath::Resolved(None, path)) = &path.kind;
- if is_some_ctor(cx, path.res) || is_ok_ctor(cx, path.res);
+ if utils::is_some_ctor(cx, path.res) || utils::is_ok_ctor(cx, path.res);
// Extract inner expression from ARGUMENT
if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar) = &args[0].kind;
@@ -208,25 +206,3 @@ fn is_some_or_ok_call<'a>(
fn has_implicit_error_from(cx: &LateContext<'_>, entire_expr: &Expr<'_>, inner_result_expr: &Expr<'_>) -> bool {
return cx.typeck_results().expr_ty(entire_expr) != cx.typeck_results().expr_ty(inner_result_expr);
}
-
-fn is_ok_ctor(cx: &LateContext<'_>, res: Res) -> bool {
- if let Some(ok_id) = cx.tcx.lang_items().result_ok_variant() {
- if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), id) = res {
- if let Some(variant_id) = cx.tcx.parent(id) {
- return variant_id == ok_id;
- }
- }
- }
- false
-}
-
-fn is_some_ctor(cx: &LateContext<'_>, res: Res) -> bool {
- if let Some(some_id) = cx.tcx.lang_items().option_some_variant() {
- if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), id) = res {
- if let Some(variant_id) = cx.tcx.parent(id) {
- return variant_id == some_id;
- }
- }
- }
- false
-}
diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs
index 3e454eecd97..59503817c0f 100644
--- a/clippy_lints/src/ranges.rs
+++ b/clippy_lints/src/ranges.rs
@@ -442,7 +442,7 @@ fn check_reversed_empty_range(cx: &LateContext<'_>, expr: &Expr<'_>) {
let mut cur_expr = expr;
while let Some(parent_expr) = get_parent_expr(cx, cur_expr) {
match higher::for_loop(parent_expr) {
- Some((_, args, _)) if args.hir_id == expr.hir_id => return true,
+ Some((_, args, _, _)) if args.hir_id == expr.hir_id => return true,
_ => cur_expr = parent_expr,
}
}
diff --git a/clippy_lints/src/utils/higher.rs b/clippy_lints/src/utils/higher.rs
index 340d340d6d3..df7f0f95782 100644
--- a/clippy_lints/src/utils/higher.rs
+++ b/clippy_lints/src/utils/higher.rs
@@ -9,6 +9,7 @@ use rustc_ast::ast;
use rustc_hir as hir;
use rustc_hir::{BorrowKind, Expr, ExprKind, StmtKind, UnOp};
use rustc_lint::LateContext;
+use rustc_span::source_map::Span;
/// Converts a hir binary operator to the corresponding `ast` type.
#[must_use]
@@ -133,11 +134,11 @@ pub fn is_from_for_desugar(local: &hir::Local<'_>) -> bool {
false
}
-/// Recover the essential nodes of a desugared for loop:
-/// `for pat in arg { body }` becomes `(pat, arg, body)`.
+/// Recover the essential nodes of a desugared for loop as well as the entire span:
+/// `for pat in arg { body }` becomes `(pat, arg, body)`. Return `(pat, arg, body, span)`.
pub fn for_loop<'tcx>(
expr: &'tcx hir::Expr<'tcx>,
-) -> Option<(&hir::Pat<'_>, &'tcx hir::Expr<'tcx>, &'tcx hir::Expr<'tcx>)> {
+) -> Option<(&hir::Pat<'_>, &'tcx hir::Expr<'tcx>, &'tcx hir::Expr<'tcx>, Span)> {
if_chain! {
if let hir::ExprKind::Match(ref iterexpr, ref arms, hir::MatchSource::ForLoopDesugar) = expr.kind;
if let hir::ExprKind::Call(_, ref iterargs) = iterexpr.kind;
@@ -148,7 +149,7 @@ pub fn for_loop<'tcx>(
if let hir::StmtKind::Local(ref local) = let_stmt.kind;
if let hir::StmtKind::Expr(ref expr) = body.kind;
then {
- return Some((&*local.pat, &iterargs[0], expr));
+ return Some((&*local.pat, &iterargs[0], expr, arms[0].span));
}
}
None
diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs
index 822863ca3e2..b3eae933062 100644
--- a/clippy_lints/src/utils/internal_lints.rs
+++ b/clippy_lints/src/utils/internal_lints.rs
@@ -841,15 +841,13 @@ pub fn check_path(cx: &LateContext<'_>, path: &[&str]) -> bool {
// implementations of native types. Check lang items.
let path_syms: Vec<_> = path.iter().map(|p| Symbol::intern(p)).collect();
let lang_items = cx.tcx.lang_items();
- for lang_item in lang_items.items() {
- if let Some(def_id) = lang_item {
- let lang_item_path = cx.get_def_path(*def_id);
- if path_syms.starts_with(&lang_item_path) {
- if let [item] = &path_syms[lang_item_path.len()..] {
- for child in cx.tcx.item_children(*def_id) {
- if child.ident.name == *item {
- return true;
- }
+ for item_def_id in lang_items.items().iter().flatten() {
+ let lang_item_path = cx.get_def_path(*item_def_id);
+ if path_syms.starts_with(&lang_item_path) {
+ if let [item] = &path_syms[lang_item_path.len()..] {
+ for child in cx.tcx.item_children(*item_def_id) {
+ if child.ident.name == *item {
+ return true;
}
}
}
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index d0db3a67533..3390c71dd8e 100644
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -37,7 +37,7 @@ use rustc_ast::ast::{self, Attribute, LitKind};
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::Applicability;
use rustc_hir as hir;
-use rustc_hir::def::{DefKind, Res};
+use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::Node;
@@ -50,7 +50,7 @@ use rustc_lint::{LateContext, Level, Lint, LintContext};
use rustc_middle::hir::exports::Export;
use rustc_middle::hir::map::Map;
use rustc_middle::ty::subst::{GenericArg, GenericArgKind};
-use rustc_middle::ty::{self, layout::IntegerExt, Ty, TyCtxt, TypeFoldable};
+use rustc_middle::ty::{self, layout::IntegerExt, DefIdTree, Ty, TyCtxt, TypeFoldable};
use rustc_semver::RustcVersion;
use rustc_session::Session;
use rustc_span::hygiene::{ExpnKind, MacroKind};
@@ -1700,6 +1700,30 @@ pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool {
}
}
+/// Check if the resolution of a given path is an `Ok` variant of `Result`.
+pub fn is_ok_ctor(cx: &LateContext<'_>, res: Res) -> bool {
+ if let Some(ok_id) = cx.tcx.lang_items().result_ok_variant() {
+ if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), id) = res {
+ if let Some(variant_id) = cx.tcx.parent(id) {
+ return variant_id == ok_id;
+ }
+ }
+ }
+ false
+}
+
+/// Check if the resolution of a given path is a `Some` variant of `Option`.
+pub fn is_some_ctor(cx: &LateContext<'_>, res: Res) -> bool {
+ if let Some(some_id) = cx.tcx.lang_items().option_some_variant() {
+ if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), id) = res {
+ if let Some(variant_id) = cx.tcx.parent(id) {
+ return variant_id == some_id;
+ }
+ }
+ }
+ false
+}
+
#[cfg(test)]
mod test {
use super::{reindent_multiline, without_block_comments};
diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs
index 149cceb39dd..c132e4de4f6 100644
--- a/clippy_lints/src/vec.rs
+++ b/clippy_lints/src/vec.rs
@@ -55,7 +55,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec {
// search for `for _ in vec![…]`
if_chain! {
- if let Some((_, arg, _)) = higher::for_loop(expr);
+ if let Some((_, arg, _, _)) = higher::for_loop(expr);
if let Some(vec_args) = higher::vec_macro(cx, arg);
if is_copy(cx, vec_type(cx.typeck_results().expr_ty_adjusted(arg)));
then {
diff --git a/tests/ui/for_loops_over_options_or_results.rs b/tests/ui/for_loops_over_options_or_results.rs
deleted file mode 100644
index 02e24b250f7..00000000000
--- a/tests/ui/for_loops_over_options_or_results.rs
+++ /dev/null
@@ -1,31 +0,0 @@
-#![warn(clippy::for_loops_over_options_or_results)]
-
-fn main() {
- let x = vec![Some(1), Some(2), Some(3)];
- for n in x {
- if let Some(n) = n {
- println!("{}", n);
- }
- }
-
- let y: Vec<Result<i32, i32>> = vec![];
- for n in y.clone() {
- if let Ok(n) = n {
- println!("{}", n);
- }
- }
-
- // This should not trigger the lint
- for n in y.clone() {
- if let Ok(n) = n {
- println!("{}", n);
- } else {
- println!("Oops!");
- }
- }
-
- // This should not trigger the lint
- for n in vec![Some(1), Some(2), Some(3)].iter().flatten() {
- println!("{}", n);
- }
-}
diff --git a/tests/ui/manual_flatten.rs b/tests/ui/manual_flatten.rs
new file mode 100644
index 00000000000..f183ceecdd8
--- /dev/null
+++ b/tests/ui/manual_flatten.rs
@@ -0,0 +1,54 @@
+#![warn(clippy::manual_flatten)]
+
+fn main() {
+ let x = vec![Some(1), Some(2), Some(3)];
+ for n in x {
+ if let Some(n) = n {
+ println!("{}", n);
+ }
+ }
+
+ let y: Vec<Result<i32, i32>> = vec![];
+ for n in y.clone() {
+ if let Ok(n) = n {
+ println!("{}", n);
+ }
+ }
+
+ let z = vec![Some(1), Some(2), Some(3)];
+ let z = z.iter();
+ for n in z {
+ if let Some(n) = n {
+ println!("{}", n);
+ }
+ }
+
+ // Using the `None` variant should not trigger the lint
+ let z = vec![Some(1), Some(2), Some(3)];
+ for n in z {
+ if n.is_none() {
+ println!("Nada.");
+ }
+ }
+
+ // Using the `Err` variant should not trigger the lint
+ for n in y.clone() {
+ if let Err(e) = n {
+ println!("Oops: {}!", e);
+ }
+ }
+
+ // Having an else clause should not trigger the lint
+ for n in y.clone() {
+ if let Ok(n) = n {
+ println!("{}", n);
+ } else {
+ println!("Oops!");
+ }
+ }
+
+ // Using manual flatten should not trigger the lint
+ for n in vec![Some(1), Some(2), Some(3)].iter().flatten() {
+ println!("{}", n);
+ }
+}
diff --git a/tests/ui/manual_flatten.stderr b/tests/ui/manual_flatten.stderr
new file mode 100644
index 00000000000..cf99a2d9ab1
--- /dev/null
+++ b/tests/ui/manual_flatten.stderr
@@ -0,0 +1,51 @@
+error: Unnecessary `if let` since only the `Some` variant of the iterator element is used.
+ --> $DIR/manual_flatten.rs:5:5
+ |
+LL | / for n in x {
+LL | | if let Some(n) = n {
+LL | | println!("{}", n);
+LL | | }
+LL | | }
+ | |_____^
+ |
+ = note: `-D clippy::manual-flatten` implied by `-D warnings`
+help: try: `x.into_iter().flatten()`
+ --> $DIR/manual_flatten.rs:5:14
+ |
+LL | for n in x {
+ | ^
+
+error: Unnecessary `if let` since only the `Ok` variant of the iterator element is used.
+ --> $DIR/manual_flatten.rs:12:5
+ |
+LL | / for n in y.clone() {
+LL | | if let Ok(n) = n {
+LL | | println!("{}", n);
+LL | | }
+LL | | }
+ | |_____^
+ |
+help: try: `y.clone().into_iter().flatten()`
+ --> $DIR/manual_flatten.rs:12:14
+ |
+LL | for n in y.clone() {
+ | ^^^^^^^^^
+
+error: Unnecessary `if let` since only the `Some` variant of the iterator element is used.
+ --> $DIR/manual_flatten.rs:20:5
+ |
+LL | / for n in z {
+LL | | if let Some(n) = n {
+LL | | println!("{}", n);
+LL | | }
+LL | | }
+ | |_____^
+ |
+help: try: `z.flatten()`
+ --> $DIR/manual_flatten.rs:20:14
+ |
+LL | for n in z {
+ | ^
+
+error: aborting due to 3 previous errors
+