From 5ff804e9b239f33fe1aab98c896248546a523b71 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Wed, 22 Oct 2025 11:46:37 +0200 Subject: [PATCH 1/2] misc: move infallible stuff out of the way --- clippy_lints/src/matches/match_as_ref.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/clippy_lints/src/matches/match_as_ref.rs b/clippy_lints/src/matches/match_as_ref.rs index 2c4d08639f61..7b23cdabf2e8 100644 --- a/clippy_lints/src/matches/match_as_ref.rs +++ b/clippy_lints/src/matches/match_as_ref.rs @@ -21,24 +21,18 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: } else { None } + && let output_ty = cx.typeck_results().expr_ty(expr) + && let input_ty = cx.typeck_results().expr_ty(ex) + && let Some(input_ty) = option_arg_ty(cx, input_ty) + && let Some(output_ty) = option_arg_ty(cx, output_ty) + && let ty::Ref(_, output_ty, _) = *output_ty.kind() { let method = match arm_ref_mutbl { Mutability::Not => "as_ref", Mutability::Mut => "as_mut", }; - let output_ty = cx.typeck_results().expr_ty(expr); - let input_ty = cx.typeck_results().expr_ty(ex); - - let cast = if let Some(input_ty) = option_arg_ty(cx, input_ty) - && let Some(output_ty) = option_arg_ty(cx, output_ty) - && let ty::Ref(_, output_ty, _) = *output_ty.kind() - && input_ty != output_ty - { - ".map(|x| x as _)" - } else { - "" - }; + let cast = if input_ty == output_ty { "" } else { ".map(|x| x as _)" }; let mut applicability = Applicability::MachineApplicable; span_lint_and_then( From 22bdd9f7c9bef81a3ebccffeb6621c9959172fe9 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Wed, 22 Oct 2025 01:03:58 +0200 Subject: [PATCH 2/2] fix(match_as_ref): suggest `as_ref` when the reference needs to be cast --- clippy_lints/src/matches/match_as_ref.rs | 45 +++++++++++++++++------ tests/ui/match_as_ref.fixed | 6 ++++ tests/ui/match_as_ref.rs | 14 ++++++++ tests/ui/match_as_ref.stderr | 46 +++++++++++++++++++++++- 4 files changed, 100 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/matches/match_as_ref.rs b/clippy_lints/src/matches/match_as_ref.rs index 7b23cdabf2e8..7ac57fae690c 100644 --- a/clippy_lints/src/matches/match_as_ref.rs +++ b/clippy_lints/src/matches/match_as_ref.rs @@ -25,13 +25,25 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: && let input_ty = cx.typeck_results().expr_ty(ex) && let Some(input_ty) = option_arg_ty(cx, input_ty) && let Some(output_ty) = option_arg_ty(cx, output_ty) - && let ty::Ref(_, output_ty, _) = *output_ty.kind() + && let ty::Ref(_, output_ty, output_mutbl) = *output_ty.kind() { let method = match arm_ref_mutbl { Mutability::Not => "as_ref", Mutability::Mut => "as_mut", }; + // ``` + // let _: Option<&T> = match opt { + // Some(ref mut t) => Some(t), + // None => None, + // }; + // ``` + // We need to suggest `t.as_ref()` in order downcast the reference from `&mut` to `&`. + // We may or may not need to cast the type as well, for which we'd need `.map()`, and that could + // theoretically take care of the reference downcasting as well, but we chose to keep these two + // operations separate + let need_as_ref = arm_ref_mutbl == Mutability::Mut && output_mutbl == Mutability::Not; + let cast = if input_ty == output_ty { "" } else { ".map(|x| x as _)" }; let mut applicability = Applicability::MachineApplicable; @@ -41,15 +53,28 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: expr.span, format!("manual implementation of `Option::{method}`"), |diag| { - diag.span_suggestion_verbose( - expr.span, - format!("use `Option::{method}()` directly"), - format!( - "{}.{method}(){cast}", - Sugg::hir_with_applicability(cx, ex, "_", &mut applicability).maybe_paren(), - ), - applicability, - ); + if need_as_ref { + diag.note("but the type is coerced to a non-mutable reference, and so `as_ref` can used instead"); + diag.span_suggestion_verbose( + expr.span, + "use `Option::as_ref()`", + format!( + "{}.as_ref(){cast}", + Sugg::hir_with_applicability(cx, ex, "_", &mut applicability).maybe_paren(), + ), + applicability, + ); + } else { + diag.span_suggestion_verbose( + expr.span, + format!("use `Option::{method}()` directly"), + format!( + "{}.{method}(){cast}", + Sugg::hir_with_applicability(cx, ex, "_", &mut applicability).maybe_paren(), + ), + applicability, + ); + } }, ); } diff --git a/tests/ui/match_as_ref.fixed b/tests/ui/match_as_ref.fixed index 3653b81eda22..09a6ed169390 100644 --- a/tests/ui/match_as_ref.fixed +++ b/tests/ui/match_as_ref.fixed @@ -84,3 +84,9 @@ fn recv_requiring_parens() { let _ = (!S).as_ref(); } + +fn issue15932() { + let _: Option<&u32> = Some(0).as_ref(); + + let _: Option<&dyn std::fmt::Debug> = Some(0).as_ref().map(|x| x as _); +} diff --git a/tests/ui/match_as_ref.rs b/tests/ui/match_as_ref.rs index d58cfd81aeab..347b6d186887 100644 --- a/tests/ui/match_as_ref.rs +++ b/tests/ui/match_as_ref.rs @@ -100,3 +100,17 @@ fn recv_requiring_parens() { Some(ref v) => Some(v), }; } + +fn issue15932() { + let _: Option<&u32> = match Some(0) { + //~^ match_as_ref + None => None, + Some(ref mut v) => Some(v), + }; + + let _: Option<&dyn std::fmt::Debug> = match Some(0) { + //~^ match_as_ref + None => None, + Some(ref mut v) => Some(v), + }; +} diff --git a/tests/ui/match_as_ref.stderr b/tests/ui/match_as_ref.stderr index 226655970ab8..df06e358f296 100644 --- a/tests/ui/match_as_ref.stderr +++ b/tests/ui/match_as_ref.stderr @@ -83,5 +83,49 @@ LL - }; LL + let _ = (!S).as_ref(); | -error: aborting due to 4 previous errors +error: manual implementation of `Option::as_mut` + --> tests/ui/match_as_ref.rs:105:27 + | +LL | let _: Option<&u32> = match Some(0) { + | ___________________________^ +LL | | +LL | | None => None, +LL | | Some(ref mut v) => Some(v), +LL | | }; + | |_____^ + | + = note: but the type is coerced to a non-mutable reference, and so `as_ref` can used instead +help: use `Option::as_ref()` + | +LL - let _: Option<&u32> = match Some(0) { +LL - +LL - None => None, +LL - Some(ref mut v) => Some(v), +LL - }; +LL + let _: Option<&u32> = Some(0).as_ref(); + | + +error: manual implementation of `Option::as_mut` + --> tests/ui/match_as_ref.rs:111:43 + | +LL | let _: Option<&dyn std::fmt::Debug> = match Some(0) { + | ___________________________________________^ +LL | | +LL | | None => None, +LL | | Some(ref mut v) => Some(v), +LL | | }; + | |_____^ + | + = note: but the type is coerced to a non-mutable reference, and so `as_ref` can used instead +help: use `Option::as_ref()` + | +LL - let _: Option<&dyn std::fmt::Debug> = match Some(0) { +LL - +LL - None => None, +LL - Some(ref mut v) => Some(v), +LL - }; +LL + let _: Option<&dyn std::fmt::Debug> = Some(0).as_ref().map(|x| x as _); + | + +error: aborting due to 6 previous errors