Skip to content

Commit 5602ef8

Browse files
committed
First attempt at coding lint
1 parent c496f4e commit 5602ef8

File tree

5 files changed

+116
-0
lines changed

5 files changed

+116
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1405,6 +1405,7 @@ Released 2018-09-13
14051405
[`option_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref
14061406
[`option_env_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_env_unwrap
14071407
[`option_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_expect_used
1408+
[`option_if_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_if_let_else
14081409
[`option_map_or_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_or_none
14091410
[`option_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unit_fn
14101411
[`option_map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unwrap_or

clippy_lints/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
733733
&non_expressive_names::SIMILAR_NAMES,
734734
&open_options::NONSENSICAL_OPEN_OPTIONS,
735735
&option_env_unwrap::OPTION_ENV_UNWRAP,
736+
&option_if_let_else::OPTION_IF_LET_ELSE,
736737
&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL,
737738
&panic_unimplemented::PANIC,
738739
&panic_unimplemented::PANIC_PARAMS,
@@ -1330,6 +1331,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13301331
LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES),
13311332
LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS),
13321333
LintId::of(&option_env_unwrap::OPTION_ENV_UNWRAP),
1334+
LintId::of(&option_if_let_else::OPTION_IF_LET_ELSE),
13331335
LintId::of(&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
13341336
LintId::of(&panic_unimplemented::PANIC_PARAMS),
13351337
LintId::of(&partialeq_ne_impl::PARTIALEQ_NE_IMPL),
@@ -1476,6 +1478,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
14761478
LintId::of(&new_without_default::NEW_WITHOUT_DEFAULT),
14771479
LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
14781480
LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES),
1481+
LintId::of(&option_if_let_else::OPTION_IF_LET_ELSE),
14791482
LintId::of(&panic_unimplemented::PANIC_PARAMS),
14801483
LintId::of(&ptr::CMP_NULL),
14811484
LintId::of(&ptr::PTR_ARG),
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
use crate::utils::sugg::Sugg;
2+
use crate::utils::{match_type, match_qpath, paths, span_lint_and_sugg};
3+
use if_chain::if_chain;
4+
5+
use rustc_errors::Applicability;
6+
use rustc_hir::*;
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_session::{declare_lint_pass, declare_tool_lint};
9+
10+
declare_clippy_lint! {
11+
/// **What it does:**
12+
/// Detects people who use `if let Some(v) = ... { y } else { x }`
13+
/// when they could use `Option::map_or` instead.
14+
///
15+
/// **Why is this bad?**
16+
/// Using the dedicated function in the Option class is clearer and
17+
/// more concise than an if let
18+
///
19+
/// **Known problems:** None.
20+
///
21+
/// **Example:**
22+
///
23+
/// ```rust
24+
/// let var: Option<u32> = Some(5u32);
25+
/// let _ = if let Some(foo) = var {
26+
/// foo
27+
/// } else {
28+
/// 5
29+
/// };
30+
/// ```
31+
///
32+
/// should be
33+
///
34+
/// ```rust
35+
/// let var: Option<u32> = Some(5u32);
36+
/// let _ = var.map_or(5, |foo| foo);
37+
/// ```
38+
pub OPTION_IF_LET_ELSE,
39+
style,
40+
"reimplementation of Option::map_or"
41+
}
42+
43+
declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]);
44+
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
48+
fn detect_option_if_let_else<'a>(
49+
cx: &LateContext<'_, '_>,
50+
expr: &'a Expr<'_>,
51+
) -> 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
67+
}
68+
}
69+
}
70+
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(
75+
cx,
76+
OPTION_IF_LET_ELSE,
77+
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,
87+
);
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);
94+
}
95+
}

src/lintlist/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1536,6 +1536,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
15361536
deprecation: None,
15371537
module: "methods",
15381538
},
1539+
Lint {
1540+
name: "option_if_let_else",
1541+
group: "style",
1542+
desc: "default lint description",
1543+
deprecation: None,
1544+
module: "option_if_let_else",
1545+
},
15391546
Lint {
15401547
name: "option_map_or_none",
15411548
group: "style",

tests/ui/option_if_let_else.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#![warn(clippy::option_if_let_else)]
2+
3+
fn main() {
4+
let optional = Some(5);
5+
if let Some(x) = optional {
6+
x+2
7+
} else {
8+
5
9+
}
10+
}

0 commit comments

Comments
 (0)