Skip to content

Commit 0a4dc7b

Browse files
tamaroningtamaroning
committed
update tests and lint
1 parent 2a38210 commit 0a4dc7b

File tree

4 files changed

+261
-95
lines changed

4 files changed

+261
-95
lines changed

‎clippy_lints/src/methods/map_then_identity_transformer.rs

Lines changed: 80 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,36 +12,91 @@ pub(super) fn check<'tcx>(
1212
cx: &LateContext<'_>,
1313
map_span: Span,
1414
map_name: &str,
15-
map_clos: &'tcx Expr<'_>,
15+
map_param: &'tcx Expr<'_>,
1616
transformer_name: &str,
17-
transformer_clos: &'tcx Expr<'_>,
17+
// Use the last parameter of the transformer because the transfomer may be `fold(_, _)`
18+
transformer_last_param: &'tcx Expr<'_>,
1819
) {
19-
if_chain!(
20-
// takes a closure of the `map`
21-
if let ExprKind::Closure(_, _, map_clos_body_id, _, _) = &map_clos.kind;
22-
// checks if the body of the closure of the `map` is an one-line expression
23-
let map_clos_val = &cx.tcx.hir().body(*map_clos_body_id).value;
24-
if is_one_line(cx, map_clos_val.span);
20+
match &transformer_last_param.kind {
21+
ExprKind::Closure(_, _, transformer_clos_body_id, _, _) => {
22+
match &map_param.kind {
23+
// map(Closure).<transformer>(Closure)
24+
ExprKind::Closure(_, _, map_clos_body_id, _, _) => {
25+
let map_clos_val = &cx.tcx.hir().body(*map_clos_body_id).value;
26+
if_chain!(
27+
// checks if the body of the closure of the `map` is an one-line expression
28+
if is_one_line(cx, map_clos_val.span);
29+
// checks if the parameter of the closure of the transformer appears once in its body
30+
if let Some(refd_param_span) = refd_param_span(cx, *transformer_clos_body_id);
31+
then {
32+
span_lint_and_then(
33+
cx,
34+
MAP_THEN_IDENTITY_TRANSFORMER,
35+
MultiSpan::from_span(map_span),
36+
&format!("this `{map_name}` can be collapsed into the `{transformer_name}`"),
37+
|diag| {
38+
let mut help_span = MultiSpan::from_spans(vec![map_clos_val.span, refd_param_span]);
39+
help_span.push_span_label(refd_param_span, "replace this variable".into());
40+
help_span.push_span_label(map_clos_val.span, "with this expression".into());
41+
diag.span_help(help_span, &format!("these `{map_name}` and `{transformer_name}` can be merged into a single `{transformer_name}`"));
42+
},
43+
);
44+
}
2545

26-
// takes a closure of the transformer
27-
if let ExprKind::Closure(_, _, transformer_clos_body_id, _, _) = &transformer_clos.kind;
28-
// checks if the parameter of the closure of the transformer appears once in its body
29-
if let Some(refd_param_span) = refd_param_span(cx, *transformer_clos_body_id);
30-
then {
31-
span_lint_and_then(
32-
cx,
33-
MAP_THEN_IDENTITY_TRANSFORMER,
34-
MultiSpan::from_span(map_span),
35-
&format!("this `{map_name}` can be collapsed into the `{transformer_name}`"),
36-
|diag| {
37-
let mut help_span = MultiSpan::from_spans(vec![map_clos_val.span, refd_param_span]);
38-
help_span.push_span_label(refd_param_span, "replace this variable".into());
39-
help_span.push_span_label(map_clos_val.span, "with this expression".into());
40-
diag.span_help(help_span, &format!("these `{map_name}` and `{transformer_name}` can be merged into a single `{transformer_name}`"));
46+
);
47+
},
48+
// map(Path).<transformer>(Closure)
49+
ExprKind::Path(_) => {
50+
if_chain!(
51+
// checks if the parameter of the `map` fits within one line
52+
if is_one_line(cx, map_param.span);
53+
// checks if the parameter of the closure of the transformer appears once in its body
54+
if let Some(refd_param_span) = refd_param_span(cx, *transformer_clos_body_id);
55+
then {
56+
span_lint_and_then(
57+
cx,
58+
MAP_THEN_IDENTITY_TRANSFORMER,
59+
MultiSpan::from_span(map_span),
60+
&format!("this `{map_name}` can be collapsed into the `{transformer_name}`"),
61+
|diag| {
62+
let mut help_span = MultiSpan::from_spans(vec![map_param.span, refd_param_span]);
63+
help_span.push_span_label(map_param.span, "apply this".into());
64+
help_span.push_span_label(refd_param_span, "to this variable".into());
65+
diag.span_help(help_span, &format!("these `{map_name}` and `{transformer_name}` can be merged into a single `{transformer_name}`"));
66+
},
67+
);
68+
}
69+
70+
);
4171
},
72+
_ => (),
73+
}
74+
},
75+
// map(Path).<transformer>(Path) or map(Closure).<transformer>(Path)
76+
ExprKind::Path(_) => {
77+
if_chain!(
78+
// checks if the parameter of the `map` fits within one line
79+
if is_one_line(cx, map_param.span);
80+
then {
81+
span_lint_and_then(
82+
cx,
83+
MAP_THEN_IDENTITY_TRANSFORMER,
84+
MultiSpan::from_span(map_span),
85+
&format!("this `{map_name}` can be collapsed into the `{transformer_name}`"),
86+
|diag| {
87+
let mut help_span = MultiSpan::from_spans(
88+
vec![map_param.span, transformer_last_param.span]
89+
);
90+
help_span.push_span_label(map_param.span, format!("and use this in the `{transformer_name}`"));
91+
help_span.push_span_label(transformer_last_param.span, "change this to a closure".into());
92+
diag.span_help(help_span, &format!("these `{map_name}` and `{transformer_name}` can be merged into a single `{transformer_name}`"));
93+
},
94+
);
95+
}
4296
);
43-
}
44-
);
97+
},
98+
_ => (),
99+
}
45100
}
46101

47102
// Given a closure `|.., x| y`, checks if `x` is referenced just exactly once in `y` and returns

‎clippy_lints/src/methods/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2288,7 +2288,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
22882288
("add" | "offset" | "sub" | "wrapping_offset" | "wrapping_add" | "wrapping_sub", [_arg]) => {
22892289
zst_offset::check(cx, expr, recv);
22902290
},
2291-
(name @ ("all" | "any" | "find" | "find_map" | "position"), [arg]) => {
2291+
(name @ ("all" | "any" | "find_map" | "position"), [arg]) => {
22922292
if let Some((name2 @ "map", [_, arg2], span2)) = method_call(recv) {
22932293
map_then_identity_transformer::check(cx, span2, name2, arg2, name, arg);
22942294
}

‎tests/ui/map_then_identity_transformer.rs

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,33 +4,50 @@
44
fn main() {
55
let a = [1, 2, 3];
66

7+
// _.map(Path).transformerp(Closure)
78
// should lint
8-
let _ = a.into_iter().map(|x| func1(x)).all(|y| y > 0);
9-
let _ = a.into_iter().map(|x| func1(x)).any(|y| y > 0);
10-
let _ = a.into_iter().map(|x| func1(x)).find(|&y| y > 0);
11-
let _ = a.into_iter().map(|x| func1(x)).find_map(|y| y.checked_add(1));
12-
let _ = a.into_iter().map(|x| func1(x)).flat_map(|y| func2(y));
13-
let _ = a.into_iter().map(|x| func1(x)).filter_map(|y| y.checked_add(1));
14-
let _ = a.into_iter().map(|x| func1(x)).fold(1, |pd, x| pd * x + 1);
15-
let _ = a.into_iter().map(|x| func1(x)).map(|y| func1(y));
16-
let _ = a.into_iter().map(|x| func1(x)).position(|y| y > 0);
9+
let _ = a.into_iter().map(func1).all(|y| y > 0);
10+
let _ = a.into_iter().map(func1).any(|y| y > 0);
11+
let _ = a.into_iter().map(func1).find_map(|y| y.checked_add(1));
12+
let _ = a.into_iter().map(func1).flat_map(|y| func2(y));
13+
let _ = a.into_iter().map(func1).filter_map(|y| y.checked_add(1));
14+
let _ = a.into_iter().map(func1).fold(1, |pd, x| pd * x + 1);
15+
let _ = a.into_iter().map(func1).map(|y| func1(y));
16+
let _ = a.into_iter().map(func1).position(|y| y > 0);
1717

18+
// _.map(Path).transformer(Closure)
1819
// should lint
19-
let _ = a.into_iter().map(|x| func1(x) * func1(x)).all(|y| y > 0);
20+
let _ = a.into_iter().map(func1).all(func3);
21+
let _ = a.into_iter().map(func1).any(func3);
2022

21-
// should not lint
22-
let _ = a.into_iter().map(|x| func1(x)).all(|y| func1(y) * func1(y) > 0);
23-
let _ = a.into_iter().map(|x| func1(x)).any(|y| func1(y) * func1(y) > 0);
24-
let _ = a.into_iter().map(|x| func1(x)).fold(1, |pd, x| pd * x * x);
23+
// _.map(Path).transformer(Closure)
24+
// should lint
25+
let _ = a.into_iter().map(|x| func1(x) + 1).all(|y| y > 0);
26+
let _ = a.into_iter().map(|x| func1(x) * func1(x)).any(|y| y > 0);
27+
let _ = a.into_iter().map(|x| func1(x) * func1(x)).fold(1, |pd, x| pd * x + 1);
28+
29+
// _.map(Closure).transformer(Path)
30+
// should lint
31+
let _ = a.into_iter().map(|x| func1(x) + 1).all(func3);
32+
let _ = a.into_iter().map(|x| func1(x) + 1).any(func3);
33+
let _ = a.into_iter().map(|x| func1(x) + 1).fold(1, func4);
2534

26-
// should not lint
35+
// should not when the transformer is `find`
36+
let _ = a.into_iter().map(func1).find(|&y| y > 0);
37+
38+
// should not lint this because the last param of the closure occurs more than once
39+
let _ = a.into_iter().map(func1).all(|y| func1(y) * func1(y) > 10);
40+
let _ = a.into_iter().map(|x| func1(x) + 1).any(|y| func1(y) * func1(y) > 10);
41+
let _ = a.into_iter().map(func1).fold(1, |pd, x| pd * x * x);
42+
43+
// should not lint this because the param of the `map` is not within one line
2744
let _ = a
2845
.into_iter()
2946
.map(|x| {
3047
// This comment has no special meaning:)
3148
x * x
3249
})
33-
.any(|y| y > 10);
50+
.any(func3);
3451
}
3552

3653
fn func1(a: i32) -> i32 {
@@ -40,3 +57,11 @@ fn func1(a: i32) -> i32 {
4057
fn func2(a: i32) -> Vec<i32> {
4158
unimplemented!();
4259
}
60+
61+
fn func3(a: i32) -> bool {
62+
unimplemented!();
63+
}
64+
65+
fn func4(a: i32, b: i32) -> i32 {
66+
unimplemented!();
67+
}

0 commit comments

Comments
 (0)