clippy_lints/src/option_if_let_else.rs

   1 use crate::utils;
   2 use crate::utils::sugg::Sugg;
   3 use crate::utils::{match_type, paths, span_lint_and_sugg};
   4 use if_chain::if_chain;
   5
   6 use rustc_errors::Applicability;
   7 use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
   8 use rustc_hir::*;
   9 use rustc_lint::{LateContext, LateLintPass};
  10 use rustc_middle::hir::map::Map;
  11 use rustc_session::{declare_lint_pass, declare_tool_lint};
  12
  13 use std::marker::PhantomData;
  14
  15 declare_clippy_lint! {
  16     /// **What it does:**
  17     /// Lints usage of  `if let Some(v) = ... { y } else { x }` which is more
  18     /// idiomatically done with `Option::map_or` (if the else bit is a simple
  19     /// expression) or `Option::map_or_else` (if the else bit is a longer
  20     /// block).
  21     ///
  22     /// **Why is this bad?**
  23     /// Using the dedicated functions of the Option type is clearer and
  24     /// more concise than an if let expression.
  25     ///
  26     /// **Known problems:**
  27     /// This lint uses whether the block is just an expression or if it has
  28     /// more statements to decide whether to use `Option::map_or` or
  29     /// `Option::map_or_else`. If you have a single expression which calls
  30     /// an expensive function, then it would be more efficient to use
  31     /// `Option::map_or_else`, but this lint would suggest `Option::map_or`.
  32     ///
  33     /// Also, this lint uses a deliberately conservative metric for checking
  34     /// if the inside of either body contains breaks or continues which will
  35     /// cause it to not suggest a fix if either block contains a loop with
  36     /// continues or breaks contained within the loop.
  37     ///
  38     /// **Example:**
  39     ///
  40     /// ```rust
  41     /// # let optional: Option<u32> = Some(0);
  42     /// let _ = if let Some(foo) = optional {
  43     ///     foo
  44     /// } else {
  45     ///     5
  46     /// };
  47     /// let _ = if let Some(foo) = optional {
  48     ///     foo
  49     /// } else {
  50     ///     let y = do_complicated_function();
  51     ///     y*y
  52     /// };
  53     /// ```
  54     ///
  55     /// should be
  56     ///
  57     /// ```rust
  58     /// # let optional: Option<u32> = Some(0);
  59     /// let _ = optional.map_or(5, |foo| foo);
  60     /// let _ = optional.map_or_else(||{
  61     ///     let y = do_complicated_function;
  62     ///     y*y
  63     /// }, |foo| foo);
  64     /// ```
  65     pub OPTION_IF_LET_ELSE,
  66     style,
  67     "reimplementation of Option::map_or"
  68 }
  69
  70 declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]);
  71
  72 /// Returns true iff the given expression is the result of calling Result::ok
  73 fn is_result_ok(cx: &LateContext<'_, '_>, expr: &'_ Expr<'_>) -> bool {
  74     if_chain! {
  75         if let ExprKind::MethodCall(ref path, _, &[ref receiver]) = &expr.kind;
  76         if path.ident.name.to_ident_string() == "ok";
  77         if match_type(cx, &cx.tables.expr_ty(&receiver), &paths::RESULT);
  78         then {
  79             true
  80         } else {
  81             false
  82         }
  83     }
  84 }
  85
  86 /// A struct containing information about occurences of the
  87 /// `if let Some(..) = .. else` construct that this lint detects.
  88 struct OptionIfLetElseOccurence {
  89     option: String,
  90     method_sugg: String,
  91     some_expr: String,
  92     none_expr: String,
  93     wrap_braces: bool,
  94 }
  95
  96 struct ReturnBreakContinueVisitor<'tcx> {
  97     seen_return_break_continue: bool,
  98     phantom_data: PhantomData<&'tcx bool>,
  99 }
 100 impl<'tcx> ReturnBreakContinueVisitor<'tcx> {
 101     fn new() -> ReturnBreakContinueVisitor<'tcx> {
 102         ReturnBreakContinueVisitor {
 103             seen_return_break_continue: false,
 104             phantom_data: PhantomData,
 105         }
 106     }
 107 }
 108 impl<'tcx> Visitor<'tcx> for ReturnBreakContinueVisitor<'tcx> {
 109     type Map = Map<'tcx>;
 110     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
 111         NestedVisitorMap::None
 112     }
 113
 114     fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
 115         if self.seen_return_break_continue {
 116             // No need to look farther if we've already seen one of them
 117             return;
 118         }
 119         match &ex.kind {
 120             ExprKind::Ret(..) | ExprKind::Break(..) | ExprKind::Continue(..) => {
 121                 self.seen_return_break_continue = true;
 122             },
 123             // Something special could be done here to handle while or for loop
 124             // desugaring, as this will detect a break if there's a while loop
 125             // or a for loop inside the expression.
 126             _ => {
 127                 rustc_hir::intravisit::walk_expr(self, ex);
 128             },
 129         }
 130     }
 131 }
 132
 133 fn contains_return_break_continue<'tcx>(expression: &'tcx Expr<'tcx>) -> bool {
 134     let mut recursive_visitor: ReturnBreakContinueVisitor<'tcx> = ReturnBreakContinueVisitor::new();
 135     recursive_visitor.visit_expr(expression);
 136     recursive_visitor.seen_return_break_continue
 137 }
 138
 139 /// If this expression is the option if let/else construct we're detecting, then
 140 /// this function returns an OptionIfLetElseOccurence struct with details if
 141 /// this construct is found, or None if this construct is not found.
 142 fn detect_option_if_let_else<'a>(cx: &LateContext<'_, 'a>, expr: &'a Expr<'a>) -> Option<OptionIfLetElseOccurence> {
 143     //(String, String, String, String)> {
 144     if_chain! {
 145         // if !utils::in_macro(expr.span); // Don't lint macros, because it behaves weirdly
 146         if let ExprKind::Match(let_body, arms, MatchSource::IfLetDesugar{contains_else_clause: true}) = &expr.kind;
 147         if arms.len() == 2;
 148         if match_type(cx, &cx.tables.expr_ty(let_body), &paths::OPTION);
 149         if !is_result_ok(cx, let_body); // Don't lint on Result::ok because a different lint does it already
 150         if let PatKind::TupleStruct(_, &[inner_pat], _) = &arms[0].pat.kind;
 151         if let PatKind::Binding(_, _, id, _) = &inner_pat.kind;
 152         if !contains_return_break_continue(arms[0].body);
 153         if !contains_return_break_continue(arms[1].body);
 154         then {
 155             let some_body = if let ExprKind::Block(Block { stmts: statements, expr: Some(expr), .. }, _)
 156                 = &arms[0].body.kind {
 157                 if let &[] = &statements {
 158                     expr
 159                 } else {
 160                     &arms[0].body
 161                 }
 162             } else {
 163                 return None;
 164             };
 165             let (none_body, method_sugg) = if let ExprKind::Block(Block { stmts: statements, expr: Some(expr), .. }, _)
 166                 = &arms[1].body.kind {
 167                 if let &[] = &statements {
 168                     (expr, "map_or")
 169                 } else {
 170                     (&arms[1].body, "map_or_else")
 171                 }
 172             } else {
 173                 return None;
 174             };
 175             let capture_name = id.name.to_ident_string();
 176             let wrap_braces = utils::get_enclosing_block(cx, expr.hir_id).map_or(false, |parent| {
 177                 if_chain! {
 178                     if let Some(Expr { kind: ExprKind::Match(
 179                                 _,
 180                                 arms,
 181                                 MatchSource::IfDesugar{contains_else_clause: true}
 182                                     | MatchSource::IfLetDesugar{contains_else_clause: true}),
 183                                 .. } ) = parent.expr;
 184                     if expr.hir_id == arms[1].body.hir_id;
 185                     then {
 186                         true
 187                     } else {
 188                         false
 189                     }
 190                 }
 191             });
 192             let parens_around_option = match &let_body.kind {
 193                 ExprKind::Call(..)
 194                         | ExprKind::MethodCall(..)
 195                         | ExprKind::Loop(..)
 196                         | ExprKind::Match(..)
 197                         | ExprKind::Block(..)
 198                         | ExprKind::Field(..)
 199                         | ExprKind::Path(_)
 200                     => false,
 201                 _ => true,
 202             };
 203             Some(OptionIfLetElseOccurence {
 204                 option: format!("{}{}{}", if parens_around_option { "(" } else { "" }, Sugg::hir(cx, let_body, ".."), if parens_around_option { ")" } else { "" }),
 205                 method_sugg: format!("{}", method_sugg),
 206                 some_expr: format!("|{}| {}", capture_name, Sugg::hir(cx, some_body, "..")),
 207                 none_expr: format!("{}{}", if method_sugg == "map_or" { "" } else { "|| " }, Sugg::hir(cx, none_body, "..")),
 208                 wrap_braces,
 209             })
 210         } else {
 211             None
 212         }
 213     }
 214 }
 215
 216 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OptionIfLetElse {
 217     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
 218         if let Some(detection) = detect_option_if_let_else(cx, expr) {
 219             span_lint_and_sugg(
 220                 cx,
 221                 OPTION_IF_LET_ELSE,
 222                 expr.span,
 223                 format!("use Option::{} instead of an if let/else", detection.method_sugg).as_str(),
 224                 "try",
 225                 format!(
 226                     "{}{}.{}({}, {}){}",
 227                     if detection.wrap_braces { "{ " } else { "" },
 228                     detection.option,
 229                     detection.method_sugg,
 230                     detection.none_expr,
 231                     detection.some_expr,
 232                     if detection.wrap_braces { " }" } else { "" },
 233                 ),
 234                 Applicability::MachineApplicable,
 235             );
 236         }
 237     }
 238 }