mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert "New lint [filter_map_bool_then]"

Browse Source 
This reverts commits 978b1daf99 and 3235d9d612.
This commit is contained in:
Catherine Flores 2023-08-10 22:03:20 +00:00
parent 5f8600939e
commit 1ec0501bca
8 changed files with 0 additions and 205 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -4844,7 +4844,6 @@ Released 2018-09-13
[`field_reassign_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default [`field_reassign_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default
[`filetype_is_file`]: https://rust-lang.github.io/rust-clippy/master/index.html#filetype_is_file [`filetype_is_file`]: https://rust-lang.github.io/rust-clippy/master/index.html#filetype_is_file
[`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map [`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map
[`filter_map_bool_then`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_bool_then
[`filter_map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_identity [`filter_map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_identity
[`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next [`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next
[`filter_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_next [`filter_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_next

1
clippy_lints/src/declared_lints.rs
View File

@ -337,7 +337,6 @@
crate::methods::EXPECT_USED_INFO, crate::methods::EXPECT_USED_INFO,
crate::methods::EXTEND_WITH_DRAIN_INFO, crate::methods::EXTEND_WITH_DRAIN_INFO,
crate::methods::FILETYPE_IS_FILE_INFO, crate::methods::FILETYPE_IS_FILE_INFO,
crate::methods::FILTER_MAP_BOOL_THEN_INFO,
crate::methods::FILTER_MAP_IDENTITY_INFO, crate::methods::FILTER_MAP_IDENTITY_INFO,
crate::methods::FILTER_MAP_NEXT_INFO, crate::methods::FILTER_MAP_NEXT_INFO,
crate::methods::FILTER_NEXT_INFO, crate::methods::FILTER_NEXT_INFO,

45
clippy_lints/src/methods/filter_map_bool_then.rs
View File

@ -1,45 +0,0 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::paths::BOOL_THEN;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::is_copy;
use clippy_utils::{is_from_proc_macro, is_trait_method, match_def_path, peel_blocks};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_span::{sym, Span};
use super::FILTER_MAP_BOOL_THEN;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg: &Expr<'_>, call_span: Span) {
if !in_external_macro(cx.sess(), expr.span)
&& is_trait_method(cx, expr, sym::Iterator)
&& let ExprKind::Closure(closure) = arg.kind
&& let body = cx.tcx.hir().body(closure.body)
&& let value = peel_blocks(body.value)
// Indexing should be fine as `filter_map` always has 1 input, we unfortunately need both
// `inputs` and `params` here as we need both the type and the span
&& let param_ty = closure.fn_decl.inputs[0]
&& let param = body.params[0]
&& is_copy(cx, cx.typeck_results().node_type(param_ty.hir_id).peel_refs())
&& let ExprKind::MethodCall(_, recv, [then_arg], _) = value.kind
&& let ExprKind::Closure(then_closure) = then_arg.kind
&& let then_body = peel_blocks(cx.tcx.hir().body(then_closure.body).value)
&& let Some(def_id) = cx.typeck_results().type_dependent_def_id(value.hir_id)
&& match_def_path(cx, def_id, &BOOL_THEN)
&& !is_from_proc_macro(cx, expr)
&& let Some(param_snippet) = snippet_opt(cx, param.span)
&& let Some(filter) = snippet_opt(cx, recv.span)
&& let Some(map) = snippet_opt(cx, then_body.span)
{
span_lint_and_sugg(
cx,
FILTER_MAP_BOOL_THEN,
call_span,
"usage of `bool::then` in `filter_map`",
"use `filter` then `map` instead",
format!("filter(|&{param_snippet}| {filter}).map(|{param_snippet}| {map})"),
Applicability::MachineApplicable,
);
}
}

34
clippy_lints/src/methods/mod.rs
View File

@ -21,7 +21,6 @@
mod extend_with_drain; mod extend_with_drain;
mod filetype_is_file; mod filetype_is_file;
mod filter_map; mod filter_map;
mod filter_map_bool_then;
mod filter_map_identity; mod filter_map_identity;
mod filter_map_next; mod filter_map_next;
mod filter_next; mod filter_next;
@ -3477,37 +3476,6 @@
"disallows `.skip(0)`" "disallows `.skip(0)`"
} }
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `bool::then` in `Iterator::filter_map`.
///
/// ### Why is this bad?
/// This can be written with `filter` then `map` instead, which would reduce nesting and
/// separates the filtering from the transformation phase. This comes with no cost to
/// performance and is just cleaner.
///
/// ### Limitations
/// Does not lint `bool::then_some`, as it eagerly evaluates its arguments rather than lazily.
/// This can create differing behavior, so better safe than sorry.
///
/// ### Example
/// ```rust
/// # fn really_expensive_fn(i: i32) -> i32 { i }
/// # let v = vec![];
/// _ = v.into_iter().filter_map(|i| (i % 2 == 0).then(|| really_expensive_fn(i)));
/// ```
/// Use instead:
/// ```rust
/// # fn really_expensive_fn(i: i32) -> i32 { i }
/// # let v = vec![];
/// _ = v.into_iter().filter(|i| i % 2 == 0).map(|i| really_expensive_fn(i));
/// ```
#[clippy::version = "1.72.0"]
pub FILTER_MAP_BOOL_THEN,
style,
"checks for usage of `bool::then` in `Iterator::filter_map`"
}
declare_clippy_lint! { declare_clippy_lint! {
/// ### What it does /// ### What it does
/// Looks for calls to `RwLock::write` where the lock is only used for reading. /// Looks for calls to `RwLock::write` where the lock is only used for reading.
@ -3676,7 +3644,6 @@ pub fn new(
FORMAT_COLLECT, FORMAT_COLLECT,
STRING_LIT_CHARS_ANY, STRING_LIT_CHARS_ANY,
ITER_SKIP_ZERO, ITER_SKIP_ZERO,
FILTER_MAP_BOOL_THEN,
READONLY_WRITE_LOCK READONLY_WRITE_LOCK
]); ]);
@ -3955,7 +3922,6 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
}, },
("filter_map", [arg]) => { ("filter_map", [arg]) => {
unnecessary_filter_map::check(cx, expr, arg, name); unnecessary_filter_map::check(cx, expr, arg, name);
filter_map_bool_then::check(cx, expr, arg, call_span);
filter_map_identity::check(cx, expr, arg, span); filter_map_identity::check(cx, expr, arg, span);
}, },
("find_map", [arg]) => { ("find_map", [arg]) => {

2
clippy_utils/src/paths.rs
View File

@ -163,5 +163,3 @@
pub const FORMATTER: [&str; 3] = ["core", "fmt", "Formatter"]; pub const FORMATTER: [&str; 3] = ["core", "fmt", "Formatter"];
pub const DEBUG_STRUCT: [&str; 4] = ["core", "fmt", "builders", "DebugStruct"]; pub const DEBUG_STRUCT: [&str; 4] = ["core", "fmt", "builders", "DebugStruct"];
pub const ORD_CMP: [&str; 4] = ["core", "cmp", "Ord", "cmp"]; pub const ORD_CMP: [&str; 4] = ["core", "cmp", "Ord", "cmp"];
#[expect(clippy::invalid_paths)] // not sure why it thinks this, it works so
pub const BOOL_THEN: [&str; 4] = ["core", "bool", "<impl bool>", "then"];

44
tests/ui/filter_map_bool_then.fixed
View File

@ -1,44 +0,0 @@
//@run-rustfix
//@aux-build:proc_macros.rs:proc-macro
#![allow(
clippy::clone_on_copy,
clippy::map_identity,
clippy::unnecessary_lazy_evaluations,
unused
)]
#![warn(clippy::filter_map_bool_then)]
#[macro_use]
extern crate proc_macros;
#[derive(Clone, PartialEq)]
struct NonCopy;
fn main() {
let v = vec![1, 2, 3, 4, 5, 6];
v.clone().iter().filter(|&i| (i % 2 == 0)).map(|i| i + 1);
v.clone().into_iter().filter(|&i| (i % 2 == 0)).map(|i| i + 1);
v.clone()
.into_iter()
.filter(|&i| (i % 2 == 0)).map(|i| i + 1);
v.clone()
.into_iter()
.filter(|&i| i != 1000)
.filter(|&i| (i % 2 == 0)).map(|i| i + 1);
v.iter()
.copied()
.filter(|&i| i != 1000)
.filter(|&i| (i.clone() % 2 == 0)).map(|i| i + 1);
// Do not lint
let v = vec![NonCopy, NonCopy];
v.clone().iter().filter_map(|i| (i == &NonCopy).then(|| i));
external! {
let v = vec![1, 2, 3, 4, 5, 6];
v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1));
}
with_span! {
span
let v = vec![1, 2, 3, 4, 5, 6];
v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1));
}
}

44
tests/ui/filter_map_bool_then.rs
View File

@ -1,44 +0,0 @@
//@run-rustfix
//@aux-build:proc_macros.rs:proc-macro
#![allow(
clippy::clone_on_copy,
clippy::map_identity,
clippy::unnecessary_lazy_evaluations,
unused
)]
#![warn(clippy::filter_map_bool_then)]
#[macro_use]
extern crate proc_macros;
#[derive(Clone, PartialEq)]
struct NonCopy;
fn main() {
let v = vec![1, 2, 3, 4, 5, 6];
v.clone().iter().filter_map(|i| (i % 2 == 0).then(|| i + 1));
v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1));
v.clone()
.into_iter()
.filter_map(|i| -> Option<_> { (i % 2 == 0).then(|| i + 1) });
v.clone()
.into_iter()
.filter(|&i| i != 1000)
.filter_map(|i| (i % 2 == 0).then(|| i + 1));
v.iter()
.copied()
.filter(|&i| i != 1000)
.filter_map(|i| (i.clone() % 2 == 0).then(|| i + 1));
// Do not lint
let v = vec![NonCopy, NonCopy];
v.clone().iter().filter_map(|i| (i == &NonCopy).then(|| i));
external! {
let v = vec![1, 2, 3, 4, 5, 6];
v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1));
}
with_span! {
span
let v = vec![1, 2, 3, 4, 5, 6];
v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1));
}
}

34
tests/ui/filter_map_bool_then.stderr
View File

@ -1,34 +0,0 @@
error: usage of `bool::then` in `filter_map`
--> $DIR/filter_map_bool_then.rs:19:22
|
LL | v.clone().iter().filter_map(|i| (i % 2 == 0).then(|| i + 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i % 2 == 0)).map(|i| i + 1)`
|
= note: `-D clippy::filter-map-bool-then` implied by `-D warnings`
error: usage of `bool::then` in `filter_map`
--> $DIR/filter_map_bool_then.rs:20:27
|
LL | v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i % 2 == 0)).map(|i| i + 1)`
error: usage of `bool::then` in `filter_map`
--> $DIR/filter_map_bool_then.rs:23:10
|
LL | .filter_map(|i| -> Option<_> { (i % 2 == 0).then(|| i + 1) });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i % 2 == 0)).map(|i| i + 1)`
error: usage of `bool::then` in `filter_map`
--> $DIR/filter_map_bool_then.rs:27:10
|
LL | .filter_map(|i| (i % 2 == 0).then(|| i + 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i % 2 == 0)).map(|i| i + 1)`
error: usage of `bool::then` in `filter_map`
--> $DIR/filter_map_bool_then.rs:31:10
|
LL | .filter_map(|i| (i.clone() % 2 == 0).then(|| i + 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i.clone() % 2 == 0)).map(|i| i + 1)`
error: aborting due to 5 previous errors