Skip to content

Commit 23c33da

Browse files
committed
When suggesting self.x for S { x }, use S { x: self.x }
Tweak output. Fix #115992.
1 parent 7d8559a commit 23c33da

13 files changed

+267
-54
lines changed

compiler/rustc_resolve/src/late.rs

+19-1
Original file line numberDiff line numberDiff line change
@@ -4140,6 +4140,12 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
41404140
});
41414141
}
41424142

4143+
fn resolve_expr_field(&mut self, f: &'ast ExprField, e: &'ast Expr) {
4144+
self.resolve_expr(&f.expr, Some(e));
4145+
self.visit_ident(f.ident);
4146+
walk_list!(self, visit_attribute, f.attrs.iter());
4147+
}
4148+
41434149
fn resolve_expr(&mut self, expr: &'ast Expr, parent: Option<&'ast Expr>) {
41444150
// First, record candidate traits for this expression if it could
41454151
// result in the invocation of a method call.
@@ -4155,7 +4161,19 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
41554161

41564162
ExprKind::Struct(ref se) => {
41574163
self.smart_resolve_path(expr.id, &se.qself, &se.path, PathSource::Struct);
4158-
visit::walk_expr(self, expr);
4164+
// This is the same as `visit::walk_expr(self, expr);`, but we want to pass the
4165+
// parent in for accurate suggestions when encountering `Foo { bar }` that should
4166+
// have been `Foo { bar: self.bar }`.
4167+
if let Some(qself) = &se.qself {
4168+
self.visit_ty(&qself.ty);
4169+
}
4170+
self.visit_path(&se.path, expr.id);
4171+
walk_list!(self, resolve_expr_field, &se.fields, expr);
4172+
match &se.rest {
4173+
StructRest::Base(expr) => self.visit_expr(expr),
4174+
StructRest::Rest(_span) => {}
4175+
StructRest::None => {}
4176+
}
41594177
}
41604178

41614179
ExprKind::Break(Some(label), _) | ExprKind::Continue(Some(label)) => {

compiler/rustc_resolve/src/late/diagnostics.rs

+62-14
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
215215
}
216216
} else {
217217
let mut span_label = None;
218-
let item_span = path.last().unwrap().ident.span;
218+
let item_ident = path.last().unwrap().ident;
219+
let item_span = item_ident.span;
219220
let (mod_prefix, mod_str, module, suggestion) = if path.len() == 1 {
220221
debug!(?self.diagnostic_metadata.current_impl_items);
221222
debug!(?self.diagnostic_metadata.current_function);
@@ -231,9 +232,35 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
231232
})
232233
{
233234
let sp = item_span.shrink_to_lo();
235+
236+
// Account for `Foo { field }` when suggesting `self.field` so we result on
237+
// `Foo { field: self.field }`.
238+
let field = match source {
239+
PathSource::Expr(Some(Expr { kind: ExprKind::Struct(expr), .. })) => {
240+
expr.fields.iter().find(|f| f.ident == item_ident)
241+
}
242+
_ => None,
243+
};
244+
let pre = if let Some(field) = field && field.is_shorthand {
245+
format!("{item_ident}: ")
246+
} else {
247+
String::new()
248+
};
249+
// Ensure we provide a structured suggestion for an assoc fn only for
250+
// expressions that are actually a fn call.
251+
let is_call = match field {
252+
Some(ast::ExprField { expr, .. }) => {
253+
matches!(expr.kind, ExprKind::Call(..))
254+
}
255+
_ => matches!(
256+
source,
257+
PathSource::Expr(Some(Expr { kind: ExprKind::Call(..), ..})),
258+
),
259+
};
260+
234261
match &item.kind {
235262
AssocItemKind::Fn(fn_)
236-
if !sig.decl.has_self() && fn_.sig.decl.has_self() => {
263+
if (!sig.decl.has_self() || !is_call) && fn_.sig.decl.has_self() => {
237264
// Ensure that we only suggest `self.` if `self` is available,
238265
// you can't call `fn foo(&self)` from `fn bar()` (#115992).
239266
// We also want to mention that the method exists.
@@ -243,20 +270,28 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
243270
));
244271
None
245272
}
273+
AssocItemKind::Fn(fn_)
274+
if !fn_.sig.decl.has_self() && !is_call => {
275+
span_label = Some((
276+
item.ident.span,
277+
"an associated function by that name is available on `Self` here",
278+
));
279+
None
280+
}
246281
AssocItemKind::Fn(fn_) if fn_.sig.decl.has_self() => Some((
247282
sp,
248283
"consider using the method on `Self`",
249-
"self.".to_string(),
284+
format!("{pre}self."),
250285
)),
251286
AssocItemKind::Fn(_) => Some((
252287
sp,
253288
"consider using the associated function on `Self`",
254-
"Self::".to_string(),
289+
format!("{pre}Self::"),
255290
)),
256291
AssocItemKind::Const(..) => Some((
257292
sp,
258293
"consider using the associated constant on `Self`",
259-
"Self::".to_string(),
294+
format!("{pre}Self::"),
260295
)),
261296
_ => None
262297
}
@@ -621,13 +656,26 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
621656
self.lookup_assoc_candidate(ident, ns, is_expected, source.is_call())
622657
{
623658
let self_is_available = self.self_value_is_available(path[0].ident.span);
659+
// Account for `Foo { field }` when suggesting `self.field` so we result on
660+
// `Foo { field: self.field }`.
661+
let pre = match source {
662+
PathSource::Expr(Some(Expr { kind: ExprKind::Struct(expr), .. }))
663+
if expr
664+
.fields
665+
.iter()
666+
.any(|f| f.ident == path[0].ident && f.is_shorthand) =>
667+
{
668+
format!("{path_str}: ")
669+
}
670+
_ => String::new(),
671+
};
624672
match candidate {
625673
AssocSuggestion::Field => {
626674
if self_is_available {
627-
err.span_suggestion(
628-
span,
675+
err.span_suggestion_verbose(
676+
span.shrink_to_lo(),
629677
"you might have meant to use the available field",
630-
format!("self.{path_str}"),
678+
format!("{pre}self."),
631679
Applicability::MachineApplicable,
632680
);
633681
} else {
@@ -640,21 +688,21 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
640688
} else {
641689
"you might have meant to refer to the method"
642690
};
643-
err.span_suggestion(
644-
span,
691+
err.span_suggestion_verbose(
692+
span.shrink_to_lo(),
645693
msg,
646-
format!("self.{path_str}"),
694+
"self.".to_string(),
647695
Applicability::MachineApplicable,
648696
);
649697
}
650698
AssocSuggestion::MethodWithSelf { .. }
651699
| AssocSuggestion::AssocFn { .. }
652700
| AssocSuggestion::AssocConst
653701
| AssocSuggestion::AssocType => {
654-
err.span_suggestion(
655-
span,
702+
err.span_suggestion_verbose(
703+
span.shrink_to_lo(),
656704
format!("you might have meant to {}", candidate.action()),
657-
format!("Self::{path_str}"),
705+
"Self::".to_string(),
658706
Applicability::MachineApplicable,
659707
);
660708
}

tests/ui/resolve/associated-fn-called-as-fn.stderr

+12-2
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,23 @@ error[E0425]: cannot find function `collect_primary` in this scope
22
--> $DIR/associated-fn-called-as-fn.rs:6:30
33
|
44
LL | '0'..='9' => collect_primary(&c),
5-
| ^^^^^^^^^^^^^^^ help: you might have meant to call the associated function: `Self::collect_primary`
5+
| ^^^^^^^^^^^^^^^
6+
|
7+
help: you might have meant to call the associated function
8+
|
9+
LL | '0'..='9' => Self::collect_primary(&c),
10+
| ++++++
611

712
error[E0425]: cannot find function `collect_primary` in this scope
813
--> $DIR/associated-fn-called-as-fn.rs:23:30
914
|
1015
LL | '0'..='9' => collect_primary(&c),
11-
| ^^^^^^^^^^^^^^^ help: you might have meant to call the associated function: `Self::collect_primary`
16+
| ^^^^^^^^^^^^^^^
17+
|
18+
help: you might have meant to call the associated function
19+
|
20+
LL | '0'..='9' => Self::collect_primary(&c),
21+
| ++++++
1222

1323
error: aborting due to 2 previous errors
1424

tests/ui/resolve/field-and-method-in-self-not-available-in-assoc-fn.rs

+3
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,8 @@ impl Foo {
1111
field; //~ ERROR cannot find value `field` in this scope
1212
Foo { field } //~ ERROR cannot find value `field` in this scope
1313
}
14+
fn clone(&self) -> Foo {
15+
Foo { field } //~ ERROR cannot find value `field` in this scope
16+
}
1417
}
1518
fn main() {}

tests/ui/resolve/field-and-method-in-self-not-available-in-assoc-fn.stderr

+15-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,20 @@ LL | fn field(&self) -> u32 {
1616
LL | Foo { field }
1717
| ^^^^^ a field by this name exists in `Self`
1818

19-
error: aborting due to 2 previous errors
19+
error[E0425]: cannot find value `field` in this scope
20+
--> $DIR/field-and-method-in-self-not-available-in-assoc-fn.rs:15:15
21+
|
22+
LL | fn field(&self) -> u32 {
23+
| ----- a method by that name is available on `Self` here
24+
...
25+
LL | Foo { field }
26+
| ^^^^^
27+
|
28+
help: you might have meant to use the available field
29+
|
30+
LL | Foo { field: self.field }
31+
| ++++++++++++
32+
33+
error: aborting due to 3 previous errors
2034

2135
For more information about this error, try `rustc --explain E0425`.

0 commit comments

Comments
 (0)