Added MSRV and more tests, improved wording

This commit is contained in:
Quinn Sinclair 2023-12-24 14:40:14 +02:00 committed by PartiallyTyped
parent 25b9ca3f64
commit 4b1ac31c18
8 changed files with 88 additions and 22 deletions

View File

@ -41,6 +41,7 @@ macro_rules! 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 }

View File

@ -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,
);
}
}

View File

@ -3779,8 +3779,8 @@
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 @@
/// ```
#[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 @@
/// ```
#[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 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
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) {

View File

@ -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()
});
}

View File

@ -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()
});
}

View File

@ -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()
});
}

View File

@ -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()
});
}

View File

@ -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()`