Skip to content

Commit 22bdd9f

Browse files
committed
fix(match_as_ref): suggest as_ref when the reference needs to be cast
1 parent 5ff804e commit 22bdd9f

File tree

4 files changed

+100
-11
lines changed

4 files changed

+100
-11
lines changed

clippy_lints/src/matches/match_as_ref.rs

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,25 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr:
2525
&& let input_ty = cx.typeck_results().expr_ty(ex)
2626
&& let Some(input_ty) = option_arg_ty(cx, input_ty)
2727
&& let Some(output_ty) = option_arg_ty(cx, output_ty)
28-
&& let ty::Ref(_, output_ty, _) = *output_ty.kind()
28+
&& let ty::Ref(_, output_ty, output_mutbl) = *output_ty.kind()
2929
{
3030
let method = match arm_ref_mutbl {
3131
Mutability::Not => "as_ref",
3232
Mutability::Mut => "as_mut",
3333
};
3434

35+
// ```
36+
// let _: Option<&T> = match opt {
37+
// Some(ref mut t) => Some(t),
38+
// None => None,
39+
// };
40+
// ```
41+
// We need to suggest `t.as_ref()` in order downcast the reference from `&mut` to `&`.
42+
// We may or may not need to cast the type as well, for which we'd need `.map()`, and that could
43+
// theoretically take care of the reference downcasting as well, but we chose to keep these two
44+
// operations separate
45+
let need_as_ref = arm_ref_mutbl == Mutability::Mut && output_mutbl == Mutability::Not;
46+
3547
let cast = if input_ty == output_ty { "" } else { ".map(|x| x as _)" };
3648

3749
let mut applicability = Applicability::MachineApplicable;
@@ -41,15 +53,28 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr:
4153
expr.span,
4254
format!("manual implementation of `Option::{method}`"),
4355
|diag| {
44-
diag.span_suggestion_verbose(
45-
expr.span,
46-
format!("use `Option::{method}()` directly"),
47-
format!(
48-
"{}.{method}(){cast}",
49-
Sugg::hir_with_applicability(cx, ex, "_", &mut applicability).maybe_paren(),
50-
),
51-
applicability,
52-
);
56+
if need_as_ref {
57+
diag.note("but the type is coerced to a non-mutable reference, and so `as_ref` can used instead");
58+
diag.span_suggestion_verbose(
59+
expr.span,
60+
"use `Option::as_ref()`",
61+
format!(
62+
"{}.as_ref(){cast}",
63+
Sugg::hir_with_applicability(cx, ex, "_", &mut applicability).maybe_paren(),
64+
),
65+
applicability,
66+
);
67+
} else {
68+
diag.span_suggestion_verbose(
69+
expr.span,
70+
format!("use `Option::{method}()` directly"),
71+
format!(
72+
"{}.{method}(){cast}",
73+
Sugg::hir_with_applicability(cx, ex, "_", &mut applicability).maybe_paren(),
74+
),
75+
applicability,
76+
);
77+
}
5378
},
5479
);
5580
}

tests/ui/match_as_ref.fixed

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,9 @@ fn recv_requiring_parens() {
8484

8585
let _ = (!S).as_ref();
8686
}
87+
88+
fn issue15932() {
89+
let _: Option<&u32> = Some(0).as_ref();
90+
91+
let _: Option<&dyn std::fmt::Debug> = Some(0).as_ref().map(|x| x as _);
92+
}

tests/ui/match_as_ref.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,17 @@ fn recv_requiring_parens() {
100100
Some(ref v) => Some(v),
101101
};
102102
}
103+
104+
fn issue15932() {
105+
let _: Option<&u32> = match Some(0) {
106+
//~^ match_as_ref
107+
None => None,
108+
Some(ref mut v) => Some(v),
109+
};
110+
111+
let _: Option<&dyn std::fmt::Debug> = match Some(0) {
112+
//~^ match_as_ref
113+
None => None,
114+
Some(ref mut v) => Some(v),
115+
};
116+
}

tests/ui/match_as_ref.stderr

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,5 +83,49 @@ LL - };
8383
LL + let _ = (!S).as_ref();
8484
|
8585

86-
error: aborting due to 4 previous errors
86+
error: manual implementation of `Option::as_mut`
87+
--> tests/ui/match_as_ref.rs:105:27
88+
|
89+
LL | let _: Option<&u32> = match Some(0) {
90+
| ___________________________^
91+
LL | |
92+
LL | | None => None,
93+
LL | | Some(ref mut v) => Some(v),
94+
LL | | };
95+
| |_____^
96+
|
97+
= note: but the type is coerced to a non-mutable reference, and so `as_ref` can used instead
98+
help: use `Option::as_ref()`
99+
|
100+
LL - let _: Option<&u32> = match Some(0) {
101+
LL -
102+
LL - None => None,
103+
LL - Some(ref mut v) => Some(v),
104+
LL - };
105+
LL + let _: Option<&u32> = Some(0).as_ref();
106+
|
107+
108+
error: manual implementation of `Option::as_mut`
109+
--> tests/ui/match_as_ref.rs:111:43
110+
|
111+
LL | let _: Option<&dyn std::fmt::Debug> = match Some(0) {
112+
| ___________________________________________^
113+
LL | |
114+
LL | | None => None,
115+
LL | | Some(ref mut v) => Some(v),
116+
LL | | };
117+
| |_____^
118+
|
119+
= note: but the type is coerced to a non-mutable reference, and so `as_ref` can used instead
120+
help: use `Option::as_ref()`
121+
|
122+
LL - let _: Option<&dyn std::fmt::Debug> = match Some(0) {
123+
LL -
124+
LL - None => None,
125+
LL - Some(ref mut v) => Some(v),
126+
LL - };
127+
LL + let _: Option<&dyn std::fmt::Debug> = Some(0).as_ref().map(|x| x as _);
128+
|
129+
130+
error: aborting due to 6 previous errors
87131

0 commit comments

Comments
 (0)