Added MSRV and more tests, improved wording

8 files changed, 88 insertions, 22 deletions

diff --git a/clippy_config/src/msrvs.rs b/clippy_config/src/msrvs.rs
index b3ef666e306..8dec477cfdd 100644
--- a/clippy_config/src/msrvs.rs
+++ b/clippy_config/src/msrvs.rs
@@ -41,6 +41,7 @@ msrv_aliases! {
     1,35,0 { OPTION_COPIED, RANGE_CONTAINS }
     1,34,0 { TRY_FROM }
     1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
+    1,29,0 { ITER_FLATTEN }
     1,28,0 { FROM_BOOL }
     1,27,0 { ITERATOR_TRY_FOLD }
     1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN }
diff --git a/clippy_lints/src/methods/iter_filter.rs b/clippy_lints/src/methods/iter_filter.rs
index 04dc4820f6a..f80bd2e2707 100644
--- a/clippy_lints/src/methods/iter_filter.rs
+++ b/clippy_lints/src/methods/iter_filter.rs
@@ -1,10 +1,10 @@
-use rustc_lint::LateContext;
+use rustc_lint::{LateContext, LintContext};
 
 use super::{ITER_FILTER_IS_OK, ITER_FILTER_IS_SOME};
 
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::{indent_of, reindent_multiline};
-use clippy_utils::{is_trait_method, peel_blocks};
+use clippy_utils::{is_trait_method, peel_blocks, span_contains_comment};
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_hir::def::Res;
@@ -54,7 +54,14 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_arg: &hir
     let is_iterator = is_trait_method(cx, expr, sym::Iterator);
     let parent_is_not_map = !parent_is_map(cx, expr);
 
-    if is_iterator && parent_is_not_map && is_method(cx, filter_arg, sym!(is_some)) {
+    if is_iterator
+        && parent_is_not_map
+        && is_method(cx, filter_arg, sym!(is_some))
+        && !span_contains_comment(
+            cx.sess().source_map(), 
+            filter_span.with_hi(expr.span.hi())
+        )
+    {
         span_lint_and_sugg(
             cx,
             ITER_FILTER_IS_SOME,
@@ -62,10 +69,17 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_arg: &hir
             "`filter` for `is_some` on iterator over `Option`",
             "consider using `flatten` instead",
             reindent_multiline(Cow::Borrowed("flatten()"), true, indent_of(cx, filter_span)).into_owned(),
-            Applicability::MaybeIncorrect,
+            Applicability::HasPlaceholders,
         );
     }
-    if is_iterator && parent_is_not_map && is_method(cx, filter_arg, sym!(is_ok)) {
+    if is_iterator
+        && parent_is_not_map
+        && is_method(cx, filter_arg, sym!(is_ok))
+        && !span_contains_comment(
+            cx.sess().source_map(), 
+            filter_span.with_hi(expr.span.hi())
+        )
+    {
         span_lint_and_sugg(
             cx,
             ITER_FILTER_IS_OK,
@@ -73,7 +87,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_arg: &hir
             "`filter` for `is_ok` on iterator over `Result`s",
             "consider using `flatten` instead",
             reindent_multiline(Cow::Borrowed("flatten()"), true, indent_of(cx, filter_span)).into_owned(),
-            Applicability::MaybeIncorrect,
+            Applicability::HasPlaceholders,
         );
     }
 }
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index d67fa28b30b..7a28f10ea87 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -3779,8 +3779,8 @@ declare_clippy_lint! {
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks for usage of `.filter(Optional::is_some)` that may be replaced with a `.flatten()` call.
-    /// This lint is potentially incorrect as it affects the type of follow-up calls.
+    /// Checks for usage of `.filter(Option::is_some)` that may be replaced with a `.flatten()` call.
+    /// This lint will require additional changes to the follow-up calls as it appects the type.
     ///
     /// ### Why is this bad?
     /// This pattern is often followed by manual unwrapping of the `Option`. The simplification
@@ -3799,14 +3799,14 @@ declare_clippy_lint! {
     /// ```
     #[clippy::version = "1.76.0"]
     pub ITER_FILTER_IS_SOME,
-    complexity,
+    pedantic,
     "filtering an iterator over `Option`s for `Some` can be achieved with `flatten`"
 }
 
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for usage of `.filter(Result::is_ok)` that may be replaced with a `.flatten()` call.
-    /// This lint is potentially incorrect as it affects the type of follow-up calls.
+    /// This lint will require additional changes to the follow-up calls as it appects the type.
     ///
     /// ### Why is this bad?
     /// This pattern is often followed by manual unwrapping of `Result`. The simplification
@@ -3825,7 +3825,7 @@ declare_clippy_lint! {
     /// ```
     #[clippy::version = "1.76.0"]
     pub ITER_FILTER_IS_OK,
-    complexity,
+    pedantic,
     "filtering an iterator over `Result`s for `Ok` can be achieved with `flatten`"
 }
 
@@ -4324,8 +4324,11 @@ impl Methods {
                             false,
                         );
                     }
+                    if self.msrv.meets(msrvs::ITER_FLATTEN) {
 
-                    iter_filter::check(cx, expr, arg, span);
+                        // use the sourcemap to get the span of the closure
+                        iter_filter::check(cx, expr, arg, span);
+                    }
                 },
                 ("find", [arg]) => {
                     if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) {
diff --git a/tests/ui/iter_filter_is_ok.fixed b/tests/ui/iter_filter_is_ok.fixed
index 9985877a02a..acfd097675c 100644
--- a/tests/ui/iter_filter_is_ok.fixed
+++ b/tests/ui/iter_filter_is_ok.fixed
@@ -5,4 +5,18 @@ fn main() {
     //~^ HELP: consider using `flatten` instead
     let _ = vec![Ok(1), Err(2), Ok(3)].into_iter().flatten();
     //~^ HELP: consider using `flatten` instead
+
+    // Don't lint below
+    let mut counter = 0;
+    let _ = vec![Ok(1), Err(2)].into_iter().filter(|o| {
+        counter += 1;
+        o.is_ok()
+    });
+    let _ = vec![Ok(1), Err(2)].into_iter().filter(|o| {
+        // Roses are red,
+        // Violets are blue,
+        // `Err` is not an `Option`,
+        // and this doesn't ryme
+        o.is_ok()
+    });
 }
diff --git a/tests/ui/iter_filter_is_ok.rs b/tests/ui/iter_filter_is_ok.rs
index 176ce902637..f13a28ad0e3 100644
--- a/tests/ui/iter_filter_is_ok.rs
+++ b/tests/ui/iter_filter_is_ok.rs
@@ -5,4 +5,18 @@ fn main() {
     //~^ HELP: consider using `flatten` instead
     let _ = vec![Ok(1), Err(2), Ok(3)].into_iter().filter(|a| a.is_ok());
     //~^ HELP: consider using `flatten` instead
+
+    // Don't lint below
+    let mut counter = 0;
+    let _ = vec![Ok(1), Err(2)].into_iter().filter(|o| {
+        counter += 1;
+        o.is_ok()
+    });
+    let _ = vec![Ok(1), Err(2)].into_iter().filter(|o| {
+        // Roses are red,
+        // Violets are blue,
+        // `Err` is not an `Option`,
+        // and this doesn't ryme
+        o.is_ok()
+    });
 }
diff --git a/tests/ui/iter_filter_is_some.fixed b/tests/ui/iter_filter_is_some.fixed
index 3732a8e72d1..f70f54b622e 100644
--- a/tests/ui/iter_filter_is_some.fixed
+++ b/tests/ui/iter_filter_is_some.fixed
@@ -1,12 +1,22 @@
 #![warn(clippy::iter_filter_is_some)]
 
-fn odds_out(x: i32) -> Option<i32> {
-    if x % 2 == 0 { Some(x) } else { None }
-}
-
 fn main() {
     let _ = vec![Some(1)].into_iter().flatten();
     //~^ HELP: consider using `flatten` instead
     let _ = vec![Some(1)].into_iter().flatten();
     //~^ HELP: consider using `flatten` instead
+
+    // Don't lint below
+    let mut counter = 0;
+    let _ = vec![Some(1)].into_iter().filter(|o| {
+        counter += 1;
+        o.is_some()
+    });
+    let _ = vec![Some(1)].into_iter().filter(|o| {
+        // Roses are red,
+        // Violets are blue,
+        // `Err` is not an `Option`,
+        // and this doesn't ryme
+        o.is_some()
+    });
 }
diff --git a/tests/ui/iter_filter_is_some.rs b/tests/ui/iter_filter_is_some.rs
index 5cc457efed5..18d450e8a98 100644
--- a/tests/ui/iter_filter_is_some.rs
+++ b/tests/ui/iter_filter_is_some.rs
@@ -1,12 +1,22 @@
 #![warn(clippy::iter_filter_is_some)]
 
-fn odds_out(x: i32) -> Option<i32> {
-    if x % 2 == 0 { Some(x) } else { None }
-}
-
 fn main() {
     let _ = vec![Some(1)].into_iter().filter(Option::is_some);
     //~^ HELP: consider using `flatten` instead
     let _ = vec![Some(1)].into_iter().filter(|o| o.is_some());
     //~^ HELP: consider using `flatten` instead
+
+    // Don't lint below
+    let mut counter = 0;
+    let _ = vec![Some(1)].into_iter().filter(|o| {
+        counter += 1;
+        o.is_some()
+    });
+    let _ = vec![Some(1)].into_iter().filter(|o| {
+        // Roses are red,
+        // Violets are blue,
+        // `Err` is not an `Option`,
+        // and this doesn't ryme
+        o.is_some()
+    });
 }
diff --git a/tests/ui/iter_filter_is_some.stderr b/tests/ui/iter_filter_is_some.stderr
index 2b1c1cdf7ef..a9ef1d547a9 100644
--- a/tests/ui/iter_filter_is_some.stderr
+++ b/tests/ui/iter_filter_is_some.stderr
@@ -1,5 +1,5 @@
 error: `filter` for `is_some` on iterator over `Option`
-  --> $DIR/iter_filter_is_some.rs:8:39
+  --> $DIR/iter_filter_is_some.rs:4:39
    |
 LL |     let _ = vec![Some(1)].into_iter().filter(Option::is_some);
    |                                       ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `flatten()`
@@ -8,7 +8,7 @@ LL |     let _ = vec![Some(1)].into_iter().filter(Option::is_some);
    = help: to override `-D warnings` add `#[allow(clippy::iter_filter_is_some)]`
 
 error: `filter` for `is_some` on iterator over `Option`
-  --> $DIR/iter_filter_is_some.rs:10:39
+  --> $DIR/iter_filter_is_some.rs:6:39
    |
 LL |     let _ = vec![Some(1)].into_iter().filter(|o| o.is_some());
    |                                       ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `flatten()`