clippy_lints/src/matches/mod.rs

   1 use clippy_utils::diagnostics::span_lint_and_help;
   2 use clippy_utils::{is_wild, meets_msrv, msrvs};
   3 use if_chain::if_chain;
   4 use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat, PatKind, QPath};
   5 use rustc_lint::{LateContext, LateLintPass};
   6 use rustc_middle::ty;
   7 use rustc_semver::RustcVersion;
   8 use rustc_session::{declare_tool_lint, impl_lint_pass};
   9
  10 mod infalliable_detructuring_match;
  11 mod match_as_ref;
  12 mod match_bool;
  13 mod match_like_matches;
  14 mod match_ref_pats;
  15 mod match_same_arms;
  16 mod match_single_binding;
  17 mod match_wild_enum;
  18 mod match_wild_err_arm;
  19 mod overlapping_arms;
  20 mod redundant_pattern_match;
  21 mod single_match;
  22
  23 declare_clippy_lint! {
  24     /// ### What it does
  25     /// Checks for matches with a single arm where an `if let`
  26     /// will usually suffice.
  27     ///
  28     /// ### Why is this bad?
  29     /// Just readability – `if let` nests less than a `match`.
  30     ///
  31     /// ### Example
  32     /// ```rust
  33     /// # fn bar(stool: &str) {}
  34     /// # let x = Some("abc");
  35     /// // Bad
  36     /// match x {
  37     ///     Some(ref foo) => bar(foo),
  38     ///     _ => (),
  39     /// }
  40     ///
  41     /// // Good
  42     /// if let Some(ref foo) = x {
  43     ///     bar(foo);
  44     /// }
  45     /// ```
  46     #[clippy::version = "pre 1.29.0"]
  47     pub SINGLE_MATCH,
  48     style,
  49     "a `match` statement with a single nontrivial arm (i.e., where the other arm is `_ => {}`) instead of `if let`"
  50 }
  51
  52 declare_clippy_lint! {
  53     /// ### What it does
  54     /// Checks for matches with two arms where an `if let else` will
  55     /// usually suffice.
  56     ///
  57     /// ### Why is this bad?
  58     /// Just readability – `if let` nests less than a `match`.
  59     ///
  60     /// ### Known problems
  61     /// Personal style preferences may differ.
  62     ///
  63     /// ### Example
  64     /// Using `match`:
  65     ///
  66     /// ```rust
  67     /// # fn bar(foo: &usize) {}
  68     /// # let other_ref: usize = 1;
  69     /// # let x: Option<&usize> = Some(&1);
  70     /// match x {
  71     ///     Some(ref foo) => bar(foo),
  72     ///     _ => bar(&other_ref),
  73     /// }
  74     /// ```
  75     ///
  76     /// Using `if let` with `else`:
  77     ///
  78     /// ```rust
  79     /// # fn bar(foo: &usize) {}
  80     /// # let other_ref: usize = 1;
  81     /// # let x: Option<&usize> = Some(&1);
  82     /// if let Some(ref foo) = x {
  83     ///     bar(foo);
  84     /// } else {
  85     ///     bar(&other_ref);
  86     /// }
  87     /// ```
  88     #[clippy::version = "pre 1.29.0"]
  89     pub SINGLE_MATCH_ELSE,
  90     pedantic,
  91     "a `match` statement with two arms where the second arm's pattern is a placeholder instead of a specific match pattern"
  92 }
  93
  94 declare_clippy_lint! {
  95     /// ### What it does
  96     /// Checks for matches where all arms match a reference,
  97     /// suggesting to remove the reference and deref the matched expression
  98     /// instead. It also checks for `if let &foo = bar` blocks.
  99     ///
 100     /// ### Why is this bad?
 101     /// It just makes the code less readable. That reference
 102     /// destructuring adds nothing to the code.
 103     ///
 104     /// ### Example
 105     /// ```rust,ignore
 106     /// // Bad
 107     /// match x {
 108     ///     &A(ref y) => foo(y),
 109     ///     &B => bar(),
 110     ///     _ => frob(&x),
 111     /// }
 112     ///
 113     /// // Good
 114     /// match *x {
 115     ///     A(ref y) => foo(y),
 116     ///     B => bar(),
 117     ///     _ => frob(x),
 118     /// }
 119     /// ```
 120     #[clippy::version = "pre 1.29.0"]
 121     pub MATCH_REF_PATS,
 122     style,
 123     "a `match` or `if let` with all arms prefixed with `&` instead of deref-ing the match expression"
 124 }
 125
 126 declare_clippy_lint! {
 127     /// ### What it does
 128     /// Checks for matches where match expression is a `bool`. It
 129     /// suggests to replace the expression with an `if...else` block.
 130     ///
 131     /// ### Why is this bad?
 132     /// It makes the code less readable.
 133     ///
 134     /// ### Example
 135     /// ```rust
 136     /// # fn foo() {}
 137     /// # fn bar() {}
 138     /// let condition: bool = true;
 139     /// match condition {
 140     ///     true => foo(),
 141     ///     false => bar(),
 142     /// }
 143     /// ```
 144     /// Use if/else instead:
 145     /// ```rust
 146     /// # fn foo() {}
 147     /// # fn bar() {}
 148     /// let condition: bool = true;
 149     /// if condition {
 150     ///     foo();
 151     /// } else {
 152     ///     bar();
 153     /// }
 154     /// ```
 155     #[clippy::version = "pre 1.29.0"]
 156     pub MATCH_BOOL,
 157     pedantic,
 158     "a `match` on a boolean expression instead of an `if..else` block"
 159 }
 160
 161 declare_clippy_lint! {
 162     /// ### What it does
 163     /// Checks for overlapping match arms.
 164     ///
 165     /// ### Why is this bad?
 166     /// It is likely to be an error and if not, makes the code
 167     /// less obvious.
 168     ///
 169     /// ### Example
 170     /// ```rust
 171     /// let x = 5;
 172     /// match x {
 173     ///     1..=10 => println!("1 ... 10"),
 174     ///     5..=15 => println!("5 ... 15"),
 175     ///     _ => (),
 176     /// }
 177     /// ```
 178     #[clippy::version = "pre 1.29.0"]
 179     pub MATCH_OVERLAPPING_ARM,
 180     style,
 181     "a `match` with overlapping arms"
 182 }
 183
 184 declare_clippy_lint! {
 185     /// ### What it does
 186     /// Checks for arm which matches all errors with `Err(_)`
 187     /// and take drastic actions like `panic!`.
 188     ///
 189     /// ### Why is this bad?
 190     /// It is generally a bad practice, similar to
 191     /// catching all exceptions in java with `catch(Exception)`
 192     ///
 193     /// ### Example
 194     /// ```rust
 195     /// let x: Result<i32, &str> = Ok(3);
 196     /// match x {
 197     ///     Ok(_) => println!("ok"),
 198     ///     Err(_) => panic!("err"),
 199     /// }
 200     /// ```
 201     #[clippy::version = "pre 1.29.0"]
 202     pub MATCH_WILD_ERR_ARM,
 203     pedantic,
 204     "a `match` with `Err(_)` arm and take drastic actions"
 205 }
 206
 207 declare_clippy_lint! {
 208     /// ### What it does
 209     /// Checks for match which is used to add a reference to an
 210     /// `Option` value.
 211     ///
 212     /// ### Why is this bad?
 213     /// Using `as_ref()` or `as_mut()` instead is shorter.
 214     ///
 215     /// ### Example
 216     /// ```rust
 217     /// let x: Option<()> = None;
 218     ///
 219     /// // Bad
 220     /// let r: Option<&()> = match x {
 221     ///     None => None,
 222     ///     Some(ref v) => Some(v),
 223     /// };
 224     ///
 225     /// // Good
 226     /// let r: Option<&()> = x.as_ref();
 227     /// ```
 228     #[clippy::version = "pre 1.29.0"]
 229     pub MATCH_AS_REF,
 230     complexity,
 231     "a `match` on an Option value instead of using `as_ref()` or `as_mut`"
 232 }
 233
 234 declare_clippy_lint! {
 235     /// ### What it does
 236     /// Checks for wildcard enum matches using `_`.
 237     ///
 238     /// ### Why is this bad?
 239     /// New enum variants added by library updates can be missed.
 240     ///
 241     /// ### Known problems
 242     /// Suggested replacements may be incorrect if guards exhaustively cover some
 243     /// variants, and also may not use correct path to enum if it's not present in the current scope.
 244     ///
 245     /// ### Example
 246     /// ```rust
 247     /// # enum Foo { A(usize), B(usize) }
 248     /// # let x = Foo::B(1);
 249     /// // Bad
 250     /// match x {
 251     ///     Foo::A(_) => {},
 252     ///     _ => {},
 253     /// }
 254     ///
 255     /// // Good
 256     /// match x {
 257     ///     Foo::A(_) => {},
 258     ///     Foo::B(_) => {},
 259     /// }
 260     /// ```
 261     #[clippy::version = "1.34.0"]
 262     pub WILDCARD_ENUM_MATCH_ARM,
 263     restriction,
 264     "a wildcard enum match arm using `_`"
 265 }
 266
 267 declare_clippy_lint! {
 268     /// ### What it does
 269     /// Checks for wildcard enum matches for a single variant.
 270     ///
 271     /// ### Why is this bad?
 272     /// New enum variants added by library updates can be missed.
 273     ///
 274     /// ### Known problems
 275     /// Suggested replacements may not use correct path to enum
 276     /// if it's not present in the current scope.
 277     ///
 278     /// ### Example
 279     /// ```rust
 280     /// # enum Foo { A, B, C }
 281     /// # let x = Foo::B;
 282     /// // Bad
 283     /// match x {
 284     ///     Foo::A => {},
 285     ///     Foo::B => {},
 286     ///     _ => {},
 287     /// }
 288     ///
 289     /// // Good
 290     /// match x {
 291     ///     Foo::A => {},
 292     ///     Foo::B => {},
 293     ///     Foo::C => {},
 294     /// }
 295     /// ```
 296     #[clippy::version = "1.45.0"]
 297     pub MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
 298     pedantic,
 299     "a wildcard enum match for a single variant"
 300 }
 301
 302 declare_clippy_lint! {
 303     /// ### What it does
 304     /// Checks for wildcard pattern used with others patterns in same match arm.
 305     ///
 306     /// ### Why is this bad?
 307     /// Wildcard pattern already covers any other pattern as it will match anyway.
 308     /// It makes the code less readable, especially to spot wildcard pattern use in match arm.
 309     ///
 310     /// ### Example
 311     /// ```rust
 312     /// // Bad
 313     /// match "foo" {
 314     ///     "a" => {},
 315     ///     "bar" | _ => {},
 316     /// }
 317     ///
 318     /// // Good
 319     /// match "foo" {
 320     ///     "a" => {},
 321     ///     _ => {},
 322     /// }
 323     /// ```
 324     #[clippy::version = "1.42.0"]
 325     pub WILDCARD_IN_OR_PATTERNS,
 326     complexity,
 327     "a wildcard pattern used with others patterns in same match arm"
 328 }
 329
 330 declare_clippy_lint! {
 331     /// ### What it does
 332     /// Checks for matches being used to destructure a single-variant enum
 333     /// or tuple struct where a `let` will suffice.
 334     ///
 335     /// ### Why is this bad?
 336     /// Just readability – `let` doesn't nest, whereas a `match` does.
 337     ///
 338     /// ### Example
 339     /// ```rust
 340     /// enum Wrapper {
 341     ///     Data(i32),
 342     /// }
 343     ///
 344     /// let wrapper = Wrapper::Data(42);
 345     ///
 346     /// let data = match wrapper {
 347     ///     Wrapper::Data(i) => i,
 348     /// };
 349     /// ```
 350     ///
 351     /// The correct use would be:
 352     /// ```rust
 353     /// enum Wrapper {
 354     ///     Data(i32),
 355     /// }
 356     ///
 357     /// let wrapper = Wrapper::Data(42);
 358     /// let Wrapper::Data(data) = wrapper;
 359     /// ```
 360     #[clippy::version = "pre 1.29.0"]
 361     pub INFALLIBLE_DESTRUCTURING_MATCH,
 362     style,
 363     "a `match` statement with a single infallible arm instead of a `let`"
 364 }
 365
 366 declare_clippy_lint! {
 367     /// ### What it does
 368     /// Checks for useless match that binds to only one value.
 369     ///
 370     /// ### Why is this bad?
 371     /// Readability and needless complexity.
 372     ///
 373     /// ### Known problems
 374     ///  Suggested replacements may be incorrect when `match`
 375     /// is actually binding temporary value, bringing a 'dropped while borrowed' error.
 376     ///
 377     /// ### Example
 378     /// ```rust
 379     /// # let a = 1;
 380     /// # let b = 2;
 381     ///
 382     /// // Bad
 383     /// match (a, b) {
 384     ///     (c, d) => {
 385     ///         // useless match
 386     ///     }
 387     /// }
 388     ///
 389     /// // Good
 390     /// let (c, d) = (a, b);
 391     /// ```
 392     #[clippy::version = "1.43.0"]
 393     pub MATCH_SINGLE_BINDING,
 394     complexity,
 395     "a match with a single binding instead of using `let` statement"
 396 }
 397
 398 declare_clippy_lint! {
 399     /// ### What it does
 400     /// Checks for unnecessary '..' pattern binding on struct when all fields are explicitly matched.
 401     ///
 402     /// ### Why is this bad?
 403     /// Correctness and readability. It's like having a wildcard pattern after
 404     /// matching all enum variants explicitly.
 405     ///
 406     /// ### Example
 407     /// ```rust
 408     /// # struct A { a: i32 }
 409     /// let a = A { a: 5 };
 410     ///
 411     /// // Bad
 412     /// match a {
 413     ///     A { a: 5, .. } => {},
 414     ///     _ => {},
 415     /// }
 416     ///
 417     /// // Good
 418     /// match a {
 419     ///     A { a: 5 } => {},
 420     ///     _ => {},
 421     /// }
 422     /// ```
 423     #[clippy::version = "1.43.0"]
 424     pub REST_PAT_IN_FULLY_BOUND_STRUCTS,
 425     restriction,
 426     "a match on a struct that binds all fields but still uses the wildcard pattern"
 427 }
 428
 429 declare_clippy_lint! {
 430     /// ### What it does
 431     /// Lint for redundant pattern matching over `Result`, `Option`,
 432     /// `std::task::Poll` or `std::net::IpAddr`
 433     ///
 434     /// ### Why is this bad?
 435     /// It's more concise and clear to just use the proper
 436     /// utility function
 437     ///
 438     /// ### Known problems
 439     /// This will change the drop order for the matched type. Both `if let` and
 440     /// `while let` will drop the value at the end of the block, both `if` and `while` will drop the
 441     /// value before entering the block. For most types this change will not matter, but for a few
 442     /// types this will not be an acceptable change (e.g. locks). See the
 443     /// [reference](https://doc.rust-lang.org/reference/destructors.html#drop-scopes) for more about
 444     /// drop order.
 445     ///
 446     /// ### Example
 447     /// ```rust
 448     /// # use std::task::Poll;
 449     /// # use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
 450     /// if let Ok(_) = Ok::<i32, i32>(42) {}
 451     /// if let Err(_) = Err::<i32, i32>(42) {}
 452     /// if let None = None::<()> {}
 453     /// if let Some(_) = Some(42) {}
 454     /// if let Poll::Pending = Poll::Pending::<()> {}
 455     /// if let Poll::Ready(_) = Poll::Ready(42) {}
 456     /// if let IpAddr::V4(_) = IpAddr::V4(Ipv4Addr::LOCALHOST) {}
 457     /// if let IpAddr::V6(_) = IpAddr::V6(Ipv6Addr::LOCALHOST) {}
 458     /// match Ok::<i32, i32>(42) {
 459     ///     Ok(_) => true,
 460     ///     Err(_) => false,
 461     /// };
 462     /// ```
 463     ///
 464     /// The more idiomatic use would be:
 465     ///
 466     /// ```rust
 467     /// # use std::task::Poll;
 468     /// # use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
 469     /// if Ok::<i32, i32>(42).is_ok() {}
 470     /// if Err::<i32, i32>(42).is_err() {}
 471     /// if None::<()>.is_none() {}
 472     /// if Some(42).is_some() {}
 473     /// if Poll::Pending::<()>.is_pending() {}
 474     /// if Poll::Ready(42).is_ready() {}
 475     /// if IpAddr::V4(Ipv4Addr::LOCALHOST).is_ipv4() {}
 476     /// if IpAddr::V6(Ipv6Addr::LOCALHOST).is_ipv6() {}
 477     /// Ok::<i32, i32>(42).is_ok();
 478     /// ```
 479     #[clippy::version = "1.31.0"]
 480     pub REDUNDANT_PATTERN_MATCHING,
 481     style,
 482     "use the proper utility function avoiding an `if let`"
 483 }
 484
 485 declare_clippy_lint! {
 486     /// ### What it does
 487     /// Checks for `match`  or `if let` expressions producing a
 488     /// `bool` that could be written using `matches!`
 489     ///
 490     /// ### Why is this bad?
 491     /// Readability and needless complexity.
 492     ///
 493     /// ### Known problems
 494     /// This lint falsely triggers, if there are arms with
 495     /// `cfg` attributes that remove an arm evaluating to `false`.
 496     ///
 497     /// ### Example
 498     /// ```rust
 499     /// let x = Some(5);
 500     ///
 501     /// // Bad
 502     /// let a = match x {
 503     ///     Some(0) => true,
 504     ///     _ => false,
 505     /// };
 506     ///
 507     /// let a = if let Some(0) = x {
 508     ///     true
 509     /// } else {
 510     ///     false
 511     /// };
 512     ///
 513     /// // Good
 514     /// let a = matches!(x, Some(0));
 515     /// ```
 516     #[clippy::version = "1.47.0"]
 517     pub MATCH_LIKE_MATCHES_MACRO,
 518     style,
 519     "a match that could be written with the matches! macro"
 520 }
 521
 522 declare_clippy_lint! {
 523     /// ### What it does
 524     /// Checks for `match` with identical arm bodies.
 525     ///
 526     /// ### Why is this bad?
 527     /// This is probably a copy & paste error. If arm bodies
 528     /// are the same on purpose, you can factor them
 529     /// [using `|`](https://doc.rust-lang.org/book/patterns.html#multiple-patterns).
 530     ///
 531     /// ### Known problems
 532     /// False positive possible with order dependent `match`
 533     /// (see issue
 534     /// [#860](https://github.com/rust-lang/rust-clippy/issues/860)).
 535     ///
 536     /// ### Example
 537     /// ```rust,ignore
 538     /// match foo {
 539     ///     Bar => bar(),
 540     ///     Quz => quz(),
 541     ///     Baz => bar(), // <= oops
 542     /// }
 543     /// ```
 544     ///
 545     /// This should probably be
 546     /// ```rust,ignore
 547     /// match foo {
 548     ///     Bar => bar(),
 549     ///     Quz => quz(),
 550     ///     Baz => baz(), // <= fixed
 551     /// }
 552     /// ```
 553     ///
 554     /// or if the original code was not a typo:
 555     /// ```rust,ignore
 556     /// match foo {
 557     ///     Bar | Baz => bar(), // <= shows the intent better
 558     ///     Quz => quz(),
 559     /// }
 560     /// ```
 561     #[clippy::version = "pre 1.29.0"]
 562     pub MATCH_SAME_ARMS,
 563     pedantic,
 564     "`match` with identical arm bodies"
 565 }
 566
 567 #[derive(Default)]
 568 pub struct Matches {
 569     msrv: Option<RustcVersion>,
 570     infallible_destructuring_match_linted: bool,
 571 }
 572
 573 impl Matches {
 574     #[must_use]
 575     pub fn new(msrv: Option<RustcVersion>) -> Self {
 576         Self {
 577             msrv,
 578             ..Matches::default()
 579         }
 580     }
 581 }
 582
 583 impl_lint_pass!(Matches => [
 584     SINGLE_MATCH,
 585     MATCH_REF_PATS,
 586     MATCH_BOOL,
 587     SINGLE_MATCH_ELSE,
 588     MATCH_OVERLAPPING_ARM,
 589     MATCH_WILD_ERR_ARM,
 590     MATCH_AS_REF,
 591     WILDCARD_ENUM_MATCH_ARM,
 592     MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
 593     WILDCARD_IN_OR_PATTERNS,
 594     MATCH_SINGLE_BINDING,
 595     INFALLIBLE_DESTRUCTURING_MATCH,
 596     REST_PAT_IN_FULLY_BOUND_STRUCTS,
 597     REDUNDANT_PATTERN_MATCHING,
 598     MATCH_LIKE_MATCHES_MACRO,
 599     MATCH_SAME_ARMS,
 600 ]);
 601
 602 impl<'tcx> LateLintPass<'tcx> for Matches {
 603     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
 604         if expr.span.from_expansion() {
 605             return;
 606         }
 607
 608         redundant_pattern_match::check(cx, expr);
 609
 610         if meets_msrv(self.msrv.as_ref(), &msrvs::MATCHES_MACRO) {
 611             if !match_like_matches::check(cx, expr) {
 612                 match_same_arms::check(cx, expr);
 613             }
 614         } else {
 615             match_same_arms::check(cx, expr);
 616         }
 617
 618         if let ExprKind::Match(ex, arms, MatchSource::Normal) = expr.kind {
 619             single_match::check(cx, ex, arms, expr);
 620             match_bool::check(cx, ex, arms, expr);
 621             overlapping_arms::check(cx, ex, arms);
 622             match_wild_err_arm::check(cx, ex, arms);
 623             match_wild_enum::check(cx, ex, arms);
 624             match_as_ref::check(cx, ex, arms, expr);
 625             check_wild_in_or_pats(cx, arms);
 626
 627             if self.infallible_destructuring_match_linted {
 628                 self.infallible_destructuring_match_linted = false;
 629             } else {
 630                 match_single_binding::check(cx, ex, arms, expr);
 631             }
 632         }
 633         if let ExprKind::Match(ex, arms, _) = expr.kind {
 634             match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr);
 635         }
 636     }
 637
 638     fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'_>) {
 639         self.infallible_destructuring_match_linted |= infalliable_detructuring_match::check(cx, local);
 640     }
 641
 642     fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
 643         if_chain! {
 644             if !pat.span.from_expansion();
 645             if let PatKind::Struct(QPath::Resolved(_, path), fields, true) = pat.kind;
 646             if let Some(def_id) = path.res.opt_def_id();
 647             let ty = cx.tcx.type_of(def_id);
 648             if let ty::Adt(def, _) = ty.kind();
 649             if def.is_struct() || def.is_union();
 650             if fields.len() == def.non_enum_variant().fields.len();
 651
 652             then {
 653                 span_lint_and_help(
 654                     cx,
 655                     REST_PAT_IN_FULLY_BOUND_STRUCTS,
 656                     pat.span,
 657                     "unnecessary use of `..` pattern in struct binding. All fields were already bound",
 658                     None,
 659                     "consider removing `..` from this binding",
 660                 );
 661             }
 662         }
 663     }
 664
 665     extract_msrv_attr!(LateContext);
 666 }
 667
 668 fn check_wild_in_or_pats(cx: &LateContext<'_>, arms: &[Arm<'_>]) {
 669     for arm in arms {
 670         if let PatKind::Or(fields) = arm.pat.kind {
 671             // look for multiple fields in this arm that contains at least one Wild pattern
 672             if fields.len() > 1 && fields.iter().any(is_wild) {
 673                 span_lint_and_help(
 674                     cx,
 675                     WILDCARD_IN_OR_PATTERNS,
 676                     arm.pat.span,
 677                     "wildcard pattern covers any other pattern as it will match anyway",
 678                     None,
 679                     "consider handling `_` separately",
 680                 );
 681             }
 682         }
 683     }
 684 }