First attempt at coding lint

Author: JarredAllen

CHANGELOG.md

@@ -1282,6 +1282,7 @@ Released 2018-09-13
 [`option_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref
 [`option_env_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_env_unwrap
 [`option_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_expect_used
+[`option_if_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_if_let_else
 [`option_map_or_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_or_none
 [`option_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unit_fn
 [`option_map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unwrap_or

README.md

@@ -5,7 +5,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 359 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 360 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 

clippy_lints/src/lib.rs

@@ -269,6 +269,7 @@ pub mod non_copy_const;
 pub mod non_expressive_names;
 pub mod open_options;
 pub mod option_env_unwrap;
+pub mod option_if_let_else;
 pub mod overflow_check_conditional;
 pub mod panic_unimplemented;
 pub mod partialeq_ne_impl;
@@ -719,6 +720,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &non_expressive_names::SIMILAR_NAMES,
         &open_options::NONSENSICAL_OPEN_OPTIONS,
         &option_env_unwrap::OPTION_ENV_UNWRAP,
+        &option_if_let_else::OPTION_IF_LET_ELSE,
         &overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL,
         &panic_unimplemented::PANIC,
         &panic_unimplemented::PANIC_PARAMS,
@@ -1298,6 +1300,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES),
         LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS),
         LintId::of(&option_env_unwrap::OPTION_ENV_UNWRAP),
+        LintId::of(&option_if_let_else::OPTION_IF_LET_ELSE),
         LintId::of(&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
         LintId::of(&panic_unimplemented::PANIC_PARAMS),
         LintId::of(&partialeq_ne_impl::PARTIALEQ_NE_IMPL),
@@ -1446,6 +1449,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&new_without_default::NEW_WITHOUT_DEFAULT),
         LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
         LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES),
+        LintId::of(&option_if_let_else::OPTION_IF_LET_ELSE),
         LintId::of(&panic_unimplemented::PANIC_PARAMS),
         LintId::of(&ptr::CMP_NULL),
         LintId::of(&ptr::PTR_ARG),

clippy_lints/src/option_if_let_else.rs

@@ -0,0 +1,95 @@
+use crate::utils::sugg::Sugg;
+use crate::utils::{match_type, match_qpath, paths, span_lint_and_sugg};
+use if_chain::if_chain;
+
+use rustc_errors::Applicability;
+use rustc_hir::*;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// **What it does:**
+    /// Detects people who use `if let Some(v) = ... { y } else { x }`
+    /// when they could use `Option::map_or` instead.
+    ///
+    /// **Why is this bad?**
+    /// Using the dedicated function in the Option class is clearer and
+    /// more concise than an if let
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// let var: Option<u32> = Some(5u32);
+    /// let _ = if let Some(foo) = var {
+    ///     foo
+    /// } else {
+    ///     5
+    /// };
+    /// ```
+    ///
+    /// should be
+    ///
+    /// ```rust
+    /// let var: Option<u32> = Some(5u32);
+    /// let _ = var.map_or(5, |foo| foo);
+    /// ```
+    pub OPTION_IF_LET_ELSE,
+    style,
+    "reimplementation of Option::map_or"
+}
+
+declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]);
+
+/// If this is the option_if_let_else thing we're detecting, then this
+/// function returns Some(Option<_> compared, value if Option is Some,
+/// value if Option is None). Otherwise, it returns None
+fn detect_option_if_let_else<'a>(
+    cx: &LateContext<'_, '_>,
+    expr: &'a Expr<'_>,
+) -> Option<(&'a Expr<'a>, &'a Expr<'a>, &'a Expr<'a>)> {
+    if_chain! {
+        if let ExprKind::Match(let_body, arms, MatchSource::IfLetDesugar { contains_else_clause: true } ) = &expr.kind;
+        if arms.len() == 2;
+        if match_type(cx, &cx.tables.expr_ty(let_body), &paths::OPTION);
+        if let PatKind::TupleStruct(path, &[inner_pat], _) = &arms[0].pat.kind;
+        if let PatKind::Wild | PatKind::Binding(..) = &inner_pat.kind;
+        then {
+            let (some_body, none_body) = if match_qpath(path, &paths::OPTION_SOME) {
+                (arms[0].body, arms[1].body)
+            } else {
+                (arms[1].body, arms[0].body)
+            };
+            Some((let_body, some_body, none_body))
+        } else {
+            None
+        }
+    }
+}
+
+/// Lint the option_if_let_else thing we're avoiding
+fn check_option_if_let_else(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
+    if let Some((option, map, else_func)) = detect_option_if_let_else(cx, expr) {
+        span_lint_and_sugg(
+            cx,
+            OPTION_IF_LET_ELSE,
+            expr.span,
+            "use Option::map_or here",
+            "try",
+            format!(
+                "{}.map_or({}, {})",
+                Sugg::hir(cx, option, ".."),
+                Sugg::hir(cx, else_func, ".."),
+                Sugg::hir(cx, map, "..")
+            ),
+            Applicability::MachineApplicable,
+        );
+    }
+}
+
+impl LateLintPass<'_, '_> for OptionIfLetElse {
+    fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
+        check_option_if_let_else(cx, expr);
+    }
+}

src/lintlist/mod.rs

@@ -6,7 +6,7 @@ pub use lint::Lint;
 pub use lint::LINT_LEVELS;
 
 // begin lint list, do not remove this comment, it’s used in `update_lints`
-pub const ALL_LINTS: [Lint; 359] = [
+pub const ALL_LINTS: [Lint; 360] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -1519,6 +1519,13 @@ pub const ALL_LINTS: [Lint; 359] = [
         deprecation: None,
         module: "methods",
     },
+    Lint {
+        name: "option_if_let_else",
+        group: "style",
+        desc: "default lint description",
+        deprecation: None,
+        module: "option_if_let_else",
+    },
     Lint {
         name: "option_map_or_none",
         group: "style",

tests/ui/option_if_let_else.rs

@@ -0,0 +1,10 @@
+#![warn(clippy::option_if_let_else)]
+
+fn main() {
+    let optional = Some(5);
+    if let Some(x) = optional {
+        x+2
+    } else {
+        5
+    }
+}