Skip to content

Commit 7a4acf9

Browse files
borsbors
committed
Auto merge of #8231 - Jarcho:implicit_clone_8227, r=camsteffen
Fix `implicit_clone` for `&&T` fixes #8227 changelog: Don't lint `implicit_clone` on `&&T`
2 parents 5cada57 + ad95279 commit 7a4acf9

File tree

4 files changed

+58
-28
lines changed

4 files changed

+58
-28
lines changed

‎clippy_lints/src/methods/implicit_clone.rs

+15-6
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,40 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet_with_context;
3+
use clippy_utils::ty::peel_mid_ty_refs;
24
use clippy_utils::{is_diag_item_method, is_diag_trait_item};
35
use if_chain::if_chain;
46
use rustc_errors::Applicability;
57
use rustc_hir as hir;
68
use rustc_lint::LateContext;
79
use rustc_middle::ty::TyS;
8-
use rustc_span::{sym, Span};
10+
use rustc_span::sym;
911

1012
use super::IMPLICIT_CLONE;
1113

12-
pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, span: Span) {
14+
pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) {
1315
if_chain! {
1416
if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
1517
if is_clone_like(cx, method_name, method_def_id);
1618
let return_type = cx.typeck_results().expr_ty(expr);
17-
let input_type = cx.typeck_results().expr_ty(recv).peel_refs();
19+
let input_type = cx.typeck_results().expr_ty(recv);
20+
let (input_type, ref_count) = peel_mid_ty_refs(input_type);
1821
if let Some(ty_name) = input_type.ty_adt_def().map(|adt_def| cx.tcx.item_name(adt_def.did));
1922
if TyS::same_type(return_type, input_type);
2023
then {
24+
let mut app = Applicability::MachineApplicable;
25+
let recv_snip = snippet_with_context(cx, recv.span, expr.span.ctxt(), "..", &mut app).0;
2126
span_lint_and_sugg(
2227
cx,
2328
IMPLICIT_CLONE,
24-
span,
29+
expr.span,
2530
&format!("implicitly cloning a `{}` by calling `{}` on its dereferenced type", ty_name, method_name),
2631
"consider using",
27-
"clone".to_string(),
28-
Applicability::MachineApplicable
32+
if ref_count > 1 {
33+
format!("({}{}).clone()", "*".repeat(ref_count - 1), recv_snip)
34+
} else {
35+
format!("{}.clone()", recv_snip)
36+
},
37+
app,
2938
);
3039
}
3140
}

‎clippy_lints/src/methods/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2338,7 +2338,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
23382338
},
23392339
("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
23402340
("to_os_string" | "to_owned" | "to_path_buf" | "to_vec", []) => {
2341-
implicit_clone::check(cx, name, expr, recv, span);
2341+
implicit_clone::check(cx, name, expr, recv);
23422342
},
23432343
("unwrap", []) => match method_call(recv) {
23442344
Some(("get", [recv, get_arg], _)) => get_unwrap::check(cx, expr, recv, get_arg, false),

‎tests/ui/implicit_clone.rs

+9
Original file line numberDiff line numberDiff line change
@@ -105,4 +105,13 @@ fn main() {
105105
let os_str = OsStr::new("foo");
106106
let _ = os_str.to_owned();
107107
let _ = os_str.to_os_string();
108+
109+
// issue #8227
110+
let pathbuf_ref = &pathbuf;
111+
let pathbuf_ref = &pathbuf_ref;
112+
let _ = pathbuf_ref.to_owned(); // Don't lint. Returns `&PathBuf`
113+
let _ = pathbuf_ref.to_path_buf();
114+
let pathbuf_ref = &pathbuf_ref;
115+
let _ = pathbuf_ref.to_owned(); // Don't lint. Returns `&&PathBuf`
116+
let _ = pathbuf_ref.to_path_buf();
108117
}

‎tests/ui/implicit_clone.stderr

+33-21
Original file line numberDiff line numberDiff line change
@@ -1,64 +1,76 @@
11
error: implicitly cloning a `Vec` by calling `to_owned` on its dereferenced type
2-
--> $DIR/implicit_clone.rs:65:17
2+
--> $DIR/implicit_clone.rs:65:13
33
|
44
LL | let _ = vec.to_owned();
5-
| ^^^^^^^^ help: consider using: `clone`
5+
| ^^^^^^^^^^^^^^ help: consider using: `vec.clone()`
66
|
77
= note: `-D clippy::implicit-clone` implied by `-D warnings`
88

99
error: implicitly cloning a `Vec` by calling `to_vec` on its dereferenced type
10-
--> $DIR/implicit_clone.rs:66:17
10+
--> $DIR/implicit_clone.rs:66:13
1111
|
1212
LL | let _ = vec.to_vec();
13-
| ^^^^^^ help: consider using: `clone`
13+
| ^^^^^^^^^^^^ help: consider using: `vec.clone()`
1414

1515
error: implicitly cloning a `Vec` by calling `to_owned` on its dereferenced type
16-
--> $DIR/implicit_clone.rs:70:21
16+
--> $DIR/implicit_clone.rs:70:13
1717
|
1818
LL | let _ = vec_ref.to_owned();
19-
| ^^^^^^^^ help: consider using: `clone`
19+
| ^^^^^^^^^^^^^^^^^^ help: consider using: `vec_ref.clone()`
2020

2121
error: implicitly cloning a `Vec` by calling `to_vec` on its dereferenced type
22-
--> $DIR/implicit_clone.rs:71:21
22+
--> $DIR/implicit_clone.rs:71:13
2323
|
2424
LL | let _ = vec_ref.to_vec();
25-
| ^^^^^^ help: consider using: `clone`
25+
| ^^^^^^^^^^^^^^^^ help: consider using: `vec_ref.clone()`
2626

2727
error: implicitly cloning a `String` by calling `to_owned` on its dereferenced type
28-
--> $DIR/implicit_clone.rs:83:17
28+
--> $DIR/implicit_clone.rs:83:13
2929
|
3030
LL | let _ = str.to_owned();
31-
| ^^^^^^^^ help: consider using: `clone`
31+
| ^^^^^^^^^^^^^^ help: consider using: `str.clone()`
3232

3333
error: implicitly cloning a `Kitten` by calling `to_owned` on its dereferenced type
34-
--> $DIR/implicit_clone.rs:87:20
34+
--> $DIR/implicit_clone.rs:87:13
3535
|
3636
LL | let _ = kitten.to_owned();
37-
| ^^^^^^^^ help: consider using: `clone`
37+
| ^^^^^^^^^^^^^^^^^ help: consider using: `kitten.clone()`
3838

3939
error: implicitly cloning a `PathBuf` by calling `to_owned` on its dereferenced type
40-
--> $DIR/implicit_clone.rs:97:21
40+
--> $DIR/implicit_clone.rs:97:13
4141
|
4242
LL | let _ = pathbuf.to_owned();
43-
| ^^^^^^^^ help: consider using: `clone`
43+
| ^^^^^^^^^^^^^^^^^^ help: consider using: `pathbuf.clone()`
4444

4545
error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenced type
46-
--> $DIR/implicit_clone.rs:98:21
46+
--> $DIR/implicit_clone.rs:98:13
4747
|
4848
LL | let _ = pathbuf.to_path_buf();
49-
| ^^^^^^^^^^^ help: consider using: `clone`
49+
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `pathbuf.clone()`
5050

5151
error: implicitly cloning a `OsString` by calling `to_owned` on its dereferenced type
52-
--> $DIR/implicit_clone.rs:101:23
52+
--> $DIR/implicit_clone.rs:101:13
5353
|
5454
LL | let _ = os_string.to_owned();
55-
| ^^^^^^^^ help: consider using: `clone`
55+
| ^^^^^^^^^^^^^^^^^^^^ help: consider using: `os_string.clone()`
5656

5757
error: implicitly cloning a `OsString` by calling `to_os_string` on its dereferenced type
58-
--> $DIR/implicit_clone.rs:102:23
58+
--> $DIR/implicit_clone.rs:102:13
5959
|
6060
LL | let _ = os_string.to_os_string();
61-
| ^^^^^^^^^^^^ help: consider using: `clone`
61+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `os_string.clone()`
6262

63-
error: aborting due to 10 previous errors
63+
error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenced type
64+
--> $DIR/implicit_clone.rs:113:13
65+
|
66+
LL | let _ = pathbuf_ref.to_path_buf();
67+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(*pathbuf_ref).clone()`
68+
69+
error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenced type
70+
--> $DIR/implicit_clone.rs:116:13
71+
|
72+
LL | let _ = pathbuf_ref.to_path_buf();
73+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(**pathbuf_ref).clone()`
74+
75+
error: aborting due to 12 previous errors
6476

0 commit comments

Comments
 (0)