From 5f2b8e67b3ab5f700e9076c3a0bba8bc2ed4a7f1 Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Tue, 7 Jun 2022 12:11:33 +0900 Subject: [PATCH 01/10] feat(new lint): new lint `use_retain` --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_style.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/use_retain.rs | 233 +++++++++++++++++++++++++ clippy_utils/src/paths.rs | 9 + tests/ui/use_retain.fixed | 184 +++++++++++++++++++ tests/ui/use_retain.rs | 190 ++++++++++++++++++++ tests/ui/use_retain.stderr | 100 +++++++++++ 10 files changed, 722 insertions(+) create mode 100644 clippy_lints/src/use_retain.rs create mode 100644 tests/ui/use_retain.fixed create mode 100644 tests/ui/use_retain.rs create mode 100644 tests/ui/use_retain.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index dcc96bc10b8a..56143e1cdb9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3840,6 +3840,7 @@ Released 2018-09-13 [`unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used [`upper_case_acronyms`]: https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms [`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug +[`use_retain`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_retain [`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self [`used_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding [`useless_asref`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_asref diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index a1565255b0b5..88e4c291b68f 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -336,6 +336,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(unwrap::PANICKING_UNWRAP), LintId::of(unwrap::UNNECESSARY_UNWRAP), LintId::of(upper_case_acronyms::UPPER_CASE_ACRONYMS), + LintId::of(use_retain::USE_RETAIN), LintId::of(useless_conversion::USELESS_CONVERSION), LintId::of(vec::USELESS_VEC), LintId::of(vec_init_then_push::VEC_INIT_THEN_PUSH), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index f706ba0620fd..187786e80dae 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -563,6 +563,7 @@ store.register_lints(&[ unwrap::UNNECESSARY_UNWRAP, unwrap_in_result::UNWRAP_IN_RESULT, upper_case_acronyms::UPPER_CASE_ACRONYMS, + use_retain::USE_RETAIN, use_self::USE_SELF, useless_conversion::USELESS_CONVERSION, vec::USELESS_VEC, diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs index d52ec50e5422..3b3505f1e194 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -117,6 +117,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(unused_unit::UNUSED_UNIT), LintId::of(upper_case_acronyms::UPPER_CASE_ACRONYMS), + LintId::of(use_retain::USE_RETAIN), LintId::of(write::PRINTLN_EMPTY_STRING), LintId::of(write::PRINT_LITERAL), LintId::of(write::PRINT_WITH_NEWLINE), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 70cf6be8b7cf..124f0d12b08c 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -410,6 +410,7 @@ mod unused_unit; mod unwrap; mod unwrap_in_result; mod upper_case_acronyms; +mod use_retain; mod use_self; mod useless_conversion; mod vec; @@ -914,6 +915,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(read_zero_byte_vec::ReadZeroByteVec)); store.register_late_pass(|| Box::new(default_instead_of_iter_empty::DefaultIterEmpty)); store.register_late_pass(move || Box::new(manual_rem_euclid::ManualRemEuclid::new(msrv))); + store.register_late_pass(|| Box::new(use_retain::UseRetain)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/use_retain.rs b/clippy_lints/src/use_retain.rs new file mode 100644 index 000000000000..ffe2786ee091 --- /dev/null +++ b/clippy_lints/src/use_retain.rs @@ -0,0 +1,233 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet; +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{get_parent_expr, match_def_path, paths, SpanlessEq}; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::def_id::DefId; +use rustc_hir::ExprKind::Assign; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::sym; + +const ACCEPTABLE_METHODS: [&[&str]; 4] = [ + &paths::HASHSET_ITER, + &paths::BTREESET_ITER, + &paths::SLICE_INTO, + &paths::VEC_DEQUE_ITER, +]; +const ACCEPTABLE_TYPES: [rustc_span::Symbol; 6] = [ + sym::BTreeSet, + sym::BTreeMap, + sym::HashSet, + sym::HashMap, + sym::Vec, + sym::VecDeque, +]; + +declare_clippy_lint! { + /// ### What it does + /// Checks for code to be replaced by `.retain()`. + /// ### Why is this bad? + /// `.retain()` is simpler. + /// ### Example + /// ```rust + /// let mut vec = vec![0, 1, 2]; + /// vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect(); + /// vec = vec.into_iter().filter(|x| x % 2 == 0).collect(); + /// ``` + /// Use instead: + /// ```rust + /// let mut vec = vec![0, 1, 2]; + /// vec.retain(|x| x % 2 == 0); + /// ``` + #[clippy::version = "1.63.0"] + pub USE_RETAIN, + style, + "`retain()` is simpler and the same functionalitys" +} +declare_lint_pass!(UseRetain => [USE_RETAIN]); + +impl<'tcx> LateLintPass<'tcx> for UseRetain { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { + if_chain! { + if let Some(parent_expr) = get_parent_expr(cx, expr); + if let Assign(left_expr, collect_expr, _) = &parent_expr.kind; + if let hir::ExprKind::MethodCall(seg, _, _) = &collect_expr.kind; + if seg.args.is_none(); + + if let hir::ExprKind::MethodCall(_, [target_expr], _) = &collect_expr.kind; + if let Some(collect_def_id) = cx.typeck_results().type_dependent_def_id(collect_expr.hir_id); + if match_def_path(cx, collect_def_id, &paths::CORE_ITER_COLLECT); + + then { + check_into_iter(cx, parent_expr, left_expr, target_expr); + check_iter(cx, parent_expr, left_expr, target_expr); + check_to_owned(cx, parent_expr, left_expr, target_expr); + } + } + } +} + +fn check_into_iter( + cx: &LateContext<'_>, + parent_expr: &hir::Expr<'_>, + left_expr: &hir::Expr<'_>, + target_expr: &hir::Expr<'_>, +) { + if_chain! { + if let hir::ExprKind::MethodCall(_, [into_iter_expr, _], _) = &target_expr.kind; + if let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id); + if match_def_path(cx, filter_def_id, &paths::CORE_ITER_FILTER); + + if let hir::ExprKind::MethodCall(_, [struct_expr], _) = &into_iter_expr.kind; + if let Some(into_iter_def_id) = cx.typeck_results().type_dependent_def_id(into_iter_expr.hir_id); + if match_def_path(cx, into_iter_def_id, &paths::CORE_ITER_INTO_ITER); + if match_acceptable_type(cx, left_expr); + + if SpanlessEq::new(cx).eq_expr(left_expr, struct_expr); + + then { + suggest(cx, parent_expr, left_expr, target_expr); + } + } +} + +fn check_iter( + cx: &LateContext<'_>, + parent_expr: &hir::Expr<'_>, + left_expr: &hir::Expr<'_>, + target_expr: &hir::Expr<'_>, +) { + if_chain! { + if let hir::ExprKind::MethodCall(_, [filter_expr], _) = &target_expr.kind; + if let Some(copied_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id); + if match_def_path(cx, copied_def_id, &paths::CORE_ITER_COPIED); + + if let hir::ExprKind::MethodCall(_, [iter_expr, _], _) = &filter_expr.kind; + if let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(filter_expr.hir_id); + if match_def_path(cx, filter_def_id, &paths::CORE_ITER_FILTER); + + if let hir::ExprKind::MethodCall(_, [struct_expr], _) = &iter_expr.kind; + if let Some(iter_expr_def_id) = cx.typeck_results().type_dependent_def_id(iter_expr.hir_id); + if match_acceptable_def_path(cx, iter_expr_def_id); + if match_acceptable_type(cx, left_expr); + if SpanlessEq::new(cx).eq_expr(left_expr, struct_expr); + + then { + suggest(cx, parent_expr, left_expr, filter_expr); + } + } +} + +fn check_to_owned( + cx: &LateContext<'_>, + parent_expr: &hir::Expr<'_>, + left_expr: &hir::Expr<'_>, + target_expr: &hir::Expr<'_>, +) { + if_chain! { + if let hir::ExprKind::MethodCall(_, [filter_expr], _) = &target_expr.kind; + if let Some(to_owned_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id); + if match_def_path(cx, to_owned_def_id, &paths::TO_OWNED_METHOD); + + if let hir::ExprKind::MethodCall(_, [chars_expr, _], _) = &filter_expr.kind; + if let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(filter_expr.hir_id); + if match_def_path(cx, filter_def_id, &paths::CORE_ITER_FILTER); + + if let hir::ExprKind::MethodCall(_, [str_expr], _) = &chars_expr.kind; + if let Some(chars_expr_def_id) = cx.typeck_results().type_dependent_def_id(chars_expr.hir_id); + if match_def_path(cx, chars_expr_def_id, &paths::STR_CHARS); + + let ty = cx.typeck_results().expr_ty(str_expr).peel_refs(); + if is_type_diagnostic_item(cx, ty, sym::String); + if SpanlessEq::new(cx).eq_expr(left_expr, str_expr); + + then { + suggest(cx, parent_expr, left_expr, filter_expr); + } + } +} + +fn suggest(cx: &LateContext<'_>, parent_expr: &hir::Expr<'_>, left_expr: &hir::Expr<'_>, filter_expr: &hir::Expr<'_>) { + if_chain! { + if let hir::ExprKind::MethodCall(_, [_, closure], _) = filter_expr.kind; + if let hir::ExprKind::Closure(_, _, filter_body_id, ..) = closure.kind; + let filter_body = cx.tcx.hir().body(filter_body_id); + if let [filter_params] = filter_body.params; + if let Some(sugg) = match filter_params.pat.kind { + hir::PatKind::Binding(_, _, filter_param_ident, None) => { + Some(format!("{}.retain(|{}| {})", snippet(cx, left_expr.span, ".."), filter_param_ident, snippet(cx, filter_body.value.span, ".."))) + }, + hir::PatKind::Tuple([key_pat, value_pat], _) => { + make_sugg(cx, key_pat, value_pat, left_expr, filter_body) + }, + hir::PatKind::Ref(pat, _) => { + match pat.kind { + hir::PatKind::Binding(_, _, filter_param_ident, None) => { + Some(format!("{}.retain(|{}| {})", snippet(cx, left_expr.span, ".."), filter_param_ident, snippet(cx, filter_body.value.span, ".."))) + }, + _ => None + } + }, + _ => None + }; + then { + span_lint_and_sugg( + cx, + USE_RETAIN, + parent_expr.span, + "this expression can be written more simply using `.retain()`", + "consider calling `.retain()` instead", + sugg, + Applicability::MachineApplicable + ); + } + } +} + +fn make_sugg( + cx: &LateContext<'_>, + key_pat: &rustc_hir::Pat<'_>, + value_pat: &rustc_hir::Pat<'_>, + left_expr: &hir::Expr<'_>, + filter_body: &hir::Body<'_>, +) -> Option { + match (&key_pat.kind, &value_pat.kind) { + (hir::PatKind::Binding(_, _, key_param_ident, None), hir::PatKind::Binding(_, _, value_param_ident, None)) => { + Some(format!( + "{}.retain(|{}, &mut {}| {})", + snippet(cx, left_expr.span, ".."), + key_param_ident, + value_param_ident, + snippet(cx, filter_body.value.span, "..") + )) + }, + (hir::PatKind::Binding(_, _, key_param_ident, None), hir::PatKind::Wild) => Some(format!( + "{}.retain(|{}, _| {})", + snippet(cx, left_expr.span, ".."), + key_param_ident, + snippet(cx, filter_body.value.span, "..") + )), + (hir::PatKind::Wild, hir::PatKind::Binding(_, _, value_param_ident, None)) => Some(format!( + "{}.retain(|_, &mut {}| {})", + snippet(cx, left_expr.span, ".."), + value_param_ident, + snippet(cx, filter_body.value.span, "..") + )), + _ => None, + } +} + +fn match_acceptable_def_path(cx: &LateContext<'_>, collect_def_id: DefId) -> bool { + return ACCEPTABLE_METHODS + .iter() + .any(|&method| match_def_path(cx, collect_def_id, method)); +} + +fn match_acceptable_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { + let expr_ty = cx.typeck_results().expr_ty(expr).peel_refs(); + return ACCEPTABLE_TYPES + .iter() + .any(|&ty| is_type_diagnostic_item(cx, expr_ty, ty)); +} diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index b31e2c1cdb1b..17cccd43a9c3 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -21,8 +21,13 @@ pub const ASREF_TRAIT: [&str; 3] = ["core", "convert", "AsRef"]; pub const BTREEMAP_CONTAINS_KEY: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "contains_key"]; pub const BTREEMAP_ENTRY: [&str; 6] = ["alloc", "collections", "btree", "map", "entry", "Entry"]; pub const BTREEMAP_INSERT: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "insert"]; +pub const BTREESET_ITER: [&str; 6] = ["alloc", "collections", "btree", "set", "BTreeSet", "iter"]; pub const CLONE_TRAIT_METHOD: [&str; 4] = ["core", "clone", "Clone", "clone"]; pub const COW: [&str; 3] = ["alloc", "borrow", "Cow"]; +pub const CORE_ITER_COLLECT: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "collect"]; +pub const CORE_ITER_COPIED: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "copied"]; +pub const CORE_ITER_FILTER: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "filter"]; +pub const CORE_ITER_INTO_ITER: [&str; 6] = ["core", "iter", "traits", "collect", "IntoIterator", "into_iter"]; pub const CSTRING_AS_C_STR: [&str; 5] = ["alloc", "ffi", "c_str", "CString", "as_c_str"]; pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"]; pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"]; @@ -50,6 +55,7 @@ pub const FUTURES_IO_ASYNCWRITEEXT: [&str; 3] = ["futures_util", "io", "AsyncWri pub const HASHMAP_CONTAINS_KEY: [&str; 6] = ["std", "collections", "hash", "map", "HashMap", "contains_key"]; pub const HASHMAP_ENTRY: [&str; 5] = ["std", "collections", "hash", "map", "Entry"]; pub const HASHMAP_INSERT: [&str; 6] = ["std", "collections", "hash", "map", "HashMap", "insert"]; +pub const HASHSET_ITER: [&str; 6] = ["std", "collections", "hash", "set", "HashSet", "iter"]; #[cfg(feature = "internal")] pub const IDENT: [&str; 3] = ["rustc_span", "symbol", "Ident"]; #[cfg(feature = "internal")] @@ -145,6 +151,7 @@ pub const SLICE_FROM_RAW_PARTS: [&str; 4] = ["core", "slice", "raw", "from_raw_p pub const SLICE_FROM_RAW_PARTS_MUT: [&str; 4] = ["core", "slice", "raw", "from_raw_parts_mut"]; pub const SLICE_GET: [&str; 4] = ["core", "slice", "", "get"]; pub const SLICE_INTO_VEC: [&str; 4] = ["alloc", "slice", "", "into_vec"]; +pub const SLICE_INTO: [&str; 4] = ["core", "slice", "", "iter"]; pub const SLICE_ITER: [&str; 4] = ["core", "slice", "iter", "Iter"]; pub const STDERR: [&str; 4] = ["std", "io", "stdio", "stderr"]; pub const STDOUT: [&str; 4] = ["std", "io", "stdio", "stdout"]; @@ -154,6 +161,7 @@ pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_s pub const STRING_AS_STR: [&str; 4] = ["alloc", "string", "String", "as_str"]; pub const STRING_NEW: [&str; 4] = ["alloc", "string", "String", "new"]; pub const STR_BYTES: [&str; 4] = ["core", "str", "", "bytes"]; +pub const STR_CHARS: [&str; 4] = ["core", "str", "", "chars"]; pub const STR_ENDS_WITH: [&str; 4] = ["core", "str", "", "ends_with"]; pub const STR_FROM_UTF8: [&str; 4] = ["core", "str", "converts", "from_utf8"]; pub const STR_LEN: [&str; 4] = ["core", "str", "", "len"]; @@ -179,6 +187,7 @@ pub const TOKIO_IO_ASYNCWRITEEXT: [&str; 5] = ["tokio", "io", "util", "async_wri pub const TRY_FROM: [&str; 4] = ["core", "convert", "TryFrom", "try_from"]; pub const VEC_AS_MUT_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_mut_slice"]; pub const VEC_AS_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_slice"]; +pub const VEC_DEQUE_ITER: [&str; 5] = ["alloc", "collections", "vec_deque", "VecDeque", "iter"]; pub const VEC_FROM_ELEM: [&str; 3] = ["alloc", "vec", "from_elem"]; pub const VEC_NEW: [&str; 4] = ["alloc", "vec", "Vec", "new"]; pub const VEC_RESIZE: [&str; 4] = ["alloc", "vec", "Vec", "resize"]; diff --git a/tests/ui/use_retain.fixed b/tests/ui/use_retain.fixed new file mode 100644 index 000000000000..d24571b36f87 --- /dev/null +++ b/tests/ui/use_retain.fixed @@ -0,0 +1,184 @@ +// run-rustfix +#![warn(clippy::use_retain)] +#![allow(unused)] +use std::collections::BTreeMap; +use std::collections::BTreeSet; +use std::collections::BinaryHeap; +use std::collections::HashMap; +use std::collections::HashSet; +use std::collections::VecDeque; + +fn main() { + binary_heap_retain(); + btree_set_retain(); + btree_map_retain(); + hash_set_retain(); + hash_map_retain(); + string_retain(); + vec_queue_retain(); + vec_retain(); +} + +fn binary_heap_retain() { + // NOTE: Do not lint now, because binary_heap_retain is nighyly API. + // https://github.com/rust-lang/rust/issues/71503 + let mut heap = BinaryHeap::from([1, 2, 3]); + heap = heap.into_iter().filter(|x| x % 2 == 0).collect(); + heap = heap.iter().filter(|&x| x % 2 == 0).copied().collect(); + + // Do not lint, because type conversion is performed + heap = heap.into_iter().filter(|x| x % 2 == 0).collect::>(); + heap = heap.iter().filter(|&x| x % 2 == 0).copied().collect::>(); + + // Do not lint, because this expression is not assign. + let mut bar: BinaryHeap = heap.iter().filter(|&x| x % 2 == 0).copied().collect(); + let mut foobar: BinaryHeap = heap.into_iter().filter(|x| x % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); +} + +fn btree_map_retain() { + let mut btree_map: BTreeMap = (0..8).map(|x| (x, x * 10)).collect(); + // Do lint. + btree_map.retain(|k, _| k % 2 == 0); + btree_map.retain(|_, &mut v| v % 2 == 0); + btree_map.retain(|k, &mut v| (k % 2 == 0) && (v % 2 == 0)); + + // Do not lint. + btree_map = btree_map + .into_iter() + .filter(|(x, _)| x % 2 == 0) + .collect::>(); + + // Do not lint, because this expression is not assign. + let mut foobar: BTreeMap = btree_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + btree_map = foobar.into_iter().filter(|(k, _)| k % 2 == 0).collect(); +} + +fn btree_set_retain() { + let mut btree_set = BTreeSet::from([1, 2, 3, 4, 5, 6]); + + // Do lint. + btree_set.retain(|x| x % 2 == 0); + btree_set.retain(|x| x % 2 == 0); + + // Do not lint, because type conversion is performed + btree_set = btree_set + .iter() + .filter(|&x| x % 2 == 0) + .copied() + .collect::>(); + + btree_set = btree_set.into_iter().filter(|x| x % 2 == 0).collect::>(); + + // Do not lint, because this expression is not assign. + let mut foobar: BTreeSet = btree_set.iter().filter(|&x| x % 2 == 0).copied().collect(); + let mut bar: BTreeSet = btree_set.into_iter().filter(|x| x % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); +} + +fn hash_map_retain() { + let mut hash_map: HashMap = (0..8).map(|x| (x, x * 10)).collect(); + // Do lint. + hash_map.retain(|k, _| k % 2 == 0); + hash_map.retain(|_, &mut v| v % 2 == 0); + hash_map.retain(|k, &mut v| (k % 2 == 0) && (v % 2 == 0)); + + // Do not lint. + hash_map = hash_map + .into_iter() + .filter(|(x, _)| x % 2 == 0) + .collect::>(); + + // Do not lint, because this expression is not assign. + let mut foobar: HashMap = hash_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + hash_map = foobar.into_iter().filter(|(k, _)| k % 2 == 0).collect(); +} + +fn hash_set_retain() { + let mut hash_set = HashSet::from([1, 2, 3, 4, 5, 6]); + // Do lint. + hash_set.retain(|x| x % 2 == 0); + hash_set.retain(|x| x % 2 == 0); + + // Do not lint, because type conversion is performed + hash_set = hash_set.into_iter().filter(|x| x % 2 == 0).collect::>(); + hash_set = hash_set + .iter() + .filter(|&x| x % 2 == 0) + .copied() + .collect::>(); + + // Do not lint, because this expression is not assign. + let mut bar: HashSet = hash_set.iter().filter(|&x| x % 2 == 0).copied().collect(); + let mut foobar: HashSet = hash_set.into_iter().filter(|x| x % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.into_iter().filter(|&x| x % 2 == 0).collect(); +} + +fn string_retain() { + let mut s = String::from("foobar"); + // Do lint. + s.retain(|c| c != 'o'); + + // Do not lint, because this expression is not assign. + let mut bar: String = s.chars().filter(|&c| c != 'o').to_owned().collect(); + + // Do not lint, because it is an assignment to a different variable. + s = bar.chars().filter(|&c| c != 'o').to_owned().collect(); +} + +fn vec_retain() { + let mut vec = vec![0, 1, 2]; + // Do lint. + vec.retain(|x| x % 2 == 0); + vec.retain(|x| x % 2 == 0); + + // Do not lint, because type conversion is performed + vec = vec.into_iter().filter(|x| x % 2 == 0).collect::>(); + vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect::>(); + + // Do not lint, because this expression is not assign. + let mut bar: Vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect(); + let mut foobar: Vec = vec.into_iter().filter(|x| x % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); +} + +fn vec_queue_retain() { + let mut vec_deque = VecDeque::new(); + vec_deque.extend(1..5); + + // Do lint. + vec_deque.retain(|x| x % 2 == 0); + vec_deque.retain(|x| x % 2 == 0); + + // Do not lint, because type conversion is performed + vec_deque = vec_deque + .iter() + .filter(|&x| x % 2 == 0) + .copied() + .collect::>(); + vec_deque = vec_deque.into_iter().filter(|x| x % 2 == 0).collect::>(); + + // Do not lint, because this expression is not assign. + let mut bar: VecDeque = vec_deque.iter().filter(|&x| x % 2 == 0).copied().collect(); + let mut foobar: VecDeque = vec_deque.into_iter().filter(|x| x % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); +} diff --git a/tests/ui/use_retain.rs b/tests/ui/use_retain.rs new file mode 100644 index 000000000000..4a51825c95f7 --- /dev/null +++ b/tests/ui/use_retain.rs @@ -0,0 +1,190 @@ +// run-rustfix +#![warn(clippy::use_retain)] +#![allow(unused)] +use std::collections::BTreeMap; +use std::collections::BTreeSet; +use std::collections::BinaryHeap; +use std::collections::HashMap; +use std::collections::HashSet; +use std::collections::VecDeque; + +fn main() { + binary_heap_retain(); + btree_set_retain(); + btree_map_retain(); + hash_set_retain(); + hash_map_retain(); + string_retain(); + vec_queue_retain(); + vec_retain(); +} + +fn binary_heap_retain() { + // NOTE: Do not lint now, because binary_heap_retain is nighyly API. + // https://github.com/rust-lang/rust/issues/71503 + let mut heap = BinaryHeap::from([1, 2, 3]); + heap = heap.into_iter().filter(|x| x % 2 == 0).collect(); + heap = heap.iter().filter(|&x| x % 2 == 0).copied().collect(); + + // Do not lint, because type conversion is performed + heap = heap.into_iter().filter(|x| x % 2 == 0).collect::>(); + heap = heap.iter().filter(|&x| x % 2 == 0).copied().collect::>(); + + // Do not lint, because this expression is not assign. + let mut bar: BinaryHeap = heap.iter().filter(|&x| x % 2 == 0).copied().collect(); + let mut foobar: BinaryHeap = heap.into_iter().filter(|x| x % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); +} + +fn btree_map_retain() { + let mut btree_map: BTreeMap = (0..8).map(|x| (x, x * 10)).collect(); + // Do lint. + btree_map = btree_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); + btree_map = btree_map.into_iter().filter(|(_, v)| v % 2 == 0).collect(); + btree_map = btree_map + .into_iter() + .filter(|(k, v)| (k % 2 == 0) && (v % 2 == 0)) + .collect(); + + // Do not lint. + btree_map = btree_map + .into_iter() + .filter(|(x, _)| x % 2 == 0) + .collect::>(); + + // Do not lint, because this expression is not assign. + let mut foobar: BTreeMap = btree_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + btree_map = foobar.into_iter().filter(|(k, _)| k % 2 == 0).collect(); +} + +fn btree_set_retain() { + let mut btree_set = BTreeSet::from([1, 2, 3, 4, 5, 6]); + + // Do lint. + btree_set = btree_set.iter().filter(|&x| x % 2 == 0).copied().collect(); + btree_set = btree_set.into_iter().filter(|x| x % 2 == 0).collect(); + + // Do not lint, because type conversion is performed + btree_set = btree_set + .iter() + .filter(|&x| x % 2 == 0) + .copied() + .collect::>(); + + btree_set = btree_set.into_iter().filter(|x| x % 2 == 0).collect::>(); + + // Do not lint, because this expression is not assign. + let mut foobar: BTreeSet = btree_set.iter().filter(|&x| x % 2 == 0).copied().collect(); + let mut bar: BTreeSet = btree_set.into_iter().filter(|x| x % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); +} + +fn hash_map_retain() { + let mut hash_map: HashMap = (0..8).map(|x| (x, x * 10)).collect(); + // Do lint. + hash_map = hash_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); + hash_map = hash_map.into_iter().filter(|(_, v)| v % 2 == 0).collect(); + hash_map = hash_map + .into_iter() + .filter(|(k, v)| (k % 2 == 0) && (v % 2 == 0)) + .collect(); + + // Do not lint. + hash_map = hash_map + .into_iter() + .filter(|(x, _)| x % 2 == 0) + .collect::>(); + + // Do not lint, because this expression is not assign. + let mut foobar: HashMap = hash_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + hash_map = foobar.into_iter().filter(|(k, _)| k % 2 == 0).collect(); +} + +fn hash_set_retain() { + let mut hash_set = HashSet::from([1, 2, 3, 4, 5, 6]); + // Do lint. + hash_set = hash_set.into_iter().filter(|x| x % 2 == 0).collect(); + hash_set = hash_set.iter().filter(|&x| x % 2 == 0).copied().collect(); + + // Do not lint, because type conversion is performed + hash_set = hash_set.into_iter().filter(|x| x % 2 == 0).collect::>(); + hash_set = hash_set + .iter() + .filter(|&x| x % 2 == 0) + .copied() + .collect::>(); + + // Do not lint, because this expression is not assign. + let mut bar: HashSet = hash_set.iter().filter(|&x| x % 2 == 0).copied().collect(); + let mut foobar: HashSet = hash_set.into_iter().filter(|x| x % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.into_iter().filter(|&x| x % 2 == 0).collect(); +} + +fn string_retain() { + let mut s = String::from("foobar"); + // Do lint. + s = s.chars().filter(|&c| c != 'o').to_owned().collect(); + + // Do not lint, because this expression is not assign. + let mut bar: String = s.chars().filter(|&c| c != 'o').to_owned().collect(); + + // Do not lint, because it is an assignment to a different variable. + s = bar.chars().filter(|&c| c != 'o').to_owned().collect(); +} + +fn vec_retain() { + let mut vec = vec![0, 1, 2]; + // Do lint. + vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect(); + vec = vec.into_iter().filter(|x| x % 2 == 0).collect(); + + // Do not lint, because type conversion is performed + vec = vec.into_iter().filter(|x| x % 2 == 0).collect::>(); + vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect::>(); + + // Do not lint, because this expression is not assign. + let mut bar: Vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect(); + let mut foobar: Vec = vec.into_iter().filter(|x| x % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); +} + +fn vec_queue_retain() { + let mut vec_deque = VecDeque::new(); + vec_deque.extend(1..5); + + // Do lint. + vec_deque = vec_deque.iter().filter(|&x| x % 2 == 0).copied().collect(); + vec_deque = vec_deque.into_iter().filter(|x| x % 2 == 0).collect(); + + // Do not lint, because type conversion is performed + vec_deque = vec_deque + .iter() + .filter(|&x| x % 2 == 0) + .copied() + .collect::>(); + vec_deque = vec_deque.into_iter().filter(|x| x % 2 == 0).collect::>(); + + // Do not lint, because this expression is not assign. + let mut bar: VecDeque = vec_deque.iter().filter(|&x| x % 2 == 0).copied().collect(); + let mut foobar: VecDeque = vec_deque.into_iter().filter(|x| x % 2 == 0).collect(); + + // Do not lint, because it is an assignment to a different variable. + bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); +} diff --git a/tests/ui/use_retain.stderr b/tests/ui/use_retain.stderr new file mode 100644 index 000000000000..331259c1626b --- /dev/null +++ b/tests/ui/use_retain.stderr @@ -0,0 +1,100 @@ +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:45:5 + | +LL | btree_map = btree_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_map.retain(|k, _| k % 2 == 0)` + | + = note: `-D clippy::use-retain` implied by `-D warnings` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:46:5 + | +LL | btree_map = btree_map.into_iter().filter(|(_, v)| v % 2 == 0).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_map.retain(|_, &mut v| v % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:47:5 + | +LL | / btree_map = btree_map +LL | | .into_iter() +LL | | .filter(|(k, v)| (k % 2 == 0) && (v % 2 == 0)) +LL | | .collect(); + | |__________________^ help: consider calling `.retain()` instead: `btree_map.retain(|k, &mut v| (k % 2 == 0) && (v % 2 == 0))` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:69:5 + | +LL | btree_set = btree_set.iter().filter(|&x| x % 2 == 0).copied().collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_set.retain(|x| x % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:70:5 + | +LL | btree_set = btree_set.into_iter().filter(|x| x % 2 == 0).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_set.retain(|x| x % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:93:5 + | +LL | hash_map = hash_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_map.retain(|k, _| k % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:94:5 + | +LL | hash_map = hash_map.into_iter().filter(|(_, v)| v % 2 == 0).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_map.retain(|_, &mut v| v % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:95:5 + | +LL | / hash_map = hash_map +LL | | .into_iter() +LL | | .filter(|(k, v)| (k % 2 == 0) && (v % 2 == 0)) +LL | | .collect(); + | |__________________^ help: consider calling `.retain()` instead: `hash_map.retain(|k, &mut v| (k % 2 == 0) && (v % 2 == 0))` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:116:5 + | +LL | hash_set = hash_set.into_iter().filter(|x| x % 2 == 0).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_set.retain(|x| x % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:117:5 + | +LL | hash_set = hash_set.iter().filter(|&x| x % 2 == 0).copied().collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_set.retain(|x| x % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:139:5 + | +LL | s = s.chars().filter(|&c| c != 'o').to_owned().collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `s.retain(|c| c != 'o')` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:151:5 + | +LL | vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec.retain(|x| x % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:152:5 + | +LL | vec = vec.into_iter().filter(|x| x % 2 == 0).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec.retain(|x| x % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:172:5 + | +LL | vec_deque = vec_deque.iter().filter(|&x| x % 2 == 0).copied().collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec_deque.retain(|x| x % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:173:5 + | +LL | vec_deque = vec_deque.into_iter().filter(|x| x % 2 == 0).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec_deque.retain(|x| x % 2 == 0)` + +error: aborting due to 15 previous errors + From b20b8f10dd1965af7854477eddf16e9bba15eed9 Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Thu, 9 Jun 2022 21:52:40 +0900 Subject: [PATCH 02/10] Update clippy_lints/src/use_retain.rs Co-authored-by: llogiq --- clippy_lints/src/use_retain.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/use_retain.rs b/clippy_lints/src/use_retain.rs index ffe2786ee091..324ae06106c2 100644 --- a/clippy_lints/src/use_retain.rs +++ b/clippy_lints/src/use_retain.rs @@ -29,7 +29,7 @@ declare_clippy_lint! { /// ### What it does /// Checks for code to be replaced by `.retain()`. /// ### Why is this bad? - /// `.retain()` is simpler. + /// `.retain()` is simpler and avoids needless allocation. /// ### Example /// ```rust /// let mut vec = vec![0, 1, 2]; From e3afc72caad1132ecad4ba75fc810a3ad309b49e Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Thu, 9 Jun 2022 22:20:59 +0900 Subject: [PATCH 03/10] remove needless return --- clippy_lints/src/use_retain.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/use_retain.rs b/clippy_lints/src/use_retain.rs index 324ae06106c2..d2479168ada5 100644 --- a/clippy_lints/src/use_retain.rs +++ b/clippy_lints/src/use_retain.rs @@ -220,14 +220,14 @@ fn make_sugg( } fn match_acceptable_def_path(cx: &LateContext<'_>, collect_def_id: DefId) -> bool { - return ACCEPTABLE_METHODS + ACCEPTABLE_METHODS .iter() - .any(|&method| match_def_path(cx, collect_def_id, method)); + .any(|&method| match_def_path(cx, collect_def_id, method)) } fn match_acceptable_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { let expr_ty = cx.typeck_results().expr_ty(expr).peel_refs(); - return ACCEPTABLE_TYPES + ACCEPTABLE_TYPES .iter() - .any(|&ty| is_type_diagnostic_item(cx, expr_ty, ty)); + .any(|&ty| is_type_diagnostic_item(cx, expr_ty, ty)) } From 3953c530f4b2cfbe00864ae824fe4a543066a7f4 Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Thu, 9 Jun 2022 22:25:07 +0900 Subject: [PATCH 04/10] change lint type from style to perf --- clippy_lints/src/lib.register_perf.rs | 1 + clippy_lints/src/lib.register_style.rs | 1 - clippy_lints/src/use_retain.rs | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/lib.register_perf.rs b/clippy_lints/src/lib.register_perf.rs index 82431863e6cf..b9d8ec725411 100644 --- a/clippy_lints/src/lib.register_perf.rs +++ b/clippy_lints/src/lib.register_perf.rs @@ -26,6 +26,7 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![ LintId::of(slow_vector_initialization::SLOW_VECTOR_INITIALIZATION), LintId::of(types::BOX_COLLECTION), LintId::of(types::REDUNDANT_ALLOCATION), + LintId::of(use_retain::USE_RETAIN), LintId::of(vec::USELESS_VEC), LintId::of(vec_init_then_push::VEC_INIT_THEN_PUSH), ]) diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs index 3b3505f1e194..d52ec50e5422 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -117,7 +117,6 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(unused_unit::UNUSED_UNIT), LintId::of(upper_case_acronyms::UPPER_CASE_ACRONYMS), - LintId::of(use_retain::USE_RETAIN), LintId::of(write::PRINTLN_EMPTY_STRING), LintId::of(write::PRINT_LITERAL), LintId::of(write::PRINT_WITH_NEWLINE), diff --git a/clippy_lints/src/use_retain.rs b/clippy_lints/src/use_retain.rs index d2479168ada5..b55e1006aacd 100644 --- a/clippy_lints/src/use_retain.rs +++ b/clippy_lints/src/use_retain.rs @@ -43,7 +43,7 @@ declare_clippy_lint! { /// ``` #[clippy::version = "1.63.0"] pub USE_RETAIN, - style, + perf, "`retain()` is simpler and the same functionalitys" } declare_lint_pass!(UseRetain => [USE_RETAIN]); From fd629c0cde26d727fbc19580455a4efd04a84828 Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Thu, 9 Jun 2022 22:34:16 +0900 Subject: [PATCH 05/10] check method --- clippy_lints/src/use_retain.rs | 2 +- clippy_utils/src/paths.rs | 1 + tests/ui/use_retain.fixed | 28 +++++++++++++++++ tests/ui/use_retain.rs | 28 +++++++++++++++++ tests/ui/use_retain.stderr | 56 ++++++++++++++++++++++++---------- 5 files changed, 98 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/use_retain.rs b/clippy_lints/src/use_retain.rs index b55e1006aacd..b6b6dc572c1e 100644 --- a/clippy_lints/src/use_retain.rs +++ b/clippy_lints/src/use_retain.rs @@ -102,7 +102,7 @@ fn check_iter( if_chain! { if let hir::ExprKind::MethodCall(_, [filter_expr], _) = &target_expr.kind; if let Some(copied_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id); - if match_def_path(cx, copied_def_id, &paths::CORE_ITER_COPIED); + if match_def_path(cx, copied_def_id, &paths::CORE_ITER_COPIED) || match_def_path(cx, copied_def_id, &paths::CORE_ITER_CLONED); if let hir::ExprKind::MethodCall(_, [iter_expr, _], _) = &filter_expr.kind; if let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(filter_expr.hir_id); diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 17cccd43a9c3..a2d5279e397c 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -25,6 +25,7 @@ pub const BTREESET_ITER: [&str; 6] = ["alloc", "collections", "btree", "set", "B pub const CLONE_TRAIT_METHOD: [&str; 4] = ["core", "clone", "Clone", "clone"]; pub const COW: [&str; 3] = ["alloc", "borrow", "Cow"]; pub const CORE_ITER_COLLECT: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "collect"]; +pub const CORE_ITER_CLONED: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "cloned"]; pub const CORE_ITER_COPIED: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "copied"]; pub const CORE_ITER_FILTER: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "filter"]; pub const CORE_ITER_INTO_ITER: [&str; 6] = ["core", "iter", "traits", "collect", "IntoIterator", "into_iter"]; diff --git a/tests/ui/use_retain.fixed b/tests/ui/use_retain.fixed index d24571b36f87..0c80d0e204ad 100644 --- a/tests/ui/use_retain.fixed +++ b/tests/ui/use_retain.fixed @@ -25,10 +25,12 @@ fn binary_heap_retain() { let mut heap = BinaryHeap::from([1, 2, 3]); heap = heap.into_iter().filter(|x| x % 2 == 0).collect(); heap = heap.iter().filter(|&x| x % 2 == 0).copied().collect(); + heap = heap.iter().filter(|&x| x % 2 == 0).cloned().collect(); // Do not lint, because type conversion is performed heap = heap.into_iter().filter(|x| x % 2 == 0).collect::>(); heap = heap.iter().filter(|&x| x % 2 == 0).copied().collect::>(); + heap = heap.iter().filter(|&x| x % 2 == 0).cloned().collect::>(); // Do not lint, because this expression is not assign. let mut bar: BinaryHeap = heap.iter().filter(|&x| x % 2 == 0).copied().collect(); @@ -65,6 +67,7 @@ fn btree_set_retain() { // Do lint. btree_set.retain(|x| x % 2 == 0); btree_set.retain(|x| x % 2 == 0); + btree_set.retain(|x| x % 2 == 0); // Do not lint, because type conversion is performed btree_set = btree_set @@ -73,6 +76,12 @@ fn btree_set_retain() { .copied() .collect::>(); + btree_set = btree_set + .iter() + .filter(|&x| x % 2 == 0) + .cloned() + .collect::>(); + btree_set = btree_set.into_iter().filter(|x| x % 2 == 0).collect::>(); // Do not lint, because this expression is not assign. @@ -81,6 +90,7 @@ fn btree_set_retain() { // Do not lint, because it is an assignment to a different variable. bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.iter().filter(|&x| x % 2 == 0).cloned().collect(); bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); } @@ -109,6 +119,7 @@ fn hash_set_retain() { // Do lint. hash_set.retain(|x| x % 2 == 0); hash_set.retain(|x| x % 2 == 0); + hash_set.retain(|x| x % 2 == 0); // Do not lint, because type conversion is performed hash_set = hash_set.into_iter().filter(|x| x % 2 == 0).collect::>(); @@ -118,12 +129,19 @@ fn hash_set_retain() { .copied() .collect::>(); + hash_set = hash_set + .iter() + .filter(|&x| x % 2 == 0) + .cloned() + .collect::>(); + // Do not lint, because this expression is not assign. let mut bar: HashSet = hash_set.iter().filter(|&x| x % 2 == 0).copied().collect(); let mut foobar: HashSet = hash_set.into_iter().filter(|x| x % 2 == 0).collect(); // Do not lint, because it is an assignment to a different variable. bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.iter().filter(|&x| x % 2 == 0).cloned().collect(); bar = foobar.into_iter().filter(|&x| x % 2 == 0).collect(); } @@ -144,10 +162,12 @@ fn vec_retain() { // Do lint. vec.retain(|x| x % 2 == 0); vec.retain(|x| x % 2 == 0); + vec.retain(|x| x % 2 == 0); // Do not lint, because type conversion is performed vec = vec.into_iter().filter(|x| x % 2 == 0).collect::>(); vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect::>(); + vec = vec.iter().filter(|&x| x % 2 == 0).cloned().collect::>(); // Do not lint, because this expression is not assign. let mut bar: Vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect(); @@ -155,6 +175,7 @@ fn vec_retain() { // Do not lint, because it is an assignment to a different variable. bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.iter().filter(|&x| x % 2 == 0).cloned().collect(); bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); } @@ -165,6 +186,7 @@ fn vec_queue_retain() { // Do lint. vec_deque.retain(|x| x % 2 == 0); vec_deque.retain(|x| x % 2 == 0); + vec_deque.retain(|x| x % 2 == 0); // Do not lint, because type conversion is performed vec_deque = vec_deque @@ -172,6 +194,11 @@ fn vec_queue_retain() { .filter(|&x| x % 2 == 0) .copied() .collect::>(); + vec_deque = vec_deque + .iter() + .filter(|&x| x % 2 == 0) + .cloned() + .collect::>(); vec_deque = vec_deque.into_iter().filter(|x| x % 2 == 0).collect::>(); // Do not lint, because this expression is not assign. @@ -180,5 +207,6 @@ fn vec_queue_retain() { // Do not lint, because it is an assignment to a different variable. bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.iter().filter(|&x| x % 2 == 0).cloned().collect(); bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); } diff --git a/tests/ui/use_retain.rs b/tests/ui/use_retain.rs index 4a51825c95f7..08c184486f95 100644 --- a/tests/ui/use_retain.rs +++ b/tests/ui/use_retain.rs @@ -25,10 +25,12 @@ fn binary_heap_retain() { let mut heap = BinaryHeap::from([1, 2, 3]); heap = heap.into_iter().filter(|x| x % 2 == 0).collect(); heap = heap.iter().filter(|&x| x % 2 == 0).copied().collect(); + heap = heap.iter().filter(|&x| x % 2 == 0).cloned().collect(); // Do not lint, because type conversion is performed heap = heap.into_iter().filter(|x| x % 2 == 0).collect::>(); heap = heap.iter().filter(|&x| x % 2 == 0).copied().collect::>(); + heap = heap.iter().filter(|&x| x % 2 == 0).cloned().collect::>(); // Do not lint, because this expression is not assign. let mut bar: BinaryHeap = heap.iter().filter(|&x| x % 2 == 0).copied().collect(); @@ -67,6 +69,7 @@ fn btree_set_retain() { // Do lint. btree_set = btree_set.iter().filter(|&x| x % 2 == 0).copied().collect(); + btree_set = btree_set.iter().filter(|&x| x % 2 == 0).cloned().collect(); btree_set = btree_set.into_iter().filter(|x| x % 2 == 0).collect(); // Do not lint, because type conversion is performed @@ -76,6 +79,12 @@ fn btree_set_retain() { .copied() .collect::>(); + btree_set = btree_set + .iter() + .filter(|&x| x % 2 == 0) + .cloned() + .collect::>(); + btree_set = btree_set.into_iter().filter(|x| x % 2 == 0).collect::>(); // Do not lint, because this expression is not assign. @@ -84,6 +93,7 @@ fn btree_set_retain() { // Do not lint, because it is an assignment to a different variable. bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.iter().filter(|&x| x % 2 == 0).cloned().collect(); bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); } @@ -115,6 +125,7 @@ fn hash_set_retain() { // Do lint. hash_set = hash_set.into_iter().filter(|x| x % 2 == 0).collect(); hash_set = hash_set.iter().filter(|&x| x % 2 == 0).copied().collect(); + hash_set = hash_set.iter().filter(|&x| x % 2 == 0).cloned().collect(); // Do not lint, because type conversion is performed hash_set = hash_set.into_iter().filter(|x| x % 2 == 0).collect::>(); @@ -124,12 +135,19 @@ fn hash_set_retain() { .copied() .collect::>(); + hash_set = hash_set + .iter() + .filter(|&x| x % 2 == 0) + .cloned() + .collect::>(); + // Do not lint, because this expression is not assign. let mut bar: HashSet = hash_set.iter().filter(|&x| x % 2 == 0).copied().collect(); let mut foobar: HashSet = hash_set.into_iter().filter(|x| x % 2 == 0).collect(); // Do not lint, because it is an assignment to a different variable. bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.iter().filter(|&x| x % 2 == 0).cloned().collect(); bar = foobar.into_iter().filter(|&x| x % 2 == 0).collect(); } @@ -149,11 +167,13 @@ fn vec_retain() { let mut vec = vec![0, 1, 2]; // Do lint. vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect(); + vec = vec.iter().filter(|&x| x % 2 == 0).cloned().collect(); vec = vec.into_iter().filter(|x| x % 2 == 0).collect(); // Do not lint, because type conversion is performed vec = vec.into_iter().filter(|x| x % 2 == 0).collect::>(); vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect::>(); + vec = vec.iter().filter(|&x| x % 2 == 0).cloned().collect::>(); // Do not lint, because this expression is not assign. let mut bar: Vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect(); @@ -161,6 +181,7 @@ fn vec_retain() { // Do not lint, because it is an assignment to a different variable. bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.iter().filter(|&x| x % 2 == 0).cloned().collect(); bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); } @@ -170,6 +191,7 @@ fn vec_queue_retain() { // Do lint. vec_deque = vec_deque.iter().filter(|&x| x % 2 == 0).copied().collect(); + vec_deque = vec_deque.iter().filter(|&x| x % 2 == 0).cloned().collect(); vec_deque = vec_deque.into_iter().filter(|x| x % 2 == 0).collect(); // Do not lint, because type conversion is performed @@ -178,6 +200,11 @@ fn vec_queue_retain() { .filter(|&x| x % 2 == 0) .copied() .collect::>(); + vec_deque = vec_deque + .iter() + .filter(|&x| x % 2 == 0) + .cloned() + .collect::>(); vec_deque = vec_deque.into_iter().filter(|x| x % 2 == 0).collect::>(); // Do not lint, because this expression is not assign. @@ -186,5 +213,6 @@ fn vec_queue_retain() { // Do not lint, because it is an assignment to a different variable. bar = foobar.iter().filter(|&x| x % 2 == 0).copied().collect(); + bar = foobar.iter().filter(|&x| x % 2 == 0).cloned().collect(); bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); } diff --git a/tests/ui/use_retain.stderr b/tests/ui/use_retain.stderr index 331259c1626b..e29d40a2e39e 100644 --- a/tests/ui/use_retain.stderr +++ b/tests/ui/use_retain.stderr @@ -1,5 +1,5 @@ error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:45:5 + --> $DIR/use_retain.rs:47:5 | LL | btree_map = btree_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_map.retain(|k, _| k % 2 == 0)` @@ -7,13 +7,13 @@ LL | btree_map = btree_map.into_iter().filter(|(k, _)| k % 2 == 0).collect() = note: `-D clippy::use-retain` implied by `-D warnings` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:46:5 + --> $DIR/use_retain.rs:48:5 | LL | btree_map = btree_map.into_iter().filter(|(_, v)| v % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_map.retain(|_, &mut v| v % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:47:5 + --> $DIR/use_retain.rs:49:5 | LL | / btree_map = btree_map LL | | .into_iter() @@ -22,31 +22,37 @@ LL | | .collect(); | |__________________^ help: consider calling `.retain()` instead: `btree_map.retain(|k, &mut v| (k % 2 == 0) && (v % 2 == 0))` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:69:5 + --> $DIR/use_retain.rs:71:5 | LL | btree_set = btree_set.iter().filter(|&x| x % 2 == 0).copied().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:70:5 + --> $DIR/use_retain.rs:72:5 + | +LL | btree_set = btree_set.iter().filter(|&x| x % 2 == 0).cloned().collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_set.retain(|x| x % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:73:5 | LL | btree_set = btree_set.into_iter().filter(|x| x % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:93:5 + --> $DIR/use_retain.rs:103:5 | LL | hash_map = hash_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_map.retain(|k, _| k % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:94:5 + --> $DIR/use_retain.rs:104:5 | LL | hash_map = hash_map.into_iter().filter(|(_, v)| v % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_map.retain(|_, &mut v| v % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:95:5 + --> $DIR/use_retain.rs:105:5 | LL | / hash_map = hash_map LL | | .into_iter() @@ -55,46 +61,64 @@ LL | | .collect(); | |__________________^ help: consider calling `.retain()` instead: `hash_map.retain(|k, &mut v| (k % 2 == 0) && (v % 2 == 0))` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:116:5 + --> $DIR/use_retain.rs:126:5 | LL | hash_set = hash_set.into_iter().filter(|x| x % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:117:5 + --> $DIR/use_retain.rs:127:5 | LL | hash_set = hash_set.iter().filter(|&x| x % 2 == 0).copied().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:139:5 + --> $DIR/use_retain.rs:128:5 + | +LL | hash_set = hash_set.iter().filter(|&x| x % 2 == 0).cloned().collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_set.retain(|x| x % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:157:5 | LL | s = s.chars().filter(|&c| c != 'o').to_owned().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `s.retain(|c| c != 'o')` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:151:5 + --> $DIR/use_retain.rs:169:5 | LL | vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:152:5 + --> $DIR/use_retain.rs:170:5 + | +LL | vec = vec.iter().filter(|&x| x % 2 == 0).cloned().collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec.retain(|x| x % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:171:5 | LL | vec = vec.into_iter().filter(|x| x % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:172:5 + --> $DIR/use_retain.rs:193:5 | LL | vec_deque = vec_deque.iter().filter(|&x| x % 2 == 0).copied().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec_deque.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:173:5 + --> $DIR/use_retain.rs:194:5 + | +LL | vec_deque = vec_deque.iter().filter(|&x| x % 2 == 0).cloned().collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec_deque.retain(|x| x % 2 == 0)` + +error: this expression can be written more simply using `.retain()` + --> $DIR/use_retain.rs:195:5 | LL | vec_deque = vec_deque.into_iter().filter(|x| x % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec_deque.retain(|x| x % 2 == 0)` -error: aborting due to 15 previous errors +error: aborting due to 19 previous errors From 14212115c4f669d33007d45c2b26623b7879ccc0 Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Thu, 9 Jun 2022 22:50:00 +0900 Subject: [PATCH 06/10] rewrite without if_chain macro --- clippy_lints/src/use_retain.rs | 146 ++++++++++++++------------------- 1 file changed, 60 insertions(+), 86 deletions(-) diff --git a/clippy_lints/src/use_retain.rs b/clippy_lints/src/use_retain.rs index b6b6dc572c1e..7c57020761cd 100644 --- a/clippy_lints/src/use_retain.rs +++ b/clippy_lints/src/use_retain.rs @@ -50,21 +50,16 @@ declare_lint_pass!(UseRetain => [USE_RETAIN]); impl<'tcx> LateLintPass<'tcx> for UseRetain { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { - if_chain! { - if let Some(parent_expr) = get_parent_expr(cx, expr); - if let Assign(left_expr, collect_expr, _) = &parent_expr.kind; - if let hir::ExprKind::MethodCall(seg, _, _) = &collect_expr.kind; - if seg.args.is_none(); - - if let hir::ExprKind::MethodCall(_, [target_expr], _) = &collect_expr.kind; - if let Some(collect_def_id) = cx.typeck_results().type_dependent_def_id(collect_expr.hir_id); - if match_def_path(cx, collect_def_id, &paths::CORE_ITER_COLLECT); - - then { - check_into_iter(cx, parent_expr, left_expr, target_expr); - check_iter(cx, parent_expr, left_expr, target_expr); - check_to_owned(cx, parent_expr, left_expr, target_expr); - } + if let Some(parent_expr) = get_parent_expr(cx, expr) + && let Assign(left_expr, collect_expr, _) = &parent_expr.kind + && let hir::ExprKind::MethodCall(seg, _, _) = &collect_expr.kind + && seg.args.is_none() + && let hir::ExprKind::MethodCall(_, [target_expr], _) = &collect_expr.kind + && let Some(collect_def_id) = cx.typeck_results().type_dependent_def_id(collect_expr.hir_id) + && match_def_path(cx, collect_def_id, &paths::CORE_ITER_COLLECT) { + check_into_iter(cx, parent_expr, left_expr, target_expr); + check_iter(cx, parent_expr, left_expr, target_expr); + check_to_owned(cx, parent_expr, left_expr, target_expr); } } } @@ -75,21 +70,15 @@ fn check_into_iter( left_expr: &hir::Expr<'_>, target_expr: &hir::Expr<'_>, ) { - if_chain! { - if let hir::ExprKind::MethodCall(_, [into_iter_expr, _], _) = &target_expr.kind; - if let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id); - if match_def_path(cx, filter_def_id, &paths::CORE_ITER_FILTER); - - if let hir::ExprKind::MethodCall(_, [struct_expr], _) = &into_iter_expr.kind; - if let Some(into_iter_def_id) = cx.typeck_results().type_dependent_def_id(into_iter_expr.hir_id); - if match_def_path(cx, into_iter_def_id, &paths::CORE_ITER_INTO_ITER); - if match_acceptable_type(cx, left_expr); - - if SpanlessEq::new(cx).eq_expr(left_expr, struct_expr); - - then { - suggest(cx, parent_expr, left_expr, target_expr); - } + if let hir::ExprKind::MethodCall(_, [into_iter_expr, _], _) = &target_expr.kind + && let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id) + && match_def_path(cx, filter_def_id, &paths::CORE_ITER_FILTER) + && let hir::ExprKind::MethodCall(_, [struct_expr], _) = &into_iter_expr.kind + && let Some(into_iter_def_id) = cx.typeck_results().type_dependent_def_id(into_iter_expr.hir_id) + && match_def_path(cx, into_iter_def_id, &paths::CORE_ITER_INTO_ITER) + && match_acceptable_type(cx, left_expr) + && SpanlessEq::new(cx).eq_expr(left_expr, struct_expr) { + suggest(cx, parent_expr, left_expr, target_expr); } } @@ -99,24 +88,19 @@ fn check_iter( left_expr: &hir::Expr<'_>, target_expr: &hir::Expr<'_>, ) { - if_chain! { - if let hir::ExprKind::MethodCall(_, [filter_expr], _) = &target_expr.kind; - if let Some(copied_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id); - if match_def_path(cx, copied_def_id, &paths::CORE_ITER_COPIED) || match_def_path(cx, copied_def_id, &paths::CORE_ITER_CLONED); - - if let hir::ExprKind::MethodCall(_, [iter_expr, _], _) = &filter_expr.kind; - if let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(filter_expr.hir_id); - if match_def_path(cx, filter_def_id, &paths::CORE_ITER_FILTER); - - if let hir::ExprKind::MethodCall(_, [struct_expr], _) = &iter_expr.kind; - if let Some(iter_expr_def_id) = cx.typeck_results().type_dependent_def_id(iter_expr.hir_id); - if match_acceptable_def_path(cx, iter_expr_def_id); - if match_acceptable_type(cx, left_expr); - if SpanlessEq::new(cx).eq_expr(left_expr, struct_expr); - - then { - suggest(cx, parent_expr, left_expr, filter_expr); - } + if let hir::ExprKind::MethodCall(_, [filter_expr], _) = &target_expr.kind + && let Some(copied_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id) + && (match_def_path(cx, copied_def_id, &paths::CORE_ITER_COPIED) + || match_def_path(cx, copied_def_id, &paths::CORE_ITER_CLONED)) + && let hir::ExprKind::MethodCall(_, [iter_expr, _], _) = &filter_expr.kind + && let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(filter_expr.hir_id) + && match_def_path(cx, filter_def_id, &paths::CORE_ITER_FILTER) + && let hir::ExprKind::MethodCall(_, [struct_expr], _) = &iter_expr.kind + && let Some(iter_expr_def_id) = cx.typeck_results().type_dependent_def_id(iter_expr.hir_id) + && match_acceptable_def_path(cx, iter_expr_def_id) + && match_acceptable_type(cx, left_expr) + && SpanlessEq::new(cx).eq_expr(left_expr, struct_expr) { + suggest(cx, parent_expr, left_expr, filter_expr); } } @@ -126,36 +110,28 @@ fn check_to_owned( left_expr: &hir::Expr<'_>, target_expr: &hir::Expr<'_>, ) { - if_chain! { - if let hir::ExprKind::MethodCall(_, [filter_expr], _) = &target_expr.kind; - if let Some(to_owned_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id); - if match_def_path(cx, to_owned_def_id, &paths::TO_OWNED_METHOD); - - if let hir::ExprKind::MethodCall(_, [chars_expr, _], _) = &filter_expr.kind; - if let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(filter_expr.hir_id); - if match_def_path(cx, filter_def_id, &paths::CORE_ITER_FILTER); - - if let hir::ExprKind::MethodCall(_, [str_expr], _) = &chars_expr.kind; - if let Some(chars_expr_def_id) = cx.typeck_results().type_dependent_def_id(chars_expr.hir_id); - if match_def_path(cx, chars_expr_def_id, &paths::STR_CHARS); - - let ty = cx.typeck_results().expr_ty(str_expr).peel_refs(); - if is_type_diagnostic_item(cx, ty, sym::String); - if SpanlessEq::new(cx).eq_expr(left_expr, str_expr); - - then { - suggest(cx, parent_expr, left_expr, filter_expr); - } + if let hir::ExprKind::MethodCall(_, [filter_expr], _) = &target_expr.kind + && let Some(to_owned_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id) + && match_def_path(cx, to_owned_def_id, &paths::TO_OWNED_METHOD) + && let hir::ExprKind::MethodCall(_, [chars_expr, _], _) = &filter_expr.kind + && let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(filter_expr.hir_id) + && match_def_path(cx, filter_def_id, &paths::CORE_ITER_FILTER) + && let hir::ExprKind::MethodCall(_, [str_expr], _) = &chars_expr.kind + && let Some(chars_expr_def_id) = cx.typeck_results().type_dependent_def_id(chars_expr.hir_id) + && match_def_path(cx, chars_expr_def_id, &paths::STR_CHARS) + && let ty = cx.typeck_results().expr_ty(str_expr).peel_refs() + && is_type_diagnostic_item(cx, ty, sym::String) + && SpanlessEq::new(cx).eq_expr(left_expr, str_expr) { + suggest(cx, parent_expr, left_expr, filter_expr); } } fn suggest(cx: &LateContext<'_>, parent_expr: &hir::Expr<'_>, left_expr: &hir::Expr<'_>, filter_expr: &hir::Expr<'_>) { - if_chain! { - if let hir::ExprKind::MethodCall(_, [_, closure], _) = filter_expr.kind; - if let hir::ExprKind::Closure(_, _, filter_body_id, ..) = closure.kind; - let filter_body = cx.tcx.hir().body(filter_body_id); - if let [filter_params] = filter_body.params; - if let Some(sugg) = match filter_params.pat.kind { + if let hir::ExprKind::MethodCall(_, [_, closure], _) = filter_expr.kind + && let hir::ExprKind::Closure(_, _, filter_body_id, ..) = closure.kind + && let filter_body = cx.tcx.hir().body(filter_body_id) + && let [filter_params] = filter_body.params + && let Some(sugg) = match filter_params.pat.kind { hir::PatKind::Binding(_, _, filter_param_ident, None) => { Some(format!("{}.retain(|{}| {})", snippet(cx, left_expr.span, ".."), filter_param_ident, snippet(cx, filter_body.value.span, ".."))) }, @@ -171,18 +147,16 @@ fn suggest(cx: &LateContext<'_>, parent_expr: &hir::Expr<'_>, left_expr: &hir::E } }, _ => None - }; - then { - span_lint_and_sugg( - cx, - USE_RETAIN, - parent_expr.span, - "this expression can be written more simply using `.retain()`", - "consider calling `.retain()` instead", - sugg, - Applicability::MachineApplicable - ); - } + } { + span_lint_and_sugg( + cx, + USE_RETAIN, + parent_expr.span, + "this expression can be written more simply using `.retain()`", + "consider calling `.retain()` instead", + sugg, + Applicability::MachineApplicable + ); } } From 4decfdec76a6b67dc87f29d6f1a8db148119211d Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Fri, 10 Jun 2022 11:31:25 +0900 Subject: [PATCH 07/10] check msrv --- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/use_retain.rs | 59 +++++++++++++++++++++++----------- clippy_utils/src/msrvs.rs | 5 +-- tests/ui/use_retain.fixed | 32 ++++++++++++++++-- tests/ui/use_retain.rs | 32 ++++++++++++++++-- tests/ui/use_retain.stderr | 38 +++++++++++----------- 6 files changed, 123 insertions(+), 45 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 124f0d12b08c..716038aa5220 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -915,7 +915,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(read_zero_byte_vec::ReadZeroByteVec)); store.register_late_pass(|| Box::new(default_instead_of_iter_empty::DefaultIterEmpty)); store.register_late_pass(move || Box::new(manual_rem_euclid::ManualRemEuclid::new(msrv))); - store.register_late_pass(|| Box::new(use_retain::UseRetain)); + store.register_late_pass(move || Box::new(use_retain::UseRetain::new(msrv))); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/use_retain.rs b/clippy_lints/src/use_retain.rs index 7c57020761cd..185201514ad3 100644 --- a/clippy_lints/src/use_retain.rs +++ b/clippy_lints/src/use_retain.rs @@ -2,12 +2,14 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::{get_parent_expr, match_def_path, paths, SpanlessEq}; +use clippy_utils::{meets_msrv, msrvs}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::ExprKind::Assign; use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::symbol::sym; const ACCEPTABLE_METHODS: [&[&str]; 4] = [ @@ -16,13 +18,13 @@ const ACCEPTABLE_METHODS: [&[&str]; 4] = [ &paths::SLICE_INTO, &paths::VEC_DEQUE_ITER, ]; -const ACCEPTABLE_TYPES: [rustc_span::Symbol; 6] = [ - sym::BTreeSet, - sym::BTreeMap, - sym::HashSet, - sym::HashMap, - sym::Vec, - sym::VecDeque, +const ACCEPTABLE_TYPES: [(rustc_span::Symbol, Option); 6] = [ + (sym::BTreeSet, Some(msrvs::BTREE_SET_RETAIN)), + (sym::BTreeMap, Some(msrvs::BTREE_MAP_RETAIN)), + (sym::HashSet, Some(msrvs::HASH_SET_RETAIN)), + (sym::HashMap, Some(msrvs::HASH_MAP_RETAIN)), + (sym::Vec, None), + (sym::VecDeque, None), ]; declare_clippy_lint! { @@ -46,7 +48,19 @@ declare_clippy_lint! { perf, "`retain()` is simpler and the same functionalitys" } -declare_lint_pass!(UseRetain => [USE_RETAIN]); + +pub struct UseRetain { + msrv: Option, +} + +impl UseRetain { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + +impl_lint_pass!(UseRetain => [USE_RETAIN]); impl<'tcx> LateLintPass<'tcx> for UseRetain { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { @@ -57,11 +71,13 @@ impl<'tcx> LateLintPass<'tcx> for UseRetain { && let hir::ExprKind::MethodCall(_, [target_expr], _) = &collect_expr.kind && let Some(collect_def_id) = cx.typeck_results().type_dependent_def_id(collect_expr.hir_id) && match_def_path(cx, collect_def_id, &paths::CORE_ITER_COLLECT) { - check_into_iter(cx, parent_expr, left_expr, target_expr); - check_iter(cx, parent_expr, left_expr, target_expr); - check_to_owned(cx, parent_expr, left_expr, target_expr); + check_into_iter(cx, parent_expr, left_expr, target_expr, self.msrv); + check_iter(cx, parent_expr, left_expr, target_expr, self.msrv); + check_to_owned(cx, parent_expr, left_expr, target_expr, self.msrv); } } + + extract_msrv_attr!(LateContext); } fn check_into_iter( @@ -69,6 +85,7 @@ fn check_into_iter( parent_expr: &hir::Expr<'_>, left_expr: &hir::Expr<'_>, target_expr: &hir::Expr<'_>, + msrv: Option, ) { if let hir::ExprKind::MethodCall(_, [into_iter_expr, _], _) = &target_expr.kind && let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id) @@ -76,7 +93,7 @@ fn check_into_iter( && let hir::ExprKind::MethodCall(_, [struct_expr], _) = &into_iter_expr.kind && let Some(into_iter_def_id) = cx.typeck_results().type_dependent_def_id(into_iter_expr.hir_id) && match_def_path(cx, into_iter_def_id, &paths::CORE_ITER_INTO_ITER) - && match_acceptable_type(cx, left_expr) + && match_acceptable_type(cx, left_expr, msrv) && SpanlessEq::new(cx).eq_expr(left_expr, struct_expr) { suggest(cx, parent_expr, left_expr, target_expr); } @@ -87,6 +104,7 @@ fn check_iter( parent_expr: &hir::Expr<'_>, left_expr: &hir::Expr<'_>, target_expr: &hir::Expr<'_>, + msrv: Option, ) { if let hir::ExprKind::MethodCall(_, [filter_expr], _) = &target_expr.kind && let Some(copied_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id) @@ -98,7 +116,7 @@ fn check_iter( && let hir::ExprKind::MethodCall(_, [struct_expr], _) = &iter_expr.kind && let Some(iter_expr_def_id) = cx.typeck_results().type_dependent_def_id(iter_expr.hir_id) && match_acceptable_def_path(cx, iter_expr_def_id) - && match_acceptable_type(cx, left_expr) + && match_acceptable_type(cx, left_expr, msrv) && SpanlessEq::new(cx).eq_expr(left_expr, struct_expr) { suggest(cx, parent_expr, left_expr, filter_expr); } @@ -109,8 +127,10 @@ fn check_to_owned( parent_expr: &hir::Expr<'_>, left_expr: &hir::Expr<'_>, target_expr: &hir::Expr<'_>, + msrv: Option, ) { - if let hir::ExprKind::MethodCall(_, [filter_expr], _) = &target_expr.kind + if meets_msrv(msrv, msrvs::STRING_RETAIN) + && let hir::ExprKind::MethodCall(_, [filter_expr], _) = &target_expr.kind && let Some(to_owned_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id) && match_def_path(cx, to_owned_def_id, &paths::TO_OWNED_METHOD) && let hir::ExprKind::MethodCall(_, [chars_expr, _], _) = &filter_expr.kind @@ -199,9 +219,10 @@ fn match_acceptable_def_path(cx: &LateContext<'_>, collect_def_id: DefId) -> boo .any(|&method| match_def_path(cx, collect_def_id, method)) } -fn match_acceptable_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { +fn match_acceptable_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>, msrv: Option) -> bool { let expr_ty = cx.typeck_results().expr_ty(expr).peel_refs(); - ACCEPTABLE_TYPES - .iter() - .any(|&ty| is_type_diagnostic_item(cx, expr_ty, ty)) + ACCEPTABLE_TYPES.iter().any(|(ty, acceptable_msrv)| { + is_type_diagnostic_item(cx, expr_ty, *ty) + && acceptable_msrv.map_or(true, |acceptable_msrv| meets_msrv(msrv, acceptable_msrv)) + }) } diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 43c0a03c42ab..b09c929f76e2 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -12,7 +12,7 @@ macro_rules! msrv_aliases { // names may refer to stabilized feature flags or library items msrv_aliases! { - 1,53,0 { OR_PATTERNS, MANUAL_BITS } + 1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN } 1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST } 1,51,0 { BORROW_AS_PTR, UNSIGNED_ABS } 1,50,0 { BOOL_THEN } @@ -30,7 +30,8 @@ msrv_aliases! { 1,34,0 { TRY_FROM } 1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES } 1,28,0 { FROM_BOOL } - 1,26,0 { RANGE_INCLUSIVE } + 1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN } + 1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN } 1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR } 1,16,0 { STR_REPEAT } 1,24,0 { IS_ASCII_DIGIT } diff --git a/tests/ui/use_retain.fixed b/tests/ui/use_retain.fixed index 0c80d0e204ad..549b8a62c0c5 100644 --- a/tests/ui/use_retain.fixed +++ b/tests/ui/use_retain.fixed @@ -1,4 +1,5 @@ // run-rustfix +#![feature(custom_inner_attributes)] #![warn(clippy::use_retain)] #![allow(unused)] use std::collections::BTreeMap; @@ -15,12 +16,16 @@ fn main() { hash_set_retain(); hash_map_retain(); string_retain(); - vec_queue_retain(); + vec_deque_retain(); vec_retain(); + _msrv_153(); + _msrv_126(); + _msrv_118(); } fn binary_heap_retain() { // NOTE: Do not lint now, because binary_heap_retain is nighyly API. + // And we need to add a test case for msrv if we update this implmention. // https://github.com/rust-lang/rust/issues/71503 let mut heap = BinaryHeap::from([1, 2, 3]); heap = heap.into_iter().filter(|x| x % 2 == 0).collect(); @@ -179,7 +184,7 @@ fn vec_retain() { bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); } -fn vec_queue_retain() { +fn vec_deque_retain() { let mut vec_deque = VecDeque::new(); vec_deque.extend(1..5); @@ -210,3 +215,26 @@ fn vec_queue_retain() { bar = foobar.iter().filter(|&x| x % 2 == 0).cloned().collect(); bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); } + +fn _msrv_153() { + #![clippy::msrv = "1.52"] + let mut btree_map: BTreeMap = (0..8).map(|x| (x, x * 10)).collect(); + btree_map = btree_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); + + let mut btree_set = BTreeSet::from([1, 2, 3, 4, 5, 6]); + btree_set = btree_set.iter().filter(|&x| x % 2 == 0).copied().collect(); +} + +fn _msrv_126() { + #![clippy::msrv = "1.25"] + let mut s = String::from("foobar"); + s = s.chars().filter(|&c| c != 'o').to_owned().collect(); +} + +fn _msrv_118() { + #![clippy::msrv = "1.17"] + let mut hash_set = HashSet::from([1, 2, 3, 4, 5, 6]); + hash_set = hash_set.into_iter().filter(|x| x % 2 == 0).collect(); + let mut hash_map: HashMap = (0..8).map(|x| (x, x * 10)).collect(); + hash_map = hash_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); +} diff --git a/tests/ui/use_retain.rs b/tests/ui/use_retain.rs index 08c184486f95..36c4176b8d5f 100644 --- a/tests/ui/use_retain.rs +++ b/tests/ui/use_retain.rs @@ -1,4 +1,5 @@ // run-rustfix +#![feature(custom_inner_attributes)] #![warn(clippy::use_retain)] #![allow(unused)] use std::collections::BTreeMap; @@ -15,12 +16,16 @@ fn main() { hash_set_retain(); hash_map_retain(); string_retain(); - vec_queue_retain(); + vec_deque_retain(); vec_retain(); + _msrv_153(); + _msrv_126(); + _msrv_118(); } fn binary_heap_retain() { // NOTE: Do not lint now, because binary_heap_retain is nighyly API. + // And we need to add a test case for msrv if we update this implmention. // https://github.com/rust-lang/rust/issues/71503 let mut heap = BinaryHeap::from([1, 2, 3]); heap = heap.into_iter().filter(|x| x % 2 == 0).collect(); @@ -185,7 +190,7 @@ fn vec_retain() { bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); } -fn vec_queue_retain() { +fn vec_deque_retain() { let mut vec_deque = VecDeque::new(); vec_deque.extend(1..5); @@ -216,3 +221,26 @@ fn vec_queue_retain() { bar = foobar.iter().filter(|&x| x % 2 == 0).cloned().collect(); bar = foobar.into_iter().filter(|x| x % 2 == 0).collect(); } + +fn _msrv_153() { + #![clippy::msrv = "1.52"] + let mut btree_map: BTreeMap = (0..8).map(|x| (x, x * 10)).collect(); + btree_map = btree_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); + + let mut btree_set = BTreeSet::from([1, 2, 3, 4, 5, 6]); + btree_set = btree_set.iter().filter(|&x| x % 2 == 0).copied().collect(); +} + +fn _msrv_126() { + #![clippy::msrv = "1.25"] + let mut s = String::from("foobar"); + s = s.chars().filter(|&c| c != 'o').to_owned().collect(); +} + +fn _msrv_118() { + #![clippy::msrv = "1.17"] + let mut hash_set = HashSet::from([1, 2, 3, 4, 5, 6]); + hash_set = hash_set.into_iter().filter(|x| x % 2 == 0).collect(); + let mut hash_map: HashMap = (0..8).map(|x| (x, x * 10)).collect(); + hash_map = hash_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); +} diff --git a/tests/ui/use_retain.stderr b/tests/ui/use_retain.stderr index e29d40a2e39e..decd406f22a9 100644 --- a/tests/ui/use_retain.stderr +++ b/tests/ui/use_retain.stderr @@ -1,5 +1,5 @@ error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:47:5 + --> $DIR/use_retain.rs:52:5 | LL | btree_map = btree_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_map.retain(|k, _| k % 2 == 0)` @@ -7,13 +7,13 @@ LL | btree_map = btree_map.into_iter().filter(|(k, _)| k % 2 == 0).collect() = note: `-D clippy::use-retain` implied by `-D warnings` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:48:5 + --> $DIR/use_retain.rs:53:5 | LL | btree_map = btree_map.into_iter().filter(|(_, v)| v % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_map.retain(|_, &mut v| v % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:49:5 + --> $DIR/use_retain.rs:54:5 | LL | / btree_map = btree_map LL | | .into_iter() @@ -22,37 +22,37 @@ LL | | .collect(); | |__________________^ help: consider calling `.retain()` instead: `btree_map.retain(|k, &mut v| (k % 2 == 0) && (v % 2 == 0))` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:71:5 + --> $DIR/use_retain.rs:76:5 | LL | btree_set = btree_set.iter().filter(|&x| x % 2 == 0).copied().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:72:5 + --> $DIR/use_retain.rs:77:5 | LL | btree_set = btree_set.iter().filter(|&x| x % 2 == 0).cloned().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:73:5 + --> $DIR/use_retain.rs:78:5 | LL | btree_set = btree_set.into_iter().filter(|x| x % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:103:5 + --> $DIR/use_retain.rs:108:5 | LL | hash_map = hash_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_map.retain(|k, _| k % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:104:5 + --> $DIR/use_retain.rs:109:5 | LL | hash_map = hash_map.into_iter().filter(|(_, v)| v % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_map.retain(|_, &mut v| v % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:105:5 + --> $DIR/use_retain.rs:110:5 | LL | / hash_map = hash_map LL | | .into_iter() @@ -61,61 +61,61 @@ LL | | .collect(); | |__________________^ help: consider calling `.retain()` instead: `hash_map.retain(|k, &mut v| (k % 2 == 0) && (v % 2 == 0))` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:126:5 + --> $DIR/use_retain.rs:131:5 | LL | hash_set = hash_set.into_iter().filter(|x| x % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:127:5 + --> $DIR/use_retain.rs:132:5 | LL | hash_set = hash_set.iter().filter(|&x| x % 2 == 0).copied().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:128:5 + --> $DIR/use_retain.rs:133:5 | LL | hash_set = hash_set.iter().filter(|&x| x % 2 == 0).cloned().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:157:5 + --> $DIR/use_retain.rs:162:5 | LL | s = s.chars().filter(|&c| c != 'o').to_owned().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `s.retain(|c| c != 'o')` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:169:5 + --> $DIR/use_retain.rs:174:5 | LL | vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:170:5 + --> $DIR/use_retain.rs:175:5 | LL | vec = vec.iter().filter(|&x| x % 2 == 0).cloned().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:171:5 + --> $DIR/use_retain.rs:176:5 | LL | vec = vec.into_iter().filter(|x| x % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:193:5 + --> $DIR/use_retain.rs:198:5 | LL | vec_deque = vec_deque.iter().filter(|&x| x % 2 == 0).copied().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec_deque.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:194:5 + --> $DIR/use_retain.rs:199:5 | LL | vec_deque = vec_deque.iter().filter(|&x| x % 2 == 0).cloned().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec_deque.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:195:5 + --> $DIR/use_retain.rs:200:5 | LL | vec_deque = vec_deque.into_iter().filter(|x| x % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec_deque.retain(|x| x % 2 == 0)` From dd3d0fdad30249505688eada4481b1d78190e971 Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Fri, 10 Jun 2022 21:02:43 +0900 Subject: [PATCH 08/10] rename use_retain => manual_retain --- CHANGELOG.md | 2 +- clippy_lints/src/lib.register_all.rs | 2 +- clippy_lints/src/lib.register_lints.rs | 2 +- clippy_lints/src/lib.register_perf.rs | 2 +- clippy_lints/src/lib.rs | 4 +- .../src/{use_retain.rs => manual_retain.rs} | 12 +++--- .../{use_retain.fixed => manual_retain.fixed} | 2 +- tests/ui/{use_retain.rs => manual_retain.rs} | 2 +- ...use_retain.stderr => manual_retain.stderr} | 40 +++++++++---------- 9 files changed, 34 insertions(+), 34 deletions(-) rename clippy_lints/src/{use_retain.rs => manual_retain.rs} (97%) rename tests/ui/{use_retain.fixed => manual_retain.fixed} (99%) rename tests/ui/{use_retain.rs => manual_retain.rs} (99%) rename tests/ui/{use_retain.stderr => manual_retain.stderr} (89%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56143e1cdb9f..cff60a21c999 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3528,6 +3528,7 @@ Released 2018-09-13 [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or [`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains [`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid +[`manual_retain`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic [`manual_split_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once [`manual_str_repeat`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat @@ -3840,7 +3841,6 @@ Released 2018-09-13 [`unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used [`upper_case_acronyms`]: https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms [`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug -[`use_retain`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_retain [`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self [`used_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding [`useless_asref`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_asref diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 88e4c291b68f..696f0c6f6b36 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -136,6 +136,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(manual_bits::MANUAL_BITS), LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE), LintId::of(manual_rem_euclid::MANUAL_REM_EUCLID), + LintId::of(manual_retain::MANUAL_RETAIN), LintId::of(manual_strip::MANUAL_STRIP), LintId::of(map_clone::MAP_CLONE), LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN), @@ -336,7 +337,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(unwrap::PANICKING_UNWRAP), LintId::of(unwrap::UNNECESSARY_UNWRAP), LintId::of(upper_case_acronyms::UPPER_CASE_ACRONYMS), - LintId::of(use_retain::USE_RETAIN), LintId::of(useless_conversion::USELESS_CONVERSION), LintId::of(vec::USELESS_VEC), LintId::of(vec_init_then_push::VEC_INIT_THEN_PUSH), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 187786e80dae..f24c65fb35b9 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -255,6 +255,7 @@ store.register_lints(&[ manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE, manual_ok_or::MANUAL_OK_OR, manual_rem_euclid::MANUAL_REM_EUCLID, + manual_retain::MANUAL_RETAIN, manual_strip::MANUAL_STRIP, map_clone::MAP_CLONE, map_err_ignore::MAP_ERR_IGNORE, @@ -563,7 +564,6 @@ store.register_lints(&[ unwrap::UNNECESSARY_UNWRAP, unwrap_in_result::UNWRAP_IN_RESULT, upper_case_acronyms::UPPER_CASE_ACRONYMS, - use_retain::USE_RETAIN, use_self::USE_SELF, useless_conversion::USELESS_CONVERSION, vec::USELESS_VEC, diff --git a/clippy_lints/src/lib.register_perf.rs b/clippy_lints/src/lib.register_perf.rs index b9d8ec725411..2233b118849b 100644 --- a/clippy_lints/src/lib.register_perf.rs +++ b/clippy_lints/src/lib.register_perf.rs @@ -13,6 +13,7 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![ LintId::of(loops::MANUAL_MEMCPY), LintId::of(loops::MISSING_SPIN_LOOP), LintId::of(loops::NEEDLESS_COLLECT), + LintId::of(manual_retain::MANUAL_RETAIN), LintId::of(methods::EXPECT_FUN_CALL), LintId::of(methods::EXTEND_WITH_DRAIN), LintId::of(methods::ITER_NTH), @@ -26,7 +27,6 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![ LintId::of(slow_vector_initialization::SLOW_VECTOR_INITIALIZATION), LintId::of(types::BOX_COLLECTION), LintId::of(types::REDUNDANT_ALLOCATION), - LintId::of(use_retain::USE_RETAIN), LintId::of(vec::USELESS_VEC), LintId::of(vec_init_then_push::VEC_INIT_THEN_PUSH), ]) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 716038aa5220..a068d5ab522b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -283,6 +283,7 @@ mod manual_bits; mod manual_non_exhaustive; mod manual_ok_or; mod manual_rem_euclid; +mod manual_retain; mod manual_strip; mod map_clone; mod map_err_ignore; @@ -410,7 +411,6 @@ mod unused_unit; mod unwrap; mod unwrap_in_result; mod upper_case_acronyms; -mod use_retain; mod use_self; mod useless_conversion; mod vec; @@ -915,7 +915,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(read_zero_byte_vec::ReadZeroByteVec)); store.register_late_pass(|| Box::new(default_instead_of_iter_empty::DefaultIterEmpty)); store.register_late_pass(move || Box::new(manual_rem_euclid::ManualRemEuclid::new(msrv))); - store.register_late_pass(move || Box::new(use_retain::UseRetain::new(msrv))); + store.register_late_pass(move || Box::new(manual_retain::ManualRetain::new(msrv))); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/use_retain.rs b/clippy_lints/src/manual_retain.rs similarity index 97% rename from clippy_lints/src/use_retain.rs rename to clippy_lints/src/manual_retain.rs index 185201514ad3..450c6d343c05 100644 --- a/clippy_lints/src/use_retain.rs +++ b/clippy_lints/src/manual_retain.rs @@ -44,25 +44,25 @@ declare_clippy_lint! { /// vec.retain(|x| x % 2 == 0); /// ``` #[clippy::version = "1.63.0"] - pub USE_RETAIN, + pub MANUAL_RETAIN, perf, "`retain()` is simpler and the same functionalitys" } -pub struct UseRetain { +pub struct ManualRetain { msrv: Option, } -impl UseRetain { +impl ManualRetain { #[must_use] pub fn new(msrv: Option) -> Self { Self { msrv } } } -impl_lint_pass!(UseRetain => [USE_RETAIN]); +impl_lint_pass!(ManualRetain => [MANUAL_RETAIN]); -impl<'tcx> LateLintPass<'tcx> for UseRetain { +impl<'tcx> LateLintPass<'tcx> for ManualRetain { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { if let Some(parent_expr) = get_parent_expr(cx, expr) && let Assign(left_expr, collect_expr, _) = &parent_expr.kind @@ -170,7 +170,7 @@ fn suggest(cx: &LateContext<'_>, parent_expr: &hir::Expr<'_>, left_expr: &hir::E } { span_lint_and_sugg( cx, - USE_RETAIN, + MANUAL_RETAIN, parent_expr.span, "this expression can be written more simply using `.retain()`", "consider calling `.retain()` instead", diff --git a/tests/ui/use_retain.fixed b/tests/ui/manual_retain.fixed similarity index 99% rename from tests/ui/use_retain.fixed rename to tests/ui/manual_retain.fixed index 549b8a62c0c5..fba503a20667 100644 --- a/tests/ui/use_retain.fixed +++ b/tests/ui/manual_retain.fixed @@ -1,6 +1,6 @@ // run-rustfix #![feature(custom_inner_attributes)] -#![warn(clippy::use_retain)] +#![warn(clippy::manual_retain)] #![allow(unused)] use std::collections::BTreeMap; use std::collections::BTreeSet; diff --git a/tests/ui/use_retain.rs b/tests/ui/manual_retain.rs similarity index 99% rename from tests/ui/use_retain.rs rename to tests/ui/manual_retain.rs index 36c4176b8d5f..81a849fe7684 100644 --- a/tests/ui/use_retain.rs +++ b/tests/ui/manual_retain.rs @@ -1,6 +1,6 @@ // run-rustfix #![feature(custom_inner_attributes)] -#![warn(clippy::use_retain)] +#![warn(clippy::manual_retain)] #![allow(unused)] use std::collections::BTreeMap; use std::collections::BTreeSet; diff --git a/tests/ui/use_retain.stderr b/tests/ui/manual_retain.stderr similarity index 89% rename from tests/ui/use_retain.stderr rename to tests/ui/manual_retain.stderr index decd406f22a9..ec635919b48f 100644 --- a/tests/ui/use_retain.stderr +++ b/tests/ui/manual_retain.stderr @@ -1,19 +1,19 @@ error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:52:5 + --> $DIR/manual_retain.rs:52:5 | LL | btree_map = btree_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_map.retain(|k, _| k % 2 == 0)` | - = note: `-D clippy::use-retain` implied by `-D warnings` + = note: `-D clippy::manual-retain` implied by `-D warnings` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:53:5 + --> $DIR/manual_retain.rs:53:5 | LL | btree_map = btree_map.into_iter().filter(|(_, v)| v % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_map.retain(|_, &mut v| v % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:54:5 + --> $DIR/manual_retain.rs:54:5 | LL | / btree_map = btree_map LL | | .into_iter() @@ -22,37 +22,37 @@ LL | | .collect(); | |__________________^ help: consider calling `.retain()` instead: `btree_map.retain(|k, &mut v| (k % 2 == 0) && (v % 2 == 0))` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:76:5 + --> $DIR/manual_retain.rs:76:5 | LL | btree_set = btree_set.iter().filter(|&x| x % 2 == 0).copied().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:77:5 + --> $DIR/manual_retain.rs:77:5 | LL | btree_set = btree_set.iter().filter(|&x| x % 2 == 0).cloned().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:78:5 + --> $DIR/manual_retain.rs:78:5 | LL | btree_set = btree_set.into_iter().filter(|x| x % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `btree_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:108:5 + --> $DIR/manual_retain.rs:108:5 | LL | hash_map = hash_map.into_iter().filter(|(k, _)| k % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_map.retain(|k, _| k % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:109:5 + --> $DIR/manual_retain.rs:109:5 | LL | hash_map = hash_map.into_iter().filter(|(_, v)| v % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_map.retain(|_, &mut v| v % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:110:5 + --> $DIR/manual_retain.rs:110:5 | LL | / hash_map = hash_map LL | | .into_iter() @@ -61,61 +61,61 @@ LL | | .collect(); | |__________________^ help: consider calling `.retain()` instead: `hash_map.retain(|k, &mut v| (k % 2 == 0) && (v % 2 == 0))` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:131:5 + --> $DIR/manual_retain.rs:131:5 | LL | hash_set = hash_set.into_iter().filter(|x| x % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:132:5 + --> $DIR/manual_retain.rs:132:5 | LL | hash_set = hash_set.iter().filter(|&x| x % 2 == 0).copied().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:133:5 + --> $DIR/manual_retain.rs:133:5 | LL | hash_set = hash_set.iter().filter(|&x| x % 2 == 0).cloned().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `hash_set.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:162:5 + --> $DIR/manual_retain.rs:162:5 | LL | s = s.chars().filter(|&c| c != 'o').to_owned().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `s.retain(|c| c != 'o')` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:174:5 + --> $DIR/manual_retain.rs:174:5 | LL | vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:175:5 + --> $DIR/manual_retain.rs:175:5 | LL | vec = vec.iter().filter(|&x| x % 2 == 0).cloned().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:176:5 + --> $DIR/manual_retain.rs:176:5 | LL | vec = vec.into_iter().filter(|x| x % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:198:5 + --> $DIR/manual_retain.rs:198:5 | LL | vec_deque = vec_deque.iter().filter(|&x| x % 2 == 0).copied().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec_deque.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:199:5 + --> $DIR/manual_retain.rs:199:5 | LL | vec_deque = vec_deque.iter().filter(|&x| x % 2 == 0).cloned().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec_deque.retain(|x| x % 2 == 0)` error: this expression can be written more simply using `.retain()` - --> $DIR/use_retain.rs:200:5 + --> $DIR/manual_retain.rs:200:5 | LL | vec_deque = vec_deque.into_iter().filter(|x| x % 2 == 0).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.retain()` instead: `vec_deque.retain(|x| x % 2 == 0)` From 3a9c0ef8a1ed339e6e8ee3ed5a7282c0bef11632 Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Sat, 18 Jun 2022 18:56:46 +0900 Subject: [PATCH 09/10] fix for git rebase --- clippy_lints/src/manual_retain.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/manual_retain.rs b/clippy_lints/src/manual_retain.rs index 450c6d343c05..c35e1e021ef1 100644 --- a/clippy_lints/src/manual_retain.rs +++ b/clippy_lints/src/manual_retain.rs @@ -148,8 +148,8 @@ fn check_to_owned( fn suggest(cx: &LateContext<'_>, parent_expr: &hir::Expr<'_>, left_expr: &hir::Expr<'_>, filter_expr: &hir::Expr<'_>) { if let hir::ExprKind::MethodCall(_, [_, closure], _) = filter_expr.kind - && let hir::ExprKind::Closure(_, _, filter_body_id, ..) = closure.kind - && let filter_body = cx.tcx.hir().body(filter_body_id) + && let hir::ExprKind::Closure{ body, ..} = closure.kind + && let filter_body = cx.tcx.hir().body(body) && let [filter_params] = filter_body.params && let Some(sugg) = match filter_params.pat.kind { hir::PatKind::Binding(_, _, filter_param_ident, None) => { From 676af4530f08aeb2aaf561b4cc64e046e5f3e80f Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Mon, 27 Jun 2022 08:20:30 +0900 Subject: [PATCH 10/10] cargo dev update_lints --- README.md | 2 +- book/src/README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index edbc626e354b..c52f873adc06 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are over 500 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are over 550 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category. diff --git a/book/src/README.md b/book/src/README.md index de1f70d7e964..d941f8b65e8e 100644 --- a/book/src/README.md +++ b/book/src/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are over 500 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are over 550 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how