Skip to content

Commit a6ccc6f

Browse files
committed
Also suggest as_mut for match_as_ref
1 parent 919601b commit a6ccc6f

File tree

3 files changed

+29
-22
lines changed

3 files changed

+29
-22
lines changed

clippy_lints/src/matches.rs

+17-10
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ declare_lint! {
148148
/// **What it does:** Checks for match which is used to add a reference to an
149149
/// `Option` value.
150150
///
151-
/// **Why is this bad?** Using `as_ref()` instead is shorter.
151+
/// **Why is this bad?** Using `as_ref()` or `as_mut()` instead is shorter.
152152
///
153153
/// **Known problems:** None.
154154
///
@@ -163,7 +163,7 @@ declare_lint! {
163163
declare_lint! {
164164
pub MATCH_AS_REF,
165165
Warn,
166-
"a match on an Option value instead of using `as_ref()`"
166+
"a match on an Option value instead of using `as_ref()` or `as_mut`"
167167
}
168168

169169
#[allow(missing_copy_implementations)]
@@ -438,15 +438,22 @@ fn check_match_as_ref(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
438438
if arms.len() == 2 &&
439439
arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
440440
arms[1].pats.len() == 1 && arms[1].guard.is_none() {
441-
if (is_ref_some_arm(&arms[0]) && is_none_arm(&arms[1])) ||
442-
(is_ref_some_arm(&arms[1]) && is_none_arm(&arms[0])) {
441+
let arm_ref: Option<BindingAnnotation> = if is_none_arm(&arms[0]) {
442+
is_ref_some_arm(&arms[1])
443+
} else if is_none_arm(&arms[1]) {
444+
is_ref_some_arm(&arms[0])
445+
} else {
446+
None
447+
};
448+
if let Some(rb) = arm_ref {
449+
let suggestion = if rb == BindingAnnotation::Ref { "as_ref" } else { "as_mut" };
443450
span_lint_and_sugg(
444451
cx,
445452
MATCH_AS_REF,
446453
expr.span,
447-
"use as_ref() instead",
454+
&format!("use {}() instead", suggestion),
448455
"try this",
449-
format!("{}.as_ref()", snippet(cx, ex.span, "_"))
456+
format!("{}.{}()", snippet(cx, ex.span, "_"), suggestion)
450457
)
451458
}
452459
}
@@ -574,7 +581,7 @@ fn is_none_arm(arm: &Arm) -> bool {
574581
}
575582

576583
// Checks if arm has the form `Some(ref v) => Some(v)` (checks for `ref` and `ref mut`)
577-
fn is_ref_some_arm(arm: &Arm) -> bool {
584+
fn is_ref_some_arm(arm: &Arm) -> Option<BindingAnnotation> {
578585
if_chain! {
579586
if let PatKind::TupleStruct(ref path, ref pats, _) = arm.pats[0].node;
580587
if pats.len() == 1 && match_qpath(path, &paths::OPTION_SOME);
@@ -585,12 +592,12 @@ fn is_ref_some_arm(arm: &Arm) -> bool {
585592
if match_qpath(some_path, &paths::OPTION_SOME) && args.len() == 1;
586593
if let ExprPath(ref qpath) = args[0].node;
587594
if let &QPath::Resolved(_, ref path2) = qpath;
588-
if path2.segments.len() == 1;
595+
if path2.segments.len() == 1 && ident.node == path2.segments[0].name;
589596
then {
590-
return ident.node == path2.segments[0].name
597+
return Some(rb)
591598
}
592599
}
593-
false
600+
None
594601
}
595602

596603
fn has_only_ref_pats(arms: &[Arm]) -> bool {

tests/ui/matches.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -316,14 +316,14 @@ fn match_wild_err_arm() {
316316
}
317317

318318
fn match_as_ref() {
319-
let owned : Option<()> = None;
320-
let borrowed = match owned {
319+
let owned: Option<()> = None;
320+
let borrowed: Option<&()> = match owned {
321321
None => None,
322322
Some(ref v) => Some(v),
323323
};
324324

325-
let mut mut_owned : Option<()> = None;
326-
let mut mut_borrowed = match mut_owned {
325+
let mut mut_owned: Option<()> = None;
326+
let borrow_mut: Option<&mut ()> = match mut_owned {
327327
None => None,
328328
Some(ref mut v) => Some(v),
329329
};

tests/ui/matches.stderr

+8-8
Original file line numberDiff line numberDiff line change
@@ -427,24 +427,24 @@ note: consider refactoring into `Ok(3) | Ok(_)`
427427
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
428428

429429
error: use as_ref() instead
430-
--> $DIR/matches.rs:320:20
430+
--> $DIR/matches.rs:320:33
431431
|
432-
320 | let borrowed = match owned {
433-
| ____________________^
432+
320 | let borrowed: Option<&()> = match owned {
433+
| _________________________________^
434434
321 | | None => None,
435435
322 | | Some(ref v) => Some(v),
436436
323 | | };
437437
| |_____^ help: try this: `owned.as_ref()`
438438
|
439439
= note: `-D match-as-ref` implied by `-D warnings`
440440

441-
error: use as_ref() instead
442-
--> $DIR/matches.rs:326:28
441+
error: use as_mut() instead
442+
--> $DIR/matches.rs:326:39
443443
|
444-
326 | let mut mut_borrowed = match mut_owned {
445-
| ____________________________^
444+
326 | let borrow_mut: Option<&mut ()> = match mut_owned {
445+
| _______________________________________^
446446
327 | | None => None,
447447
328 | | Some(ref mut v) => Some(v),
448448
329 | | };
449-
| |_____^ help: try this: `mut_owned.as_ref()`
449+
| |_____^ help: try this: `mut_owned.as_mut()`
450450

0 commit comments

Comments
 (0)