Skip to content

Commit 8ae0a7f

Browse files
ada4aada4a
committed
suggest making the variable mutable if necessary
1 parent 14dfc03 commit 8ae0a7f

File tree

6 files changed

+105
-32
lines changed

6 files changed

+105
-32
lines changed

‎clippy_lints/src/methods/map_identity.rs‎

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
use clippy_utils::diagnostics::span_lint_and_sugg;
1+
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::ty::is_type_diagnostic_item;
3-
use clippy_utils::{is_expr_untyped_identity_function, is_trait_method, path_to_local};
4-
use rustc_ast::BindingMode;
3+
use clippy_utils::{is_expr_untyped_identity_function, is_mutable, is_trait_method, path_to_local};
54
use rustc_errors::Applicability;
6-
use rustc_hir::{self as hir, Node, PatKind};
5+
use rustc_hir::{self as hir, ExprKind, Node, PatKind};
76
use rustc_lint::LateContext;
87
use rustc_span::{Span, Symbol, sym};
98

@@ -23,26 +22,57 @@ pub(super) fn check(
2322
|| is_type_diagnostic_item(cx, caller_ty, sym::Result)
2423
|| is_type_diagnostic_item(cx, caller_ty, sym::Option))
2524
&& is_expr_untyped_identity_function(cx, map_arg)
26-
&& let Some(sugg_span) = expr.span.trim_start(caller.span)
25+
&& let Some(call_span) = expr.span.trim_start(caller.span)
2726
{
28-
// If the result of `.map(identity)` is used as a mutable reference,
29-
// the caller must not be an immutable binding.
30-
if cx.typeck_results().expr_ty_adjusted(expr).is_mutable_ptr()
31-
&& let Some(hir_id) = path_to_local(caller)
32-
&& let Node::Pat(pat) = cx.tcx.hir_node(hir_id)
33-
&& !matches!(pat.kind, PatKind::Binding(BindingMode::MUT, ..))
34-
{
35-
return;
27+
let mut sugg = vec![(call_span, String::new())];
28+
let mut apply = true;
29+
if !is_mutable(cx, caller) {
30+
if let Some(hir_id) = path_to_local(caller)
31+
&& let Node::Pat(pat) = cx.tcx.hir_node(hir_id)
32+
&& let PatKind::Binding(_, _, ident, _) = pat.kind
33+
{
34+
sugg.push((ident.span.shrink_to_lo(), String::from("mut ")));
35+
} else {
36+
// If we can't make the binding mutable, make the suggestion `Unspecified` to prevent it from being
37+
// automatically applied, and add a complementary help message.
38+
apply = false;
39+
}
3640
}
3741

38-
span_lint_and_sugg(
42+
let method_requiring_mut = if let Node::Expr(expr) = cx.tcx.parent_hir_node(expr.hir_id)
43+
&& let ExprKind::MethodCall(method, ..) = expr.kind
44+
{
45+
Some(method.ident)
46+
} else {
47+
None
48+
};
49+
50+
span_lint_and_then(
3951
cx,
4052
MAP_IDENTITY,
41-
sugg_span,
53+
call_span,
4254
"unnecessary map of the identity function",
43-
format!("remove the call to `{name}`"),
44-
String::new(),
45-
Applicability::MachineApplicable,
55+
|diag| {
56+
diag.multipart_suggestion(
57+
format!("remove the call to `{name}`"),
58+
sugg,
59+
if apply {
60+
Applicability::MachineApplicable
61+
} else {
62+
Applicability::Unspecified
63+
},
64+
);
65+
if !apply {
66+
if let Some(method_requiring_mut) = method_requiring_mut {
67+
diag.span_note(
68+
caller.span,
69+
format!("this must be made mutable to use `{method_requiring_mut}`"),
70+
);
71+
} else {
72+
diag.span_note(caller.span, "this must be made mutable".to_string());
73+
}
74+
}
75+
},
4676
);
4777
}
4878
}

‎tests/ui/map_identity.fixed‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,12 @@ fn issue11764() {
7272
}
7373

7474
fn issue13904() {
75-
// don't lint: `it.next()` would not be legal as `it` is immutable
76-
let it = [1, 2, 3].into_iter();
77-
let _ = it.map(|x| x).next();
75+
// lint, but there's a catch:
76+
// when we remove the `.map()`, `it.next()` would require `it` to be mutable
77+
// therefore, include that in the suggestion as well
78+
let mut it = [1, 2, 3].into_iter();
79+
let _ = it.next();
80+
//~^ map_identity
7881

7982
// lint
8083
#[allow(unused_mut)]

‎tests/ui/map_identity.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,12 @@ fn issue11764() {
7878
}
7979

8080
fn issue13904() {
81-
// don't lint: `it.next()` would not be legal as `it` is immutable
81+
// lint, but there's a catch:
82+
// when we remove the `.map()`, `it.next()` would require `it` to be mutable
83+
// therefore, include that in the suggestion as well
8284
let it = [1, 2, 3].into_iter();
8385
let _ = it.map(|x| x).next();
86+
//~^ map_identity
8487

8588
// lint
8689
#[allow(unused_mut)]

‎tests/ui/map_identity.stderr‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,58 +76,70 @@ 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:88:15
79+
--> tests/ui/map_identity.rs:85:15
80+
|
81+
LL | let _ = it.map(|x| x).next();
82+
| ^^^^^^^^^^^
83+
|
84+
help: remove the call to `map`
85+
|
86+
LL ~ let mut it = [1, 2, 3].into_iter();
87+
LL ~ let _ = it.next();
88+
|
89+
90+
error: unnecessary map of the identity function
91+
--> tests/ui/map_identity.rs:91:15
8092
|
8193
LL | let _ = it.map(|x| x).next();
8294
| ^^^^^^^^^^^ help: remove the call to `map`
8395

8496
error: unnecessary map of the identity function
85-
--> tests/ui/map_identity.rs:93:19
97+
--> tests/ui/map_identity.rs:96:19
8698
|
8799
LL | let _ = { it }.map(|x| x).next();
88100
| ^^^^^^^^^^^ help: remove the call to `map`
89101

90102
error: unnecessary map of the identity function
91-
--> tests/ui/map_identity.rs:105:30
103+
--> tests/ui/map_identity.rs:108:30
92104
|
93105
LL | let _ = x.iter().copied().map(|[x, y]| [x, y]);
94106
| ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
95107

96108
error: unnecessary map of the identity function
97-
--> tests/ui/map_identity.rs:131:26
109+
--> tests/ui/map_identity.rs:134:26
98110
|
99111
LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar });
100112
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
101113

102114
error: unnecessary map of the identity function
103-
--> tests/ui/map_identity.rs:135:26
115+
--> tests/ui/map_identity.rs:138:26
104116
|
105117
LL | let _ = x.into_iter().map(|Foo { foo, bar }| foo::Foo { foo, bar });
106118
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
107119

108120
error: unnecessary map of the identity function
109-
--> tests/ui/map_identity.rs:143:26
121+
--> tests/ui/map_identity.rs:146:26
110122
|
111123
LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo: foo, bar: bar });
112124
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
113125

114126
error: unnecessary map of the identity function
115-
--> tests/ui/map_identity.rs:147:26
127+
--> tests/ui/map_identity.rs:150:26
116128
|
117129
LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { bar, foo });
118130
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
119131

120132
error: unnecessary map of the identity function
121-
--> tests/ui/map_identity.rs:157:26
133+
--> tests/ui/map_identity.rs:160:26
122134
|
123135
LL | let _ = x.into_iter().map(|Foo2(foo, bar)| Foo2(foo, bar));
124136
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
125137

126138
error: unnecessary map of the identity function
127-
--> tests/ui/map_identity.rs:161:26
139+
--> tests/ui/map_identity.rs:164:26
128140
|
129141
LL | let _ = x.into_iter().map(|Foo2(foo, bar)| foo::Foo2(foo, bar));
130142
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
131143

132-
error: aborting due to 20 previous errors
144+
error: aborting due to 21 previous errors
133145

‎tests/ui/map_identity_unfixable.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
//@no-rustfix
2+
#![warn(clippy::map_identity)]
3+
4+
fn main() {
5+
let mut index = [true, true, false, false, false, true].iter();
6+
let subindex = (index.by_ref().take(3), 42);
7+
let _ = subindex.0.map(|n| n).next();
8+
//~^ map_identity
9+
}

‎tests/ui/map_identity_unfixable.stderr‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: unnecessary map of the identity function
2+
--> tests/ui/map_identity_unfixable.rs:7:23
3+
|
4+
LL | let _ = subindex.0.map(|n| n).next();
5+
| ^^^^^^^^^^^ help: remove the call to `map`
6+
|
7+
note: this must be made mutable to use `next`
8+
--> tests/ui/map_identity_unfixable.rs:7:13
9+
|
10+
LL | let _ = subindex.0.map(|n| n).next();
11+
| ^^^^^^^^^^
12+
= note: `-D clippy::map-identity` implied by `-D warnings`
13+
= help: to override `-D warnings` add `#[allow(clippy::map_identity)]`
14+
15+
error: aborting due to 1 previous error
16+

0 commit comments

Comments
 (0)