Skip to content

Commit ea77fcc

Browse files
ada4aada4a
committed
clean-up
1 parent 8ff0cf8 commit ea77fcc

File tree

9 files changed

+140
-223
lines changed

9 files changed

+140
-223
lines changed

‎clippy_lints/src/unwrap.rs‎

Lines changed: 52 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -156,39 +156,52 @@ fn collect_unwrap_info<'tcx>(
156156
}
157157
}
158158

159-
match expr.kind {
160-
ExprKind::Binary(op, left, right)
161-
if matches!(
162-
(invert, op.node),
163-
(false, BinOpKind::And | BinOpKind::BitAnd) | (true, BinOpKind::Or | BinOpKind::BitOr)
164-
) =>
165-
{
166-
let mut unwrap_info = collect_unwrap_info(cx, if_expr, left, branch, invert, false);
167-
unwrap_info.extend(collect_unwrap_info(cx, if_expr, right, branch, invert, false));
168-
unwrap_info
169-
},
170-
ExprKind::Unary(UnOp::Not, expr) => collect_unwrap_info(cx, if_expr, expr, branch, !invert, false),
171-
ExprKind::MethodCall(method_name, receiver, [], _)
172-
if let Some(local_id) = receiver.res_local_id()
173-
&& let ty = cx.typeck_results().expr_ty(receiver)
174-
&& let name = method_name.ident.name
175-
&& let Some((kind, unwrappable)) = option_or_result_call(cx, ty, name) =>
176-
{
177-
let safe_to_unwrap = unwrappable != invert;
178-
179-
vec![UnwrapInfo {
180-
local_id,
181-
if_expr,
182-
check: expr,
183-
check_name: name,
184-
branch,
185-
safe_to_unwrap,
186-
kind,
187-
is_entire_condition,
188-
}]
189-
},
190-
_ => vec![],
159+
fn inner<'tcx>(
160+
cx: &LateContext<'tcx>,
161+
if_expr: &'tcx Expr<'_>,
162+
expr: &'tcx Expr<'_>,
163+
branch: &'tcx Expr<'_>,
164+
invert: bool,
165+
is_entire_condition: bool,
166+
out: &mut Vec<UnwrapInfo<'tcx>>,
167+
) {
168+
match expr.kind {
169+
ExprKind::Binary(op, left, right)
170+
if matches!(
171+
(invert, op.node),
172+
(false, BinOpKind::And | BinOpKind::BitAnd) | (true, BinOpKind::Or | BinOpKind::BitOr)
173+
) =>
174+
{
175+
inner(cx, if_expr, left, branch, invert, false, out);
176+
inner(cx, if_expr, right, branch, invert, false, out);
177+
},
178+
ExprKind::Unary(UnOp::Not, expr) => inner(cx, if_expr, expr, branch, !invert, false, out),
179+
ExprKind::MethodCall(method_name, receiver, [], _)
180+
if let Some(local_id) = receiver.res_local_id()
181+
&& let ty = cx.typeck_results().expr_ty(receiver)
182+
&& let name = method_name.ident.name
183+
&& let Some((kind, unwrappable)) = option_or_result_call(cx, ty, name) =>
184+
{
185+
let safe_to_unwrap = unwrappable != invert;
186+
187+
out.push(UnwrapInfo {
188+
local_id,
189+
if_expr,
190+
check: expr,
191+
check_name: name,
192+
branch,
193+
safe_to_unwrap,
194+
kind,
195+
is_entire_condition,
196+
});
197+
},
198+
_ => {},
199+
}
191200
}
201+
202+
let mut out = vec![];
203+
inner(cx, if_expr, expr, branch, invert, is_entire_condition, &mut out);
204+
out
192205
}
193206

194207
/// A HIR visitor delegate that checks if a local variable of type `Option` or `Result` is mutated,
@@ -321,21 +334,14 @@ impl<'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'_, 'tcx> {
321334
&& let Some(id) = self_arg.res_local_id()
322335
&& matches!(method_name.ident.name, sym::unwrap | sym::expect | sym::unwrap_err)
323336
&& let call_to_unwrap = matches!(method_name.ident.name, sym::unwrap | sym::expect)
324-
&& let Some(unwrappable) = self.unwrappables.iter()
325-
.find(|u| u.local_id == id)
337+
&& let Some(unwrappable) = self.unwrappables.iter().find(|u| u.local_id == id)
326338
// Span contexts should not differ with the conditional branch
327339
&& let span_ctxt = expr.span.ctxt()
328340
&& unwrappable.branch.span.ctxt() == span_ctxt
329341
&& unwrappable.check.span.ctxt() == span_ctxt
330342
{
331343
if call_to_unwrap == unwrappable.safe_to_unwrap {
332-
let is_entire_condition = unwrappable.is_entire_condition;
333344
let unwrappable_variable_name = self.cx.tcx.hir_name(unwrappable.local_id);
334-
let suggested_pattern = if call_to_unwrap {
335-
unwrappable.kind.success_variant_pattern()
336-
} else {
337-
unwrappable.kind.error_variant_pattern()
338-
};
339345

340346
span_lint_hir_and_then(
341347
self.cx,
@@ -347,12 +353,17 @@ impl<'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'_, 'tcx> {
347353
method_name.ident.name, unwrappable.check_name,
348354
),
349355
|diag| {
350-
if is_entire_condition {
356+
if unwrappable.is_entire_condition {
351357
diag.span_suggestion(
352358
unwrappable.check.span.with_lo(unwrappable.if_expr.span.lo()),
353359
"try",
354360
format!(
355361
"if let {suggested_pattern} = {borrow_prefix}{unwrappable_variable_name}",
362+
suggested_pattern = if call_to_unwrap {
363+
unwrappable.kind.success_variant_pattern()
364+
} else {
365+
unwrappable.kind.error_variant_pattern()
366+
},
356367
borrow_prefix = match as_ref_kind {
357368
Some(AsRefKind::AsRef) => "&",
358369
Some(AsRefKind::AsMut) => "&mut ",

‎tests/ui/checked_unwrap/complex_conditionals.rs‎

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,19 @@
1-
#![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
2-
#![allow(
3-
clippy::if_same_then_else,
4-
clippy::branches_sharing_code,
5-
clippy::unnecessary_literal_unwrap
6-
)]
1+
#![warn(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
2+
#![expect(clippy::branches_sharing_code, clippy::unnecessary_literal_unwrap)]
73

84
fn test_complex_conditions() {
95
let x: Result<(), ()> = Ok(());
106
let y: Result<(), ()> = Ok(());
117
if x.is_ok() && y.is_err() {
12-
// unnecessary
138
x.unwrap();
149
//~^ unnecessary_unwrap
1510

16-
// will panic
1711
x.unwrap_err();
1812
//~^ panicking_unwrap
1913

20-
// will panic
2114
y.unwrap();
2215
//~^ panicking_unwrap
2316

24-
// unnecessary
2517
y.unwrap_err();
2618
//~^ unnecessary_unwrap
2719
} else {
@@ -37,45 +29,35 @@ fn test_complex_conditions() {
3729
x.unwrap();
3830
y.unwrap();
3931
} else {
40-
// will panic
4132
x.unwrap();
4233
//~^ panicking_unwrap
4334

44-
// unnecessary
4535
x.unwrap_err();
4636
//~^ unnecessary_unwrap
4737

48-
// will panic
4938
y.unwrap();
5039
//~^ panicking_unwrap
5140

52-
// unnecessary
5341
y.unwrap_err();
5442
//~^ unnecessary_unwrap
5543
}
5644
let z: Result<(), ()> = Ok(());
5745
if x.is_ok() && !(y.is_ok() || z.is_err()) {
58-
// unnecessary
5946
x.unwrap();
6047
//~^ unnecessary_unwrap
6148

62-
// will panic
6349
x.unwrap_err();
6450
//~^ panicking_unwrap
6551

66-
// will panic
6752
y.unwrap();
6853
//~^ panicking_unwrap
6954

70-
// unnecessary
7155
y.unwrap_err();
7256
//~^ unnecessary_unwrap
7357

74-
// unnecessary
7558
z.unwrap();
7659
//~^ unnecessary_unwrap
7760

78-
// will panic
7961
z.unwrap_err();
8062
//~^ panicking_unwrap
8163
}
@@ -85,27 +67,21 @@ fn test_complex_conditions() {
8567
y.unwrap();
8668
z.unwrap();
8769
} else {
88-
// will panic
8970
x.unwrap();
9071
//~^ panicking_unwrap
9172

92-
// unnecessary
9373
x.unwrap_err();
9474
//~^ unnecessary_unwrap
9575

96-
// unnecessary
9776
y.unwrap();
9877
//~^ unnecessary_unwrap
9978

100-
// will panic
10179
y.unwrap_err();
10280
//~^ panicking_unwrap
10381

104-
// will panic
10582
z.unwrap();
10683
//~^ panicking_unwrap
10784

108-
// unnecessary
10985
z.unwrap_err();
11086
//~^ unnecessary_unwrap
11187
}

0 commit comments

Comments
 (0)