Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

suggest Option::as_deref(_mut) on type mismatch in option combinator if it passes typeck #111659

Merged
merged 2 commits into from
Jun 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -357,6 +357,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 @@ -3660,15 +3669,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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this case suggest as_ref?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially had it suggest as_ref() where applicable too, but there's already a help message for calling and_then with an fn(&T) on some Option<T>, though it suggests removing the borrow on the function parameter and I'm not sure how it would work out when there are two suggestions made by the compiler (is that fine?)

help: do not borrow the argument
     |
14   - fn generic<T>(_: &T) {}
14   + fn generic<T>(_: T) {}
     |
help: call `Option::as_ref()` first
     |
18   |     let x = produces_string().as_ref().and_then(generic);
     |                              +++++++++

//~^ 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`.