|
| 1 | +use clippy_utils::diagnostics::span_lint_and_then; |
| 2 | +use clippy_utils::macros::HirNode; |
| 3 | +use clippy_utils::sugg::Sugg; |
| 4 | +use clippy_utils::{is_trait_method, path_to_local}; |
| 5 | +use rustc_errors::Applicability; |
| 6 | +use rustc_hir::{self as hir, Expr, ExprKind, Node}; |
| 7 | +use rustc_lint::{LateContext, LateLintPass}; |
| 8 | +use rustc_middle::ty::{self, Instance, Mutability}; |
| 9 | +use rustc_session::declare_lint_pass; |
| 10 | +use rustc_span::def_id::DefId; |
| 11 | +use rustc_span::symbol::sym; |
| 12 | +use rustc_span::ExpnKind; |
| 13 | + |
| 14 | +declare_clippy_lint! { |
| 15 | + /// ### What it does |
| 16 | + /// Checks for code like `foo = bar.clone();` |
| 17 | + /// |
| 18 | + /// ### Why is this bad? |
| 19 | + /// Custom `Clone::clone_from()` or `ToOwned::clone_into` implementations allow the objects |
| 20 | + /// to share resources and therefore avoid allocations. |
| 21 | + /// |
| 22 | + /// ### Example |
| 23 | + /// ```rust |
| 24 | + /// struct Thing; |
| 25 | + /// |
| 26 | + /// impl Clone for Thing { |
| 27 | + /// fn clone(&self) -> Self { todo!() } |
| 28 | + /// fn clone_from(&mut self, other: &Self) { todo!() } |
| 29 | + /// } |
| 30 | + /// |
| 31 | + /// pub fn assign_to_ref(a: &mut Thing, b: Thing) { |
| 32 | + /// *a = b.clone(); |
| 33 | + /// } |
| 34 | + /// ``` |
| 35 | + /// Use instead: |
| 36 | + /// ```rust |
| 37 | + /// struct Thing; |
| 38 | + /// impl Clone for Thing { |
| 39 | + /// fn clone(&self) -> Self { todo!() } |
| 40 | + /// fn clone_from(&mut self, other: &Self) { todo!() } |
| 41 | + /// } |
| 42 | + /// |
| 43 | + /// pub fn assign_to_ref(a: &mut Thing, b: Thing) { |
| 44 | + /// a.clone_from(&b); |
| 45 | + /// } |
| 46 | + /// ``` |
| 47 | + #[clippy::version = "1.77.0"] |
| 48 | + pub ASSIGNING_CLONES, |
| 49 | + perf, |
| 50 | + "assigning the result of cloning may be inefficient" |
| 51 | +} |
| 52 | +declare_lint_pass!(AssigningClones => [ASSIGNING_CLONES]); |
| 53 | + |
| 54 | +impl<'tcx> LateLintPass<'tcx> for AssigningClones { |
| 55 | + fn check_expr(&mut self, cx: &LateContext<'tcx>, assign_expr: &'tcx hir::Expr<'_>) { |
| 56 | + // Do not fire the lint in macros |
| 57 | + let expn_data = assign_expr.span().ctxt().outer_expn_data(); |
| 58 | + match expn_data.kind { |
| 59 | + ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) | ExpnKind::Macro(..) => return, |
| 60 | + ExpnKind::Root => {}, |
| 61 | + } |
| 62 | + |
| 63 | + let ExprKind::Assign(lhs, rhs, _span) = assign_expr.kind else { |
| 64 | + return; |
| 65 | + }; |
| 66 | + |
| 67 | + let Some(call) = extract_call(cx, rhs) else { |
| 68 | + return; |
| 69 | + }; |
| 70 | + |
| 71 | + if is_ok_to_suggest(cx, lhs, &call) { |
| 72 | + suggest(cx, assign_expr, lhs, &call); |
| 73 | + } |
| 74 | + } |
| 75 | +} |
| 76 | + |
| 77 | +// Try to resolve the call to `Clone::clone` or `ToOwned::to_owned`. |
| 78 | +fn extract_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<CallCandidate<'tcx>> { |
| 79 | + let fn_def_id = clippy_utils::fn_def_id(cx, expr)?; |
| 80 | + |
| 81 | + // Fast paths to only check method calls without arguments or function calls with a single argument |
| 82 | + let (target, kind, resolved_method) = match expr.kind { |
| 83 | + ExprKind::MethodCall(path, receiver, [], _span) => { |
| 84 | + let args = cx.typeck_results().node_args(expr.hir_id); |
| 85 | + |
| 86 | + // If we could not resolve the method, don't apply the lint |
| 87 | + let Ok(Some(resolved_method)) = Instance::resolve(cx.tcx, cx.param_env, fn_def_id, args) else { |
| 88 | + return None; |
| 89 | + }; |
| 90 | + if is_trait_method(cx, expr, sym::Clone) && path.ident.name == sym::clone { |
| 91 | + (TargetTrait::Clone, CallKind::MethodCall { receiver }, resolved_method) |
| 92 | + } else if is_trait_method(cx, expr, sym::ToOwned) && path.ident.name.as_str() == "to_owned" { |
| 93 | + (TargetTrait::ToOwned, CallKind::MethodCall { receiver }, resolved_method) |
| 94 | + } else { |
| 95 | + return None; |
| 96 | + } |
| 97 | + }, |
| 98 | + ExprKind::Call(function, [arg]) => { |
| 99 | + let kind = cx.typeck_results().node_type(function.hir_id).kind(); |
| 100 | + |
| 101 | + // If we could not resolve the method, don't apply the lint |
| 102 | + let Ok(Some(resolved_method)) = (match kind { |
| 103 | + ty::FnDef(_, args) => Instance::resolve(cx.tcx, cx.param_env, fn_def_id, args), |
| 104 | + _ => Ok(None), |
| 105 | + }) else { |
| 106 | + return None; |
| 107 | + }; |
| 108 | + if cx.tcx.is_diagnostic_item(sym::to_owned_method, fn_def_id) { |
| 109 | + ( |
| 110 | + TargetTrait::ToOwned, |
| 111 | + CallKind::FunctionCall { self_arg: arg }, |
| 112 | + resolved_method, |
| 113 | + ) |
| 114 | + } else if let Some(trait_did) = cx.tcx.trait_of_item(fn_def_id) |
| 115 | + && cx.tcx.is_diagnostic_item(sym::Clone, trait_did) |
| 116 | + { |
| 117 | + ( |
| 118 | + TargetTrait::Clone, |
| 119 | + CallKind::FunctionCall { self_arg: arg }, |
| 120 | + resolved_method, |
| 121 | + ) |
| 122 | + } else { |
| 123 | + return None; |
| 124 | + } |
| 125 | + }, |
| 126 | + _ => return None, |
| 127 | + }; |
| 128 | + |
| 129 | + Some(CallCandidate { |
| 130 | + target, |
| 131 | + kind, |
| 132 | + method_def_id: resolved_method.def_id(), |
| 133 | + }) |
| 134 | +} |
| 135 | + |
| 136 | +// Return true if we find that the called method has a custom implementation and isn't derived or |
| 137 | +// provided by default by the corresponding trait. |
| 138 | +fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallCandidate<'tcx>) -> bool { |
| 139 | + // If the left-hand side is a local variable, it might be uninitialized at this point. |
| 140 | + // In that case we do not want to suggest the lint. |
| 141 | + if let Some(local) = path_to_local(lhs) { |
| 142 | + // TODO: This check currently bails if the local variable has no initializer. |
| 143 | + // That is overly conservative - the lint should fire even if there was no initializer, |
| 144 | + // but the variable has been initialized before `lhs` was evaluated. |
| 145 | + if let Some(Node::Local(local)) = cx.tcx.hir().parent_id_iter(local).next().map(|p| cx.tcx.hir_node(p)) |
| 146 | + && local.init.is_none() |
| 147 | + { |
| 148 | + return false; |
| 149 | + } |
| 150 | + } |
| 151 | + |
| 152 | + let Some(impl_block) = cx.tcx.impl_of_method(call.method_def_id) else { |
| 153 | + return false; |
| 154 | + }; |
| 155 | + |
| 156 | + // If the method implementation comes from #[derive(Clone)], then don't suggest the lint. |
| 157 | + // Automatically generated Clone impls do not currently override `clone_from`. |
| 158 | + // See e.g. https://github.com/rust-lang/rust/pull/98445#issuecomment-1190681305 for more details. |
| 159 | + if cx.tcx.is_builtin_derived(impl_block) { |
| 160 | + return false; |
| 161 | + } |
| 162 | + |
| 163 | + // Find the function for which we want to check that it is implemented. |
| 164 | + let provided_fn = match call.target { |
| 165 | + TargetTrait::Clone => cx.tcx.get_diagnostic_item(sym::Clone).and_then(|clone| { |
| 166 | + cx.tcx |
| 167 | + .provided_trait_methods(clone) |
| 168 | + .find(|item| item.name == sym::clone_from) |
| 169 | + }), |
| 170 | + TargetTrait::ToOwned => cx.tcx.get_diagnostic_item(sym::ToOwned).and_then(|to_owned| { |
| 171 | + cx.tcx |
| 172 | + .provided_trait_methods(to_owned) |
| 173 | + .find(|item| item.name.as_str() == "clone_into") |
| 174 | + }), |
| 175 | + }; |
| 176 | + let Some(provided_fn) = provided_fn else { |
| 177 | + return false; |
| 178 | + }; |
| 179 | + |
| 180 | + // Now take a look if the impl block defines an implementation for the method that we're interested |
| 181 | + // in. If not, then we're using a default implementation, which is not interesting, so we will |
| 182 | + // not suggest the lint. |
| 183 | + let implemented_fns = cx.tcx.impl_item_implementor_ids(impl_block); |
| 184 | + implemented_fns.contains_key(&provided_fn.def_id) |
| 185 | +} |
| 186 | + |
| 187 | +fn suggest<'tcx>( |
| 188 | + cx: &LateContext<'tcx>, |
| 189 | + assign_expr: &hir::Expr<'tcx>, |
| 190 | + lhs: &hir::Expr<'tcx>, |
| 191 | + call: &CallCandidate<'tcx>, |
| 192 | +) { |
| 193 | + span_lint_and_then(cx, ASSIGNING_CLONES, assign_expr.span, call.message(), |diag| { |
| 194 | + let mut applicability = Applicability::MachineApplicable; |
| 195 | + |
| 196 | + diag.span_suggestion( |
| 197 | + assign_expr.span, |
| 198 | + call.suggestion_msg(), |
| 199 | + call.suggested_replacement(cx, lhs, &mut applicability), |
| 200 | + applicability, |
| 201 | + ); |
| 202 | + }); |
| 203 | +} |
| 204 | + |
| 205 | +#[derive(Copy, Clone, Debug)] |
| 206 | +enum CallKind<'tcx> { |
| 207 | + MethodCall { receiver: &'tcx Expr<'tcx> }, |
| 208 | + FunctionCall { self_arg: &'tcx Expr<'tcx> }, |
| 209 | +} |
| 210 | + |
| 211 | +#[derive(Copy, Clone, Debug)] |
| 212 | +enum TargetTrait { |
| 213 | + Clone, |
| 214 | + ToOwned, |
| 215 | +} |
| 216 | + |
| 217 | +#[derive(Debug)] |
| 218 | +struct CallCandidate<'tcx> { |
| 219 | + target: TargetTrait, |
| 220 | + kind: CallKind<'tcx>, |
| 221 | + // DefId of the called method from an impl block that implements the target trait |
| 222 | + method_def_id: DefId, |
| 223 | +} |
| 224 | + |
| 225 | +impl<'tcx> CallCandidate<'tcx> { |
| 226 | + #[inline] |
| 227 | + fn message(&self) -> &'static str { |
| 228 | + match self.target { |
| 229 | + TargetTrait::Clone => "assigning the result of `Clone::clone()` may be inefficient", |
| 230 | + TargetTrait::ToOwned => "assigning the result of `ToOwned::to_owned()` may be inefficient", |
| 231 | + } |
| 232 | + } |
| 233 | + |
| 234 | + #[inline] |
| 235 | + fn suggestion_msg(&self) -> &'static str { |
| 236 | + match self.target { |
| 237 | + TargetTrait::Clone => "use `clone_from()`", |
| 238 | + TargetTrait::ToOwned => "use `clone_into()`", |
| 239 | + } |
| 240 | + } |
| 241 | + |
| 242 | + fn suggested_replacement( |
| 243 | + &self, |
| 244 | + cx: &LateContext<'tcx>, |
| 245 | + lhs: &hir::Expr<'tcx>, |
| 246 | + applicability: &mut Applicability, |
| 247 | + ) -> String { |
| 248 | + match self.target { |
| 249 | + TargetTrait::Clone => { |
| 250 | + match self.kind { |
| 251 | + CallKind::MethodCall { receiver } => { |
| 252 | + let receiver_sugg = if let ExprKind::Unary(hir::UnOp::Deref, ref_expr) = lhs.kind { |
| 253 | + // `*lhs = self_expr.clone();` -> `lhs.clone_from(self_expr)` |
| 254 | + Sugg::hir_with_applicability(cx, ref_expr, "_", applicability) |
| 255 | + } else { |
| 256 | + // `lhs = self_expr.clone();` -> `lhs.clone_from(self_expr)` |
| 257 | + Sugg::hir_with_applicability(cx, lhs, "_", applicability) |
| 258 | + } |
| 259 | + .maybe_par(); |
| 260 | + |
| 261 | + // Determine whether we need to reference the argument to clone_from(). |
| 262 | + let clone_receiver_type = cx.typeck_results().expr_ty(receiver); |
| 263 | + let clone_receiver_adj_type = cx.typeck_results().expr_ty_adjusted(receiver); |
| 264 | + let mut arg_sugg = Sugg::hir_with_applicability(cx, receiver, "_", applicability); |
| 265 | + if clone_receiver_type != clone_receiver_adj_type { |
| 266 | + // The receiver may have been a value type, so we need to add an `&` to |
| 267 | + // be sure the argument to clone_from will be a reference. |
| 268 | + arg_sugg = arg_sugg.addr(); |
| 269 | + }; |
| 270 | + |
| 271 | + format!("{receiver_sugg}.clone_from({arg_sugg})") |
| 272 | + }, |
| 273 | + CallKind::FunctionCall { self_arg, .. } => { |
| 274 | + let self_sugg = if let ExprKind::Unary(hir::UnOp::Deref, ref_expr) = lhs.kind { |
| 275 | + // `*lhs = Clone::clone(self_expr);` -> `Clone::clone_from(lhs, self_expr)` |
| 276 | + Sugg::hir_with_applicability(cx, ref_expr, "_", applicability) |
| 277 | + } else { |
| 278 | + // `lhs = Clone::clone(self_expr);` -> `Clone::clone_from(&mut lhs, self_expr)` |
| 279 | + Sugg::hir_with_applicability(cx, lhs, "_", applicability).mut_addr() |
| 280 | + }; |
| 281 | + // The RHS had to be exactly correct before the call, there is no auto-deref for function calls. |
| 282 | + let rhs_sugg = Sugg::hir_with_applicability(cx, self_arg, "_", applicability); |
| 283 | + |
| 284 | + format!("Clone::clone_from({self_sugg}, {rhs_sugg})") |
| 285 | + }, |
| 286 | + } |
| 287 | + }, |
| 288 | + TargetTrait::ToOwned => { |
| 289 | + let rhs_sugg = if let ExprKind::Unary(hir::UnOp::Deref, ref_expr) = lhs.kind { |
| 290 | + // `*lhs = rhs.to_owned()` -> `rhs.clone_into(lhs)` |
| 291 | + // `*lhs = ToOwned::to_owned(rhs)` -> `ToOwned::clone_into(rhs, lhs)` |
| 292 | + let sugg = Sugg::hir_with_applicability(cx, ref_expr, "_", applicability).maybe_par(); |
| 293 | + let inner_type = cx.typeck_results().expr_ty(ref_expr); |
| 294 | + // If after unwrapping the dereference, the type is not a mutable reference, we add &mut to make it |
| 295 | + // deref to a mutable reference. |
| 296 | + if matches!(inner_type.kind(), ty::Ref(_, _, Mutability::Mut)) { |
| 297 | + sugg |
| 298 | + } else { |
| 299 | + sugg.mut_addr() |
| 300 | + } |
| 301 | + } else { |
| 302 | + // `lhs = rhs.to_owned()` -> `rhs.clone_into(&mut lhs)` |
| 303 | + // `lhs = ToOwned::to_owned(rhs)` -> `ToOwned::clone_into(rhs, &mut lhs)` |
| 304 | + Sugg::hir_with_applicability(cx, lhs, "_", applicability) |
| 305 | + .maybe_par() |
| 306 | + .mut_addr() |
| 307 | + }; |
| 308 | + |
| 309 | + match self.kind { |
| 310 | + CallKind::MethodCall { receiver } => { |
| 311 | + let receiver_sugg = Sugg::hir_with_applicability(cx, receiver, "_", applicability); |
| 312 | + format!("{receiver_sugg}.clone_into({rhs_sugg})") |
| 313 | + }, |
| 314 | + CallKind::FunctionCall { self_arg, .. } => { |
| 315 | + let self_sugg = Sugg::hir_with_applicability(cx, self_arg, "_", applicability); |
| 316 | + format!("ToOwned::clone_into({self_sugg}, {rhs_sugg})") |
| 317 | + }, |
| 318 | + } |
| 319 | + }, |
| 320 | + } |
| 321 | + } |
| 322 | +} |
0 commit comments