Skip to content

Commit ddbd7eb

Browse files
committed
Debugging attempts
1 parent 6117bb7 commit ddbd7eb

File tree

3 files changed

+58
-42
lines changed

3 files changed

+58
-42
lines changed
Lines changed: 45 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::utils::sugg::Sugg;
2-
use crate::utils::{match_type, match_qpath, paths, span_lint_and_sugg};
2+
use crate::utils::{match_qpath, match_type, paths, span_lint, span_lint_and_sugg};
33
use if_chain::if_chain;
44

55
use rustc_errors::Applicability;
@@ -42,54 +42,61 @@ declare_clippy_lint! {
4242

4343
declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]);
4444

45-
/// If this is the option_if_let_else thing we're detecting, then this
46-
/// function returns Some(Option<_> compared, value if Option is Some,
47-
/// value if Option is None). Otherwise, it returns None
45+
/// If this is the option if let/else thing we're detecting, then this
46+
/// function returns Some(Option<_> compared, expression if Option is
47+
/// Some, expression if Option is None). Otherwise, it returns None
4848
fn detect_option_if_let_else<'a>(
4949
cx: &LateContext<'_, '_>,
5050
expr: &'a Expr<'_>,
5151
) -> Option<(&'a Expr<'a>, &'a Expr<'a>, &'a Expr<'a>)> {
52-
if_chain! {
53-
if let ExprKind::Match(let_body, arms, MatchSource::IfLetDesugar { contains_else_clause: true } ) = &expr.kind;
54-
if arms.len() == 2;
55-
if match_type(cx, &cx.tables.expr_ty(let_body), &paths::OPTION);
56-
if let PatKind::TupleStruct(path, &[inner_pat], _) = &arms[0].pat.kind;
57-
if let PatKind::Wild | PatKind::Binding(..) = &inner_pat.kind;
58-
then {
59-
let (some_body, none_body) = if match_qpath(path, &paths::OPTION_SOME) {
60-
(arms[0].body, arms[1].body)
61-
} else {
62-
(arms[1].body, arms[0].body)
63-
};
64-
Some((let_body, some_body, none_body))
65-
} else {
66-
None
52+
if let ExprKind::Match(let_body, arms, MatchSource::IfLetDesugar{contains_else_clause: true}) = &expr.kind {
53+
dbg!("Found if let/else stmt");
54+
if arms.len() == 2 {
55+
dbg!("If let/else statement has two arms");
56+
if match_type(cx, &cx.tables.expr_ty(let_body), &paths::OPTION) {
57+
dbg!("rhs of if let is Option<T>");
58+
if let PatKind::TupleStruct(path, &[inner_pat], _) = &arms[0].pat.kind {
59+
dbg!("arm0 is TupleStruct");
60+
if let PatKind::Wild | PatKind::Binding(..) = &inner_pat.kind {
61+
dbg!("inside of Some(x) matches anything");
62+
let (some_body, none_body) = if match_qpath(path, &paths::OPTION_SOME) {
63+
(arms[0].body, arms[1].body)
64+
} else {
65+
(arms[1].body, arms[0].body)
66+
};
67+
return Some((let_body, some_body, none_body));
68+
}
69+
}
70+
}
6771
}
6872
}
73+
None
6974
}
7075

71-
/// Lint the option_if_let_else thing we're avoiding
72-
fn check_option_if_let_else(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
73-
if let Some((option, map, else_func)) = detect_option_if_let_else(cx, expr) {
74-
span_lint_and_sugg(
76+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OptionIfLetElse {
77+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
78+
span_lint(
7579
cx,
7680
OPTION_IF_LET_ELSE,
7781
expr.span,
78-
"use Option::map_or here",
79-
"try",
80-
format!(
81-
"{}.map_or({}, {})",
82-
Sugg::hir(cx, option, ".."),
83-
Sugg::hir(cx, else_func, ".."),
84-
Sugg::hir(cx, map, "..")
85-
),
86-
Applicability::MachineApplicable,
82+
"OptionIfLetElse::check_expr was run on this span",
8783
);
88-
}
89-
}
90-
91-
impl LateLintPass<'_, '_> for OptionIfLetElse {
92-
fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
93-
check_option_if_let_else(cx, expr);
84+
return;
85+
if let Some((option, map, else_func)) = detect_option_if_let_else(cx, expr) {
86+
span_lint_and_sugg(
87+
cx,
88+
OPTION_IF_LET_ELSE,
89+
expr.span,
90+
"use Option::map_or_else instead of an if let/else",
91+
"try",
92+
format!(
93+
"{}.map_or_else({}, {})",
94+
Sugg::hir(cx, option, ".."),
95+
Sugg::hir(cx, else_func, ".."),
96+
Sugg::hir(cx, map, "..")
97+
),
98+
Applicability::MachineApplicable,
99+
);
100+
}
94101
}
95102
}

src/lintlist/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1522,7 +1522,7 @@ pub const ALL_LINTS: [Lint; 360] = [
15221522
Lint {
15231523
name: "option_if_let_else",
15241524
group: "style",
1525-
desc: "default lint description",
1525+
desc: "reimplementation of Option::map_or",
15261526
deprecation: None,
15271527
module: "option_if_let_else",
15281528
},

tests/ui/option_if_let_else.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,19 @@
11
#![warn(clippy::option_if_let_else)]
22

3+
fn bad1(string: Option<&str>) -> (bool, &str) {
4+
if let Some(x) = string {
5+
(true, x)
6+
} else {
7+
(false, "hello")
8+
}
9+
}
10+
311
fn main() {
412
let optional = Some(5);
5-
if let Some(x) = optional {
6-
x+2
13+
let _ = if let Some(x) = optional {
14+
x + 2
715
} else {
816
5
9-
}
17+
};
18+
let _ = bad1(None);
1019
}

0 commit comments

Comments
 (0)