Skip to content

Commit

Permalink
Auto merge of #8231 - Jarcho:implicit_clone_8227, r=camsteffen
Browse files Browse the repository at this point in the history
Fix `implicit_clone` for `&&T`

fixes #8227

changelog: Don't lint `implicit_clone` on `&&T`
  • Loading branch information
bors committed Jan 14, 2022
2 parents 5cada57 + ad95279 commit 7a4acf9
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 28 deletions.
21 changes: 15 additions & 6 deletions clippy_lints/src/methods/implicit_clone.rs
Original file line number Diff line number Diff line change
@@ -1,31 +1,40 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_context;
use clippy_utils::ty::peel_mid_ty_refs;
use clippy_utils::{is_diag_item_method, is_diag_trait_item};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty::TyS;
use rustc_span::{sym, Span};
use rustc_span::sym;

use super::IMPLICIT_CLONE;

pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, span: Span) {
pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) {
if_chain! {
if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
if is_clone_like(cx, method_name, method_def_id);
let return_type = cx.typeck_results().expr_ty(expr);
let input_type = cx.typeck_results().expr_ty(recv).peel_refs();
let input_type = cx.typeck_results().expr_ty(recv);
let (input_type, ref_count) = peel_mid_ty_refs(input_type);
if let Some(ty_name) = input_type.ty_adt_def().map(|adt_def| cx.tcx.item_name(adt_def.did));
if TyS::same_type(return_type, input_type);
then {
let mut app = Applicability::MachineApplicable;
let recv_snip = snippet_with_context(cx, recv.span, expr.span.ctxt(), "..", &mut app).0;
span_lint_and_sugg(
cx,
IMPLICIT_CLONE,
span,
expr.span,
&format!("implicitly cloning a `{}` by calling `{}` on its dereferenced type", ty_name, method_name),
"consider using",
"clone".to_string(),
Applicability::MachineApplicable
if ref_count > 1 {
format!("({}{}).clone()", "*".repeat(ref_count - 1), recv_snip)
} else {
format!("{}.clone()", recv_snip)
},
app,
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2338,7 +2338,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
},
("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
("to_os_string" | "to_owned" | "to_path_buf" | "to_vec", []) => {
implicit_clone::check(cx, name, expr, recv, span);
implicit_clone::check(cx, name, expr, recv);
},
("unwrap", []) => match method_call(recv) {
Some(("get", [recv, get_arg], _)) => get_unwrap::check(cx, expr, recv, get_arg, false),
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/implicit_clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,13 @@ fn main() {
let os_str = OsStr::new("foo");
let _ = os_str.to_owned();
let _ = os_str.to_os_string();

// issue #8227
let pathbuf_ref = &pathbuf;
let pathbuf_ref = &pathbuf_ref;
let _ = pathbuf_ref.to_owned(); // Don't lint. Returns `&PathBuf`
let _ = pathbuf_ref.to_path_buf();
let pathbuf_ref = &pathbuf_ref;
let _ = pathbuf_ref.to_owned(); // Don't lint. Returns `&&PathBuf`
let _ = pathbuf_ref.to_path_buf();
}
54 changes: 33 additions & 21 deletions tests/ui/implicit_clone.stderr
Original file line number Diff line number Diff line change
@@ -1,64 +1,76 @@
error: implicitly cloning a `Vec` by calling `to_owned` on its dereferenced type
--> $DIR/implicit_clone.rs:65:17
--> $DIR/implicit_clone.rs:65:13
|
LL | let _ = vec.to_owned();
| ^^^^^^^^ help: consider using: `clone`
| ^^^^^^^^^^^^^^ help: consider using: `vec.clone()`
|
= note: `-D clippy::implicit-clone` implied by `-D warnings`

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

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

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

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

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

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

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

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

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

error: aborting due to 10 previous errors
error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenced type
--> $DIR/implicit_clone.rs:113:13
|
LL | let _ = pathbuf_ref.to_path_buf();
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(*pathbuf_ref).clone()`

error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenced type
--> $DIR/implicit_clone.rs:116:13
|
LL | let _ = pathbuf_ref.to_path_buf();
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(**pathbuf_ref).clone()`

error: aborting due to 12 previous errors

0 comments on commit 7a4acf9

Please sign in to comment.