More robust as_ref/as_deref suggestions

Author: compiler-errors

compiler/rustc_hir_typeck/messages.ftl

@@ -25,6 +25,8 @@ hir_typeck_const_select_must_be_fn = this argument must be a function item
 
 hir_typeck_convert_to_str = try converting the passed type into a `&str`
 
+hir_typeck_convert_using_method = try using `{$sugg}` to convert `{$found}` to `{$expected}`
+
 hir_typeck_ctor_is_private = tuple struct constructor `{$def}` is private
 
 hir_typeck_expected_default_return_type = expected `()` because of default return type

compiler/rustc_hir_typeck/src/errors.rs

@@ -327,3 +327,18 @@ pub struct CtorIsPrivate {
     pub span: Span,
     pub def: String,
 }
+
+#[derive(Subdiagnostic)]
+#[suggestion(
+    hir_typeck_convert_using_method,
+    code = "{sugg}",
+    applicability = "machine-applicable",
+    style = "verbose"
+)]
+pub struct SuggestConvertViaMethod<'tcx> {
+    #[primary_span]
+    pub span: Span,
+    pub sugg: &'static str,
+    pub expected: Ty<'tcx>,
+    pub found: Ty<'tcx>,
+}

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -1,6 +1,8 @@
 use super::FnCtxt;
 
-use crate::errors::{AddReturnTypeSuggestion, ExpectedReturnTypeLabel, SuggestBoxing};
+use crate::errors::{
+    AddReturnTypeSuggestion, ExpectedReturnTypeLabel, SuggestBoxing, SuggestConvertViaMethod,
+};
 use crate::fluent_generated as fluent;
 use crate::method::probe::{IsSuggestion, Mode, ProbeScope};
 use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX};
@@ -275,6 +277,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
     ) -> bool {
         let expr = expr.peel_blocks();
+        let methods = self.get_conversion_methods(expr.span, expected, found, expr.hir_id);
+
         if let Some((suggestion, msg, applicability, verbose, annotation)) =
             self.suggest_deref_or_ref(expr, found, expected)
         {
@@ -325,9 +329,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 }
             }
             return true;
-        } else if self.suggest_else_fn_with_closure(err, expr, found, expected) {
+        }
+
+        if self.suggest_else_fn_with_closure(err, expr, found, expected) {
             return true;
-        } else if self.suggest_fn_call(err, expr, found, |output| self.can_coerce(output, expected))
+        }
+
+        if self.suggest_fn_call(err, expr, found, |output| self.can_coerce(output, expected))
             && let ty::FnDef(def_id, ..) = *found.kind()
             && let Some(sp) = self.tcx.hir().span_if_local(def_id)
         {
@@ -343,97 +351,142 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 err.span_label(sp, format!("{descr} `{name}` defined here"));
             }
             return true;
-        } else if self.suggest_cast(err, expr, found, expected, expected_ty_expr) {
+        }
+
+        if self.suggest_cast(err, expr, found, expected, expected_ty_expr) {
             return true;
-        } else {
-            let methods = self.get_conversion_methods(expr.span, expected, found, expr.hir_id);
-            if !methods.is_empty() {
-                let mut suggestions = methods.iter()
-                    .filter_map(|conversion_method| {
-                        let receiver_method_ident = expr.method_ident();
-                        if let Some(method_ident) = receiver_method_ident
-                            && method_ident.name == conversion_method.name
-                        {
-                            return None // do not suggest code that is already there (#53348)
-                        }
+        }
 
-                        let method_call_list = [sym::to_vec, sym::to_string];
-                        let mut sugg = if let ExprKind::MethodCall(receiver_method, ..) = expr.kind
-                            && receiver_method.ident.name == sym::clone
-                            && method_call_list.contains(&conversion_method.name)
-                            // If receiver is `.clone()` and found type has one of those methods,
-                            // we guess that the user wants to convert from a slice type (`&[]` or `&str`)
-                            // to an owned type (`Vec` or `String`). These conversions clone internally,
-                            // so we remove the user's `clone` call.
-                        {
-                            vec![(
-                                receiver_method.ident.span,
-                                conversion_method.name.to_string()
-                            )]
-                        } else if expr.precedence().order()
-                            < ExprPrecedence::MethodCall.order()
-                        {
-                            vec![
-                                (expr.span.shrink_to_lo(), "(".to_string()),
-                                (expr.span.shrink_to_hi(), format!(").{}()", conversion_method.name)),
-                            ]
-                        } else {
-                            vec![(expr.span.shrink_to_hi(), format!(".{}()", conversion_method.name))]
-                        };
-                        let struct_pat_shorthand_field = self.maybe_get_struct_pattern_shorthand_field(expr);
-                        if let Some(name) = struct_pat_shorthand_field {
-                            sugg.insert(
-                                0,
-                                (expr.span.shrink_to_lo(), format!("{}: ", name)),
-                            );
-                        }
-                        Some(sugg)
-                    })
-                    .peekable();
-                if suggestions.peek().is_some() {
-                    err.multipart_suggestions(
-                        "try using a conversion method",
-                        suggestions,
-                        Applicability::MaybeIncorrect,
-                    );
-                    return true;
-                }
-            } else if let ty::Adt(found_adt, found_substs) = found.kind()
-                && self.tcx.is_diagnostic_item(sym::Option, found_adt.did())
-                && let ty::Adt(expected_adt, expected_substs) = expected.kind()
-                && self.tcx.is_diagnostic_item(sym::Option, expected_adt.did())
-                && let ty::Ref(_, inner_ty, _) = expected_substs.type_at(0).kind()
-                && inner_ty.is_str()
-            {
-                let ty = found_substs.type_at(0);
-                let mut peeled = ty;
-                let mut ref_cnt = 0;
-                while let ty::Ref(_, inner, _) = peeled.kind() {
-                    peeled = *inner;
-                    ref_cnt += 1;
-                }
-                if let ty::Adt(adt, _) = peeled.kind()
-                    && Some(adt.did()) == self.tcx.lang_items().string()
-                {
-                    let sugg = if ref_cnt == 0 {
-                        ".as_deref()"
+        if !methods.is_empty() {
+            let mut suggestions = methods
+                .iter()
+                .filter_map(|conversion_method| {
+                    let receiver_method_ident = expr.method_ident();
+                    if let Some(method_ident) = receiver_method_ident
+                        && method_ident.name == conversion_method.name
+                    {
+                        return None // do not suggest code that is already there (#53348)
+                    }
+
+                    let method_call_list = [sym::to_vec, sym::to_string];
+                    let mut sugg = if let ExprKind::MethodCall(receiver_method, ..) = expr.kind
+                        && receiver_method.ident.name == sym::clone
+                        && method_call_list.contains(&conversion_method.name)
+                        // If receiver is `.clone()` and found type has one of those methods,
+                        // we guess that the user wants to convert from a slice type (`&[]` or `&str`)
+                        // to an owned type (`Vec` or `String`). These conversions clone internally,
+                        // so we remove the user's `clone` call.
+                    {
+                        vec![(
+                            receiver_method.ident.span,
+                            conversion_method.name.to_string()
+                        )]
+                    } else if expr.precedence().order()
+                        < ExprPrecedence::MethodCall.order()
+                    {
+                        vec![
+                            (expr.span.shrink_to_lo(), "(".to_string()),
+                            (expr.span.shrink_to_hi(), format!(").{}()", conversion_method.name)),
+                        ]
                     } else {
-                        ".map(|x| x.as_str())"
+                        vec![(expr.span.shrink_to_hi(), format!(".{}()", conversion_method.name))]
                     };
-                    err.span_suggestion_verbose(
-                        expr.span.shrink_to_hi(),
-                        fluent::hir_typeck_convert_to_str,
-                        sugg,
-                        Applicability::MachineApplicable,
-                    );
-                    return true;
-                }
+                    let struct_pat_shorthand_field =
+                        self.maybe_get_struct_pattern_shorthand_field(expr);
+                    if let Some(name) = struct_pat_shorthand_field {
+                        sugg.insert(0, (expr.span.shrink_to_lo(), format!("{}: ", name)));
+                    }
+                    Some(sugg)
+                })
+                .peekable();
+            if suggestions.peek().is_some() {
+                err.multipart_suggestions(
+                    "try using a conversion method",
+                    suggestions,
+                    Applicability::MaybeIncorrect,
+                );
+                return true;
+            }
+        }
+
+        if let Some((found_ty_inner, expected_ty_inner, error_tys)) =
+            self.deconstruct_option_or_result(found, expected)
+            && let ty::Ref(_, peeled, hir::Mutability::Not) = *expected_ty_inner.kind()
+        {
+            // Check that given `Result<_, E>`, our expected ty is `Result<_, &E>`
+            let error_tys_equate_as_ref = error_tys.map_or(true, |(found, expected)| {
+                self.can_eq(self.param_env, self.tcx.mk_imm_ref(self.tcx.lifetimes.re_erased, found), expected)
+            });
+            // FIXME: This could/should be extended to suggest `as_mut` and `as_deref_mut`,
+            // but those checks need to be a bit more delicate and the benefit is diminishing.
+            if self.can_eq(self.param_env, found_ty_inner, peeled) && error_tys_equate_as_ref {
+                err.subdiagnostic(SuggestConvertViaMethod {
+                    span: expr.span.shrink_to_hi(),
+                    sugg: ".as_ref()",
+                    expected,
+                    found,
+                });
+                return true;
+            } else if let Some((deref_ty, _)) =
+                self.autoderef(expr.span, found_ty_inner).silence_errors().nth(1)
+                && self.can_eq(self.param_env, deref_ty, peeled)
+                && error_tys_equate_as_ref
+            {
+                err.subdiagnostic(SuggestConvertViaMethod {
+                    span: expr.span.shrink_to_hi(),
+                    sugg: ".as_deref()",
+                    expected,
+                    found,
+                });
+                return true;
+            } else if let ty::Adt(adt, _) = found_ty_inner.peel_refs().kind()
+                && Some(adt.did()) == self.tcx.lang_items().string()
+                && peeled.is_str()
+                && error_tys.map_or(true, |(found, expected)| {
+                    self.can_eq(self.param_env, found, expected)
+                })
+            {
+                err.span_suggestion_verbose(
+                    expr.span.shrink_to_hi(),
+                    fluent::hir_typeck_convert_to_str,
+                    ".map(|x| x.as_str())",
+                    Applicability::MachineApplicable,
+                );
+                return true;
             }
         }
 
         false
     }
 
+    fn deconstruct_option_or_result(
+        &self,
+        found_ty: Ty<'tcx>,
+        expected_ty: Ty<'tcx>,
+    ) -> Option<(Ty<'tcx>, Ty<'tcx>, Option<(Ty<'tcx>, Ty<'tcx>)>)> {
+        let ty::Adt(found_adt, found_substs) = found_ty.peel_refs().kind() else {
+            return None;
+        };
+        let ty::Adt(expected_adt, expected_substs) = expected_ty.kind() else {
+            return None;
+        };
+        if self.tcx.is_diagnostic_item(sym::Option, found_adt.did())
+            && self.tcx.is_diagnostic_item(sym::Option, expected_adt.did())
+        {
+            Some((found_substs.type_at(0), expected_substs.type_at(0), None))
+        } else if self.tcx.is_diagnostic_item(sym::Result, found_adt.did())
+            && self.tcx.is_diagnostic_item(sym::Result, expected_adt.did())
+        {
+            Some((
+                found_substs.type_at(0),
+                expected_substs.type_at(0),
+                Some((found_substs.type_at(1), expected_substs.type_at(1))),
+            ))
+        } else {
+            None
+        }
+    }
+
     /// When encountering the expected boxed value allocated in the stack, suggest allocating it
     /// in the heap by calling `Box::new()`.
     pub(in super::super) fn suggest_boxing_when_appropriate(

compiler/rustc_infer/messages.ftl

@@ -278,9 +278,6 @@ infer_ril_introduced_by = requirement introduced by this return type
 infer_ril_introduced_here = `'static` requirement introduced here
 infer_ril_static_introduced_by = "`'static` lifetime requirement introduced by the return type
 
-infer_sarwa_option = you can convert from `&Option<T>` to `Option<&T>` using `.as_ref()`
-infer_sarwa_result = you can convert from `&Result<T, E>` to `Result<&T, &E>` using `.as_ref()`
-
 infer_sbfrit_box_return_expr = if you change the return type to expect trait objects, box the returned expressions
 
 infer_sbfrit_change_return_type = you could change the return type to be a boxed trait object

compiler/rustc_infer/src/errors/mod.rs

@@ -1246,30 +1246,6 @@ pub struct FnConsiderCasting {
     pub casting: String,
 }
 
-#[derive(Subdiagnostic)]
-pub enum SuggestAsRefWhereAppropriate<'a> {
-    #[suggestion(
-        infer_sarwa_option,
-        code = "{snippet}.as_ref()",
-        applicability = "machine-applicable"
-    )]
-    Option {
-        #[primary_span]
-        span: Span,
-        snippet: &'a str,
-    },
-    #[suggestion(
-        infer_sarwa_result,
-        code = "{snippet}.as_ref()",
-        applicability = "machine-applicable"
-    )]
-    Result {
-        #[primary_span]
-        span: Span,
-        snippet: &'a str,
-    },
-}
-
 #[derive(Subdiagnostic)]
 pub enum SuggestAccessingField<'a> {
     #[suggestion(

compiler/rustc_infer/src/infer/error_reporting/mod.rs

@@ -1897,7 +1897,6 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
 
             if should_suggest_fixes {
                 self.suggest_tuple_pattern(cause, &exp_found, diag);
-                self.suggest_as_ref_where_appropriate(span, &exp_found, diag);
                 self.suggest_accessing_field_where_appropriate(cause, &exp_found, diag);
                 self.suggest_await_on_expect_found(cause, span, &exp_found, diag);
                 self.suggest_function_pointers(cause, span, &exp_found, diag);

compiler/rustc_infer/src/infer/error_reporting/suggest.rs

@@ -13,9 +13,9 @@ use rustc_span::{sym, BytePos, Span};
 
 use crate::errors::{
     ConsiderAddingAwait, FnConsiderCasting, FnItemsAreDistinct, FnUniqTypes,
-    FunctionPointerSuggestion, SuggestAccessingField, SuggestAsRefWhereAppropriate,
-    SuggestBoxingForReturnImplTrait, SuggestRemoveSemiOrReturnBinding, SuggestTuplePatternMany,
-    SuggestTuplePatternOne, TypeErrorAdditionalDiags,
+    FunctionPointerSuggestion, SuggestAccessingField, SuggestBoxingForReturnImplTrait,
+    SuggestRemoveSemiOrReturnBinding, SuggestTuplePatternMany, SuggestTuplePatternOne,
+    TypeErrorAdditionalDiags,
 };
 
 use super::TypeErrCtxt;
@@ -289,27 +289,6 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
         }
     }
 
-    /// When encountering a case where `.as_ref()` on a `Result` or `Option` would be appropriate,
-    /// suggests it.
-    pub(super) fn suggest_as_ref_where_appropriate(
-        &self,
-        span: Span,
-        exp_found: &ty::error::ExpectedFound<Ty<'tcx>>,
-        diag: &mut Diagnostic,
-    ) {
-        if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span)
-            && let Some(msg) = self.should_suggest_as_ref_kind(exp_found.expected, exp_found.found)
-        {
-            // HACK: fix issue# 100605, suggesting convert from &Option<T> to Option<&T>, remove the extra `&`
-            let snippet = snippet.trim_start_matches('&');
-            let subdiag = match msg {
-                SuggestAsRefKind::Option => SuggestAsRefWhereAppropriate::Option { span, snippet },
-                SuggestAsRefKind::Result => SuggestAsRefWhereAppropriate::Result { span, snippet },
-            };
-            diag.subdiagnostic(subdiag);
-        }
-    }
-
     pub(super) fn suggest_function_pointers(
         &self,
         cause: &ObligationCause<'tcx>,