Skip to content

Commit 3261416

Browse files
committed
fix: option_if_let_else FP when value partially moved
1 parent a9c61ec commit 3261416

File tree

3 files changed

+115
-2
lines changed

3 files changed

+115
-2
lines changed

clippy_lints/src/option_if_let_else.rs

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,23 @@
1+
use std::ops::ControlFlow;
2+
13
use clippy_utils::diagnostics::span_lint_and_sugg;
24
use clippy_utils::sugg::Sugg;
5+
use clippy_utils::ty::is_copy;
36
use clippy_utils::{
47
CaptureKind, can_move_expr_to_closure, eager_or_lazy, higher, is_else_clause, is_in_const_context,
58
is_res_lang_ctor, peel_blocks, peel_hir_expr_while,
69
};
10+
use rustc_data_structures::fx::FxHashSet;
711
use rustc_errors::Applicability;
812
use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk};
913
use rustc_hir::def::Res;
14+
use rustc_hir::intravisit::{Visitor, walk_expr, walk_path};
1015
use rustc_hir::{
11-
Arm, BindingMode, Expr, ExprKind, MatchSource, Mutability, Pat, PatExpr, PatExprKind, PatKind, Path, QPath, UnOp,
16+
Arm, BindingMode, Expr, ExprKind, HirId, MatchSource, Mutability, Node, Pat, PatExpr, PatExprKind, PatKind, Path,
17+
QPath, UnOp,
1218
};
1319
use rustc_lint::{LateContext, LateLintPass};
20+
use rustc_middle::hir::nested_filter;
1421
use rustc_session::declare_lint_pass;
1522
use rustc_span::SyntaxContext;
1623

@@ -110,11 +117,12 @@ fn format_option_in_sugg(cond_sugg: Sugg<'_>, as_ref: bool, as_mut: bool) -> Str
110117
)
111118
}
112119

120+
#[expect(clippy::too_many_lines)]
113121
fn try_get_option_occurrence<'tcx>(
114122
cx: &LateContext<'tcx>,
115123
ctxt: SyntaxContext,
116124
pat: &Pat<'tcx>,
117-
expr: &Expr<'_>,
125+
expr: &'tcx Expr<'_>,
118126
if_then: &'tcx Expr<'_>,
119127
if_else: &'tcx Expr<'_>,
120128
) -> Option<OptionOccurrence> {
@@ -184,6 +192,22 @@ fn try_get_option_occurrence<'tcx>(
184192
}
185193
}
186194

195+
if !is_copy(cx, cx.typeck_results().expr_ty(expr)) {
196+
let mut condition_visitor = ConditionVisitor {
197+
cx,
198+
identifiers: FxHashSet::default(),
199+
};
200+
condition_visitor.visit_expr(cond_expr);
201+
202+
let mut reference_visitor = ReferenceVisitor {
203+
cx,
204+
identifiers: condition_visitor.identifiers,
205+
};
206+
if reference_visitor.visit_expr(none_body).is_break() {
207+
return None;
208+
}
209+
}
210+
187211
let mut app = Applicability::Unspecified;
188212

189213
let (none_body, is_argless_call) = match none_body.kind {
@@ -219,6 +243,55 @@ fn try_get_option_occurrence<'tcx>(
219243
None
220244
}
221245

246+
struct ConditionVisitor<'a, 'tcx> {
247+
cx: &'a LateContext<'tcx>,
248+
identifiers: FxHashSet<HirId>,
249+
}
250+
251+
impl<'tcx> Visitor<'tcx> for ConditionVisitor<'_, 'tcx> {
252+
type NestedFilter = nested_filter::All;
253+
254+
fn visit_path(&mut self, path: &Path<'tcx>, _: HirId) {
255+
if let Res::Local(local_id) = path.res
256+
&& let Node::Pat(pat) = self.cx.tcx.hir_node(local_id)
257+
&& let PatKind::Binding(_, local_id, ..) = pat.kind
258+
{
259+
self.identifiers.insert(local_id);
260+
}
261+
walk_path(self, path);
262+
}
263+
264+
fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
265+
self.cx.tcx
266+
}
267+
}
268+
269+
struct ReferenceVisitor<'a, 'tcx> {
270+
cx: &'a LateContext<'tcx>,
271+
identifiers: FxHashSet<HirId>,
272+
}
273+
274+
impl<'tcx> Visitor<'tcx> for ReferenceVisitor<'_, 'tcx> {
275+
type NestedFilter = nested_filter::All;
276+
type Result = ControlFlow<()>;
277+
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) -> ControlFlow<()> {
278+
if let ExprKind::Path(ref path) = expr.kind
279+
&& let QPath::Resolved(_, path) = path
280+
&& let Res::Local(local_id) = path.res
281+
&& let Node::Pat(pat) = self.cx.tcx.hir_node(local_id)
282+
&& let PatKind::Binding(_, local_id, ..) = pat.kind
283+
&& self.identifiers.contains(&local_id)
284+
{
285+
return ControlFlow::Break(());
286+
}
287+
walk_expr(self, expr)
288+
}
289+
290+
fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
291+
self.cx.tcx
292+
}
293+
}
294+
222295
fn try_get_inner_pat_and_is_result<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<(&'tcx Pat<'tcx>, bool)> {
223296
if let PatKind::TupleStruct(ref qpath, [inner_pat], ..) = pat.kind {
224297
let res = cx.qpath_res(qpath, pat.hir_id);

tests/ui/option_if_let_else.fixed

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,3 +268,23 @@ fn issue11893() {
268268
panic!("Haven't thought about this condition.");
269269
}
270270
}
271+
272+
mod issue13964 {
273+
#[derive(Debug)]
274+
struct A(Option<String>);
275+
276+
fn foo(a: A) {
277+
let _ = match a.0 {
278+
Some(x) => x,
279+
None => panic!("{a:?} is invalid."),
280+
};
281+
}
282+
283+
fn bar(a: A) {
284+
let _ = if let Some(x) = a.0 {
285+
x
286+
} else {
287+
panic!("{a:?} is invalid.")
288+
};
289+
}
290+
}

tests/ui/option_if_let_else.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,3 +331,23 @@ fn issue11893() {
331331
panic!("Haven't thought about this condition.");
332332
}
333333
}
334+
335+
mod issue13964 {
336+
#[derive(Debug)]
337+
struct A(Option<String>);
338+
339+
fn foo(a: A) {
340+
let _ = match a.0 {
341+
Some(x) => x,
342+
None => panic!("{a:?} is invalid."),
343+
};
344+
}
345+
346+
fn bar(a: A) {
347+
let _ = if let Some(x) = a.0 {
348+
x
349+
} else {
350+
panic!("{a:?} is invalid.")
351+
};
352+
}
353+
}

0 commit comments

Comments
 (0)