From 6d2ba95d0099f1a656da44c7f868be0074e5b5e1 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Sun, 14 May 2023 19:10:52 +0200 Subject: [PATCH 1/2] suggest `Option::as_deref(_mut)` --- .../src/traits/error_reporting/suggestions.rs | 105 ++++++++++++++++-- .../suggest-option-asderef-unfixable.rs | 40 +++++++ .../suggest-option-asderef-unfixable.stderr | 96 ++++++++++++++++ .../suggest-option-asderef.fixed | 30 +++++ .../suggest-option-asderef.rs | 30 +++++ .../suggest-option-asderef.stderr | 63 +++++++++++ 6 files changed, 355 insertions(+), 9 deletions(-) create mode 100644 tests/ui/mismatched_types/suggest-option-asderef-unfixable.rs create mode 100644 tests/ui/mismatched_types/suggest-option-asderef-unfixable.stderr create mode 100644 tests/ui/mismatched_types/suggest-option-asderef.fixed create mode 100644 tests/ui/mismatched_types/suggest-option-asderef.rs create mode 100644 tests/ui/mismatched_types/suggest-option-asderef.stderr diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 49b309abcda3a..9f2740a3898db 100644 --- 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; @@ -27,6 +27,7 @@ use rustc_infer::infer::error_reporting::TypeErrCtxt; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_infer::infer::{DefineOpaqueTypes, InferOk, LateBoundRegionConversionTime}; use rustc_middle::hir::map; +use rustc_middle::query::Key; use rustc_middle::ty::error::TypeError::{self, Sorts}; use rustc_middle::ty::{ self, suggest_arbitrary_trait_bound, suggest_constraining_type_param, AdtKind, @@ -357,6 +358,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, @@ -3660,15 +3670,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(ty_adt_did) = typeck_results.expr_ty_adjusted(expr).ty_adt_id() else { return }; + if !tcx.is_diagnostic_item(sym::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 `::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 new file mode 100644 index 0000000000000..cc9ba5514fef1 --- /dev/null +++ b/tests/ui/mismatched_types/suggest-option-asderef-unfixable.rs @@ -0,0 +1,40 @@ +fn produces_string() -> Option { + 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) -> 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 new file mode 100644 index 0000000000000..079909eb48d1d --- /dev/null +++ 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::::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::::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::::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::::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) -> 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::::and_then` + --> $SRC_DIR/core/src/option.rs:LL:COL +help: do not borrow the argument + | +LL - fn generic_ref(_: &T) -> Option<()> { +LL + fn generic_ref(_: 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::::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 new file mode 100644 index 0000000000000..08805999341ff --- /dev/null +++ b/tests/ui/mismatched_types/suggest-option-asderef.fixed @@ -0,0 +1,30 @@ +// run-rustfix + +fn produces_string() -> Option { + Some("my cool string".to_owned()) +} + +fn takes_str(_: &str) -> Option<()> { + Some(()) +} + +fn takes_str_mut(_: &mut str) -> Option<()> { + Some(()) +} + +fn generic(_: 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> = produces_string().as_deref().map(takes_str); + //~^ ERROR type mismatch in function arguments + //~| HELP call `Option::as_deref()` first + let _: 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 new file mode 100644 index 0000000000000..3cfb2ffa828c6 --- /dev/null +++ b/tests/ui/mismatched_types/suggest-option-asderef.rs @@ -0,0 +1,30 @@ +// run-rustfix + +fn produces_string() -> Option { + Some("my cool string".to_owned()) +} + +fn takes_str(_: &str) -> Option<()> { + Some(()) +} + +fn takes_str_mut(_: &mut str) -> Option<()> { + Some(()) +} + +fn generic(_: 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> = produces_string().map(takes_str); + //~^ ERROR type mismatch in function arguments + //~| HELP call `Option::as_deref()` first + let _: 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 new file mode 100644 index 0000000000000..46da19d2bf4f2 --- /dev/null +++ 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::::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> = 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::::map` + --> $SRC_DIR/core/src/option.rs:LL:COL +help: call `Option::as_deref()` first + | +LL | let _: 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> = 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::::map` + --> $SRC_DIR/core/src/option.rs:LL:COL +help: call `Option::as_deref_mut()` first + | +LL | let _: 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`. From 268b08b01bfd95d315c03a9e3a69f254176b2a35 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Sat, 3 Jun 2023 17:17:56 +0200 Subject: [PATCH 2/2] do not use ty_adt_id from internal trait --- .../src/traits/error_reporting/suggestions.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 9f2740a3898db..1044613c585c3 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -27,7 +27,6 @@ use rustc_infer::infer::error_reporting::TypeErrCtxt; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_infer::infer::{DefineOpaqueTypes, InferOk, LateBoundRegionConversionTime}; use rustc_middle::hir::map; -use rustc_middle::query::Key; use rustc_middle::ty::error::TypeError::{self, Sorts}; use rustc_middle::ty::{ self, suggest_arbitrary_trait_bound, suggest_constraining_type_param, AdtKind, @@ -3701,8 +3700,8 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { let Some(typeck_results) = self.typeck_results.as_ref() else { return }; // Make sure we're dealing with the `Option` type. - let Some(ty_adt_did) = typeck_results.expr_ty_adjusted(expr).ty_adt_id() else { return }; - if !tcx.is_diagnostic_item(sym::Option, ty_adt_did) { + 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; }