Skip to content

Commit a75e271

Browse files
borsbors
committed
Auto merge of #12282 - maekawatoshiki:fix, r=xFrednet
Handle false positive with `map_clone` lint ### Summary - Fixes rust-lang/rust-clippy#12271 - (This is my first contribution to clippy and any suggestion would be appreciated) changelog: [`map_clone`]: Handle false positive with `map_clone` lint
2 parents 660b058 + 5f8d8f1 commit a75e271

File tree

4 files changed

+53
-20
lines changed

4 files changed

+53
-20
lines changed

‎clippy_lints/src/methods/map_clone.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,11 @@ pub(super) fn check(cx: &LateContext<'_>, e: &hir::Expr<'_>, recv: &hir::Expr<'_
8686
}
8787
}
8888
},
89-
hir::ExprKind::Call(call, [_]) => {
90-
if let hir::ExprKind::Path(qpath) = call.kind {
89+
hir::ExprKind::Call(call, args) => {
90+
if let hir::ExprKind::Path(qpath) = call.kind
91+
&& let [arg] = args
92+
&& ident_eq(name, arg)
93+
{
9194
handle_path(cx, call, &qpath, e, recv);
9295
}
9396
},
@@ -118,7 +121,9 @@ fn handle_path(
118121
if let ty::Adt(_, args) = cx.typeck_results().expr_ty(recv).kind()
119122
&& let args = args.as_slice()
120123
&& let Some(ty) = args.iter().find_map(|generic_arg| generic_arg.as_type())
121-
&& ty.is_ref()
124+
&& let ty::Ref(_, ty, Mutability::Not) = ty.kind()
125+
&& let ty::FnDef(_, lst) = cx.typeck_results().expr_ty(arg).kind()
126+
&& lst.iter().all(|l| l.as_type() == Some(*ty))
122127
{
123128
lint_path(cx, e.span, recv.span, is_copy(cx, ty.peel_refs()));
124129
}

‎tests/ui/map_clone.fixed‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
clippy::redundant_clone,
77
clippy::redundant_closure,
88
clippy::useless_asref,
9-
clippy::useless_vec
9+
clippy::useless_vec,
10+
clippy::empty_loop
1011
)]
1112

1213
fn main() {
@@ -117,4 +118,17 @@ fn main() {
117118
let y = x.as_ref().map(|x| String::clone(x));
118119
let x: Result<String, ()> = Ok(String::new());
119120
let y = x.as_ref().map(|x| String::clone(x));
121+
122+
// Issue #12271
123+
{
124+
// Don't lint these
125+
let x: Option<&u8> = None;
126+
let y = x.map(|x| String::clone(loop {}));
127+
let x: Option<&u8> = None;
128+
let y = x.map(|x| u8::clone(loop {}));
129+
let x: Vec<&u8> = vec![];
130+
let y = x.into_iter().map(|x| String::clone(loop {}));
131+
let x: Vec<&u8> = vec![];
132+
let y = x.into_iter().map(|x| u8::clone(loop {}));
133+
}
120134
}

‎tests/ui/map_clone.rs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
clippy::redundant_clone,
77
clippy::redundant_closure,
88
clippy::useless_asref,
9-
clippy::useless_vec
9+
clippy::useless_vec,
10+
clippy::empty_loop
1011
)]
1112

1213
fn main() {
@@ -117,4 +118,17 @@ fn main() {
117118
let y = x.as_ref().map(|x| String::clone(x));
118119
let x: Result<String, ()> = Ok(String::new());
119120
let y = x.as_ref().map(|x| String::clone(x));
121+
122+
// Issue #12271
123+
{
124+
// Don't lint these
125+
let x: Option<&u8> = None;
126+
let y = x.map(|x| String::clone(loop {}));
127+
let x: Option<&u8> = None;
128+
let y = x.map(|x| u8::clone(loop {}));
129+
let x: Vec<&u8> = vec![];
130+
let y = x.into_iter().map(|x| String::clone(loop {}));
131+
let x: Vec<&u8> = vec![];
132+
let y = x.into_iter().map(|x| u8::clone(loop {}));
133+
}
120134
}

‎tests/ui/map_clone.stderr‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: you are using an explicit closure for copying elements
2-
--> tests/ui/map_clone.rs:13:22
2+
--> tests/ui/map_clone.rs:14:22
33
|
44
LL | let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect();
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `vec![5_i8; 6].iter().copied()`
@@ -8,85 +8,85 @@ LL | let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect();
88
= help: to override `-D warnings` add `#[allow(clippy::map_clone)]`
99

1010
error: you are using an explicit closure for cloning elements
11-
--> tests/ui/map_clone.rs:14:26
11+
--> tests/ui/map_clone.rs:15:26
1212
|
1313
LL | let _: Vec<String> = vec![String::new()].iter().map(|x| x.clone()).collect();
1414
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `vec![String::new()].iter().cloned()`
1515

1616
error: you are using an explicit closure for copying elements
17-
--> tests/ui/map_clone.rs:15:23
17+
--> tests/ui/map_clone.rs:16:23
1818
|
1919
LL | let _: Vec<u32> = vec![42, 43].iter().map(|&x| x).collect();
2020
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `vec![42, 43].iter().copied()`
2121

2222
error: you are using an explicit closure for copying elements
23-
--> tests/ui/map_clone.rs:17:26
23+
--> tests/ui/map_clone.rs:18:26
2424
|
2525
LL | let _: Option<u64> = Some(&16).map(|b| *b);
2626
| ^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `Some(&16).copied()`
2727

2828
error: you are using an explicit closure for copying elements
29-
--> tests/ui/map_clone.rs:18:25
29+
--> tests/ui/map_clone.rs:19:25
3030
|
3131
LL | let _: Option<u8> = Some(&1).map(|x| x.clone());
3232
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `Some(&1).copied()`
3333

3434
error: you are needlessly cloning iterator elements
35-
--> tests/ui/map_clone.rs:29:29
35+
--> tests/ui/map_clone.rs:30:29
3636
|
3737
LL | let _ = std::env::args().map(|v| v.clone());
3838
| ^^^^^^^^^^^^^^^^^^^ help: remove the `map` call
3939

4040
error: you are explicitly cloning with `.map()`
41-
--> tests/ui/map_clone.rs:68:13
41+
--> tests/ui/map_clone.rs:69:13
4242
|
4343
LL | let y = x.map(|x| String::clone(x));
4444
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.cloned()`
4545

4646
error: you are explicitly cloning with `.map()`
47-
--> tests/ui/map_clone.rs:70:13
47+
--> tests/ui/map_clone.rs:71:13
4848
|
4949
LL | let y = x.map(Clone::clone);
5050
| ^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.cloned()`
5151

5252
error: you are explicitly cloning with `.map()`
53-
--> tests/ui/map_clone.rs:73:13
53+
--> tests/ui/map_clone.rs:74:13
5454
|
5555
LL | let y = x.map(String::clone);
5656
| ^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.cloned()`
5757

5858
error: you are explicitly cloning with `.map()`
59-
--> tests/ui/map_clone.rs:79:13
59+
--> tests/ui/map_clone.rs:80:13
6060
|
6161
LL | let y = x.map(|x| u32::clone(x));
6262
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `x.copied()`
6363

6464
error: you are explicitly cloning with `.map()`
65-
--> tests/ui/map_clone.rs:82:13
65+
--> tests/ui/map_clone.rs:83:13
6666
|
6767
LL | let y = x.map(|x| Clone::clone(x));
6868
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `x.copied()`
6969

7070
error: you are explicitly cloning with `.map()`
71-
--> tests/ui/map_clone.rs:94:13
71+
--> tests/ui/map_clone.rs:95:13
7272
|
7373
LL | let y = x.map(|x| String::clone(x));
7474
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.cloned()`
7575

7676
error: you are explicitly cloning with `.map()`
77-
--> tests/ui/map_clone.rs:97:13
77+
--> tests/ui/map_clone.rs:98:13
7878
|
7979
LL | let y = x.map(|x| Clone::clone(x));
8080
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.cloned()`
8181

8282
error: you are explicitly cloning with `.map()`
83-
--> tests/ui/map_clone.rs:103:13
83+
--> tests/ui/map_clone.rs:104:13
8484
|
8585
LL | let y = x.map(|x| u32::clone(x));
8686
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `x.copied()`
8787

8888
error: you are explicitly cloning with `.map()`
89-
--> tests/ui/map_clone.rs:106:13
89+
--> tests/ui/map_clone.rs:107:13
9090
|
9191
LL | let y = x.map(|x| Clone::clone(x));
9292
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `x.copied()`

0 commit comments

Comments
 (0)