From 83555914edf10727bab5a97113ce2b3faa9abb7b Mon Sep 17 00:00:00 2001 From: Gabriel Fernandes Date: Sun, 11 Feb 2024 21:11:13 -0300 Subject: [PATCH] [mem_replace_with_default] No longer triggers on unused expression --- clippy_lints/src/mem_replace.rs | 6 ++++-- tests/ui/mem_replace.fixed | 8 ++++++++ tests/ui/mem_replace.rs | 8 ++++++++ tests/ui/mem_replace.stderr | 10 +++++----- tests/ui/mem_replace_macro.rs | 4 ++-- tests/ui/mem_replace_macro.stderr | 6 +++--- 6 files changed, 30 insertions(+), 12 deletions(-) diff --git a/clippy_lints/src/mem_replace.rs b/clippy_lints/src/mem_replace.rs index fa4f4a47e38e..1cdb7921f816 100644 --- a/clippy_lints/src/mem_replace.rs +++ b/clippy_lints/src/mem_replace.rs @@ -3,7 +3,9 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lin use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_non_aggregate_primitive_type; -use clippy_utils::{is_default_equivalent, is_res_lang_ctor, path_res, peel_ref_operators, std_or_core}; +use clippy_utils::{ + is_default_equivalent, is_expr_used_or_unified, is_res_lang_ctor, path_res, peel_ref_operators, std_or_core, +}; use rustc_errors::Applicability; use rustc_hir::LangItem::OptionNone; use rustc_hir::{Expr, ExprKind}; @@ -232,7 +234,7 @@ impl<'tcx> LateLintPass<'tcx> for MemReplace { // Check that second argument is `Option::None` if is_res_lang_ctor(cx, path_res(cx, src), OptionNone) { check_replace_option_with_none(cx, dest, expr.span); - } else if self.msrv.meets(msrvs::MEM_TAKE) { + } else if self.msrv.meets(msrvs::MEM_TAKE) && is_expr_used_or_unified(cx.tcx, expr) { check_replace_with_default(cx, src, dest, expr.span); } check_replace_with_uninit(cx, src, dest, expr.span); diff --git a/tests/ui/mem_replace.fixed b/tests/ui/mem_replace.fixed index 453d0bcc57c5..78d8b3e9bce6 100644 --- a/tests/ui/mem_replace.fixed +++ b/tests/ui/mem_replace.fixed @@ -71,6 +71,14 @@ fn dont_lint_primitive() { let _ = std::mem::replace(&mut pint, 0); } +// lint is disabled for expressions that are not used because changing to `take` is not the +// recommended fix. Additionally, the `replace` is #[must_use], so that lint will provide +// the correct suggestion +fn dont_lint_not_used() { + let mut s = String::from("foo"); + std::mem::replace(&mut s, String::default()); +} + fn main() { replace_option_with_none(); replace_with_default(); diff --git a/tests/ui/mem_replace.rs b/tests/ui/mem_replace.rs index 0c4e0f6032d8..28915bf6daee 100644 --- a/tests/ui/mem_replace.rs +++ b/tests/ui/mem_replace.rs @@ -71,6 +71,14 @@ fn dont_lint_primitive() { let _ = std::mem::replace(&mut pint, 0); } +// lint is disabled for expressions that are not used because changing to `take` is not the +// recommended fix. Additionally, the `replace` is #[must_use], so that lint will provide +// the correct suggestion +fn dont_lint_not_used() { + let mut s = String::from("foo"); + std::mem::replace(&mut s, String::default()); +} + fn main() { replace_option_with_none(); replace_with_default(); diff --git a/tests/ui/mem_replace.stderr b/tests/ui/mem_replace.stderr index ae5f4e5340a2..c8ddf585fd2a 100644 --- a/tests/ui/mem_replace.stderr +++ b/tests/ui/mem_replace.stderr @@ -119,31 +119,31 @@ LL | let _ = std::mem::replace(&mut slice, &[]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut slice)` error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take` - --> $DIR/mem_replace.rs:89:13 + --> $DIR/mem_replace.rs:97:13 | LL | let _ = std::mem::replace(&mut s, String::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut s)` error: replacing an `Option` with `None` - --> $DIR/mem_replace.rs:119:13 + --> $DIR/mem_replace.rs:127:13 | LL | let _ = std::mem::replace(&mut f.0, None); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `f.0.take()` error: replacing an `Option` with `None` - --> $DIR/mem_replace.rs:120:13 + --> $DIR/mem_replace.rs:128:13 | LL | let _ = std::mem::replace(&mut *f, None); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `(*f).take()` error: replacing an `Option` with `None` - --> $DIR/mem_replace.rs:121:13 + --> $DIR/mem_replace.rs:129:13 | LL | let _ = std::mem::replace(&mut b.opt, None); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `b.opt.take()` error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take` - --> $DIR/mem_replace.rs:123:13 + --> $DIR/mem_replace.rs:131:13 | LL | let _ = std::mem::replace(&mut b.val, String::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut b.val)` diff --git a/tests/ui/mem_replace_macro.rs b/tests/ui/mem_replace_macro.rs index 132873858b7c..c14ea2942f3e 100644 --- a/tests/ui/mem_replace_macro.rs +++ b/tests/ui/mem_replace_macro.rs @@ -7,6 +7,6 @@ use proc_macros::{external, inline_macros}; #[inline_macros] fn main() { let s = &mut String::from("foo"); - inline!(std::mem::replace($s, Default::default())); - external!(std::mem::replace($s, Default::default())); + let _ = inline!(std::mem::replace($s, Default::default())); + let _ = external!(std::mem::replace($s, Default::default())); } diff --git a/tests/ui/mem_replace_macro.stderr b/tests/ui/mem_replace_macro.stderr index c6435e94e967..63043e6bf261 100644 --- a/tests/ui/mem_replace_macro.stderr +++ b/tests/ui/mem_replace_macro.stderr @@ -1,8 +1,8 @@ error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take` - --> $DIR/mem_replace_macro.rs:10:13 + --> $DIR/mem_replace_macro.rs:10:21 | -LL | inline!(std::mem::replace($s, Default::default())); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _ = inline!(std::mem::replace($s, Default::default())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::mem-replace-with-default` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::mem_replace_with_default)]`