Skip to content

Commit 39bde6d

Browse files
committed
Reorganize code in mem_replace.rs
Make each lint be more self-contained, and trigger only the next lint when the previous one didn't already rewrite the expression.
1 parent ff87bea commit 39bde6d

File tree

1 file changed

+50
-36
lines changed

1 file changed

+50
-36
lines changed

clippy_lints/src/mem_replace.rs

Lines changed: 50 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -103,26 +103,31 @@ declare_clippy_lint! {
103103
impl_lint_pass!(MemReplace =>
104104
[MEM_REPLACE_OPTION_WITH_NONE, MEM_REPLACE_WITH_UNINIT, MEM_REPLACE_WITH_DEFAULT]);
105105

106-
fn check_replace_option_with_none(cx: &LateContext<'_>, dest: &Expr<'_>, expr_span: Span) {
107-
// Since this is a late pass (already type-checked),
108-
// and we already know that the second argument is an
109-
// `Option`, we do not need to check the first
110-
// argument's type. All that's left is to get
111-
// the replacee's expr after peeling off the `&mut`
112-
let sugg_expr = peel_ref_operators(cx, dest);
113-
let mut applicability = Applicability::MachineApplicable;
114-
span_lint_and_sugg(
115-
cx,
116-
MEM_REPLACE_OPTION_WITH_NONE,
117-
expr_span,
118-
"replacing an `Option` with `None`",
119-
"consider `Option::take()` instead",
120-
format!(
121-
"{}.take()",
122-
Sugg::hir_with_context(cx, sugg_expr, expr_span.ctxt(), "", &mut applicability).maybe_par()
123-
),
124-
applicability,
125-
);
106+
fn check_replace_option_with_none(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) -> bool {
107+
if is_res_lang_ctor(cx, path_res(cx, src), OptionNone) {
108+
// Since this is a late pass (already type-checked),
109+
// and we already know that the second argument is an
110+
// `Option`, we do not need to check the first
111+
// argument's type. All that's left is to get
112+
// the replacee's expr after peeling off the `&mut`
113+
let sugg_expr = peel_ref_operators(cx, dest);
114+
let mut applicability = Applicability::MachineApplicable;
115+
span_lint_and_sugg(
116+
cx,
117+
MEM_REPLACE_OPTION_WITH_NONE,
118+
expr_span,
119+
"replacing an `Option` with `None`",
120+
"consider `Option::take()` instead",
121+
format!(
122+
"{}.take()",
123+
Sugg::hir_with_context(cx, sugg_expr, expr_span.ctxt(), "", &mut applicability).maybe_par()
124+
),
125+
applicability,
126+
);
127+
true
128+
} else {
129+
false
130+
}
126131
}
127132

128133
fn check_replace_with_uninit(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) {
@@ -181,34 +186,44 @@ fn check_replace_with_uninit(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'
181186
}
182187
}
183188

184-
fn check_replace_with_default(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) {
185-
// disable lint for primitives
186-
let expr_type = cx.typeck_results().expr_ty_adjusted(src);
187-
if is_non_aggregate_primitive_type(expr_type) {
188-
return;
189-
}
190-
if is_default_equivalent(cx, src) && !expr_span.in_external_macro(cx.tcx.sess.source_map()) {
191-
let Some(top_crate) = std_or_core(cx) else { return };
189+
fn check_replace_with_default(
190+
cx: &LateContext<'_>,
191+
src: &Expr<'_>,
192+
dest: &Expr<'_>,
193+
expr: &Expr<'_>,
194+
msrv: &Msrv,
195+
) -> bool {
196+
if msrv.meets(msrvs::MEM_TAKE) && is_expr_used_or_unified(cx.tcx, expr)
197+
// disable lint for primitives
198+
&& let expr_type = cx.typeck_results().expr_ty_adjusted(src)
199+
&& !is_non_aggregate_primitive_type(expr_type)
200+
&& is_default_equivalent(cx, src)
201+
&& !expr.span.in_external_macro(cx.tcx.sess.source_map())
202+
&& let Some(top_crate) = std_or_core(cx)
203+
{
192204
span_lint_and_then(
193205
cx,
194206
MEM_REPLACE_WITH_DEFAULT,
195-
expr_span,
207+
expr.span,
196208
format!(
197209
"replacing a value of type `T` with `T::default()` is better expressed using `{top_crate}::mem::take`"
198210
),
199211
|diag| {
200-
if !expr_span.from_expansion() {
212+
if !expr.span.from_expansion() {
201213
let suggestion = format!("{top_crate}::mem::take({})", snippet(cx, dest.span, ""));
202214

203215
diag.span_suggestion(
204-
expr_span,
216+
expr.span,
205217
"consider using",
206218
suggestion,
207219
Applicability::MachineApplicable,
208220
);
209221
}
210222
},
211223
);
224+
true
225+
} else {
226+
false
212227
}
213228
}
214229

@@ -233,12 +248,11 @@ impl<'tcx> LateLintPass<'tcx> for MemReplace {
233248
&& cx.tcx.is_diagnostic_item(sym::mem_replace, def_id)
234249
{
235250
// Check that second argument is `Option::None`
236-
if is_res_lang_ctor(cx, path_res(cx, src), OptionNone) {
237-
check_replace_option_with_none(cx, dest, expr.span);
238-
} else if self.msrv.meets(msrvs::MEM_TAKE) && is_expr_used_or_unified(cx.tcx, expr) {
239-
check_replace_with_default(cx, src, dest, expr.span);
251+
if !check_replace_option_with_none(cx, src, dest, expr.span)
252+
&& !check_replace_with_default(cx, src, dest, expr, &self.msrv)
253+
{
254+
check_replace_with_uninit(cx, src, dest, expr.span);
240255
}
241-
check_replace_with_uninit(cx, src, dest, expr.span);
242256
}
243257
}
244258
extract_msrv_attr!(LateContext);

0 commit comments

Comments
 (0)