diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 326ce83a84f..2183c6e4fa6 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -148,7 +148,7 @@ declare_lint! { /// **What it does:** Checks for match which is used to add a reference to an /// `Option` value. /// -/// **Why is this bad?** Using `as_ref()` instead is shorter. +/// **Why is this bad?** Using `as_ref()` or `as_mut()` instead is shorter. /// /// **Known problems:** None. /// @@ -163,7 +163,7 @@ declare_lint! { declare_lint! { pub MATCH_AS_REF, Warn, - "a match on an Option value instead of using `as_ref()`" + "a match on an Option value instead of using `as_ref()` or `as_mut`" } #[allow(missing_copy_implementations)] @@ -438,15 +438,22 @@ fn check_match_as_ref(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) { if arms.len() == 2 && arms[0].pats.len() == 1 && arms[0].guard.is_none() && arms[1].pats.len() == 1 && arms[1].guard.is_none() { - if (is_ref_some_arm(&arms[0]) && is_none_arm(&arms[1])) || - (is_ref_some_arm(&arms[1]) && is_none_arm(&arms[0])) { + let arm_ref: Option = if is_none_arm(&arms[0]) { + is_ref_some_arm(&arms[1]) + } else if is_none_arm(&arms[1]) { + is_ref_some_arm(&arms[0]) + } else { + None + }; + if let Some(rb) = arm_ref { + let suggestion = if rb == BindingAnnotation::Ref { "as_ref" } else { "as_mut" }; span_lint_and_sugg( cx, MATCH_AS_REF, expr.span, - "use as_ref() instead", + &format!("use {}() instead", suggestion), "try this", - format!("{}.as_ref()", snippet(cx, ex.span, "_")) + format!("{}.{}()", snippet(cx, ex.span, "_"), suggestion) ) } } @@ -574,7 +581,7 @@ fn is_none_arm(arm: &Arm) -> bool { } // Checks if arm has the form `Some(ref v) => Some(v)` (checks for `ref` and `ref mut`) -fn is_ref_some_arm(arm: &Arm) -> bool { +fn is_ref_some_arm(arm: &Arm) -> Option { if_chain! { if let PatKind::TupleStruct(ref path, ref pats, _) = arm.pats[0].node; if pats.len() == 1 && match_qpath(path, &paths::OPTION_SOME); @@ -585,12 +592,12 @@ fn is_ref_some_arm(arm: &Arm) -> bool { if match_qpath(some_path, &paths::OPTION_SOME) && args.len() == 1; if let ExprPath(ref qpath) = args[0].node; if let &QPath::Resolved(_, ref path2) = qpath; - if path2.segments.len() == 1; + if path2.segments.len() == 1 && ident.node == path2.segments[0].name; then { - return ident.node == path2.segments[0].name + return Some(rb) } } - false + None } fn has_only_ref_pats(arms: &[Arm]) -> bool { diff --git a/tests/ui/matches.rs b/tests/ui/matches.rs index 72a36338aa2..67a901f65b2 100644 --- a/tests/ui/matches.rs +++ b/tests/ui/matches.rs @@ -316,14 +316,14 @@ fn match_wild_err_arm() { } fn match_as_ref() { - let owned : Option<()> = None; - let borrowed = match owned { + let owned: Option<()> = None; + let borrowed: Option<&()> = match owned { None => None, Some(ref v) => Some(v), }; - let mut mut_owned : Option<()> = None; - let mut mut_borrowed = match mut_owned { + let mut mut_owned: Option<()> = None; + let borrow_mut: Option<&mut ()> = match mut_owned { None => None, Some(ref mut v) => Some(v), }; diff --git a/tests/ui/matches.stderr b/tests/ui/matches.stderr index 3d7b8f78473..62c77c778be 100644 --- a/tests/ui/matches.stderr +++ b/tests/ui/matches.stderr @@ -427,10 +427,10 @@ note: consider refactoring into `Ok(3) | Ok(_)` = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: use as_ref() instead - --> $DIR/matches.rs:320:20 + --> $DIR/matches.rs:320:33 | -320 | let borrowed = match owned { - | ____________________^ +320 | let borrowed: Option<&()> = match owned { + | _________________________________^ 321 | | None => None, 322 | | Some(ref v) => Some(v), 323 | | }; @@ -438,13 +438,13 @@ error: use as_ref() instead | = note: `-D match-as-ref` implied by `-D warnings` -error: use as_ref() instead - --> $DIR/matches.rs:326:28 +error: use as_mut() instead + --> $DIR/matches.rs:326:39 | -326 | let mut mut_borrowed = match mut_owned { - | ____________________________^ +326 | let borrow_mut: Option<&mut ()> = match mut_owned { + | _______________________________________^ 327 | | None => None, 328 | | Some(ref mut v) => Some(v), 329 | | }; - | |_____^ help: try this: `mut_owned.as_ref()` + | |_____^ help: try this: `mut_owned.as_mut()`