about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2020-07-06 15:56:52 +0000
committerbors <bors@rust-lang.org>2020-07-06 15:56:52 +0000
commitac856922f80e9ef8cd95c3004699e4bc8fa0c978 (patch)
tree49cbbe0c038ae067a7c256f94c876b4dc04985a6
parent57cdf2dc16e1833008ae5b120cb2c045d267dda8 (diff)
parentc8f700ea697f74ef8f86891b050c859cf457e3ab (diff)
downloadrust-ac856922f80e9ef8cd95c3004699e4bc8fa0c978.tar.gz
rust-ac856922f80e9ef8cd95c3004699e4bc8fa0c978.zip
Auto merge of #5301 - JarredAllen:option_if_let_else, r=flip1995
Suggest `Option::map_or`(_else) for `if let Some { y } else { x }`

Fixes #5203

There are two issues with this code that I have noticed:

- Use of `Option::map_or` causes it to always evaluate the code in the else block. If that block is computationally expensive or if it updates some state (such as getting the next value from an iterator), then this change would cause the code to behave differently. In either of those circumstances, it should suggest Option::map_or_else, which takes both cases as a closure and runs one. However, I don't know how to check if the expression would change some state, so I left the lint's applicability as MaybeIncorrect.

- There are lints which can trigger on specific sub-cases of this lint (`if_let_some_result`, `question_mark`, and `while_let_loop`) and suggest different changes (usually better ones because they're more specific). Is this acceptable for clippy to give multiple suggestions, or should I have the code check if those other lints trigger and then not trigger this lint if they do?

changelog: Add lint [`option_if_let_else`]
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/attrs.rs20
-rw-r--r--clippy_lints/src/if_let_mutex.rs9
-rw-r--r--clippy_lints/src/len_zero.rs16
-rw-r--r--clippy_lints/src/lib.rs4
-rw-r--r--clippy_lints/src/literal_representation.rs9
-rw-r--r--clippy_lints/src/loops.rs32
-rw-r--r--clippy_lints/src/methods/mod.rs10
-rw-r--r--clippy_lints/src/methods/unnecessary_filter_map.rs11
-rw-r--r--clippy_lints/src/minmax.rs25
-rw-r--r--clippy_lints/src/misc.rs14
-rw-r--r--clippy_lints/src/option_if_let_else.rs267
-rw-r--r--clippy_lints/src/returns.rs18
-rw-r--r--clippy_lints/src/shadow.rs12
-rw-r--r--clippy_lints/src/types.rs23
-rw-r--r--clippy_lints/src/use_self.rs9
-rw-r--r--clippy_lints/src/utils/attrs.rs69
-rw-r--r--clippy_lints/src/utils/mod.rs62
-rw-r--r--clippy_lints/src/utils/numeric_literal.rs6
-rw-r--r--clippy_lints/src/utils/sugg.rs22
-rw-r--r--clippy_lints/src/write.rs13
-rw-r--r--src/lintlist/mod.rs7
-rw-r--r--tests/compile-test.rs12
-rw-r--r--tests/ui/option_if_let_else.fixed74
-rw-r--r--tests/ui/option_if_let_else.rs92
-rw-r--r--tests/ui/option_if_let_else.stderr151
26 files changed, 760 insertions, 228 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ed8f16e65bb..1a081bb85fe 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1577,6 +1577,7 @@ Released 2018-09-13
 [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
 [`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_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_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_option
diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs
index 2d0855f6895..d68d0d8ccf5 100644
--- a/clippy_lints/src/attrs.rs
+++ b/clippy_lints/src/attrs.rs
@@ -481,15 +481,14 @@ fn is_relevant_trait(cx: &LateContext<'_>, item: &TraitItem<'_>) -> bool {
 }
 
 fn is_relevant_block(cx: &LateContext<'_>, tables: &ty::TypeckTables<'_>, block: &Block<'_>) -> bool {
-    if let Some(stmt) = block.stmts.first() {
-        match &stmt.kind {
+    block.stmts.first().map_or(
+        block.expr.as_ref().map_or(false, |e| is_relevant_expr(cx, tables, e)),
+        |stmt| match &stmt.kind {
             StmtKind::Local(_) => true,
             StmtKind::Expr(expr) | StmtKind::Semi(expr) => is_relevant_expr(cx, tables, expr),
             _ => false,
-        }
-    } else {
-        block.expr.as_ref().map_or(false, |e| is_relevant_expr(cx, tables, e))
-    }
+        },
+    )
 }
 
 fn is_relevant_expr(cx: &LateContext<'_>, tables: &ty::TypeckTables<'_>, expr: &Expr<'_>) -> bool {
@@ -499,11 +498,10 @@ fn is_relevant_expr(cx: &LateContext<'_>, tables: &ty::TypeckTables<'_>, expr: &
         ExprKind::Ret(None) | ExprKind::Break(_, None) => false,
         ExprKind::Call(path_expr, _) => {
             if let ExprKind::Path(qpath) = &path_expr.kind {
-                if let Some(fun_id) = tables.qpath_res(qpath, path_expr.hir_id).opt_def_id() {
-                    !match_def_path(cx, fun_id, &paths::BEGIN_PANIC)
-                } else {
-                    true
-                }
+                tables
+                    .qpath_res(qpath, path_expr.hir_id)
+                    .opt_def_id()
+                    .map_or(true, |fun_id| !match_def_path(cx, fun_id, &paths::BEGIN_PANIC))
             } else {
                 true
             }
diff --git a/clippy_lints/src/if_let_mutex.rs b/clippy_lints/src/if_let_mutex.rs
index f911cb68ea5..fbd2eeacc6e 100644
--- a/clippy_lints/src/if_let_mutex.rs
+++ b/clippy_lints/src/if_let_mutex.rs
@@ -135,13 +135,10 @@ impl<'tcx> Visitor<'tcx> for ArmVisitor<'_, 'tcx> {
     }
 }
 
-impl<'tcx> ArmVisitor<'_, 'tcx> {
+impl<'tcx, 'l> ArmVisitor<'tcx, 'l> {
     fn same_mutex(&self, cx: &LateContext<'_>, op_mutex: &Expr<'_>) -> bool {
-        if let Some(arm_mutex) = self.found_mutex {
-            SpanlessEq::new(cx).eq_expr(op_mutex, arm_mutex)
-        } else {
-            false
-        }
+        self.found_mutex
+            .map_or(false, |arm_mutex| SpanlessEq::new(cx).eq_expr(op_mutex, arm_mutex))
     }
 }
 
diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs
index 26d96428771..1b09328ceab 100644
--- a/clippy_lints/src/len_zero.rs
+++ b/clippy_lints/src/len_zero.rs
@@ -302,16 +302,12 @@ fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
 
     let ty = &walk_ptrs_ty(cx.tables().expr_ty(expr));
     match ty.kind {
-        ty::Dynamic(ref tt, ..) => {
-            if let Some(principal) = tt.principal() {
-                cx.tcx
-                    .associated_items(principal.def_id())
-                    .in_definition_order()
-                    .any(|item| is_is_empty(cx, &item))
-            } else {
-                false
-            }
-        },
+        ty::Dynamic(ref tt, ..) => tt.principal().map_or(false, |principal| {
+            cx.tcx
+                .associated_items(principal.def_id())
+                .in_definition_order()
+                .any(|item| is_is_empty(cx, &item))
+        }),
         ty::Projection(ref proj) => has_is_empty_impl(cx, proj.item_def_id),
         ty::Adt(id, _) => has_is_empty_impl(cx, id.did),
         ty::Array(..) | ty::Slice(..) | ty::Str => true,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 1d5be893ffb..fe34e4390d6 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -264,6 +264,7 @@ mod non_copy_const;
 mod non_expressive_names;
 mod open_options;
 mod option_env_unwrap;
+mod option_if_let_else;
 mod overflow_check_conditional;
 mod panic_unimplemented;
 mod partialeq_ne_impl;
@@ -734,6 +735,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,
@@ -1052,6 +1054,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default());
     store.register_late_pass(|| box unnamed_address::UnnamedAddress);
     store.register_late_pass(|| box dereference::Dereferencing);
+    store.register_late_pass(|| box option_if_let_else::OptionIfLetElse);
     store.register_late_pass(|| box future_not_send::FutureNotSend);
     store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls);
     store.register_late_pass(|| box if_let_mutex::IfLetMutex);
@@ -1158,6 +1161,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&needless_continue::NEEDLESS_CONTINUE),
         LintId::of(&needless_pass_by_value::NEEDLESS_PASS_BY_VALUE),
         LintId::of(&non_expressive_names::SIMILAR_NAMES),
+        LintId::of(&option_if_let_else::OPTION_IF_LET_ELSE),
         LintId::of(&ranges::RANGE_PLUS_ONE),
         LintId::of(&shadow::SHADOW_UNRELATED),
         LintId::of(&strings::STRING_ADD_ASSIGN),
diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs
index 7ba43562d7d..a36fdca5d5d 100644
--- a/clippy_lints/src/literal_representation.rs
+++ b/clippy_lints/src/literal_representation.rs
@@ -264,10 +264,13 @@ impl LiteralDigitGrouping {
 
         let (part, mistyped_suffixes, missing_char) = if let Some((_, exponent)) = &mut num_lit.exponent {
             (exponent, &["32", "64"][..], 'f')
-        } else if let Some(fraction) = &mut num_lit.fraction {
-            (fraction, &["32", "64"][..], 'f')
         } else {
-            (&mut num_lit.integer, &["8", "16", "32", "64"][..], 'i')
+            num_lit
+                .fraction
+                .as_mut()
+                .map_or((&mut num_lit.integer, &["8", "16", "32", "64"][..], 'i'), |fraction| {
+                    (fraction, &["32", "64"][..], 'f')
+                })
         };
 
         let mut split = part.rsplit('_');
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index d821b513484..b803d753b6d 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -686,13 +686,9 @@ fn never_loop_expr(expr: &Expr<'_>, main_loop_id: HirId) -> NeverLoopResult {
                 NeverLoopResult::AlwaysBreak
             }
         },
-        ExprKind::Break(_, ref e) | ExprKind::Ret(ref e) => {
-            if let Some(ref e) = *e {
-                combine_seq(never_loop_expr(e, main_loop_id), NeverLoopResult::AlwaysBreak)
-            } else {
-                NeverLoopResult::AlwaysBreak
-            }
-        },
+        ExprKind::Break(_, ref e) | ExprKind::Ret(ref e) => e.as_ref().map_or(NeverLoopResult::AlwaysBreak, |e| {
+            combine_seq(never_loop_expr(e, main_loop_id), NeverLoopResult::AlwaysBreak)
+        }),
         ExprKind::InlineAsm(ref asm) => asm
             .operands
             .iter()
@@ -1881,13 +1877,9 @@ fn is_ref_iterable_type(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
 fn is_iterable_array<'tcx>(ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool {
     // IntoIterator is currently only implemented for array sizes <= 32 in rustc
     match ty.kind {
-        ty::Array(_, n) => {
-            if let Some(val) = n.try_eval_usize(cx.tcx, cx.param_env) {
-                (0..=32).contains(&val)
-            } else {
-                false
-            }
-        },
+        ty::Array(_, n) => n
+            .try_eval_usize(cx.tcx, cx.param_env)
+            .map_or(false, |val| (0..=32).contains(&val)),
         _ => false,
     }
 }
@@ -1899,11 +1891,7 @@ fn extract_expr_from_first_stmt<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<
         return None;
     }
     if let StmtKind::Local(ref local) = block.stmts[0].kind {
-        if let Some(expr) = local.init {
-            Some(expr)
-        } else {
-            None
-        }
+        local.init //.map(|expr| expr)
     } else {
         None
     }
@@ -2023,15 +2011,13 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
                 if let PatKind::Binding(.., ident, _) = local.pat.kind {
                     self.name = Some(ident.name);
 
-                    self.state = if let Some(ref init) = local.init {
+                    self.state = local.init.as_ref().map_or(VarState::Declared, |init| {
                         if is_integer_const(&self.cx, init, 0) {
                             VarState::Warn
                         } else {
                             VarState::Declared
                         }
-                    } else {
-                        VarState::Declared
-                    }
+                    })
                 }
             }
         }
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 160304865c5..f1c8894c0ee 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -2460,13 +2460,9 @@ fn derefs_to_slice<'tcx>(
             ty::Slice(_) => true,
             ty::Adt(def, _) if def.is_box() => may_slice(cx, ty.boxed_ty()),
             ty::Adt(..) => is_type_diagnostic_item(cx, ty, sym!(vec_type)),
-            ty::Array(_, size) => {
-                if let Some(size) = size.try_eval_usize(cx.tcx, cx.param_env) {
-                    size < 32
-                } else {
-                    false
-                }
-            },
+            ty::Array(_, size) => size
+                .try_eval_usize(cx.tcx, cx.param_env)
+                .map_or(false, |size| size < 32),
             ty::Ref(_, inner, _) => may_slice(cx, inner),
             _ => false,
         }
diff --git a/clippy_lints/src/methods/unnecessary_filter_map.rs b/clippy_lints/src/methods/unnecessary_filter_map.rs
index fdcba110542..75e123eb593 100644
--- a/clippy_lints/src/methods/unnecessary_filter_map.rs
+++ b/clippy_lints/src/methods/unnecessary_filter_map.rs
@@ -77,13 +77,10 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc
             }
             (true, true)
         },
-        hir::ExprKind::Block(ref block, _) => {
-            if let Some(expr) = &block.expr {
-                check_expression(cx, arg_id, &expr)
-            } else {
-                (false, false)
-            }
-        },
+        hir::ExprKind::Block(ref block, _) => block
+            .expr
+            .as_ref()
+            .map_or((false, false), |expr| check_expression(cx, arg_id, &expr)),
         hir::ExprKind::Match(_, arms, _) => {
             let mut found_mapping = false;
             let mut found_filtering = false;
diff --git a/clippy_lints/src/minmax.rs b/clippy_lints/src/minmax.rs
index 0a2d577396a..c8aa98d3489 100644
--- a/clippy_lints/src/minmax.rs
+++ b/clippy_lints/src/minmax.rs
@@ -53,7 +53,7 @@ impl<'tcx> LateLintPass<'tcx> for MinMaxPass {
     }
 }
 
-#[derive(PartialEq, Eq, Debug)]
+#[derive(PartialEq, Eq, Debug, Clone, Copy)]
 enum MinMax {
     Min,
     Max,
@@ -86,16 +86,15 @@ fn fetch_const<'a>(cx: &LateContext<'_>, args: &'a [Expr<'a>], m: MinMax) -> Opt
     if args.len() != 2 {
         return None;
     }
-    if let Some(c) = constant_simple(cx, cx.tables(), &args[0]) {
-        if constant_simple(cx, cx.tables(), &args[1]).is_none() {
-            // otherwise ignore
-            Some((m, c, &args[1]))
-        } else {
-            None
-        }
-    } else if let Some(c) = constant_simple(cx, cx.tables(), &args[1]) {
-        Some((m, c, &args[0]))
-    } else {
-        None
-    }
+    constant_simple(cx, cx.tables(), &args[0]).map_or_else(
+        || constant_simple(cx, cx.tables(), &args[1]).map(|c| (m, c, &args[0])),
+        |c| {
+            if constant_simple(cx, cx.tables(), &args[1]).is_none() {
+                // otherwise ignore
+                Some((m, c, &args[1]))
+            } else {
+                None
+            }
+        },
+    )
 }
diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs
index 6a256627bd1..3d4225f36a7 100644
--- a/clippy_lints/src/misc.rs
+++ b/clippy_lints/src/misc.rs
@@ -682,16 +682,10 @@ fn check_to_owned(cx: &LateContext<'_>, expr: &Expr<'_>, other: &Expr<'_>, left:
 /// `unused_variables`'s idea
 /// of what it means for an expression to be "used".
 fn is_used(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
-    if let Some(parent) = get_parent_expr(cx, expr) {
-        match parent.kind {
-            ExprKind::Assign(_, ref rhs, _) | ExprKind::AssignOp(_, _, ref rhs) => {
-                SpanlessEq::new(cx).eq_expr(rhs, expr)
-            },
-            _ => is_used(cx, parent),
-        }
-    } else {
-        true
-    }
+    get_parent_expr(cx, expr).map_or(true, |parent| match parent.kind {
+        ExprKind::Assign(_, ref rhs, _) | ExprKind::AssignOp(_, _, ref rhs) => SpanlessEq::new(cx).eq_expr(rhs, expr),
+        _ => is_used(cx, parent),
+    })
 }
 
 /// Tests whether an expression is in a macro expansion (e.g., something
diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs
new file mode 100644
index 00000000000..8dbe58763bf
--- /dev/null
+++ b/clippy_lints/src/option_if_let_else.rs
@@ -0,0 +1,267 @@
+use crate::utils;
+use crate::utils::sugg::Sugg;
+use crate::utils::{match_type, paths, span_lint_and_sugg};
+use if_chain::if_chain;
+
+use rustc_errors::Applicability;
+use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
+use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, MatchSource, Mutability, PatKind, UnOp};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::hir::map::Map;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// **What it does:**
+    /// Lints usage of  `if let Some(v) = ... { y } else { x }` which is more
+    /// idiomatically done with `Option::map_or` (if the else bit is a simple
+    /// expression) or `Option::map_or_else` (if the else bit is a longer
+    /// block).
+    ///
+    /// **Why is this bad?**
+    /// Using the dedicated functions of the Option type is clearer and
+    /// more concise than an if let expression.
+    ///
+    /// **Known problems:**
+    /// This lint uses whether the block is just an expression or if it has
+    /// more statements to decide whether to use `Option::map_or` or
+    /// `Option::map_or_else`. If you have a single expression which calls
+    /// an expensive function, then it would be more efficient to use
+    /// `Option::map_or_else`, but this lint would suggest `Option::map_or`.
+    ///
+    /// Also, this lint uses a deliberately conservative metric for checking
+    /// if the inside of either body contains breaks or continues which will
+    /// cause it to not suggest a fix if either block contains a loop with
+    /// continues or breaks contained within the loop.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// # let optional: Option<u32> = Some(0);
+    /// # fn do_complicated_function() -> u32 { 5 };
+    /// let _ = if let Some(foo) = optional {
+    ///     foo
+    /// } else {
+    ///     5
+    /// };
+    /// let _ = if let Some(foo) = optional {
+    ///     foo
+    /// } else {
+    ///     let y = do_complicated_function();
+    ///     y*y
+    /// };
+    /// ```
+    ///
+    /// should be
+    ///
+    /// ```rust
+    /// # let optional: Option<u32> = Some(0);
+    /// # fn do_complicated_function() -> u32 { 5 };
+    /// let _ = optional.map_or(5, |foo| foo);
+    /// let _ = optional.map_or_else(||{
+    ///     let y = do_complicated_function();
+    ///     y*y
+    /// }, |foo| foo);
+    /// ```
+    pub OPTION_IF_LET_ELSE,
+    pedantic,
+    "reimplementation of Option::map_or"
+}
+
+declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]);
+
+/// Returns true iff the given expression is the result of calling `Result::ok`
+fn is_result_ok(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool {
+    if let ExprKind::MethodCall(ref path, _, &[ref receiver], _) = &expr.kind {
+        path.ident.name.to_ident_string() == "ok" && match_type(cx, &cx.tables().expr_ty(&receiver), &paths::RESULT)
+    } else {
+        false
+    }
+}
+
+/// A struct containing information about occurences of the
+/// `if let Some(..) = .. else` construct that this lint detects.
+struct OptionIfLetElseOccurence {
+    option: String,
+    method_sugg: String,
+    some_expr: String,
+    none_expr: String,
+    wrap_braces: bool,
+}
+
+struct ReturnBreakContinueMacroVisitor {
+    seen_return_break_continue: bool,
+}
+impl ReturnBreakContinueMacroVisitor {
+    fn new() -> ReturnBreakContinueMacroVisitor {
+        ReturnBreakContinueMacroVisitor {
+            seen_return_break_continue: false,
+        }
+    }
+}
+impl<'tcx> Visitor<'tcx> for ReturnBreakContinueMacroVisitor {
+    type Map = Map<'tcx>;
+    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+        NestedVisitorMap::None
+    }
+
+    fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
+        if self.seen_return_break_continue {
+            // No need to look farther if we've already seen one of them
+            return;
+        }
+        match &ex.kind {
+            ExprKind::Ret(..) | ExprKind::Break(..) | ExprKind::Continue(..) => {
+                self.seen_return_break_continue = true;
+            },
+            // Something special could be done here to handle while or for loop
+            // desugaring, as this will detect a break if there's a while loop
+            // or a for loop inside the expression.
+            _ => {
+                if utils::in_macro(ex.span) {
+                    self.seen_return_break_continue = true;
+                } else {
+                    rustc_hir::intravisit::walk_expr(self, ex);
+                }
+            },
+        }
+    }
+}
+
+fn contains_return_break_continue_macro(expression: &Expr<'_>) -> bool {
+    let mut recursive_visitor = ReturnBreakContinueMacroVisitor::new();
+    recursive_visitor.visit_expr(expression);
+    recursive_visitor.seen_return_break_continue
+}
+
+/// Extracts the body of a given arm. If the arm contains only an expression,
+/// then it returns the expression. Otherwise, it returns the entire block
+fn extract_body_from_arm<'a>(arm: &'a Arm<'a>) -> Option<&'a Expr<'a>> {
+    if let ExprKind::Block(
+        Block {
+            stmts: statements,
+            expr: Some(expr),
+            ..
+        },
+        _,
+    ) = &arm.body.kind
+    {
+        if let [] = statements {
+            Some(&expr)
+        } else {
+            Some(&arm.body)
+        }
+    } else {
+        None
+    }
+}
+
+/// If this is the else body of an if/else expression, then we need to wrap
+/// it in curcly braces. Otherwise, we don't.
+fn should_wrap_in_braces(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    utils::get_enclosing_block(cx, expr.hir_id).map_or(false, |parent| {
+        if let Some(Expr {
+            kind:
+                ExprKind::Match(
+                    _,
+                    arms,
+                    MatchSource::IfDesugar {
+                        contains_else_clause: true,
+                    }
+                    | MatchSource::IfLetDesugar {
+                        contains_else_clause: true,
+                    },
+                ),
+            ..
+        }) = parent.expr
+        {
+            expr.hir_id == arms[1].body.hir_id
+        } else {
+            false
+        }
+    })
+}
+
+fn format_option_in_sugg(cx: &LateContext<'_>, cond_expr: &Expr<'_>, as_ref: bool, as_mut: bool) -> String {
+    format!(
+        "{}{}",
+        Sugg::hir(cx, cond_expr, "..").maybe_par(),
+        if as_mut {
+            ".as_mut()"
+        } else if as_ref {
+            ".as_ref()"
+        } else {
+            ""
+        }
+    )
+}
+
+/// If this expression is the option if let/else construct we're detecting, then
+/// this function returns an `OptionIfLetElseOccurence` struct with details if
+/// this construct is found, or None if this construct is not found.
+fn detect_option_if_let_else(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<OptionIfLetElseOccurence> {
+    if_chain! {
+        if !utils::in_macro(expr.span); // Don't lint macros, because it behaves weirdly
+        if let ExprKind::Match(cond_expr, arms, MatchSource::IfLetDesugar{contains_else_clause: true}) = &expr.kind;
+        if arms.len() == 2;
+        if !is_result_ok(cx, cond_expr); // Don't lint on Result::ok because a different lint does it already
+        if let PatKind::TupleStruct(struct_qpath, &[inner_pat], _) = &arms[0].pat.kind;
+        if utils::match_qpath(struct_qpath, &paths::OPTION_SOME);
+        if let PatKind::Binding(bind_annotation, _, id, _) = &inner_pat.kind;
+        if !contains_return_break_continue_macro(arms[0].body);
+        if !contains_return_break_continue_macro(arms[1].body);
+        then {
+            let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" };
+            let some_body = extract_body_from_arm(&arms[0])?;
+            let none_body = extract_body_from_arm(&arms[1])?;
+            let method_sugg = match &none_body.kind {
+                ExprKind::Block(..) => "map_or_else",
+                _ => "map_or",
+            };
+            let capture_name = id.name.to_ident_string();
+            let wrap_braces = should_wrap_in_braces(cx, expr);
+            let (as_ref, as_mut) = match &cond_expr.kind {
+                ExprKind::AddrOf(_, Mutability::Not, _) => (true, false),
+                ExprKind::AddrOf(_, Mutability::Mut, _) => (false, true),
+                _ => (bind_annotation == &BindingAnnotation::Ref, bind_annotation == &BindingAnnotation::RefMut),
+            };
+            let cond_expr = match &cond_expr.kind {
+                // Pointer dereferencing happens automatically, so we can omit it in the suggestion
+                ExprKind::Unary(UnOp::UnDeref, expr) | ExprKind::AddrOf(_, _, expr) => expr,
+                _ => cond_expr,
+            };
+            Some(OptionIfLetElseOccurence {
+                option: format_option_in_sugg(cx, cond_expr, as_ref, as_mut),
+                method_sugg: method_sugg.to_string(),
+                some_expr: format!("|{}{}| {}", capture_mut, capture_name, Sugg::hir(cx, some_body, "..")),
+                none_expr: format!("{}{}", if method_sugg == "map_or" { "" } else { "|| " }, Sugg::hir(cx, none_body, "..")),
+                wrap_braces,
+            })
+        } else {
+            None
+        }
+    }
+}
+
+impl<'a> LateLintPass<'a> for OptionIfLetElse {
+    fn check_expr(&mut self, cx: &LateContext<'a>, expr: &Expr<'_>) {
+        if let Some(detection) = detect_option_if_let_else(cx, expr) {
+            span_lint_and_sugg(
+                cx,
+                OPTION_IF_LET_ELSE,
+                expr.span,
+                format!("use Option::{} instead of an if let/else", detection.method_sugg).as_str(),
+                "try",
+                format!(
+                    "{}{}.{}({}, {}){}",
+                    if detection.wrap_braces { "{ " } else { "" },
+                    detection.option,
+                    detection.method_sugg,
+                    detection.none_expr,
+                    detection.some_expr,
+                    if detection.wrap_braces { " }" } else { "" },
+                ),
+                Applicability::MaybeIncorrect,
+            );
+        }
+    }
+}
diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index 3c939744173..faef7e724dd 100644
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -259,15 +259,15 @@ fn is_unit_expr(expr: &ast::Expr) -> bool {
 
 fn lint_unneeded_unit_return(cx: &EarlyContext<'_>, ty: &ast::Ty, span: Span) {
     let (ret_span, appl) = if let Ok(fn_source) = cx.sess().source_map().span_to_snippet(span.with_hi(ty.span.hi())) {
-        if let Some(rpos) = fn_source.rfind("->") {
-            #[allow(clippy::cast_possible_truncation)]
-            (
-                ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)),
-                Applicability::MachineApplicable,
-            )
-        } else {
-            (ty.span, Applicability::MaybeIncorrect)
-        }
+        fn_source
+            .rfind("->")
+            .map_or((ty.span, Applicability::MaybeIncorrect), |rpos| {
+                (
+                    #[allow(clippy::cast_possible_truncation)]
+                    ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)),
+                    Applicability::MachineApplicable,
+                )
+            })
     } else {
         (ty.span, Applicability::MaybeIncorrect)
     };
diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs
index 7da47ee4ff9..4cdff63f118 100644
--- a/clippy_lints/src/shadow.rs
+++ b/clippy_lints/src/shadow.rs
@@ -165,14 +165,10 @@ fn check_local<'tcx>(cx: &LateContext<'tcx>, local: &'tcx Local<'_>, bindings: &
 
 fn is_binding(cx: &LateContext<'_>, pat_id: HirId) -> bool {
     let var_ty = cx.tables().node_type_opt(pat_id);
-    if let Some(var_ty) = var_ty {
-        match var_ty.kind {
-            ty::Adt(..) => false,
-            _ => true,
-        }
-    } else {
-        false
-    }
+    var_ty.map_or(false, |var_ty| match var_ty.kind {
+        ty::Adt(..) => false,
+        _ => true,
+    })
 }
 
 fn check_pat<'tcx>(
diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs
index b1345f0de5e..d6f31a99bb3 100644
--- a/clippy_lints/src/types.rs
+++ b/clippy_lints/src/types.rs
@@ -1205,16 +1205,19 @@ fn span_lossless_lint(cx: &LateContext<'_>, expr: &Expr<'_>, op: &Expr<'_>, cast
     // has parens on the outside, they are no longer needed.
     let mut applicability = Applicability::MachineApplicable;
     let opt = snippet_opt(cx, op.span);
-    let sugg = if let Some(ref snip) = opt {
-        if should_strip_parens(op, snip) {
-            &snip[1..snip.len() - 1]
-        } else {
-            snip.as_str()
-        }
-    } else {
-        applicability = Applicability::HasPlaceholders;
-        ".."
-    };
+    let sugg = opt.as_ref().map_or_else(
+        || {
+            applicability = Applicability::HasPlaceholders;
+            ".."
+        },
+        |snip| {
+            if should_strip_parens(op, snip) {
+                &snip[1..snip.len() - 1]
+            } else {
+                snip.as_str()
+            }
+        },
+    );
 
     span_lint_and_sugg(
         cx,
diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs
index f85db1e2006..39a8c020872 100644
--- a/clippy_lints/src/use_self.rs
+++ b/clippy_lints/src/use_self.rs
@@ -167,14 +167,13 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
             if let TyKind::Path(QPath::Resolved(_, ref item_path)) = item_type.kind;
             then {
                 let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args;
-                let should_check = if let Some(ref params) = *parameters {
-                    !params.parenthesized && !params.args.iter().any(|arg| match arg {
+                let should_check = parameters.as_ref().map_or(
+                    true,
+                    |params| !params.parenthesized && !params.args.iter().any(|arg| match arg {
                         GenericArg::Lifetime(_) => true,
                         _ => false,
                     })
-                } else {
-                    true
-                };
+                );
 
                 if should_check {
                     let visitor = &mut UseSelfVisitor {
diff --git a/clippy_lints/src/utils/attrs.rs b/clippy_lints/src/utils/attrs.rs
index 4dcf6c105ec..4bb4b087c55 100644
--- a/clippy_lints/src/utils/attrs.rs
+++ b/clippy_lints/src/utils/attrs.rs
@@ -65,42 +65,45 @@ pub fn get_attr<'a>(
         };
         let attr_segments = &attr.path.segments;
         if attr_segments.len() == 2 && attr_segments[0].ident.to_string() == "clippy" {
-            if let Some(deprecation_status) =
-                BUILTIN_ATTRIBUTES
-                    .iter()
-                    .find_map(|(builtin_name, deprecation_status)| {
-                        if *builtin_name == attr_segments[1].ident.to_string() {
-                            Some(deprecation_status)
-                        } else {
-                            None
-                        }
-                    })
-            {
-                let mut diag = sess.struct_span_err(attr_segments[1].ident.span, "Usage of deprecated attribute");
-                match *deprecation_status {
-                    DeprecationStatus::Deprecated => {
-                        diag.emit();
-                        false
-                    },
-                    DeprecationStatus::Replaced(new_name) => {
-                        diag.span_suggestion(
-                            attr_segments[1].ident.span,
-                            "consider using",
-                            new_name.to_string(),
-                            Applicability::MachineApplicable,
-                        );
-                        diag.emit();
+            BUILTIN_ATTRIBUTES
+                .iter()
+                .find_map(|(builtin_name, deprecation_status)| {
+                    if *builtin_name == attr_segments[1].ident.to_string() {
+                        Some(deprecation_status)
+                    } else {
+                        None
+                    }
+                })
+                .map_or_else(
+                    || {
+                        sess.span_err(attr_segments[1].ident.span, "Usage of unknown attribute");
                         false
                     },
-                    DeprecationStatus::None => {
-                        diag.cancel();
-                        attr_segments[1].ident.to_string() == name
+                    |deprecation_status| {
+                        let mut diag =
+                            sess.struct_span_err(attr_segments[1].ident.span, "Usage of deprecated attribute");
+                        match *deprecation_status {
+                            DeprecationStatus::Deprecated => {
+                                diag.emit();
+                                false
+                            },
+                            DeprecationStatus::Replaced(new_name) => {
+                                diag.span_suggestion(
+                                    attr_segments[1].ident.span,
+                                    "consider using",
+                                    new_name.to_string(),
+                                    Applicability::MachineApplicable,
+                                );
+                                diag.emit();
+                                false
+                            },
+                            DeprecationStatus::None => {
+                                diag.cancel();
+                                attr_segments[1].ident.to_string() == name
+                            },
+                        }
                     },
-                }
-            } else {
-                sess.span_err(attr_segments[1].ident.span, "Usage of unknown attribute");
-                false
-            }
+                )
         } else {
             false
         }
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index 99ba7d64331..3a3b79925ff 100644
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -153,11 +153,7 @@ pub fn is_type_diagnostic_item(cx: &LateContext<'_>, ty: Ty<'_>, diag_item: Symb
 pub fn match_trait_method(cx: &LateContext<'_>, expr: &Expr<'_>, path: &[&str]) -> bool {
     let def_id = cx.tables().type_dependent_def_id(expr.hir_id).unwrap();
     let trt_id = cx.tcx.trait_of_item(def_id);
-    if let Some(trt_id) = trt_id {
-        match_def_path(cx, trt_id, path)
-    } else {
-        false
-    }
+    trt_id.map_or(false, |trt_id| match_def_path(cx, trt_id, path))
 }
 
 /// Checks if an expression references a variable of the given name.
@@ -600,21 +596,15 @@ pub fn snippet_block_with_applicability<'a, T: LintContext>(
 /// //  ^^^^^^^^^^
 /// ```
 pub fn first_line_of_span<T: LintContext>(cx: &T, span: Span) -> Span {
-    if let Some(first_char_pos) = first_char_in_first_line(cx, span) {
-        span.with_lo(first_char_pos)
-    } else {
-        span
-    }
+    first_char_in_first_line(cx, span).map_or(span, |first_char_pos| span.with_lo(first_char_pos))
 }
 
 fn first_char_in_first_line<T: LintContext>(cx: &T, span: Span) -> Option<BytePos> {
     let line_span = line_span(cx, span);
-    if let Some(snip) = snippet_opt(cx, line_span) {
+    snippet_opt(cx, line_span).and_then(|snip| {
         snip.find(|c: char| !c.is_whitespace())
             .map(|pos| line_span.lo() + BytePos::from_usize(pos))
-    } else {
-        None
-    }
+    })
 }
 
 /// Returns the indentation of the line of a span
@@ -626,11 +616,7 @@ fn first_char_in_first_line<T: LintContext>(cx: &T, span: Span) -> Option<BytePo
 /// //          ^^ -- will return 4
 /// ```
 pub fn indent_of<T: LintContext>(cx: &T, span: Span) -> Option<usize> {
-    if let Some(snip) = snippet_opt(cx, line_span(cx, span)) {
-        snip.find(|c: char| !c.is_whitespace())
-    } else {
-        None
-    }
+    snippet_opt(cx, line_span(cx, span)).and_then(|snip| snip.find(|c: char| !c.is_whitespace()))
 }
 
 /// Extends the span to the beginning of the spans line, incl. whitespaces.
@@ -738,25 +724,21 @@ pub fn get_enclosing_block<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Optio
     let enclosing_node = map
         .get_enclosing_scope(hir_id)
         .and_then(|enclosing_id| map.find(enclosing_id));
-    if let Some(node) = enclosing_node {
-        match node {
-            Node::Block(block) => Some(block),
-            Node::Item(&Item {
-                kind: ItemKind::Fn(_, _, eid),
-                ..
-            })
-            | Node::ImplItem(&ImplItem {
-                kind: ImplItemKind::Fn(_, eid),
-                ..
-            }) => match cx.tcx.hir().body(eid).value.kind {
-                ExprKind::Block(ref block, _) => Some(block),
-                _ => None,
-            },
+    enclosing_node.and_then(|node| match node {
+        Node::Block(block) => Some(block),
+        Node::Item(&Item {
+            kind: ItemKind::Fn(_, _, eid),
+            ..
+        })
+        | Node::ImplItem(&ImplItem {
+            kind: ImplItemKind::Fn(_, eid),
+            ..
+        }) => match cx.tcx.hir().body(eid).value.kind {
+            ExprKind::Block(ref block, _) => Some(block),
             _ => None,
-        }
-    } else {
-        None
-    }
+        },
+        _ => None,
+    })
 }
 
 /// Returns the base type for HIR references and pointers.
@@ -1328,11 +1310,7 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
         _ => None,
     };
 
-    if let Some(did) = did {
-        must_use_attr(&cx.tcx.get_attrs(did)).is_some()
-    } else {
-        false
-    }
+    did.map_or(false, |did| must_use_attr(&cx.tcx.get_attrs(did)).is_some())
 }
 
 pub fn is_no_std_crate(krate: &Crate<'_>) -> bool {
diff --git a/clippy_lints/src/utils/numeric_literal.rs b/clippy_lints/src/utils/numeric_literal.rs
index 99413153d49..7a79741b30b 100644
--- a/clippy_lints/src/utils/numeric_literal.rs
+++ b/clippy_lints/src/utils/numeric_literal.rs
@@ -200,12 +200,10 @@ impl<'a> NumericLiteral<'a> {
 
 fn split_suffix<'a>(src: &'a str, lit_kind: &LitKind) -> (&'a str, Option<&'a str>) {
     debug_assert!(lit_kind.is_numeric());
-    if let Some(suffix_length) = lit_suffix_length(lit_kind) {
+    lit_suffix_length(lit_kind).map_or((src, None), |suffix_length| {
         let (unsuffixed, suffix) = src.split_at(src.len() - suffix_length);
         (unsuffixed, Some(suffix))
-    } else {
-        (src, None)
-    }
+    })
 }
 
 fn lit_suffix_length(lit_kind: &LitKind) -> Option<usize> {
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index 8fc97f2fd64..0ac7714fbeb 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -492,20 +492,20 @@ fn astbinop2assignop(op: ast::BinOp) -> AssocOp {
 /// before it on its line.
 fn indentation<T: LintContext>(cx: &T, span: Span) -> Option<String> {
     let lo = cx.sess().source_map().lookup_char_pos(span.lo());
-    if let Some(line) = lo.file.get_line(lo.line - 1 /* line numbers in `Loc` are 1-based */) {
-        if let Some((pos, _)) = line.char_indices().find(|&(_, c)| c != ' ' && c != '\t') {
-            // We can mix char and byte positions here because we only consider `[ \t]`.
-            if lo.col == CharPos(pos) {
-                Some(line[..pos].into())
+    lo.file
+        .get_line(lo.line - 1 /* line numbers in `Loc` are 1-based */)
+        .and_then(|line| {
+            if let Some((pos, _)) = line.char_indices().find(|&(_, c)| c != ' ' && c != '\t') {
+                // We can mix char and byte positions here because we only consider `[ \t]`.
+                if lo.col == CharPos(pos) {
+                    Some(line[..pos].into())
+                } else {
+                    None
+                }
             } else {
                 None
             }
-        } else {
-            None
-        }
-    } else {
-        None
-    }
+        })
 }
 
 /// Convenience extension trait for `DiagnosticBuilder`.
diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs
index 732f4b28e06..063f94582b9 100644
--- a/clippy_lints/src/write.rs
+++ b/clippy_lints/src/write.rs
@@ -297,12 +297,13 @@ impl EarlyLintPass for Write {
             if let (Some(fmt_str), expr) = self.check_tts(cx, &mac.args.inner_tokens(), true) {
                 if fmt_str.symbol == Symbol::intern("") {
                     let mut applicability = Applicability::MachineApplicable;
-                    let suggestion = if let Some(e) = expr {
-                        snippet_with_applicability(cx, e.span, "v", &mut applicability)
-                    } else {
-                        applicability = Applicability::HasPlaceholders;
-                        Cow::Borrowed("v")
-                    };
+                    let suggestion = expr.map_or_else(
+                        || {
+                            applicability = Applicability::HasPlaceholders;
+                            Cow::Borrowed("v")
+                        },
+                        |e| snippet_with_applicability(cx, e.span, "v", &mut Applicability::MachineApplicable),
+                    );
 
                     span_lint_and_sugg(
                         cx,
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index 6402efc2521..e681f47f949 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -1621,6 +1621,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         module: "option_env_unwrap",
     },
     Lint {
+        name: "option_if_let_else",
+        group: "pedantic",
+        desc: "reimplementation of Option::map_or",
+        deprecation: None,
+        module: "option_if_let_else",
+    },
+    Lint {
         name: "option_map_or_none",
         group: "style",
         desc: "using `Option.map_or(None, f)`, which is more succinctly expressed as `and_then(f)`",
diff --git a/tests/compile-test.rs b/tests/compile-test.rs
index 99505fc6b29..eb6d495acbe 100644
--- a/tests/compile-test.rs
+++ b/tests/compile-test.rs
@@ -12,19 +12,11 @@ use std::path::{Path, PathBuf};
 mod cargo;
 
 fn host_lib() -> PathBuf {
-    if let Some(path) = option_env!("HOST_LIBS") {
-        PathBuf::from(path)
-    } else {
-        cargo::CARGO_TARGET_DIR.join(env!("PROFILE"))
-    }
+    option_env!("HOST_LIBS").map_or(cargo::CARGO_TARGET_DIR.join(env!("PROFILE")), PathBuf::from)
 }
 
 fn clippy_driver_path() -> PathBuf {
-    if let Some(path) = option_env!("CLIPPY_DRIVER_PATH") {
-        PathBuf::from(path)
-    } else {
-        cargo::TARGET_LIB.join("clippy-driver")
-    }
+    option_env!("CLIPPY_DRIVER_PATH").map_or(cargo::TARGET_LIB.join("clippy-driver"), PathBuf::from)
 }
 
 // When we'll want to use `extern crate ..` for a dependency that is used
diff --git a/tests/ui/option_if_let_else.fixed b/tests/ui/option_if_let_else.fixed
new file mode 100644
index 00000000000..695a460cc4e
--- /dev/null
+++ b/tests/ui/option_if_let_else.fixed
@@ -0,0 +1,74 @@
+// run-rustfix
+#![warn(clippy::option_if_let_else)]
+
+fn bad1(string: Option<&str>) -> (bool, &str) {
+    string.map_or((false, "hello"), |x| (true, x))
+}
+
+fn else_if_option(string: Option<&str>) -> Option<(bool, &str)> {
+    if string.is_none() {
+        None
+    } else { string.map_or(Some((false, "")), |x| Some((true, x))) }
+}
+
+fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {
+    let _ = string.map_or(0, |s| s.len());
+    let _ = num.as_ref().map_or(&0, |s| s);
+    let _ = num.as_mut().map_or(&mut 0, |s| {
+        *s += 1;
+        s
+    });
+    let _ = num.as_ref().map_or(&0, |s| s);
+    let _ = num.map_or(0, |mut s| {
+        s += 1;
+        s
+    });
+    let _ = num.as_mut().map_or(&mut 0, |s| {
+        *s += 1;
+        s
+    });
+}
+
+fn longer_body(arg: Option<u32>) -> u32 {
+    arg.map_or(13, |x| {
+        let y = x * x;
+        y * y
+    })
+}
+
+fn test_map_or_else(arg: Option<u32>) {
+    let _ = arg.map_or_else(|| {
+        let mut y = 1;
+        y = (y + 2 / y) / 2;
+        y = (y + 2 / y) / 2;
+        y
+    }, |x| x * x * x * x);
+}
+
+fn negative_tests(arg: Option<u32>) -> u32 {
+    let _ = if let Some(13) = arg { "unlucky" } else { "lucky" };
+    for _ in 0..10 {
+        let _ = if let Some(x) = arg {
+            x
+        } else {
+            continue;
+        };
+    }
+    let _ = if let Some(x) = arg {
+        return x;
+    } else {
+        5
+    };
+    7
+}
+
+fn main() {
+    let optional = Some(5);
+    let _ = optional.map_or(5, |x| x + 2);
+    let _ = bad1(None);
+    let _ = else_if_option(None);
+    unop_bad(&None, None);
+    let _ = longer_body(None);
+    test_map_or_else(None);
+    let _ = negative_tests(None);
+}
diff --git a/tests/ui/option_if_let_else.rs b/tests/ui/option_if_let_else.rs
new file mode 100644
index 00000000000..dee80d26bd9
--- /dev/null
+++ b/tests/ui/option_if_let_else.rs
@@ -0,0 +1,92 @@
+// run-rustfix
+#![warn(clippy::option_if_let_else)]
+
+fn bad1(string: Option<&str>) -> (bool, &str) {
+    if let Some(x) = string {
+        (true, x)
+    } else {
+        (false, "hello")
+    }
+}
+
+fn else_if_option(string: Option<&str>) -> Option<(bool, &str)> {
+    if string.is_none() {
+        None
+    } else if let Some(x) = string {
+        Some((true, x))
+    } else {
+        Some((false, ""))
+    }
+}
+
+fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {
+    let _ = if let Some(s) = *string { s.len() } else { 0 };
+    let _ = if let Some(s) = &num { s } else { &0 };
+    let _ = if let Some(s) = &mut num {
+        *s += 1;
+        s
+    } else {
+        &mut 0
+    };
+    let _ = if let Some(ref s) = num { s } else { &0 };
+    let _ = if let Some(mut s) = num {
+        s += 1;
+        s
+    } else {
+        0
+    };
+    let _ = if let Some(ref mut s) = num {
+        *s += 1;
+        s
+    } else {
+        &mut 0
+    };
+}
+
+fn longer_body(arg: Option<u32>) -> u32 {
+    if let Some(x) = arg {
+        let y = x * x;
+        y * y
+    } else {
+        13
+    }
+}
+
+fn test_map_or_else(arg: Option<u32>) {
+    let _ = if let Some(x) = arg {
+        x * x * x * x
+    } else {
+        let mut y = 1;
+        y = (y + 2 / y) / 2;
+        y = (y + 2 / y) / 2;
+        y
+    };
+}
+
+fn negative_tests(arg: Option<u32>) -> u32 {
+    let _ = if let Some(13) = arg { "unlucky" } else { "lucky" };
+    for _ in 0..10 {
+        let _ = if let Some(x) = arg {
+            x
+        } else {
+            continue;
+        };
+    }
+    let _ = if let Some(x) = arg {
+        return x;
+    } else {
+        5
+    };
+    7
+}
+
+fn main() {
+    let optional = Some(5);
+    let _ = if let Some(x) = optional { x + 2 } else { 5 };
+    let _ = bad1(None);
+    let _ = else_if_option(None);
+    unop_bad(&None, None);
+    let _ = longer_body(None);
+    test_map_or_else(None);
+    let _ = negative_tests(None);
+}
diff --git a/tests/ui/option_if_let_else.stderr b/tests/ui/option_if_let_else.stderr
new file mode 100644
index 00000000000..7005850efaf
--- /dev/null
+++ b/tests/ui/option_if_let_else.stderr
@@ -0,0 +1,151 @@
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:5:5
+   |
+LL | /     if let Some(x) = string {
+LL | |         (true, x)
+LL | |     } else {
+LL | |         (false, "hello")
+LL | |     }
+   | |_____^ help: try: `string.map_or((false, "hello"), |x| (true, x))`
+   |
+   = note: `-D clippy::option-if-let-else` implied by `-D warnings`
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:15:12
+   |
+LL |       } else if let Some(x) = string {
+   |  ____________^
+LL | |         Some((true, x))
+LL | |     } else {
+LL | |         Some((false, ""))
+LL | |     }
+   | |_____^ help: try: `{ string.map_or(Some((false, "")), |x| Some((true, x))) }`
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:23:13
+   |
+LL |     let _ = if let Some(s) = *string { s.len() } else { 0 };
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `string.map_or(0, |s| s.len())`
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:24:13
+   |
+LL |     let _ = if let Some(s) = &num { s } else { &0 };
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)`
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:25:13
+   |
+LL |       let _ = if let Some(s) = &mut num {
+   |  _____________^
+LL | |         *s += 1;
+LL | |         s
+LL | |     } else {
+LL | |         &mut 0
+LL | |     };
+   | |_____^
+   |
+help: try
+   |
+LL |     let _ = num.as_mut().map_or(&mut 0, |s| {
+LL |         *s += 1;
+LL |         s
+LL |     });
+   |
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:31:13
+   |
+LL |     let _ = if let Some(ref s) = num { s } else { &0 };
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)`
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:32:13
+   |
+LL |       let _ = if let Some(mut s) = num {
+   |  _____________^
+LL | |         s += 1;
+LL | |         s
+LL | |     } else {
+LL | |         0
+LL | |     };
+   | |_____^
+   |
+help: try
+   |
+LL |     let _ = num.map_or(0, |mut s| {
+LL |         s += 1;
+LL |         s
+LL |     });
+   |
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:38:13
+   |
+LL |       let _ = if let Some(ref mut s) = num {
+   |  _____________^
+LL | |         *s += 1;
+LL | |         s
+LL | |     } else {
+LL | |         &mut 0
+LL | |     };
+   | |_____^
+   |
+help: try
+   |
+LL |     let _ = num.as_mut().map_or(&mut 0, |s| {
+LL |         *s += 1;
+LL |         s
+LL |     });
+   |
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:47:5
+   |
+LL | /     if let Some(x) = arg {
+LL | |         let y = x * x;
+LL | |         y * y
+LL | |     } else {
+LL | |         13
+LL | |     }
+   | |_____^
+   |
+help: try
+   |
+LL |     arg.map_or(13, |x| {
+LL |         let y = x * x;
+LL |         y * y
+LL |     })
+   |
+
+error: use Option::map_or_else instead of an if let/else
+  --> $DIR/option_if_let_else.rs:56:13
+   |
+LL |       let _ = if let Some(x) = arg {
+   |  _____________^
+LL | |         x * x * x * x
+LL | |     } else {
+LL | |         let mut y = 1;
+...  |
+LL | |         y
+LL | |     };
+   | |_____^
+   |
+help: try
+   |
+LL |     let _ = arg.map_or_else(|| {
+LL |         let mut y = 1;
+LL |         y = (y + 2 / y) / 2;
+LL |         y = (y + 2 / y) / 2;
+LL |         y
+LL |     }, |x| x * x * x * x);
+   |
+
+error: use Option::map_or instead of an if let/else
+  --> $DIR/option_if_let_else.rs:85:13
+   |
+LL |     let _ = if let Some(x) = optional { x + 2 } else { 5 };
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)`
+
+error: aborting due to 11 previous errors
+