Skip to content

Commit

Permalink
Auto merge of rust-lang#12278 - GabrielBFern:master, r=Jarcho
Browse files Browse the repository at this point in the history
[`mem_replace_with_default`] No longer triggers on unused expression

changelog:[`mem_replace_with_default`]: No longer triggers on unused expression

Change [`mem_replace_with_default`] to not trigger on unused expression because the lint from `#[must_use]` handle this case better.

fixes: rust-lang#5586
  • Loading branch information
bors committed Feb 13, 2024
2 parents 4350678 + 8355591 commit 3e3a09e
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 12 deletions.
6 changes: 4 additions & 2 deletions clippy_lints/src/mem_replace.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/mem_replace.fixed
Expand Up @@ -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();
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/mem_replace.rs
Expand Up @@ -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();
Expand Down
10 changes: 5 additions & 5 deletions tests/ui/mem_replace.stderr
Expand Up @@ -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)`
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/mem_replace_macro.rs
Expand Up @@ -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()));
}
6 changes: 3 additions & 3 deletions 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)]`
Expand Down

0 comments on commit 3e3a09e

Please sign in to comment.