Documentation changes and bugfixes

Author: JarredAllen

clippy_lints/src/option_if_let_else.rs

@@ -10,16 +10,19 @@ 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.
+    /// when they could use `Option::map_or` or `Option::map_or_else`
+    /// instead.
     ///
     /// **Why is this bad?**
     /// Using the dedicated function in the Option class is clearer and
-    /// more concise than an if let
+    /// more concise than an if let expression.
     ///
     /// **Known problems:**
-    /// Using `Option::map_or` requires it to compute the or value every
-    /// function call, even if the value is unneeded. In some circumstances,
-    /// it might be better to use Option::map_or_else. For example,
+    /// Using `Option::map_or` (as the lint presently suggests) requires
+    /// it to compute the else value every function call, even if the
+    /// value is unneeded. It might be better to use Option::map_or_else
+    /// if the code is either very expensive or if it changes some state
+    /// (i.e. mutates a value). For example,
     /// ```rust
     /// # let optional = Some(0);
     /// let _ = if let Some(x) = optional {
@@ -67,9 +70,10 @@ declare_clippy_lint! {
 
 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, expression if Option is
-/// Some, expression if Option is None). Otherwise, it returns None
+/// If this expression is the option if let/else construct we're
+/// detecting, then this function returns Some(Option<_> compared,
+/// expression if Option is Some, expression if Option is None).
+/// Otherwise, it returns None.
 fn detect_option_if_let_else<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr<'_>) -> Option<(String, String, String)> {
     if_chain! {
         if let ExprKind::Match(let_body, arms, MatchSource::IfLetDesugar{contains_else_clause: true}) = &expr.kind;
@@ -112,9 +116,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OptionIfLetElse {
                 cx,
                 OPTION_IF_LET_ELSE,
                 expr.span,
-                "use Option::map_or_else instead of an if let/else",
+                "use Option::map_or instead of an if let/else",
                 "try",
-                format!("{}.map_or_else({}, {})", option, else_func, map),
+                format!("{}.map_or({}, {})", option, else_func, map),
                 Applicability::MaybeIncorrect,
             );
         }

tests/ui/author/blocks.rs

@@ -1,5 +1,5 @@
 #![feature(stmt_expr_attributes)]
-#![allow(redundant_semicolons, clippy::no_effect)]
+#![allow(redundant_semicolon, clippy::no_effect)]
 
 #[rustfmt::skip]
 fn main() {

tests/ui/option_if_let_else.stderr

@@ -1,16 +1,16 @@
-error: use Option::map_or_else instead of an if let/else
+error: use Option::map_or instead of an if let/else
   --> $DIR/option_if_let_else.rs:4:5
    |
 LL | /     if let Some(x) = string {
 LL | |         (true, x)
 LL | |     } else {
 LL | |         (false, "hello")
 LL | |     }
-   | |_____^ help: try: `string.map_or_else((false, "hello"), |x| (true, x))`
+   | |_____^ 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_else instead of an if let/else
+error: use Option::map_or instead of an if let/else
   --> $DIR/option_if_let_else.rs:12:5
    |
 LL | /     if let Some(x) = arg {
@@ -23,17 +23,17 @@ LL | |     }
    |
 help: try
    |
-LL |     arg.map_or_else(13, |x| {
+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
+error: use Option::map_or instead of an if let/else
   --> $DIR/option_if_let_else.rs:26:13
    |
 LL |     let _ = if let Some(x) = optional { x + 2 } else { 5 };
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or_else(5, |x| x + 2)`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)`
 
 error: aborting due to 3 previous errors
 

tests/ui/swap.rs

@@ -5,7 +5,7 @@
     clippy::blacklisted_name,
     clippy::no_effect,
     clippy::redundant_clone,
-    redundant_semicolons,
+    redundant_semicolon,
     unused_assignments
 )]