Skip to content

Commit

Permalink
Rollup merge of #111659 - y21:suggest-as-deref, r=cjgillot
Browse files Browse the repository at this point in the history
suggest `Option::as_deref(_mut)` on type mismatch in option combinator if it passes typeck

Fixes #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`
  • Loading branch information
matthiaskrgr authored Jun 3, 2023
2 parents 2f5e6bb + 268b08b commit 91f222f
Show file tree
Hide file tree
Showing 6 changed files with 354 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
);
}
}
}
Expand Down
40 changes: 40 additions & 0 deletions tests/ui/mismatched_types/suggest-option-asderef-unfixable.rs
Original file line number Diff line number Diff line change
@@ -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
}
96 changes: 96 additions & 0 deletions tests/ui/mismatched_types/suggest-option-asderef-unfixable.stderr
Original file line number Diff line number Diff line change
@@ -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`.
30 changes: 30 additions & 0 deletions tests/ui/mismatched_types/suggest-option-asderef.fixed
Original file line number Diff line number Diff line change
@@ -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);
}
30 changes: 30 additions & 0 deletions tests/ui/mismatched_types/suggest-option-asderef.rs
Original file line number Diff line number Diff line change
@@ -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);
}
63 changes: 63 additions & 0 deletions tests/ui/mismatched_types/suggest-option-asderef.stderr
Original file line number Diff line number Diff line change
@@ -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`.

0 comments on commit 91f222f

Please sign in to comment.