bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Auto merge of #116089 - estebank:issue-115992-2, r=compiler-errors

Browse source 
When suggesting `self.x` for `S { x }`, use `S { x: self.x }`

Fix #115992.

r? `@compiler-errors`

Follow up to #116086.
This commit is contained in:
bors 2023-09-29 05:45:18 +00:00
commit c1f86f0bc8
14 changed files with 306 additions and 75 deletions
Download patch file Download diff file Expand all files Collapse all files

20
compiler/rustc_resolve/src/late.rs
View file

@ -4140,6 +4140,12 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
});
}
fn resolve_expr_field(&mut self, f: &'ast ExprField, e: &'ast Expr) {
self.resolve_expr(&f.expr, Some(e));
self.visit_ident(f.ident);
walk_list!(self, visit_attribute, f.attrs.iter());
}
fn resolve_expr(&mut self, expr: &'ast Expr, parent: Option<&'ast Expr>) {
// First, record candidate traits for this expression if it could
// result in the invocation of a method call.
@ -4155,7 +4161,19 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
ExprKind::Struct(ref se) => {
self.smart_resolve_path(expr.id, &se.qself, &se.path, PathSource::Struct);
visit::walk_expr(self, expr);
// This is the same as `visit::walk_expr(self, expr);`, but we want to pass the
// parent in for accurate suggestions when encountering `Foo { bar }` that should
// have been `Foo { bar: self.bar }`.
if let Some(qself) = &se.qself {
self.visit_ty(&qself.ty);
}
self.visit_path(&se.path, expr.id);
walk_list!(self, resolve_expr_field, &se.fields, expr);
match &se.rest {
StructRest::Base(expr) => self.visit_expr(expr),
StructRest::Rest(_span) => {}
StructRest::None => {}
}
}
ExprKind::Break(Some(label), _) | ExprKind::Continue(Some(label)) => {

90
compiler/rustc_resolve/src/late/diagnostics.rs
View file

@ -41,7 +41,7 @@ type Res = def::Res<ast::NodeId>;
/// A field or associated item from self type suggested in case of resolution failure.
enum AssocSuggestion {
Field,
Field(Span),
MethodWithSelf { called: bool },
AssocFn { called: bool },
AssocType,
@ -51,7 +51,7 @@ enum AssocSuggestion {
impl AssocSuggestion {
fn action(&self) -> &'static str {
match self {
AssocSuggestion::Field => "use the available field",
AssocSuggestion::Field(_) => "use the available field",
AssocSuggestion::MethodWithSelf { called: true } => {
"call the method with the fully-qualified path"
}
@ -215,7 +215,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
}
} else {
let mut span_label = None;
let item_span = path.last().unwrap().ident.span;
let item_ident = path.last().unwrap().ident;
let item_span = item_ident.span;
let (mod_prefix, mod_str, module, suggestion) = if path.len() == 1 {
debug!(?self.diagnostic_metadata.current_impl_items);
debug!(?self.diagnostic_metadata.current_function);
@ -231,9 +232,35 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
})
{
let sp = item_span.shrink_to_lo();
// Account for `Foo { field }` when suggesting `self.field` so we result on
// `Foo { field: self.field }`.
let field = match source {
PathSource::Expr(Some(Expr { kind: ExprKind::Struct(expr), .. })) => {
expr.fields.iter().find(|f| f.ident == item_ident)
}
_ => None,
};
let pre = if let Some(field) = field && field.is_shorthand {
format!("{item_ident}: ")
} else {
String::new()
};
// Ensure we provide a structured suggestion for an assoc fn only for
// expressions that are actually a fn call.
let is_call = match field {
Some(ast::ExprField { expr, .. }) => {
matches!(expr.kind, ExprKind::Call(..))
}
_ => matches!(
source,
PathSource::Expr(Some(Expr { kind: ExprKind::Call(..), ..})),
),
};
match &item.kind {
AssocItemKind::Fn(fn_)
if !sig.decl.has_self() && fn_.sig.decl.has_self() => {
if (!sig.decl.has_self() || !is_call) && fn_.sig.decl.has_self() => {
// Ensure that we only suggest `self.` if `self` is available,
// you can't call `fn foo(&self)` from `fn bar()` (#115992).
// We also want to mention that the method exists.
@ -243,20 +270,28 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
));
None
}
AssocItemKind::Fn(fn_)
if !fn_.sig.decl.has_self() && !is_call => {
span_label = Some((
item.ident.span,
"an associated function by that name is available on `Self` here",
));
None
}
AssocItemKind::Fn(fn_) if fn_.sig.decl.has_self() => Some((
sp,
"consider using the method on `Self`",
"self.".to_string(),
format!("{pre}self."),
)),
AssocItemKind::Fn(_) => Some((
sp,
"consider using the associated function on `Self`",
"Self::".to_string(),
format!("{pre}Self::"),
)),
AssocItemKind::Const(..) => Some((
sp,
"consider using the associated constant on `Self`",
"Self::".to_string(),
format!("{pre}Self::"),
)),
_ => None
}
@ -621,17 +656,30 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
self.lookup_assoc_candidate(ident, ns, is_expected, source.is_call())
{
let self_is_available = self.self_value_is_available(path[0].ident.span);
// Account for `Foo { field }` when suggesting `self.field` so we result on
// `Foo { field: self.field }`.
let pre = match source {
PathSource::Expr(Some(Expr { kind: ExprKind::Struct(expr), .. }))
if expr
.fields
.iter()
.any(|f| f.ident == path[0].ident && f.is_shorthand) =>
{
format!("{path_str}: ")
}
_ => String::new(),
};
match candidate {
AssocSuggestion::Field => {
AssocSuggestion::Field(field_span) => {
if self_is_available {
err.span_suggestion(
span,
err.span_suggestion_verbose(
span.shrink_to_lo(),
"you might have meant to use the available field",
format!("self.{path_str}"),
format!("{pre}self."),
Applicability::MachineApplicable,
);
} else {
err.span_label(span, "a field by this name exists in `Self`");
err.span_label(field_span, "a field by that name exists in `Self`");
}
}
AssocSuggestion::MethodWithSelf { called } if self_is_available => {
@ -640,10 +688,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
} else {
"you might have meant to refer to the method"
};
err.span_suggestion(
span,
err.span_suggestion_verbose(
span.shrink_to_lo(),
msg,
format!("self.{path_str}"),
"self.".to_string(),
Applicability::MachineApplicable,
);
}
@ -651,10 +699,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
| AssocSuggestion::AssocFn { .. }
| AssocSuggestion::AssocConst
| AssocSuggestion::AssocType => {
err.span_suggestion(
span,
err.span_suggestion_verbose(
span.shrink_to_lo(),
format!("you might have meant to {}", candidate.action()),
format!("Self::{path_str}"),
"Self::".to_string(),
Applicability::MachineApplicable,
);
}
@ -1667,11 +1715,11 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
resolution.full_res()
{
if let Some(field_ids) = self.r.field_def_ids(did) {
if field_ids
if let Some(field_id) = field_ids
.iter()
.any(|&field_id| ident.name == self.r.tcx.item_name(field_id))
.find(|&&field_id| ident.name == self.r.tcx.item_name(field_id))
{
return Some(AssocSuggestion::Field);
return Some(AssocSuggestion::Field(self.r.def_span(*field_id)));
}
}
}