Skip to content

Commit 1f94ae4

Browse files
committed
feat(new lint): new lint use_retain
1 parent 542d474 commit 1f94ae4

File tree

10 files changed

+723
-0
lines changed

10 files changed

+723
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3836,6 +3836,7 @@ Released 2018-09-13
38363836
[`unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used
38373837
[`upper_case_acronyms`]: https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
38383838
[`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug
3839+
[`use_retain`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_retain
38393840
[`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self
38403841
[`used_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding
38413842
[`useless_asref`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_asref

clippy_lints/src/lib.register_all.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
332332
LintId::of(unwrap::PANICKING_UNWRAP),
333333
LintId::of(unwrap::UNNECESSARY_UNWRAP),
334334
LintId::of(upper_case_acronyms::UPPER_CASE_ACRONYMS),
335+
LintId::of(use_retain::USE_RETAIN),
335336
LintId::of(useless_conversion::USELESS_CONVERSION),
336337
LintId::of(vec::USELESS_VEC),
337338
LintId::of(vec_init_then_push::VEC_INIT_THEN_PUSH),

clippy_lints/src/lib.register_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,7 @@ store.register_lints(&[
559559
unwrap::UNNECESSARY_UNWRAP,
560560
unwrap_in_result::UNWRAP_IN_RESULT,
561561
upper_case_acronyms::UPPER_CASE_ACRONYMS,
562+
use_retain::USE_RETAIN,
562563
use_self::USE_SELF,
563564
useless_conversion::USELESS_CONVERSION,
564565
vec::USELESS_VEC,

clippy_lints/src/lib.register_style.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
115115
LintId::of(unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
116116
LintId::of(unused_unit::UNUSED_UNIT),
117117
LintId::of(upper_case_acronyms::UPPER_CASE_ACRONYMS),
118+
LintId::of(use_retain::USE_RETAIN),
118119
LintId::of(write::PRINTLN_EMPTY_STRING),
119120
LintId::of(write::PRINT_LITERAL),
120121
LintId::of(write::PRINT_WITH_NEWLINE),

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,7 @@ mod unused_unit;
407407
mod unwrap;
408408
mod unwrap_in_result;
409409
mod upper_case_acronyms;
410+
mod use_retain;
410411
mod use_self;
411412
mod useless_conversion;
412413
mod vec;
@@ -907,6 +908,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
907908
store.register_late_pass(|| Box::new(swap_ptr_to_ref::SwapPtrToRef));
908909
store.register_late_pass(|| Box::new(mismatching_type_param_order::TypeParamMismatch));
909910
store.register_late_pass(|| Box::new(as_underscore::AsUnderscore));
911+
store.register_late_pass(|| Box::new(use_retain::UseRetain));
910912
// add lints here, do not remove this comment, it's used in `new_lint`
911913
}
912914

clippy_lints/src/use_retain.rs

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet;
3+
use clippy_utils::ty::is_type_diagnostic_item;
4+
use clippy_utils::{get_parent_expr, match_def_path, paths, SpanlessEq};
5+
use rustc_errors::Applicability;
6+
use rustc_hir as hir;
7+
use rustc_hir::def_id::DefId;
8+
use rustc_hir::ExprKind::Assign;
9+
use rustc_lint::{LateContext, LateLintPass};
10+
use rustc_session::{declare_lint_pass, declare_tool_lint};
11+
use rustc_span::symbol::sym;
12+
13+
const ACCEPTABLE_METHODS: [&[&str]; 4] = [
14+
&paths::HASHSET_ITER,
15+
&paths::BTREESET_ITER,
16+
&paths::SLICE_INTO,
17+
&paths::VEC_DEQUE_ITER,
18+
];
19+
const ACCEPTABLE_TYPES: [rustc_span::Symbol; 6] = [
20+
sym::BTreeSet,
21+
sym::BTreeMap,
22+
sym::HashSet,
23+
sym::HashMap,
24+
sym::Vec,
25+
sym::VecDeque,
26+
];
27+
28+
declare_clippy_lint! {
29+
/// ### What it does
30+
/// Checks for code to be replaced by `.retain()`.
31+
/// ### Why is this bad?
32+
/// `.retain()` is simpler.
33+
/// ### Example
34+
/// ```rust
35+
/// let mut vec = vec![0, 1, 2];
36+
/// vec = vec.iter().filter(|&x| x % 2 == 0).copied().collect();
37+
/// vec = vec.into_iter().filter(|x| x % 2 == 0).collect();
38+
/// ```
39+
/// Use instead:
40+
/// ```rust
41+
/// let mut vec = vec![0, 1, 2];
42+
/// vec.retain(|x| x % 2 == 0);
43+
/// ```
44+
#[clippy::version = "1.63.0"]
45+
pub USE_RETAIN,
46+
style,
47+
"`retain()` is simpler and the same functionalitys"
48+
}
49+
declare_lint_pass!(UseRetain => [USE_RETAIN]);
50+
51+
impl<'tcx> LateLintPass<'tcx> for UseRetain {
52+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
53+
if_chain! {
54+
if let Some(parent_expr) = get_parent_expr(cx, expr);
55+
if let Assign(left_expr, collect_expr, _) = &parent_expr.kind;
56+
if let hir::ExprKind::MethodCall(seg, _, _) = &collect_expr.kind;
57+
if seg.args.is_none();
58+
59+
if let hir::ExprKind::MethodCall(_, [target_expr], _) = &collect_expr.kind;
60+
if let Some(collect_def_id) = cx.typeck_results().type_dependent_def_id(collect_expr.hir_id);
61+
if match_def_path(cx, collect_def_id, &paths::CORE_ITER_COLLECT);
62+
63+
then {
64+
check_into_iter(cx, parent_expr, left_expr, target_expr);
65+
check_iter(cx, parent_expr, left_expr, target_expr);
66+
check_to_owned(cx, parent_expr, left_expr, target_expr);
67+
}
68+
}
69+
}
70+
}
71+
72+
fn check_into_iter(
73+
cx: &LateContext<'_>,
74+
parent_expr: &hir::Expr<'_>,
75+
left_expr: &hir::Expr<'_>,
76+
target_expr: &hir::Expr<'_>,
77+
) {
78+
if_chain! {
79+
if let hir::ExprKind::MethodCall(_, [into_iter_expr, _], _) = &target_expr.kind;
80+
if let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id);
81+
if match_def_path(cx, filter_def_id, &paths::CORE_ITER_FILTER);
82+
83+
if let hir::ExprKind::MethodCall(_, [struct_expr], _) = &into_iter_expr.kind;
84+
if let Some(into_iter_def_id) = cx.typeck_results().type_dependent_def_id(into_iter_expr.hir_id);
85+
if match_def_path(cx, into_iter_def_id, &paths::CORE_ITER_INTO_ITER);
86+
if match_acceptable_type(cx, left_expr);
87+
88+
if SpanlessEq::new(cx).eq_expr(left_expr, struct_expr);
89+
90+
then {
91+
suggest(cx, parent_expr, left_expr, target_expr);
92+
}
93+
}
94+
}
95+
96+
fn check_iter(
97+
cx: &LateContext<'_>,
98+
parent_expr: &hir::Expr<'_>,
99+
left_expr: &hir::Expr<'_>,
100+
target_expr: &hir::Expr<'_>,
101+
) {
102+
if_chain! {
103+
if let hir::ExprKind::MethodCall(_, [filter_expr], _) = &target_expr.kind;
104+
if let Some(copied_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id);
105+
if match_def_path(cx, copied_def_id, &paths::CORE_ITER_COPIED);
106+
107+
if let hir::ExprKind::MethodCall(_, [iter_expr, _], _) = &filter_expr.kind;
108+
if let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(filter_expr.hir_id);
109+
if match_def_path(cx, filter_def_id, &paths::CORE_ITER_FILTER);
110+
111+
if let hir::ExprKind::MethodCall(_, [struct_expr], _) = &iter_expr.kind;
112+
if let Some(iter_expr_def_id) = cx.typeck_results().type_dependent_def_id(iter_expr.hir_id);
113+
if match_acceptable_def_path(cx, iter_expr_def_id);
114+
if match_acceptable_type(cx, left_expr);
115+
if SpanlessEq::new(cx).eq_expr(left_expr, struct_expr);
116+
117+
then {
118+
suggest(cx, parent_expr, left_expr, filter_expr);
119+
}
120+
}
121+
}
122+
123+
fn check_to_owned(
124+
cx: &LateContext<'_>,
125+
parent_expr: &hir::Expr<'_>,
126+
left_expr: &hir::Expr<'_>,
127+
target_expr: &hir::Expr<'_>,
128+
) {
129+
if_chain! {
130+
if let hir::ExprKind::MethodCall(_, [filter_expr], _) = &target_expr.kind;
131+
if let Some(to_owned_def_id) = cx.typeck_results().type_dependent_def_id(target_expr.hir_id);
132+
if match_def_path(cx, to_owned_def_id, &paths::TO_OWNED_METHOD);
133+
134+
if let hir::ExprKind::MethodCall(_, [chars_expr, _], _) = &filter_expr.kind;
135+
if let Some(filter_def_id) = cx.typeck_results().type_dependent_def_id(filter_expr.hir_id);
136+
if match_def_path(cx, filter_def_id, &paths::CORE_ITER_FILTER);
137+
138+
if let hir::ExprKind::MethodCall(_, [str_expr], _) = &chars_expr.kind;
139+
if let Some(chars_expr_def_id) = cx.typeck_results().type_dependent_def_id(chars_expr.hir_id);
140+
if match_def_path(cx, chars_expr_def_id, &paths::STR_CHARS);
141+
142+
let ty = cx.typeck_results().expr_ty(str_expr).peel_refs();
143+
if is_type_diagnostic_item(cx, ty, sym::String);
144+
if SpanlessEq::new(cx).eq_expr(left_expr, str_expr);
145+
146+
then {
147+
suggest(cx, parent_expr, left_expr, filter_expr);
148+
}
149+
}
150+
}
151+
152+
fn suggest(cx: &LateContext<'_>, parent_expr: &hir::Expr<'_>, left_expr: &hir::Expr<'_>, filter_expr: &hir::Expr<'_>) {
153+
if_chain! {
154+
if let hir::ExprKind::MethodCall(_, [_, closure], _) = filter_expr.kind;
155+
if let hir::ExprKind::Closure(_, _, filter_body_id, ..) = closure.kind;
156+
let filter_body = cx.tcx.hir().body(filter_body_id);
157+
if let [filter_params] = filter_body.params;
158+
then {
159+
if let Some(sugg) = match filter_params.pat.kind {
160+
hir::PatKind::Binding(_, _, filter_param_ident, None) => {
161+
Some(format!("{}.retain(|{}| {})", snippet(cx, left_expr.span, ".."), filter_param_ident, snippet(cx, filter_body.value.span, "..")))
162+
},
163+
hir::PatKind::Tuple([key_pat, value_pat], _) => {
164+
make_sugg(cx, key_pat, value_pat, left_expr, filter_body)
165+
},
166+
hir::PatKind::Ref(pat, _) => {
167+
match pat.kind {
168+
hir::PatKind::Binding(_, _, filter_param_ident, None) => {
169+
Some(format!("{}.retain(|{}| {})", snippet(cx, left_expr.span, ".."), filter_param_ident, snippet(cx, filter_body.value.span, "..")))
170+
},
171+
_ => None
172+
}
173+
},
174+
_ => None
175+
} {
176+
span_lint_and_sugg(
177+
cx,
178+
USE_RETAIN,
179+
parent_expr.span,
180+
"this expression can be written more simply using `.retain()`",
181+
"consider calling `.retain()` instead",
182+
sugg,
183+
Applicability::MachineApplicable
184+
);
185+
}
186+
}
187+
}
188+
}
189+
190+
fn make_sugg(
191+
cx: &LateContext<'_>,
192+
key_pat: &rustc_hir::Pat<'_>,
193+
value_pat: &rustc_hir::Pat<'_>,
194+
left_expr: &hir::Expr<'_>,
195+
filter_body: &hir::Body<'_>,
196+
) -> Option<String> {
197+
match (&key_pat.kind, &value_pat.kind) {
198+
(hir::PatKind::Binding(_, _, key_param_ident, None), hir::PatKind::Binding(_, _, value_param_ident, None)) => {
199+
Some(format!(
200+
"{}.retain(|{}, &mut {}| {})",
201+
snippet(cx, left_expr.span, ".."),
202+
key_param_ident,
203+
value_param_ident,
204+
snippet(cx, filter_body.value.span, "..")
205+
))
206+
},
207+
(hir::PatKind::Binding(_, _, key_param_ident, None), hir::PatKind::Wild) => Some(format!(
208+
"{}.retain(|{}, _| {})",
209+
snippet(cx, left_expr.span, ".."),
210+
key_param_ident,
211+
snippet(cx, filter_body.value.span, "..")
212+
)),
213+
(hir::PatKind::Wild, hir::PatKind::Binding(_, _, value_param_ident, None)) => Some(format!(
214+
"{}.retain(|_, &mut {}| {})",
215+
snippet(cx, left_expr.span, ".."),
216+
value_param_ident,
217+
snippet(cx, filter_body.value.span, "..")
218+
)),
219+
_ => None,
220+
}
221+
}
222+
223+
fn match_acceptable_def_path(cx: &LateContext<'_>, collect_def_id: DefId) -> bool {
224+
return ACCEPTABLE_METHODS
225+
.iter()
226+
.any(|&method| match_def_path(cx, collect_def_id, method));
227+
}
228+
229+
fn match_acceptable_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
230+
let expr_ty = cx.typeck_results().expr_ty(expr).peel_refs();
231+
return ACCEPTABLE_TYPES
232+
.iter()
233+
.any(|&ty| is_type_diagnostic_item(cx, expr_ty, ty));
234+
}

clippy_utils/src/paths.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,13 @@ pub const ASREF_TRAIT: [&str; 3] = ["core", "convert", "AsRef"];
2121
pub const BTREEMAP_CONTAINS_KEY: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "contains_key"];
2222
pub const BTREEMAP_ENTRY: [&str; 6] = ["alloc", "collections", "btree", "map", "entry", "Entry"];
2323
pub const BTREEMAP_INSERT: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "insert"];
24+
pub const BTREESET_ITER: [&str; 6] = ["alloc", "collections", "btree", "set", "BTreeSet", "iter"];
2425
pub const CLONE_TRAIT_METHOD: [&str; 4] = ["core", "clone", "Clone", "clone"];
2526
pub const COW: [&str; 3] = ["alloc", "borrow", "Cow"];
27+
pub const CORE_ITER_COLLECT: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "collect"];
28+
pub const CORE_ITER_COPIED: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "copied"];
29+
pub const CORE_ITER_FILTER: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "filter"];
30+
pub const CORE_ITER_INTO_ITER: [&str; 6] = ["core", "iter", "traits", "collect", "IntoIterator", "into_iter"];
2631
pub const CSTRING_AS_C_STR: [&str; 5] = ["alloc", "ffi", "c_str", "CString", "as_c_str"];
2732
pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"];
2833
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
5055
pub const HASHMAP_CONTAINS_KEY: [&str; 6] = ["std", "collections", "hash", "map", "HashMap", "contains_key"];
5156
pub const HASHMAP_ENTRY: [&str; 5] = ["std", "collections", "hash", "map", "Entry"];
5257
pub const HASHMAP_INSERT: [&str; 6] = ["std", "collections", "hash", "map", "HashMap", "insert"];
58+
pub const HASHSET_ITER: [&str; 6] = ["std", "collections", "hash", "set", "HashSet", "iter"];
5359
#[cfg(feature = "internal")]
5460
pub const IDENT: [&str; 3] = ["rustc_span", "symbol", "Ident"];
5561
#[cfg(feature = "internal")]
@@ -144,6 +150,7 @@ pub const SLICE_FROM_RAW_PARTS: [&str; 4] = ["core", "slice", "raw", "from_raw_p
144150
pub const SLICE_FROM_RAW_PARTS_MUT: [&str; 4] = ["core", "slice", "raw", "from_raw_parts_mut"];
145151
pub const SLICE_GET: [&str; 4] = ["core", "slice", "<impl [T]>", "get"];
146152
pub const SLICE_INTO_VEC: [&str; 4] = ["alloc", "slice", "<impl [T]>", "into_vec"];
153+
pub const SLICE_INTO: [&str; 4] = ["core", "slice", "<impl [T]>", "iter"];
147154
pub const SLICE_ITER: [&str; 4] = ["core", "slice", "iter", "Iter"];
148155
pub const STDERR: [&str; 4] = ["std", "io", "stdio", "stderr"];
149156
pub const STDOUT: [&str; 4] = ["std", "io", "stdio", "stdout"];
@@ -153,6 +160,7 @@ pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_s
153160
pub const STRING_AS_STR: [&str; 4] = ["alloc", "string", "String", "as_str"];
154161
pub const STRING_NEW: [&str; 4] = ["alloc", "string", "String", "new"];
155162
pub const STR_BYTES: [&str; 4] = ["core", "str", "<impl str>", "bytes"];
163+
pub const STR_CHARS: [&str; 4] = ["core", "str", "<impl str>", "chars"];
156164
pub const STR_ENDS_WITH: [&str; 4] = ["core", "str", "<impl str>", "ends_with"];
157165
pub const STR_FROM_UTF8: [&str; 4] = ["core", "str", "converts", "from_utf8"];
158166
pub const STR_LEN: [&str; 4] = ["core", "str", "<impl str>", "len"];
@@ -178,6 +186,7 @@ pub const TOKIO_IO_ASYNCWRITEEXT: [&str; 5] = ["tokio", "io", "util", "async_wri
178186
pub const TRY_FROM: [&str; 4] = ["core", "convert", "TryFrom", "try_from"];
179187
pub const VEC_AS_MUT_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_mut_slice"];
180188
pub const VEC_AS_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_slice"];
189+
pub const VEC_DEQUE_ITER: [&str; 5] = ["alloc", "collections", "vec_deque", "VecDeque", "iter"];
181190
pub const VEC_FROM_ELEM: [&str; 3] = ["alloc", "vec", "from_elem"];
182191
pub const VEC_NEW: [&str; 4] = ["alloc", "vec", "Vec", "new"];
183192
pub const VEC_RESIZE: [&str; 4] = ["alloc", "vec", "Vec", "resize"];

0 commit comments

Comments
 (0)