src/matches.rs

   1 use rustc::hir::*;
   2 use rustc::lint::*;
   3 use rustc::middle::const_val::ConstVal;
   4 use rustc::ty;
   5 use rustc_const_eval::EvalHint::ExprTypeChecked;
   6 use rustc_const_eval::eval_const_expr_partial;
   7 use rustc_const_math::ConstInt;
   8 use std::cmp::Ordering;
   9 use syntax::ast::LitKind;
  10 use syntax::codemap::Span;
  11 use utils::paths;
  12 use utils::{match_type, snippet, span_note_and_lint, span_lint_and_then, in_external_macro, expr_block};
  13
  14 /// **What it does:** This lint checks for matches with a single arm where an `if let` will usually suffice.
  15 ///
  16 /// **Why is this bad?** Just readability – `if let` nests less than a `match`.
  17 ///
  18 /// **Known problems:** None
  19 ///
  20 /// **Example:**
  21 /// ```
  22 /// match x {
  23 ///     Some(ref foo) -> bar(foo),
  24 ///     _ => ()
  25 /// }
  26 /// ```
  27 declare_lint! {
  28     pub SINGLE_MATCH, Warn,
  29     "a match statement with a single nontrivial arm (i.e, where the other arm \
  30      is `_ => {}`) is used; recommends `if let` instead"
  31 }
  32
  33 /// **What it does:** This lint checks for matches with a two arms where an `if let` will usually suffice.
  34 ///
  35 /// **Why is this bad?** Just readability – `if let` nests less than a `match`.
  36 ///
  37 /// **Known problems:** Personal style preferences may differ
  38 ///
  39 /// **Example:**
  40 /// ```
  41 /// match x {
  42 ///     Some(ref foo) -> bar(foo),
  43 ///     _ => bar(other_ref),
  44 /// }
  45 /// ```
  46 declare_lint! {
  47     pub SINGLE_MATCH_ELSE, Allow,
  48     "a match statement with a two arms where the second arm's pattern is a wildcard; \
  49      recommends `if let` instead"
  50 }
  51
  52 /// **What it does:** This lint checks for matches where all arms match a reference, suggesting to remove the reference and deref the matched expression instead. It also checks for `if let &foo = bar` blocks.
  53 ///
  54 /// **Why is this bad?** It just makes the code less readable. That reference destructuring adds nothing to the code.
  55 ///
  56 /// **Known problems:** None
  57 ///
  58 /// **Example:**
  59 ///
  60 /// ```
  61 /// match x {
  62 ///     &A(ref y) => foo(y),
  63 ///     &B => bar(),
  64 ///     _ => frob(&x),
  65 /// }
  66 /// ```
  67 declare_lint! {
  68     pub MATCH_REF_PATS, Warn,
  69     "a match or `if let` has all arms prefixed with `&`; the match expression can be \
  70      dereferenced instead"
  71 }
  72
  73 /// **What it does:** This lint checks for matches where match expression is a `bool`. It suggests to replace the expression with an `if...else` block.
  74 ///
  75 /// **Why is this bad?** It makes the code less readable.
  76 ///
  77 /// **Known problems:** None
  78 ///
  79 /// **Example:**
  80 ///
  81 /// ```
  82 /// let condition: bool = true;
  83 /// match condition {
  84 ///     true => foo(),
  85 ///     false => bar(),
  86 /// }
  87 /// ```
  88 declare_lint! {
  89     pub MATCH_BOOL, Warn,
  90     "a match on boolean expression; recommends `if..else` block instead"
  91 }
  92
  93 /// **What it does:** This lint checks for overlapping match arms.
  94 ///
  95 /// **Why is this bad?** It is likely to be an error and if not, makes the code less obvious.
  96 ///
  97 /// **Known problems:** None
  98 ///
  99 /// **Example:**
 100 ///
 101 /// ```
 102 /// let x = 5;
 103 /// match x {
 104 ///     1 ... 10 => println!("1 ... 10"),
 105 ///     5 ... 15 => println!("5 ... 15"),
 106 ///     _ => (),
 107 /// }
 108 /// ```
 109 declare_lint! {
 110     pub MATCH_OVERLAPPING_ARM, Warn, "a match has overlapping arms"
 111 }
 112
 113 #[allow(missing_copy_implementations)]
 114 pub struct MatchPass;
 115
 116 impl LintPass for MatchPass {
 117     fn get_lints(&self) -> LintArray {
 118         lint_array!(SINGLE_MATCH, MATCH_REF_PATS, MATCH_BOOL, SINGLE_MATCH_ELSE)
 119     }
 120 }
 121
 122 impl LateLintPass for MatchPass {
 123     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
 124         if in_external_macro(cx, expr.span) {
 125             return;
 126         }
 127         if let ExprMatch(ref ex, ref arms, MatchSource::Normal) = expr.node {
 128             check_single_match(cx, ex, arms, expr);
 129             check_match_bool(cx, ex, arms, expr);
 130             check_overlapping_arms(cx, ex, arms);
 131         }
 132         if let ExprMatch(ref ex, ref arms, source) = expr.node {
 133             check_match_ref_pats(cx, ex, arms, source, expr);
 134         }
 135     }
 136 }
 137
 138 #[cfg_attr(rustfmt, rustfmt_skip)]
 139 fn check_single_match(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
 140     if arms.len() == 2 &&
 141       arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
 142       arms[1].pats.len() == 1 && arms[1].guard.is_none() {
 143         let els = if is_unit_expr(&arms[1].body) {
 144             None
 145         } else if let ExprBlock(_) = arms[1].body.node {
 146             // matches with blocks that contain statements are prettier as `if let + else`
 147             Some(&*arms[1].body)
 148         } else {
 149             // allow match arms with just expressions
 150             return;
 151         };
 152         let ty = cx.tcx.expr_ty(ex);
 153         if ty.sty != ty::TyBool || cx.current_level(MATCH_BOOL) == Allow {
 154             check_single_match_single_pattern(cx, ex, arms, expr, els);
 155             check_single_match_opt_like(cx, ex, arms, expr, ty, els);
 156         }
 157     }
 158 }
 159
 160 fn check_single_match_single_pattern(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr, els: Option<&Expr>) {
 161     if arms[1].pats[0].node == PatKind::Wild {
 162         let lint = if els.is_some() {
 163             SINGLE_MATCH_ELSE
 164         } else {
 165             SINGLE_MATCH
 166         };
 167         let els_str = els.map_or(String::new(), |els| format!(" else {}", expr_block(cx, els, None, "..")));
 168         span_lint_and_then(cx,
 169                            lint,
 170                            expr.span,
 171                            "you seem to be trying to use match for destructuring a single pattern. \
 172                            Consider using `if let`",
 173                            |db| {
 174                                db.span_suggestion(expr.span,
 175                                                   "try this",
 176                                                   format!("if let {} = {} {}{}",
 177                                                           snippet(cx, arms[0].pats[0].span, ".."),
 178                                                           snippet(cx, ex.span, ".."),
 179                                                           expr_block(cx, &arms[0].body, None, ".."),
 180                                                           els_str));
 181                            });
 182     }
 183 }
 184
 185 fn check_single_match_opt_like(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr, ty: ty::Ty, els: Option<&Expr>) {
 186     // list of candidate Enums we know will never get any more members
 187     let candidates = &[(&paths::COW, "Borrowed"),
 188                        (&paths::COW, "Cow::Borrowed"),
 189                        (&paths::COW, "Cow::Owned"),
 190                        (&paths::COW, "Owned"),
 191                        (&paths::OPTION, "None"),
 192                        (&paths::RESULT, "Err"),
 193                        (&paths::RESULT, "Ok")];
 194
 195     let path = match arms[1].pats[0].node {
 196         PatKind::TupleStruct(ref path, Some(ref inner)) => {
 197             // contains any non wildcard patterns? e.g. Err(err)
 198             if inner.iter().any(|pat| pat.node != PatKind::Wild) {
 199                 return;
 200             }
 201             path.to_string()
 202         }
 203         PatKind::TupleStruct(ref path, None) => path.to_string(),
 204         PatKind::Ident(BindByValue(MutImmutable), ident, None) => ident.node.to_string(),
 205         _ => return,
 206     };
 207
 208     for &(ty_path, pat_path) in candidates {
 209         if &path == pat_path && match_type(cx, ty, ty_path) {
 210             let lint = if els.is_some() {
 211                 SINGLE_MATCH_ELSE
 212             } else {
 213                 SINGLE_MATCH
 214             };
 215             let els_str = els.map_or(String::new(), |els| format!(" else {}", expr_block(cx, els, None, "..")));
 216             span_lint_and_then(cx,
 217                                lint,
 218                                expr.span,
 219                                "you seem to be trying to use match for destructuring a single pattern. Consider \
 220                                 using `if let`",
 221                                |db| {
 222                                    db.span_suggestion(expr.span,
 223                                                       "try this",
 224                                                       format!("if let {} = {} {}{}",
 225                                                               snippet(cx, arms[0].pats[0].span, ".."),
 226                                                               snippet(cx, ex.span, ".."),
 227                                                               expr_block(cx, &arms[0].body, None, ".."),
 228                                                               els_str));
 229                                });
 230         }
 231     }
 232 }
 233
 234 fn check_match_bool(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
 235     // type of expression == bool
 236     if cx.tcx.expr_ty(ex).sty == ty::TyBool {
 237         let sugg = if arms.len() == 2 && arms[0].pats.len() == 1 {
 238             // no guards
 239             let exprs = if let PatKind::Lit(ref arm_bool) = arms[0].pats[0].node {
 240                 if let ExprLit(ref lit) = arm_bool.node {
 241                     match lit.node {
 242                         LitKind::Bool(true) => Some((&*arms[0].body, &*arms[1].body)),
 243                         LitKind::Bool(false) => Some((&*arms[1].body, &*arms[0].body)),
 244                         _ => None,
 245                     }
 246                 } else {
 247                     None
 248                 }
 249             } else {
 250                 None
 251             };
 252
 253             if let Some((ref true_expr, ref false_expr)) = exprs {
 254                 match (is_unit_expr(true_expr), is_unit_expr(false_expr)) {
 255                     (false, false) => {
 256                         Some(format!("if {} {} else {}",
 257                                      snippet(cx, ex.span, "b"),
 258                                      expr_block(cx, true_expr, None, ".."),
 259                                      expr_block(cx, false_expr, None, "..")))
 260                     }
 261                     (false, true) => {
 262                         Some(format!("if {} {}", snippet(cx, ex.span, "b"), expr_block(cx, true_expr, None, "..")))
 263                     }
 264                     (true, false) => {
 265                         Some(format!("try\nif !{} {}",
 266                                      snippet(cx, ex.span, "b"),
 267                                      expr_block(cx, false_expr, None, "..")))
 268                     }
 269                     (true, true) => None,
 270                 }
 271             } else {
 272                 None
 273             }
 274         } else {
 275             None
 276         };
 277
 278         span_lint_and_then(cx,
 279                            MATCH_BOOL,
 280                            expr.span,
 281                            "you seem to be trying to match on a boolean expression. Consider using an if..else block:",
 282                            move |db| {
 283                                if let Some(sugg) = sugg {
 284                                    db.span_suggestion(expr.span, "try this", sugg);
 285                                }
 286                            });
 287     }
 288 }
 289
 290 fn check_overlapping_arms(cx: &LateContext, ex: &Expr, arms: &[Arm]) {
 291     if arms.len() >= 2 && cx.tcx.expr_ty(ex).is_integral() {
 292         let ranges = all_ranges(cx, arms);
 293         let type_ranges = type_ranges(&ranges);
 294         if !type_ranges.is_empty() {
 295             if let Some((start, end)) = overlapping(&type_ranges) {
 296                 span_note_and_lint(cx,
 297                                    MATCH_OVERLAPPING_ARM,
 298                                    start.span,
 299                                    "some ranges overlap",
 300                                    end.span,
 301                                    "overlaps with this");
 302             }
 303         }
 304     }
 305 }
 306
 307 fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], source: MatchSource, expr: &Expr) {
 308     if has_only_ref_pats(arms) {
 309         if let ExprAddrOf(Mutability::MutImmutable, ref inner) = ex.node {
 310             let template = match_template(cx, expr.span, source, "", inner);
 311             span_lint_and_then(cx,
 312                                MATCH_REF_PATS,
 313                                expr.span,
 314                                "you don't need to add `&` to both the expression and the patterns",
 315                                |db| {
 316                                    db.span_suggestion(expr.span, "try", template);
 317                                });
 318         } else {
 319             let template = match_template(cx, expr.span, source, "*", ex);
 320             span_lint_and_then(cx,
 321                                MATCH_REF_PATS,
 322                                expr.span,
 323                                "you don't need to add `&` to all patterns",
 324                                |db| {
 325                                    db.span_suggestion(expr.span,
 326                                                       "instead of prefixing all patterns with `&`, you can \
 327                                                        dereference the expression",
 328                                                       template);
 329                                });
 330         }
 331     }
 332 }
 333
 334 /// Get all arms that are unbounded `PatRange`s.
 335 fn all_ranges(cx: &LateContext, arms: &[Arm]) -> Vec<SpannedRange<ConstVal>> {
 336     arms.iter()
 337         .filter_map(|arm| {
 338             if let Arm { ref pats, guard: None, .. } = *arm {
 339                 Some(pats.iter().filter_map(|pat| {
 340                     if_let_chain! {[
 341                         let PatKind::Range(ref lhs, ref rhs) = pat.node,
 342                         let Ok(lhs) = eval_const_expr_partial(cx.tcx, &lhs, ExprTypeChecked, None),
 343                         let Ok(rhs) = eval_const_expr_partial(cx.tcx, &rhs, ExprTypeChecked, None)
 344                     ], {
 345                         return Some(SpannedRange { span: pat.span, node: (lhs, rhs) });
 346                     }}
 347
 348                     if_let_chain! {[
 349                         let PatKind::Lit(ref value) = pat.node,
 350                         let Ok(value) = eval_const_expr_partial(cx.tcx, &value, ExprTypeChecked, None)
 351                     ], {
 352                         return Some(SpannedRange { span: pat.span, node: (value.clone(), value) });
 353                     }}
 354
 355                     None
 356                 }))
 357             } else {
 358                 None
 359             }
 360         })
 361         .flat_map(IntoIterator::into_iter)
 362         .collect()
 363 }
 364
 365 #[derive(Debug, Eq, PartialEq)]
 366 pub struct SpannedRange<T> {
 367     pub span: Span,
 368     pub node: (T, T),
 369 }
 370
 371 type TypedRanges = Vec<SpannedRange<ConstInt>>;
 372
 373 /// Get all `Int` ranges or all `Uint` ranges. Mixed types are an error anyway and other types than
 374 /// `Uint` and `Int` probably don't make sense.
 375 fn type_ranges(ranges: &[SpannedRange<ConstVal>]) -> TypedRanges {
 376     ranges.iter()
 377           .filter_map(|range| {
 378               if let (ConstVal::Integral(start), ConstVal::Integral(end)) = range.node {
 379                   Some(SpannedRange {
 380                       span: range.span,
 381                       node: (start, end),
 382                   })
 383               } else {
 384                   None
 385               }
 386           })
 387           .collect()
 388 }
 389
 390 fn is_unit_expr(expr: &Expr) -> bool {
 391     match expr.node {
 392         ExprTup(ref v) if v.is_empty() => true,
 393         ExprBlock(ref b) if b.stmts.is_empty() && b.expr.is_none() => true,
 394         _ => false,
 395     }
 396 }
 397
 398 fn has_only_ref_pats(arms: &[Arm]) -> bool {
 399     let mapped = arms.iter()
 400                      .flat_map(|a| &a.pats)
 401                      .map(|p| {
 402                          match p.node {
 403                              PatKind::Ref(..) => Some(true),  // &-patterns
 404                              PatKind::Wild => Some(false),   // an "anything" wildcard is also fine
 405                              _ => None,                    // any other pattern is not fine
 406                          }
 407                      })
 408                      .collect::<Option<Vec<bool>>>();
 409     // look for Some(v) where there's at least one true element
 410     mapped.map_or(false, |v| v.iter().any(|el| *el))
 411 }
 412
 413 fn match_template(cx: &LateContext, span: Span, source: MatchSource, op: &str, expr: &Expr) -> String {
 414     let expr_snippet = snippet(cx, expr.span, "..");
 415     match source {
 416         MatchSource::Normal => format!("match {}{} {{ .. }}", op, expr_snippet),
 417         MatchSource::IfLetDesugar { .. } => format!("if let .. = {}{} {{ .. }}", op, expr_snippet),
 418         MatchSource::WhileLetDesugar => format!("while let .. = {}{} {{ .. }}", op, expr_snippet),
 419         MatchSource::ForLoopDesugar => span_bug!(span, "for loop desugared to match with &-patterns!"),
 420         MatchSource::TryDesugar => span_bug!(span, "`?` operator desugared to match with &-patterns!"),
 421     }
 422 }
 423
 424 pub fn overlapping<T>(ranges: &[SpannedRange<T>]) -> Option<(&SpannedRange<T>, &SpannedRange<T>)>
 425     where T: Copy + Ord
 426 {
 427     #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 428     enum Kind<'a, T: 'a> {
 429         Start(T, &'a SpannedRange<T>),
 430         End(T, &'a SpannedRange<T>),
 431     }
 432
 433     impl<'a, T: Copy> Kind<'a, T> {
 434         fn range(&self) -> &'a SpannedRange<T> {
 435             match *self {
 436                 Kind::Start(_, r) |
 437                 Kind::End(_, r) => r,
 438             }
 439         }
 440
 441         fn value(self) -> T {
 442             match self {
 443                 Kind::Start(t, _) |
 444                 Kind::End(t, _) => t,
 445             }
 446         }
 447     }
 448
 449     impl<'a, T: Copy + Ord> PartialOrd for Kind<'a, T> {
 450         fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
 451             Some(self.cmp(other))
 452         }
 453     }
 454
 455     impl<'a, T: Copy + Ord> Ord for Kind<'a, T> {
 456         fn cmp(&self, other: &Self) -> Ordering {
 457             self.value().cmp(&other.value())
 458         }
 459     }
 460
 461     let mut values = Vec::with_capacity(2 * ranges.len());
 462
 463     for r in ranges {
 464         values.push(Kind::Start(r.node.0, r));
 465         values.push(Kind::End(r.node.1, r));
 466     }
 467
 468     values.sort();
 469
 470     for (a, b) in values.iter().zip(values.iter().skip(1)) {
 471         match (a, b) {
 472             (&Kind::Start(_, ra), &Kind::End(_, rb)) => {
 473                 if ra.node != rb.node {
 474                     return Some((ra, rb));
 475                 }
 476             }
 477             (&Kind::End(a, _), &Kind::Start(b, _)) if a != b => (),
 478             _ => return Some((a.range(), b.range())),
 479         }
 480     }
 481
 482     None
 483 }