Skip to content

Commit 3e3a09e

Browse files
committed
Auto merge of #12278 - GabrielBFern:master, r=Jarcho
[`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: #5586
2 parents 4350678 + 8355591 commit 3e3a09e

6 files changed

+30
-12
lines changed

clippy_lints/src/mem_replace.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lin
33
use clippy_utils::source::{snippet, snippet_with_applicability};
44
use clippy_utils::sugg::Sugg;
55
use clippy_utils::ty::is_non_aggregate_primitive_type;
6-
use clippy_utils::{is_default_equivalent, is_res_lang_ctor, path_res, peel_ref_operators, std_or_core};
6+
use clippy_utils::{
7+
is_default_equivalent, is_expr_used_or_unified, is_res_lang_ctor, path_res, peel_ref_operators, std_or_core,
8+
};
79
use rustc_errors::Applicability;
810
use rustc_hir::LangItem::OptionNone;
911
use rustc_hir::{Expr, ExprKind};
@@ -232,7 +234,7 @@ impl<'tcx> LateLintPass<'tcx> for MemReplace {
232234
// Check that second argument is `Option::None`
233235
if is_res_lang_ctor(cx, path_res(cx, src), OptionNone) {
234236
check_replace_option_with_none(cx, dest, expr.span);
235-
} else if self.msrv.meets(msrvs::MEM_TAKE) {
237+
} else if self.msrv.meets(msrvs::MEM_TAKE) && is_expr_used_or_unified(cx.tcx, expr) {
236238
check_replace_with_default(cx, src, dest, expr.span);
237239
}
238240
check_replace_with_uninit(cx, src, dest, expr.span);

tests/ui/mem_replace.fixed

+8
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,14 @@ fn dont_lint_primitive() {
7171
let _ = std::mem::replace(&mut pint, 0);
7272
}
7373

74+
// lint is disabled for expressions that are not used because changing to `take` is not the
75+
// recommended fix. Additionally, the `replace` is #[must_use], so that lint will provide
76+
// the correct suggestion
77+
fn dont_lint_not_used() {
78+
let mut s = String::from("foo");
79+
std::mem::replace(&mut s, String::default());
80+
}
81+
7482
fn main() {
7583
replace_option_with_none();
7684
replace_with_default();

tests/ui/mem_replace.rs

+8
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,14 @@ fn dont_lint_primitive() {
7171
let _ = std::mem::replace(&mut pint, 0);
7272
}
7373

74+
// lint is disabled for expressions that are not used because changing to `take` is not the
75+
// recommended fix. Additionally, the `replace` is #[must_use], so that lint will provide
76+
// the correct suggestion
77+
fn dont_lint_not_used() {
78+
let mut s = String::from("foo");
79+
std::mem::replace(&mut s, String::default());
80+
}
81+
7482
fn main() {
7583
replace_option_with_none();
7684
replace_with_default();

tests/ui/mem_replace.stderr

+5-5
Original file line numberDiff line numberDiff line change
@@ -119,31 +119,31 @@ LL | let _ = std::mem::replace(&mut slice, &[]);
119119
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut slice)`
120120

121121
error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
122-
--> $DIR/mem_replace.rs:89:13
122+
--> $DIR/mem_replace.rs:97:13
123123
|
124124
LL | let _ = std::mem::replace(&mut s, String::default());
125125
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut s)`
126126

127127
error: replacing an `Option` with `None`
128-
--> $DIR/mem_replace.rs:119:13
128+
--> $DIR/mem_replace.rs:127:13
129129
|
130130
LL | let _ = std::mem::replace(&mut f.0, None);
131131
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `f.0.take()`
132132

133133
error: replacing an `Option` with `None`
134-
--> $DIR/mem_replace.rs:120:13
134+
--> $DIR/mem_replace.rs:128:13
135135
|
136136
LL | let _ = std::mem::replace(&mut *f, None);
137137
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `(*f).take()`
138138

139139
error: replacing an `Option` with `None`
140-
--> $DIR/mem_replace.rs:121:13
140+
--> $DIR/mem_replace.rs:129:13
141141
|
142142
LL | let _ = std::mem::replace(&mut b.opt, None);
143143
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `b.opt.take()`
144144

145145
error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
146-
--> $DIR/mem_replace.rs:123:13
146+
--> $DIR/mem_replace.rs:131:13
147147
|
148148
LL | let _ = std::mem::replace(&mut b.val, String::default());
149149
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut b.val)`

tests/ui/mem_replace_macro.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ use proc_macros::{external, inline_macros};
77
#[inline_macros]
88
fn main() {
99
let s = &mut String::from("foo");
10-
inline!(std::mem::replace($s, Default::default()));
11-
external!(std::mem::replace($s, Default::default()));
10+
let _ = inline!(std::mem::replace($s, Default::default()));
11+
let _ = external!(std::mem::replace($s, Default::default()));
1212
}

tests/ui/mem_replace_macro.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
2-
--> $DIR/mem_replace_macro.rs:10:13
2+
--> $DIR/mem_replace_macro.rs:10:21
33
|
4-
LL | inline!(std::mem::replace($s, Default::default()));
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4+
LL | let _ = inline!(std::mem::replace($s, Default::default()));
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::mem-replace-with-default` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::mem_replace_with_default)]`

0 commit comments

Comments
 (0)