Skip to content

Commit 73d40e2

Browse files
ada4aada4a
committed
create the different suggestions in each branch separately
- the interplay between `apply` and the contents of `sugg` was a bit confusing imo - we were calculating `method_requiring_mut` even if unneeded (we could've just calculated it in the `if !apply` branch, but still) - this allows to give a more descriptive message in the case of 2 machine-applicable suggestions, and match on that in the tests
1 parent 0e7fb67 commit 73d40e2

File tree

4 files changed

+73
-48
lines changed

4 files changed

+73
-48
lines changed

‎clippy_lints/src/methods/map_identity.rs‎

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
1-
use clippy_utils::diagnostics::span_lint_and_then;
1+
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
2+
use clippy_utils::source::snippet_with_applicability;
23
use clippy_utils::ty::is_type_diagnostic_item;
34
use clippy_utils::{is_expr_untyped_identity_function, is_mutable, is_trait_method, path_to_local_with_projections};
45
use rustc_errors::Applicability;
56
use rustc_hir::{self as hir, ExprKind, Node, PatKind};
6-
use rustc_lint::LateContext;
7+
use rustc_lint::{LateContext, LintContext};
78
use rustc_span::{Span, Symbol, sym};
89

910
use super::MAP_IDENTITY;
1011

12+
const MSG: &str = "unnecessary map of the identity function";
13+
1114
pub(super) fn check(
1215
cx: &LateContext<'_>,
1316
expr: &hir::Expr<'_>,
@@ -24,7 +27,7 @@ pub(super) fn check(
2427
&& is_expr_untyped_identity_function(cx, map_arg)
2528
&& let Some(call_span) = expr.span.trim_start(caller.span)
2629
{
27-
let mut sugg = vec![(call_span, String::new())];
30+
let main_sugg = (call_span, String::new());
2831
let mut app = Applicability::MachineApplicable;
2932

3033
let needs_to_be_mutable = cx.typeck_results().expr_ty_adjusted(expr).is_mutable_ptr();
@@ -34,39 +37,51 @@ pub(super) fn check(
3437
&& let PatKind::Binding(_, _, ident, _) = pat.kind
3538
{
3639
// We can reach the binding -- suggest making it mutable
37-
sugg.push((ident.span.shrink_to_lo(), String::from("mut ")));
40+
let suggs = vec![main_sugg, (ident.span.shrink_to_lo(), String::from("mut "))];
41+
42+
let ident = snippet_with_applicability(cx.sess(), ident.span, "_", &mut app);
43+
44+
span_lint_and_then(cx, MAP_IDENTITY, call_span, MSG, |diag| {
45+
diag.multipart_suggestion(
46+
format!("remove the call to `{name}`, and make `{ident}` mutable"),
47+
suggs,
48+
app,
49+
);
50+
});
3851
} else {
3952
// If we can't make the binding mutable, prevent the suggestion from being automatically applied,
4053
// and add a complementary help message.
4154
app = Applicability::Unspecified;
42-
}
43-
}
4455

45-
let method_requiring_mut = if let Node::Expr(expr) = cx.tcx.parent_hir_node(expr.hir_id)
46-
&& let ExprKind::MethodCall(method, ..) = expr.kind
47-
{
48-
Some(method.ident)
49-
} else {
50-
None
51-
};
56+
let method_requiring_mut = if let Node::Expr(expr) = cx.tcx.parent_hir_node(expr.hir_id)
57+
&& let ExprKind::MethodCall(method, ..) = expr.kind
58+
{
59+
Some(method.ident)
60+
} else {
61+
None
62+
};
5263

53-
span_lint_and_then(
54-
cx,
55-
MAP_IDENTITY,
56-
call_span,
57-
"unnecessary map of the identity function",
58-
|diag| {
59-
diag.multipart_suggestion(format!("remove the call to `{name}`"), sugg, app);
64+
span_lint_and_then(cx, MAP_IDENTITY, call_span, MSG, |diag| {
65+
diag.span_suggestion(main_sugg.0, format!("remove the call to `{name}`"), main_sugg.1, app);
6066

61-
if app != Applicability::MachineApplicable {
6267
let note = if let Some(method_requiring_mut) = method_requiring_mut {
6368
format!("this must be made mutable to use `{method_requiring_mut}`")
6469
} else {
6570
"this must be made mutable".to_string()
6671
};
6772
diag.span_note(caller.span, note);
68-
}
69-
},
70-
);
73+
});
74+
}
75+
} else {
76+
span_lint_and_sugg(
77+
cx,
78+
MAP_IDENTITY,
79+
main_sugg.0,
80+
MSG,
81+
format!("remove the call to `{name}`"),
82+
main_sugg.1,
83+
app,
84+
);
85+
}
7186
}
7287
}

‎tests/ui/map_identity.fixed‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
//@require-annotations-for-level: ERROR
12
#![warn(clippy::map_identity)]
23
#![allow(clippy::needless_return, clippy::disallowed_names)]
34

@@ -78,23 +79,27 @@ fn issue13904() {
7879
let mut it = [1, 2, 3].into_iter();
7980
let _ = it.next();
8081
//~^ map_identity
82+
//~| HELP: remove the call to `map`, and make `it` mutable
8183

8284
// lint
8385
let mut index = [1, 2, 3].into_iter();
8486
let mut subindex = (index.by_ref().take(3), 42);
8587
let _ = subindex.0.next();
8688
//~^ map_identity
89+
//~| HELP: remove the call to `map`, and make `subindex` mutable
8790

8891
// lint
8992
#[allow(unused_mut)]
9093
let mut it = [1, 2, 3].into_iter();
9194
let _ = it.next();
9295
//~^ map_identity
96+
//~| HELP: remove the call to `map`
9397

9498
// lint
9599
let it = [1, 2, 3].into_iter();
96100
let _ = { it }.next();
97101
//~^ map_identity
102+
//~| HELP: remove the call to `map`
98103
}
99104

100105
// same as `issue11764`, but for arrays

‎tests/ui/map_identity.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
//@require-annotations-for-level: ERROR
12
#![warn(clippy::map_identity)]
23
#![allow(clippy::needless_return, clippy::disallowed_names)]
34

@@ -84,23 +85,27 @@ fn issue13904() {
8485
let it = [1, 2, 3].into_iter();
8586
let _ = it.map(|x| x).next();
8687
//~^ map_identity
88+
//~| HELP: remove the call to `map`, and make `it` mutable
8789

8890
// lint
8991
let mut index = [1, 2, 3].into_iter();
9092
let subindex = (index.by_ref().take(3), 42);
9193
let _ = subindex.0.map(|n| n).next();
9294
//~^ map_identity
95+
//~| HELP: remove the call to `map`, and make `subindex` mutable
9396

9497
// lint
9598
#[allow(unused_mut)]
9699
let mut it = [1, 2, 3].into_iter();
97100
let _ = it.map(|x| x).next();
98101
//~^ map_identity
102+
//~| HELP: remove the call to `map`
99103

100104
// lint
101105
let it = [1, 2, 3].into_iter();
102106
let _ = { it }.map(|x| x).next();
103107
//~^ map_identity
108+
//~| HELP: remove the call to `map`
104109
}
105110

106111
// same as `issue11764`, but for arrays

‎tests/ui/map_identity.stderr‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: unnecessary map of the identity function
2-
--> tests/ui/map_identity.rs:7:47
2+
--> tests/ui/map_identity.rs:8:47
33
|
44
LL | let _: Vec<_> = x.iter().map(not_identity).map(|x| return x).collect();
55
| ^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
@@ -8,25 +8,25 @@ LL | let _: Vec<_> = x.iter().map(not_identity).map(|x| return x).collect();
88
= help: to override `-D warnings` add `#[allow(clippy::map_identity)]`
99

1010
error: unnecessary map of the identity function
11-
--> tests/ui/map_identity.rs:9:57
11+
--> tests/ui/map_identity.rs:10:57
1212
|
1313
LL | let _: Vec<_> = x.iter().map(std::convert::identity).map(|y| y).collect();
1414
| ^^^^^^^^^^^ help: remove the call to `map`
1515

1616
error: unnecessary map of the identity function
17-
--> tests/ui/map_identity.rs:9:29
17+
--> tests/ui/map_identity.rs:10:29
1818
|
1919
LL | let _: Vec<_> = x.iter().map(std::convert::identity).map(|y| y).collect();
2020
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
2121

2222
error: unnecessary map of the identity function
23-
--> tests/ui/map_identity.rs:12:32
23+
--> tests/ui/map_identity.rs:13:32
2424
|
2525
LL | let _: Option<u8> = Some(3).map(|x| x);
2626
| ^^^^^^^^^^^ help: remove the call to `map`
2727

2828
error: unnecessary map of the identity function
29-
--> tests/ui/map_identity.rs:14:36
29+
--> tests/ui/map_identity.rs:15:36
3030
|
3131
LL | let _: Result<i8, f32> = Ok(-3).map(|x| {
3232
| ____________________________________^
@@ -36,19 +36,19 @@ LL | | });
3636
| |______^ help: remove the call to `map`
3737

3838
error: unnecessary map of the identity function
39-
--> tests/ui/map_identity.rs:25:36
39+
--> tests/ui/map_identity.rs:26:36
4040
|
4141
LL | let _: Result<u32, u32> = Ok(1).map_err(|a| a);
4242
| ^^^^^^^^^^^^^^^ help: remove the call to `map_err`
4343

4444
error: unnecessary map of the identity function
45-
--> tests/ui/map_identity.rs:36:22
45+
--> tests/ui/map_identity.rs:37:22
4646
|
4747
LL | let _ = x.clone().map(|(x, y)| (x, y));
4848
| ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
4949

5050
error: unnecessary map of the identity function
51-
--> tests/ui/map_identity.rs:38:22
51+
--> tests/ui/map_identity.rs:39:22
5252
|
5353
LL | let _ = x.clone().map(|(x, y)| {
5454
| ______________________^
@@ -58,97 +58,97 @@ LL | | });
5858
| |______^ help: remove the call to `map`
5959

6060
error: unnecessary map of the identity function
61-
--> tests/ui/map_identity.rs:42:22
61+
--> tests/ui/map_identity.rs:43:22
6262
|
6363
LL | let _ = x.clone().map(|(x, y)| return (x, y));
6464
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
6565

6666
error: unnecessary map of the identity function
67-
--> tests/ui/map_identity.rs:46:22
67+
--> tests/ui/map_identity.rs:47:22
6868
|
6969
LL | let _ = y.clone().map(|(x, y, (z, (w,)))| (x, y, (z, (w,))));
7070
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
7171

7272
error: unnecessary map of the identity function
73-
--> tests/ui/map_identity.rs:76:30
73+
--> tests/ui/map_identity.rs:77:30
7474
|
7575
LL | let _ = x.iter().copied().map(|(x, y)| (x, y));
7676
| ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
7777

7878
error: unnecessary map of the identity function
79-
--> tests/ui/map_identity.rs:85:15
79+
--> tests/ui/map_identity.rs:86:15
8080
|
8181
LL | let _ = it.map(|x| x).next();
8282
| ^^^^^^^^^^^
8383
|
84-
help: remove the call to `map`
84+
help: remove the call to `map`, and make `it` mutable
8585
|
8686
LL ~ let mut it = [1, 2, 3].into_iter();
8787
LL ~ let _ = it.next();
8888
|
8989

9090
error: unnecessary map of the identity function
91-
--> tests/ui/map_identity.rs:91:23
91+
--> tests/ui/map_identity.rs:93:23
9292
|
9393
LL | let _ = subindex.0.map(|n| n).next();
9494
| ^^^^^^^^^^^
9595
|
96-
help: remove the call to `map`
96+
help: remove the call to `map`, and make `subindex` mutable
9797
|
9898
LL ~ let mut subindex = (index.by_ref().take(3), 42);
9999
LL ~ let _ = subindex.0.next();
100100
|
101101

102102
error: unnecessary map of the identity function
103-
--> tests/ui/map_identity.rs:97:15
103+
--> tests/ui/map_identity.rs:100:15
104104
|
105105
LL | let _ = it.map(|x| x).next();
106106
| ^^^^^^^^^^^ help: remove the call to `map`
107107

108108
error: unnecessary map of the identity function
109-
--> tests/ui/map_identity.rs:102:19
109+
--> tests/ui/map_identity.rs:106:19
110110
|
111111
LL | let _ = { it }.map(|x| x).next();
112112
| ^^^^^^^^^^^ help: remove the call to `map`
113113

114114
error: unnecessary map of the identity function
115-
--> tests/ui/map_identity.rs:114:30
115+
--> tests/ui/map_identity.rs:119:30
116116
|
117117
LL | let _ = x.iter().copied().map(|[x, y]| [x, y]);
118118
| ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
119119

120120
error: unnecessary map of the identity function
121-
--> tests/ui/map_identity.rs:140:26
121+
--> tests/ui/map_identity.rs:145:26
122122
|
123123
LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar });
124124
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
125125

126126
error: unnecessary map of the identity function
127-
--> tests/ui/map_identity.rs:144:26
127+
--> tests/ui/map_identity.rs:149:26
128128
|
129129
LL | let _ = x.into_iter().map(|Foo { foo, bar }| foo::Foo { foo, bar });
130130
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
131131

132132
error: unnecessary map of the identity function
133-
--> tests/ui/map_identity.rs:152:26
133+
--> tests/ui/map_identity.rs:157:26
134134
|
135135
LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo: foo, bar: bar });
136136
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
137137

138138
error: unnecessary map of the identity function
139-
--> tests/ui/map_identity.rs:156:26
139+
--> tests/ui/map_identity.rs:161:26
140140
|
141141
LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { bar, foo });
142142
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
143143

144144
error: unnecessary map of the identity function
145-
--> tests/ui/map_identity.rs:166:26
145+
--> tests/ui/map_identity.rs:171:26
146146
|
147147
LL | let _ = x.into_iter().map(|Foo2(foo, bar)| Foo2(foo, bar));
148148
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
149149

150150
error: unnecessary map of the identity function
151-
--> tests/ui/map_identity.rs:170:26
151+
--> tests/ui/map_identity.rs:175:26
152152
|
153153
LL | let _ = x.into_iter().map(|Foo2(foo, bar)| foo::Foo2(foo, bar));
154154
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`

0 commit comments

Comments
 (0)