Rollup merge of rust-lang#111659 - y21:suggest-as-deref, r=cjgillot

matthiaskrgr · web-flow · commit 91f222f93179 · 2023-06-03T20:38:10.000+02:00
suggest `Option::as_deref(_mut)` on type mismatch in option combinator if it passes typeck Fixes rust-lang#106342. This adds a suggestion to call `.as_deref()` (or `.as_deref_mut()` resp.) if typeck fails due to a type mismatch in the function passed to an `Option` combinator such as `.map()` or `.and_then()`. For example: ```rs fn foo(_: &str) {} Some(String::new()).map(foo); ``` The `.map()` method requires its argument to satisfy `F: FnOnce(String)`, but it received `fn(&str)`, which won't pass. However, placing a `.as_deref()` before the `.map()` call fixes this since `&str == &<String as Deref>::Target`
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -13,7 +13,7 @@ use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_errors::{
     error_code, pluralize, struct_span_err, Applicability, Diagnostic, DiagnosticBuilder,
-    ErrorGuaranteed, MultiSpan, Style,
+    ErrorGuaranteed, MultiSpan, Style, SuggestionStyle,
 };
 use rustc_hir as hir;
 use rustc_hir::def::DefKind;
@@ -362,6 +362,15 @@ pub trait TypeErrCtxtExt<'tcx> {
         err: &mut Diagnostic,
         trait_pred: ty::PolyTraitPredicate<'tcx>,
     );
+
+    fn suggest_option_method_if_applicable(
+        &self,
+        failed_pred: ty::Predicate<'tcx>,
+        param_env: ty::ParamEnv<'tcx>,
+        err: &mut Diagnostic,
+        expr: &hir::Expr<'_>,
+    );
+
     fn note_function_argument_obligation(
         &self,
         body_id: LocalDefId,
@@ -3521,15 +3530,92 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                 err.replace_span_with(path.ident.span, true);
             }
         }
-        if let Some(Node::Expr(hir::Expr {
-            kind:
-                hir::ExprKind::Call(hir::Expr { span, .. }, _)
-                | hir::ExprKind::MethodCall(hir::PathSegment { ident: Ident { span, .. }, .. }, ..),
-            ..
-        })) = hir.find(call_hir_id)
+
+        if let Some(Node::Expr(expr)) = hir.find(call_hir_id) {
+            if let hir::ExprKind::Call(hir::Expr { span, .. }, _)
+            | hir::ExprKind::MethodCall(
+                hir::PathSegment { ident: Ident { span, .. }, .. },
+                ..,
+            ) = expr.kind
+            {
+                if Some(*span) != err.span.primary_span() {
+                    err.span_label(*span, "required by a bound introduced by this call");
+                }
+            }
+
+            if let hir::ExprKind::MethodCall(_, expr, ..) = expr.kind {
+                self.suggest_option_method_if_applicable(failed_pred, param_env, err, expr);
+            }
+        }
+    }
+
+    fn suggest_option_method_if_applicable(
+        &self,
+        failed_pred: ty::Predicate<'tcx>,
+        param_env: ty::ParamEnv<'tcx>,
+        err: &mut Diagnostic,
+        expr: &hir::Expr<'_>,
+    ) {
+        let tcx = self.tcx;
+        let infcx = self.infcx;
+        let Some(typeck_results) = self.typeck_results.as_ref() else { return };
+
+        // Make sure we're dealing with the `Option` type.
+        let Some(option_ty_adt) = typeck_results.expr_ty_adjusted(expr).ty_adt_def() else { return };
+        if !tcx.is_diagnostic_item(sym::Option, option_ty_adt.did()) {
+            return;
+        }
+
+        // Given the predicate `fn(&T): FnOnce<(U,)>`, extract `fn(&T)` and `(U,)`,
+        // then suggest `Option::as_deref(_mut)` if `U` can deref to `T`
+        if let ty::PredicateKind::Clause(ty::Clause::Trait(ty::TraitPredicate { trait_ref, .. }))
+            = failed_pred.kind().skip_binder()
+            && tcx.is_fn_trait(trait_ref.def_id)
+            && let [self_ty, found_ty] = trait_ref.substs.as_slice()
+            && let Some(fn_ty) = self_ty.as_type().filter(|ty| ty.is_fn())
+            && let fn_sig @ ty::FnSig {
+                abi: abi::Abi::Rust,
+                c_variadic: false,
+                unsafety: hir::Unsafety::Normal,
+                ..
+            } = fn_ty.fn_sig(tcx).skip_binder()
+
+            // Extract first param of fn sig with peeled refs, e.g. `fn(&T)` -> `T`
+            && let Some(&ty::Ref(_, target_ty, needs_mut)) = fn_sig.inputs().first().map(|t| t.kind())
+            && !target_ty.has_escaping_bound_vars()
+
+            // Extract first tuple element out of fn trait, e.g. `FnOnce<(U,)>` -> `U`
+            && let Some(ty::Tuple(tys)) = found_ty.as_type().map(Ty::kind)
+            && let &[found_ty] = tys.as_slice()
+            && !found_ty.has_escaping_bound_vars()
+
+            // Extract `<U as Deref>::Target` assoc type and check that it is `T`
+            && let Some(deref_target_did) = tcx.lang_items().deref_target()
+            && let projection = tcx.mk_projection(deref_target_did, tcx.mk_substs(&[ty::GenericArg::from(found_ty)]))
+            && let Ok(deref_target) = tcx.try_normalize_erasing_regions(param_env, projection)
+            && deref_target == target_ty
         {
-            if Some(*span) != err.span.primary_span() {
-                err.span_label(*span, "required by a bound introduced by this call");
+            let help = if let hir::Mutability::Mut = needs_mut
+                && let Some(deref_mut_did) = tcx.lang_items().deref_mut_trait()
+                && infcx
+                    .type_implements_trait(deref_mut_did, iter::once(found_ty), param_env)
+                    .must_apply_modulo_regions()
+            {
+                Some(("call `Option::as_deref_mut()` first", ".as_deref_mut()"))
+            } else if let hir::Mutability::Not = needs_mut {
+                Some(("call `Option::as_deref()` first", ".as_deref()"))
+            } else {
+                None
+            };
+
+            if let Some((msg, sugg)) = help {
+                err.span_suggestion_with_style(
+                    expr.span.shrink_to_hi(),
+                    msg,
+                    sugg,
+                    Applicability::MaybeIncorrect,
+                    SuggestionStyle::ShowAlways
+                );
             }
         }
     }
diff --git a/tests/ui/mismatched_types/suggest-option-asderef-unfixable.rs b/tests/ui/mismatched_types/suggest-option-asderef-unfixable.rs
@@ -0,0 +1,40 @@
+fn produces_string() -> Option<String> {
+    Some("my cool string".to_owned())
+}
+
+fn takes_str_but_too_many_refs(_: &&str) -> Option<()> {
+    Some(())
+}
+
+fn no_args() -> Option<()> {
+    Some(())
+}
+
+fn generic_ref<T>(_: &T) -> Option<()> {
+    Some(())
+}
+
+extern "C" fn takes_str_but_wrong_abi(_: &str) -> Option<()> {
+    Some(())
+}
+
+unsafe fn takes_str_but_unsafe(_: &str) -> Option<()> {
+    Some(())
+}
+
+struct TypeWithoutDeref;
+
+fn main() {
+    let _ = produces_string().and_then(takes_str_but_too_many_refs);
+    //~^ ERROR type mismatch in function arguments
+    let _ = produces_string().and_then(takes_str_but_wrong_abi);
+    //~^ ERROR expected a `FnOnce<(String,)>` closure, found `for<'a> extern "C" fn(&'a str) -> Option<()> {takes_str_but_wrong_abi}`
+    let _ = produces_string().and_then(takes_str_but_unsafe);
+    //~^ ERROR expected a `FnOnce<(String,)>` closure, found `for<'a> unsafe fn(&'a str) -> Option<()> {takes_str_but_unsafe}`
+    let _ = produces_string().and_then(no_args);
+    //~^ ERROR function is expected to take 1 argument, but it takes 0 arguments
+    let _ = produces_string().and_then(generic_ref);
+    //~^ ERROR type mismatch in function arguments
+    let _ = Some(TypeWithoutDeref).and_then(takes_str_but_too_many_refs);
+    //~^ ERROR type mismatch in function arguments
+}
diff --git a/tests/ui/mismatched_types/suggest-option-asderef-unfixable.stderr b/tests/ui/mismatched_types/suggest-option-asderef-unfixable.stderr
@@ -0,0 +1,96 @@
+error[E0631]: type mismatch in function arguments
+  --> $DIR/suggest-option-asderef-unfixable.rs:28:40
+   |
+LL | fn takes_str_but_too_many_refs(_: &&str) -> Option<()> {
+   | ------------------------------------------------------ found signature defined here
+...
+LL |     let _ = produces_string().and_then(takes_str_but_too_many_refs);
+   |                               -------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected due to this
+   |                               |
+   |                               required by a bound introduced by this call
+   |
+   = note: expected function signature `fn(String) -> _`
+              found function signature `for<'a, 'b> fn(&'a &'b str) -> _`
+note: required by a bound in `Option::<T>::and_then`
+  --> $SRC_DIR/core/src/option.rs:LL:COL
+
+error[E0277]: expected a `FnOnce<(String,)>` closure, found `for<'a> extern "C" fn(&'a str) -> Option<()> {takes_str_but_wrong_abi}`
+  --> $DIR/suggest-option-asderef-unfixable.rs:30:40
+   |
+LL |     let _ = produces_string().and_then(takes_str_but_wrong_abi);
+   |                               -------- ^^^^^^^^^^^^^^^^^^^^^^^ expected an `FnOnce<(String,)>` closure, found `for<'a> extern "C" fn(&'a str) -> Option<()> {takes_str_but_wrong_abi}`
+   |                               |
+   |                               required by a bound introduced by this call
+   |
+   = help: the trait `FnOnce<(String,)>` is not implemented for fn item `for<'a> extern "C" fn(&'a str) -> Option<()> {takes_str_but_wrong_abi}`
+note: required by a bound in `Option::<T>::and_then`
+  --> $SRC_DIR/core/src/option.rs:LL:COL
+
+error[E0277]: expected a `FnOnce<(String,)>` closure, found `for<'a> unsafe fn(&'a str) -> Option<()> {takes_str_but_unsafe}`
+  --> $DIR/suggest-option-asderef-unfixable.rs:32:40
+   |
+LL |     let _ = produces_string().and_then(takes_str_but_unsafe);
+   |                               -------- ^^^^^^^^^^^^^^^^^^^^ call the function in a closure: `|| unsafe { /* code */ }`
+   |                               |
+   |                               required by a bound introduced by this call
+   |
+   = help: the trait `FnOnce<(String,)>` is not implemented for fn item `for<'a> unsafe fn(&'a str) -> Option<()> {takes_str_but_unsafe}`
+   = note: unsafe function cannot be called generically without an unsafe block
+note: required by a bound in `Option::<T>::and_then`
+  --> $SRC_DIR/core/src/option.rs:LL:COL
+
+error[E0593]: function is expected to take 1 argument, but it takes 0 arguments
+  --> $DIR/suggest-option-asderef-unfixable.rs:34:40
+   |
+LL | fn no_args() -> Option<()> {
+   | -------------------------- takes 0 arguments
+...
+LL |     let _ = produces_string().and_then(no_args);
+   |                               -------- ^^^^^^^ expected function that takes 1 argument
+   |                               |
+   |                               required by a bound introduced by this call
+   |
+note: required by a bound in `Option::<T>::and_then`
+  --> $SRC_DIR/core/src/option.rs:LL:COL
+
+error[E0631]: type mismatch in function arguments
+  --> $DIR/suggest-option-asderef-unfixable.rs:36:40
+   |
+LL | fn generic_ref<T>(_: &T) -> Option<()> {
+   | -------------------------------------- found signature defined here
+...
+LL |     let _ = produces_string().and_then(generic_ref);
+   |                               -------- ^^^^^^^^^^^ expected due to this
+   |                               |
+   |                               required by a bound introduced by this call
+   |
+   = note: expected function signature `fn(String) -> _`
+              found function signature `for<'a> fn(&'a _) -> _`
+note: required by a bound in `Option::<T>::and_then`
+  --> $SRC_DIR/core/src/option.rs:LL:COL
+help: do not borrow the argument
+   |
+LL - fn generic_ref<T>(_: &T) -> Option<()> {
+LL + fn generic_ref<T>(_: T) -> Option<()> {
+   |
+
+error[E0631]: type mismatch in function arguments
+  --> $DIR/suggest-option-asderef-unfixable.rs:38:45
+   |
+LL | fn takes_str_but_too_many_refs(_: &&str) -> Option<()> {
+   | ------------------------------------------------------ found signature defined here
+...
+LL |     let _ = Some(TypeWithoutDeref).and_then(takes_str_but_too_many_refs);
+   |                                    -------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected due to this
+   |                                    |
+   |                                    required by a bound introduced by this call
+   |
+   = note: expected function signature `fn(TypeWithoutDeref) -> _`
+              found function signature `for<'a, 'b> fn(&'a &'b str) -> _`
+note: required by a bound in `Option::<T>::and_then`
+  --> $SRC_DIR/core/src/option.rs:LL:COL
+
+error: aborting due to 6 previous errors
+
+Some errors have detailed explanations: E0277, E0593, E0631.
+For more information about an error, try `rustc --explain E0277`.
diff --git a/tests/ui/mismatched_types/suggest-option-asderef.fixed b/tests/ui/mismatched_types/suggest-option-asderef.fixed
@@ -0,0 +1,30 @@
+// run-rustfix
+
+fn produces_string() -> Option<String> {
+    Some("my cool string".to_owned())
+}
+
+fn takes_str(_: &str) -> Option<()> {
+    Some(())
+}
+
+fn takes_str_mut(_: &mut str) -> Option<()> {
+    Some(())
+}
+
+fn generic<T>(_: T) -> Option<()> {
+    Some(())
+}
+
+fn main() {
+    let _: Option<()> = produces_string().as_deref().and_then(takes_str);
+    //~^ ERROR type mismatch in function arguments
+    //~| HELP call `Option::as_deref()` first
+    let _: Option<Option<()>> = produces_string().as_deref().map(takes_str);
+    //~^ ERROR type mismatch in function arguments
+    //~| HELP call `Option::as_deref()` first
+    let _: Option<Option<()>> = produces_string().as_deref_mut().map(takes_str_mut);
+    //~^ ERROR type mismatch in function arguments
+    //~| HELP call `Option::as_deref_mut()` first
+    let _ = produces_string().and_then(generic);
+}
diff --git a/tests/ui/mismatched_types/suggest-option-asderef.rs b/tests/ui/mismatched_types/suggest-option-asderef.rs
@@ -0,0 +1,30 @@
+// run-rustfix
+
+fn produces_string() -> Option<String> {
+    Some("my cool string".to_owned())
+}
+
+fn takes_str(_: &str) -> Option<()> {
+    Some(())
+}
+
+fn takes_str_mut(_: &mut str) -> Option<()> {
+    Some(())
+}
+
+fn generic<T>(_: T) -> Option<()> {
+    Some(())
+}
+
+fn main() {
+    let _: Option<()> = produces_string().and_then(takes_str);
+    //~^ ERROR type mismatch in function arguments
+    //~| HELP call `Option::as_deref()` first
+    let _: Option<Option<()>> = produces_string().map(takes_str);
+    //~^ ERROR type mismatch in function arguments
+    //~| HELP call `Option::as_deref()` first
+    let _: Option<Option<()>> = produces_string().map(takes_str_mut);
+    //~^ ERROR type mismatch in function arguments
+    //~| HELP call `Option::as_deref_mut()` first
+    let _ = produces_string().and_then(generic);
+}
diff --git a/tests/ui/mismatched_types/suggest-option-asderef.stderr b/tests/ui/mismatched_types/suggest-option-asderef.stderr
@@ -0,0 +1,63 @@
+error[E0631]: type mismatch in function arguments
+  --> $DIR/suggest-option-asderef.rs:20:52
+   |
+LL | fn takes_str(_: &str) -> Option<()> {
+   | ----------------------------------- found signature defined here
+...
+LL |     let _: Option<()> = produces_string().and_then(takes_str);
+   |                                           -------- ^^^^^^^^^ expected due to this
+   |                                           |
+   |                                           required by a bound introduced by this call
+   |
+   = note: expected function signature `fn(String) -> _`
+              found function signature `for<'a> fn(&'a str) -> _`
+note: required by a bound in `Option::<T>::and_then`
+  --> $SRC_DIR/core/src/option.rs:LL:COL
+help: call `Option::as_deref()` first
+   |
+LL |     let _: Option<()> = produces_string().as_deref().and_then(takes_str);
+   |                                          +++++++++++
+
+error[E0631]: type mismatch in function arguments
+  --> $DIR/suggest-option-asderef.rs:23:55
+   |
+LL | fn takes_str(_: &str) -> Option<()> {
+   | ----------------------------------- found signature defined here
+...
+LL |     let _: Option<Option<()>> = produces_string().map(takes_str);
+   |                                                   --- ^^^^^^^^^ expected due to this
+   |                                                   |
+   |                                                   required by a bound introduced by this call
+   |
+   = note: expected function signature `fn(String) -> _`
+              found function signature `for<'a> fn(&'a str) -> _`
+note: required by a bound in `Option::<T>::map`
+  --> $SRC_DIR/core/src/option.rs:LL:COL
+help: call `Option::as_deref()` first
+   |
+LL |     let _: Option<Option<()>> = produces_string().as_deref().map(takes_str);
+   |                                                  +++++++++++
+
+error[E0631]: type mismatch in function arguments
+  --> $DIR/suggest-option-asderef.rs:26:55
+   |
+LL | fn takes_str_mut(_: &mut str) -> Option<()> {
+   | ------------------------------------------- found signature defined here
+...
+LL |     let _: Option<Option<()>> = produces_string().map(takes_str_mut);
+   |                                                   --- ^^^^^^^^^^^^^ expected due to this
+   |                                                   |
+   |                                                   required by a bound introduced by this call
+   |
+   = note: expected function signature `fn(String) -> _`
+              found function signature `for<'a> fn(&'a mut str) -> _`
+note: required by a bound in `Option::<T>::map`
+  --> $SRC_DIR/core/src/option.rs:LL:COL
+help: call `Option::as_deref_mut()` first
+   |
+LL |     let _: Option<Option<()>> = produces_string().as_deref_mut().map(takes_str_mut);
+   |                                                  +++++++++++++++
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0631`.
