Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 37 additions & 7 deletions clippy_lints/src/assigning_clones.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use rustc_hir::{self as hir, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass, impl_lint_pass};
use rustc_middle::mir;
use rustc_middle::ty::{self, Instance, Mutability};
use rustc_span::{Span, SyntaxContext};
use rustc_span::{Span, SyntaxContext, kw};

use crate::methods::is_to_owned_like;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -85,15 +87,34 @@ impl<'tcx> LateLintPass<'tcx> for AssigningClones {
&& ctxt.is_root()
&& let which_trait = match fn_name {
sym::clone if fn_def.assoc_fn_parent(cx).is_diag_item(cx, sym::Clone) => CloneTrait::Clone,
sym::to_owned
if fn_def.assoc_fn_parent(cx).is_diag_item(cx, sym::ToOwned)
&& self.msrv.meets(cx, msrvs::CLONE_INTO) =>
_ if let Some(parent_def) = fn_def.opt_parent(cx)
&& is_to_owned_like(cx, rhs, fn_name, parent_def)
&& self.msrv.meets(cx, msrvs::CLONE_INTO)
=>
{
CloneTrait::ToOwned
},
_ => return,
}
&& let Ok(Some(resolved_fn)) = Instance::try_resolve(cx.tcx, cx.typing_env(), fn_def.1, fn_gen_args)
&& let fn_arg_ty = typeck.expr_ty_adjusted(fn_arg)
&& let Some((resolved_def, resolved_args)) = if matches!(fn_name, sym::clone | sym::to_owned) {
Some((fn_def.1, fn_gen_args))
} else if let Some(def) = cx.tcx.get_diagnostic_item(sym::to_owned_method) {
// Borrowed from `clippy_utils::is_default_equivalent_call`.
let args = ty::GenericArgs::for_item(cx.tcx, def, |param, _| {
if let ty::GenericParamDefKind::Lifetime = param.kind {
cx.tcx.lifetimes.re_erased.into()
} else if param.index == 0 && param.name == kw::SelfUpper {
fn_arg_ty.into()
} else {
param.to_error(cx.tcx)
}
});
Some((def, args))
} else {
None
}
&& let Ok(Some(resolved_fn)) = Instance::try_resolve(cx.tcx, cx.typing_env(), resolved_def, resolved_args)
// TODO: This check currently bails if the local variable has no initializer.
// That is overly conservative - the lint should fire even if there was no initializer,
// but the variable has been initialized before `lhs` was evaluated.
Expand Down Expand Up @@ -122,8 +143,17 @@ impl<'tcx> LateLintPass<'tcx> for AssigningClones {
ASSIGNING_CLONES,
e.span,
match which_trait {
CloneTrait::Clone => "assigning the result of `Clone::clone()` may be inefficient",
CloneTrait::ToOwned => "assigning the result of `ToOwned::to_owned()` may be inefficient",
CloneTrait::Clone => "assigning the result of `Clone::clone()` may be inefficient".to_string(),
CloneTrait::ToOwned => {
if matches!(fn_name, sym::to_owned) {
"assigning the result of `ToOwned::to_owned()` may be inefficient".to_string()
} else {
format!(
"assigning the result of `{}::{fn_name}()` may be inefficient",
fn_arg_ty.peel_refs()
)
}
},
},
|diag| {
let mut app = Applicability::Unspecified;
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ use crate::matches::manual_filter;

pub use implicit_clone::is_clone_like;
pub use path_ends_with_ext::DEFAULT_ALLOWED_DOTFILES;
pub use unnecessary_to_owned::is_to_owned_like;

declare_clippy_lint! {
/// ### What it does
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/unnecessary_to_owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ fn is_cloned_or_copied(cx: &LateContext<'_>, method_name: Symbol, method_parent_

/// Returns true if the named method can be used to convert the receiver to its "owned"
/// representation.
fn is_to_owned_like<'a>(
pub fn is_to_owned_like<'a>(
cx: &LateContext<'a>,
call_expr: &Expr<'a>,
method_name: Symbol,
Expand Down
2 changes: 1 addition & 1 deletion src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ fn main() -> ExitCode {
// uses
if let Some(pos) = orig_args.iter().position(|arg| arg == "--rustc") {
orig_args.remove(pos);
orig_args[0] = "rustc".to_string();
"rustc".clone_into(&mut orig_args[0]);

let mut args: Vec<String> = orig_args.clone();
pass_sysroot_env_if_given(&mut args, sys_root_env);
Expand Down
41 changes: 41 additions & 0 deletions tests/ui/assigning_clones.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,47 @@ impl<T: Clone> Clone for DerefWrapperWithClone<T> {
}
}

fn issue16517(a: &str, g: &[i32]) {
use std::ffi::{OsStr, OsString};
use std::ops::Deref;
use std::path::{Path, PathBuf};

let mut b = String::new();
a.clone_into(&mut b);
//~^ assigning_clones

let c = OsStr::new("hello");
let mut d = OsString::new();
c.clone_into(&mut d);
//~^ assigning_clones

let e = Path::new("hello");
let mut f = PathBuf::new();
e.clone_into(&mut f);
//~^ assigning_clones

let mut h = Vec::new();
g.clone_into(&mut h);
//~^ assigning_clones

struct Foo {
x: String,
}

impl Deref for Foo {
type Target = str;

fn deref(&self) -> &str {
&self.x
}
}

let i = Foo { x: "hello".to_string() };
let mut j = String::new();
i.clone_into(&mut j);
//~^ assigning_clones
}

#[cfg(test)]
mod test {
#[derive(Default)]
Expand Down
41 changes: 41 additions & 0 deletions tests/ui/assigning_clones.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,47 @@ impl<T: Clone> Clone for DerefWrapperWithClone<T> {
}
}

fn issue16517(a: &str, g: &[i32]) {
use std::ffi::{OsStr, OsString};
use std::ops::Deref;
use std::path::{Path, PathBuf};

let mut b = String::new();
b = a.to_string();
//~^ assigning_clones

let c = OsStr::new("hello");
let mut d = OsString::new();
d = c.to_os_string();
//~^ assigning_clones

let e = Path::new("hello");
let mut f = PathBuf::new();
f = e.to_path_buf();
//~^ assigning_clones

let mut h = Vec::new();
h = g.to_vec();
//~^ assigning_clones

struct Foo {
x: String,
}

impl Deref for Foo {
type Target = str;

fn deref(&self) -> &str {
&self.x
}
}

let i = Foo { x: "hello".to_string() };
let mut j = String::new();
j = i.to_string();
//~^ assigning_clones
}

#[cfg(test)]
mod test {
#[derive(Default)]
Expand Down
32 changes: 31 additions & 1 deletion tests/ui/assigning_clones.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -181,5 +181,35 @@ error: assigning the result of `ToOwned::to_owned()` may be inefficient
LL | s = ToOwned::to_owned(&format!("{} {}", "hello", "world"));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(&format!("{} {}", "hello", "world"), &mut s)`

error: aborting due to 30 previous errors
error: assigning the result of `str::to_string()` may be inefficient
--> tests/ui/assigning_clones.rs:432:5
|
LL | b = a.to_string();
| ^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `a.clone_into(&mut b)`

error: assigning the result of `std::ffi::OsStr::to_os_string()` may be inefficient
--> tests/ui/assigning_clones.rs:437:5
|
LL | d = c.to_os_string();
| ^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `c.clone_into(&mut d)`

error: assigning the result of `std::path::Path::to_path_buf()` may be inefficient
--> tests/ui/assigning_clones.rs:442:5
|
LL | f = e.to_path_buf();
| ^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `e.clone_into(&mut f)`

error: assigning the result of `[i32]::to_vec()` may be inefficient
--> tests/ui/assigning_clones.rs:446:5
|
LL | h = g.to_vec();
| ^^^^^^^^^^^^^^ help: use `clone_into()`: `g.clone_into(&mut h)`

error: assigning the result of `str::to_string()` may be inefficient
--> tests/ui/assigning_clones.rs:463:5
|
LL | j = i.to_string();
| ^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `i.clone_into(&mut j)`

error: aborting due to 35 previous errors