Skip to content

Commit 44e0747

Browse files
committed
Auto merge of #7216 - ThibsG:OptionIfLetElse7006, r=llogiq
Stop linting `else if let` pattern in [`option_if_let_else`] lint For readability concerns, it is counterproductive to lint `else if let` pattern. Unfortunately the suggested code is much less readable. Fixes: #7006 changelog: stop linting `else if let` pattern in [`option_if_let_else`] lint
2 parents 48dad26 + 368e621 commit 44e0747

File tree

3 files changed

+9
-14
lines changed

3 files changed

+9
-14
lines changed

clippy_lints/src/option_if_let_else.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::sugg::Sugg;
33
use clippy_utils::ty::is_type_diagnostic_item;
44
use clippy_utils::usage::contains_return_break_continue_macro;
5-
use clippy_utils::{eager_or_lazy, get_enclosing_block, in_macro, is_lang_ctor};
5+
use clippy_utils::{eager_or_lazy, get_enclosing_block, in_macro, is_else_clause, is_lang_ctor};
66
use if_chain::if_chain;
77
use rustc_errors::Applicability;
88
use rustc_hir::LangItem::OptionSome;
@@ -161,13 +161,15 @@ fn detect_option_if_let_else<'tcx>(
161161
if_chain! {
162162
if !in_macro(expr.span); // Don't lint macros, because it behaves weirdly
163163
if let ExprKind::Match(cond_expr, arms, MatchSource::IfLetDesugar{contains_else_clause: true}) = &expr.kind;
164+
if !is_else_clause(cx.tcx, expr);
164165
if arms.len() == 2;
165166
if !is_result_ok(cx, cond_expr); // Don't lint on Result::ok because a different lint does it already
166167
if let PatKind::TupleStruct(struct_qpath, &[inner_pat], _) = &arms[0].pat.kind;
167168
if is_lang_ctor(cx, struct_qpath, OptionSome);
168169
if let PatKind::Binding(bind_annotation, _, id, _) = &inner_pat.kind;
169170
if !contains_return_break_continue_macro(arms[0].body);
170171
if !contains_return_break_continue_macro(arms[1].body);
172+
171173
then {
172174
let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" };
173175
let some_body = extract_body_from_arm(&arms[0])?;

tests/ui/option_if_let_else.fixed

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ fn bad1(string: Option<&str>) -> (bool, &str) {
1010
fn else_if_option(string: Option<&str>) -> Option<(bool, &str)> {
1111
if string.is_none() {
1212
None
13-
} else { string.map_or(Some((false, "")), |x| Some((true, x))) }
13+
} else if let Some(x) = string {
14+
Some((true, x))
15+
} else {
16+
Some((false, ""))
17+
}
1418
}
1519

1620
fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {

tests/ui/option_if_let_else.stderr

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,6 @@ LL | | }
1010
|
1111
= note: `-D clippy::option-if-let-else` implied by `-D warnings`
1212

13-
error: use Option::map_or instead of an if let/else
14-
--> $DIR/option_if_let_else.rs:17:12
15-
|
16-
LL | } else if let Some(x) = string {
17-
| ____________^
18-
LL | | Some((true, x))
19-
LL | | } else {
20-
LL | | Some((false, ""))
21-
LL | | }
22-
| |_____^ help: try: `{ string.map_or(Some((false, "")), |x| Some((true, x))) }`
23-
2413
error: use Option::map_or instead of an if let/else
2514
--> $DIR/option_if_let_else.rs:25:13
2615
|
@@ -159,5 +148,5 @@ error: use Option::map_or instead of an if let/else
159148
LL | let _ = if let Some(x) = optional { x + 2 } else { 5 };
160149
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)`
161150

162-
error: aborting due to 12 previous errors
151+
error: aborting due to 11 previous errors
163152

0 commit comments

Comments
 (0)