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

Method suggestion code tweaks #117655

Merged
merged 2 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 3 additions & 3 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::errors::{
YieldExprOutsideOfCoroutine,
};
use crate::fatally_break_rust;
use crate::method::{MethodCallComponents, SelfSource};
use crate::method::SelfSource;
use crate::type_error_struct;
use crate::Expectation::{self, ExpectCastableToType, ExpectHasType, NoExpectation};
use crate::{
Expand Down Expand Up @@ -512,7 +512,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) -> Ty<'tcx> {
let tcx = self.tcx;
let (res, opt_ty, segs) =
self.resolve_ty_and_res_fully_qualified_call(qpath, expr.hir_id, expr.span);
self.resolve_ty_and_res_fully_qualified_call(qpath, expr.hir_id, expr.span, Some(args));
let ty = match res {
Res::Err => {
self.suggest_assoc_method_call(segs);
Expand Down Expand Up @@ -1332,7 +1332,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
segment.ident,
SelfSource::MethodCall(rcvr),
error,
Some(MethodCallComponents { receiver: rcvr, args, full_expr: expr }),
Some(args),
expected,
false,
) {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
qpath: &'tcx QPath<'tcx>,
hir_id: hir::HirId,
span: Span,
args: Option<&'tcx [hir::Expr<'tcx>]>,
) -> (Res, Option<RawTy<'tcx>>, &'tcx [hir::PathSegment<'tcx>]) {
debug!(
"resolve_ty_and_res_fully_qualified_call: qpath={:?} hir_id={:?} span={:?}",
Expand Down Expand Up @@ -898,7 +899,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
item_name,
SelfSource::QPath(qself),
error,
None,
args,
Expectation::NoExpectation,
trait_missing_method && span.edition().at_least_rust_2021(), // emits missing method for trait only after edition 2021
) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/method/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod prelude2021;
pub mod probe;
mod suggest;

pub use self::suggest::{MethodCallComponents, SelfSource};
pub use self::suggest::SelfSource;
pub use self::MethodError::*;

use crate::errors::OpMethodGenericParams;
Expand Down
161 changes: 80 additions & 81 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,6 @@ use rustc_hir::intravisit::Visitor;
use std::cmp::{self, Ordering};
use std::iter;

/// After identifying that `full_expr` is a method call, we use this type to keep the expression's
/// components readily available to us to point at the right place in diagnostics.
#[derive(Debug, Clone, Copy)]
pub struct MethodCallComponents<'tcx> {
pub receiver: &'tcx hir::Expr<'tcx>,
pub args: &'tcx [hir::Expr<'tcx>],
pub full_expr: &'tcx hir::Expr<'tcx>,
}

impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn is_fn_ty(&self, ty: Ty<'tcx>, span: Span) -> bool {
let tcx = self.tcx;
Expand Down Expand Up @@ -124,7 +115,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
item_name: Ident,
source: SelfSource<'tcx>,
error: MethodError<'tcx>,
args: Option<MethodCallComponents<'tcx>>,
args: Option<&'tcx [hir::Expr<'tcx>]>,
expected: Expectation<'tcx>,
trait_missing_method: bool,
) -> Option<DiagnosticBuilder<'_, ErrorGuaranteed>> {
Expand Down Expand Up @@ -167,6 +158,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.note_candidates_on_method_error(
rcvr_ty,
item_name,
source,
args,
span,
&mut err,
Expand Down Expand Up @@ -266,23 +258,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn suggest_missing_writer(
&self,
rcvr_ty: Ty<'tcx>,
args: MethodCallComponents<'tcx>,
rcvr_expr: &hir::Expr<'tcx>,
) -> DiagnosticBuilder<'_, ErrorGuaranteed> {
let (ty_str, _ty_file) = self.tcx.short_ty_string(rcvr_ty);
let mut err = struct_span_err!(
self.tcx.sess,
args.receiver.span,
rcvr_expr.span,
E0599,
"cannot write into `{}`",
ty_str
);
err.span_note(
args.receiver.span,
rcvr_expr.span,
"must implement `io::Write`, `fmt::Write`, or have a `write_fmt` method",
);
if let ExprKind::Lit(_) = args.receiver.kind {
if let ExprKind::Lit(_) = rcvr_expr.kind {
err.span_help(
args.receiver.span.shrink_to_lo(),
rcvr_expr.span.shrink_to_lo(),
"a writer is needed before this format string",
);
};
Expand All @@ -296,7 +288,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
rcvr_ty: Ty<'tcx>,
item_name: Ident,
source: SelfSource<'tcx>,
args: Option<MethodCallComponents<'tcx>>,
args: Option<&'tcx [hir::Expr<'tcx>]>,
sugg_span: Span,
no_match_data: &mut NoMatchData<'tcx>,
expected: Expectation<'tcx>,
Expand Down Expand Up @@ -377,23 +369,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
tcx.is_diagnostic_item(sym::write_macro, def_id)
|| tcx.is_diagnostic_item(sym::writeln_macro, def_id)
}) && item_name.name == Symbol::intern("write_fmt");
let mut err = if is_write && let Some(args) = args {
self.suggest_missing_writer(rcvr_ty, args)
} else {
tcx.sess.create_err(NoAssociatedItem {
span,
item_kind,
item_name,
ty_prefix: if trait_missing_method {
// FIXME(mu001999) E0599 maybe not suitable here because it is for types
Cow::from("trait")
} else {
rcvr_ty.prefix_string(self.tcx)
},
ty_str: ty_str_reported,
trait_missing_method,
})
};
let mut err =
if is_write && let SelfSource::MethodCall(rcvr_expr) = source
{
self.suggest_missing_writer(rcvr_ty, rcvr_expr)
} else {
tcx.sess.create_err(NoAssociatedItem {
span,
item_kind,
item_name,
ty_prefix: if trait_missing_method {
// FIXME(mu001999) E0599 maybe not suitable here because it is for types
Cow::from("trait")
} else {
rcvr_ty.prefix_string(self.tcx)
},
ty_str: ty_str_reported,
trait_missing_method,
})
};
if tcx.sess.source_map().is_multiline(sugg_span) {
err.span_label(sugg_span.with_hi(span.lo()), "");
}
Expand All @@ -409,7 +403,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err.downgrade_to_delayed_bug();
}

self.find_builder_fn(&mut err, rcvr_ty, source);
if matches!(source, SelfSource::QPath(_)) && args.is_some() {
self.find_builder_fn(&mut err, rcvr_ty);
}

if tcx.ty_is_opaque_future(rcvr_ty) && item_name.name == sym::poll {
err.help(format!(
"method `poll` found on `Pin<&mut {ty_str}>`, \
Expand Down Expand Up @@ -523,6 +520,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.note_candidates_on_method_error(
rcvr_ty,
item_name,
source,
args,
span,
&mut err,
Expand All @@ -533,6 +531,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.note_candidates_on_method_error(
rcvr_ty,
item_name,
source,
args,
span,
&mut err,
Expand Down Expand Up @@ -976,7 +975,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
unsatisfied_bounds = true;
}
} else if let ty::Adt(def, targs) = rcvr_ty.kind()
&& let Some(args) = args
&& let SelfSource::MethodCall(rcvr_expr) = source
{
// This is useful for methods on arbitrary self types that might have a simple
// mutability difference, like calling a method on `Pin<&mut Self>` that is on
Expand All @@ -999,8 +998,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
rcvr_ty,
&item_segment,
span,
args.full_expr,
args.receiver,
tcx.hir().get_parent(rcvr_expr.hir_id).expect_expr(),
rcvr_expr,
) {
err.span_note(
tcx.def_span(method.def_id),
Expand Down Expand Up @@ -1169,7 +1168,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
span,
rcvr_ty,
item_name,
args.map(|MethodCallComponents { args, .. }| args.len() + 1),
args.map(|args| args.len() + 1),
source,
no_match_data.out_of_scope_traits.clone(),
&unsatisfied_predicates,
Expand Down Expand Up @@ -1250,7 +1249,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
rcvr_ty: Ty<'tcx>,
item_name: Ident,
args: Option<MethodCallComponents<'tcx>>,
self_source: SelfSource<'tcx>,
args: Option<&'tcx [hir::Expr<'tcx>]>,
span: Span,
err: &mut Diagnostic,
sources: &mut Vec<CandidateSource>,
Expand Down Expand Up @@ -1339,6 +1339,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if let Some(sugg) = print_disambiguation_help(
item_name,
args,
self_source,
err,
path,
ty,
Expand Down Expand Up @@ -1378,6 +1379,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if let Some(sugg) = print_disambiguation_help(
item_name,
args,
self_source,
err,
path,
rcvr_ty,
Expand Down Expand Up @@ -1410,18 +1412,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

/// Look at all the associated functions without receivers in the type's inherent impls
/// to look for builders that return `Self`, `Option<Self>` or `Result<Self, _>`.
fn find_builder_fn(&self, err: &mut Diagnostic, rcvr_ty: Ty<'tcx>, source: SelfSource<'tcx>) {
fn find_builder_fn(&self, err: &mut Diagnostic, rcvr_ty: Ty<'tcx>) {
let ty::Adt(adt_def, _) = rcvr_ty.kind() else {
return;
};
let SelfSource::QPath(ty) = source else {
return;
};
let hir = self.tcx.hir();
if let Some(Node::Pat(_)) = hir.find(hir.parent_id(ty.hir_id)) {
// Do not suggest a fn call when a pattern is expected.
return;
}
Comment on lines -1421 to -1424
Copy link
Contributor

Choose a reason for hiding this comment

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

I tend to pass in everything needed to make a consideration into these kind of functions so that the outer logic reads linearly with fewer branches, but I don't mind either way.

let mut items = self
.tcx
.inherent_impls(adt_def.did())
Expand Down Expand Up @@ -1504,7 +1498,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
rcvr_ty: Ty<'tcx>,
source: SelfSource<'tcx>,
item_name: Ident,
args: Option<MethodCallComponents<'tcx>>,
args: Option<&'tcx [hir::Expr<'tcx>]>,
sugg_span: Span,
) {
let mut has_unsuggestable_args = false;
Expand Down Expand Up @@ -1578,38 +1572,40 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
None
};
let mut applicability = Applicability::MachineApplicable;
let args = if let Some(MethodCallComponents { receiver, args, .. }) = args {
// The first arg is the same kind as the receiver
let explicit_args = if first_arg.is_some() {
std::iter::once(receiver).chain(args.iter()).collect::<Vec<_>>()
let args = if let SelfSource::MethodCall(receiver) = source
&& let Some(args) = args
{
// The first arg is the same kind as the receiver
let explicit_args = if first_arg.is_some() {
std::iter::once(receiver).chain(args.iter()).collect::<Vec<_>>()
} else {
// There is no `Self` kind to infer the arguments from
if has_unsuggestable_args {
applicability = Applicability::HasPlaceholders;
}
args.iter().collect()
};
format!(
"({}{})",
first_arg.unwrap_or(""),
explicit_args
.iter()
.map(|arg| self
.tcx
.sess
.source_map()
.span_to_snippet(arg.span)
.unwrap_or_else(|_| {
applicability = Applicability::HasPlaceholders;
"_".to_owned()
}))
.collect::<Vec<_>>()
.join(", "),
)
} else {
// There is no `Self` kind to infer the arguments from
if has_unsuggestable_args {
applicability = Applicability::HasPlaceholders;
}
args.iter().collect()
applicability = Applicability::HasPlaceholders;
"(...)".to_owned()
};
format!(
"({}{})",
first_arg.unwrap_or(""),
explicit_args
.iter()
.map(|arg| self
.tcx
.sess
.source_map()
.span_to_snippet(arg.span)
.unwrap_or_else(|_| {
applicability = Applicability::HasPlaceholders;
"_".to_owned()
}))
.collect::<Vec<_>>()
.join(", "),
)
} else {
applicability = Applicability::HasPlaceholders;
"(...)".to_owned()
};
err.span_suggestion(
sugg_span,
"use associated function syntax instead",
Expand Down Expand Up @@ -3268,7 +3264,8 @@ pub fn all_traits(tcx: TyCtxt<'_>) -> Vec<TraitInfo> {

fn print_disambiguation_help<'tcx>(
item_name: Ident,
args: Option<MethodCallComponents<'tcx>>,
args: Option<&'tcx [hir::Expr<'tcx>]>,
source: SelfSource<'tcx>,
err: &mut Diagnostic,
trait_name: String,
rcvr_ty: Ty<'_>,
Expand All @@ -3281,7 +3278,9 @@ fn print_disambiguation_help<'tcx>(
fn_has_self_parameter: bool,
) -> Option<String> {
Some(
if let (ty::AssocKind::Fn, Some(MethodCallComponents { receiver, args, .. })) = (kind, args)
if matches!(kind, ty::AssocKind::Fn)
&& let SelfSource::MethodCall(receiver) = source
&& let Some(args) = args
{
let args = format!(
"({}{})",
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn check_pat(&self, pat: &'tcx Pat<'tcx>, expected: Ty<'tcx>, pat_info: PatInfo<'tcx, '_>) {
let PatInfo { binding_mode: def_bm, top_info: ti, .. } = pat_info;
let path_res = match &pat.kind {
PatKind::Path(qpath) => {
Some(self.resolve_ty_and_res_fully_qualified_call(qpath, pat.hir_id, pat.span))
}
PatKind::Path(qpath) => Some(
self.resolve_ty_and_res_fully_qualified_call(qpath, pat.hir_id, pat.span, None),
),
_ => None,
};
let adjust_mode = self.calc_adjust_mode(pat, path_res.map(|(res, ..)| res));
Expand Down Expand Up @@ -1060,7 +1060,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// Resolve the path and check the definition for errors.
let (res, opt_ty, segments) =
self.resolve_ty_and_res_fully_qualified_call(qpath, pat.hir_id, pat.span);
self.resolve_ty_and_res_fully_qualified_call(qpath, pat.hir_id, pat.span, None);
if res == Res::Err {
let e = tcx.sess.delay_span_bug(pat.span, "`Res::Err` but no error emitted");
self.set_tainted_by_errors(e);
Expand Down