Skip to content

Commit 3980342

Browse files
estebankestebank
committed
Use structured suggestion for disambiguating method calls
Fix rust-lang#65635.
1 parent 7dbfb0a commit 3980342

18 files changed

+248
-74
lines changed

‎src/librustc/ty/mod.rs

+11
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,17 @@ pub enum AssocKind {
212212
Type
213213
}
214214

215+
impl AssocKind {
216+
pub fn suggestion_descr(&self) -> &'static str {
217+
match self {
218+
ty::AssocKind::Method => "method call",
219+
ty::AssocKind::Type |
220+
ty::AssocKind::OpaqueTy => "associated type",
221+
ty::AssocKind::Const => "associated constant",
222+
}
223+
}
224+
}
225+
215226
impl AssocItem {
216227
pub fn def_kind(&self) -> DefKind {
217228
match self.kind {

‎src/librustc_typeck/check/expr.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1422,8 +1422,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
14221422
field: ast::Ident,
14231423
) -> Ty<'tcx> {
14241424
let expr_t = self.check_expr_with_needs(base, needs);
1425-
let expr_t = self.structurally_resolved_type(base.span,
1426-
expr_t);
1425+
let expr_t = self.structurally_resolved_type(base.span, expr_t);
14271426
let mut private_candidate = None;
14281427
let mut autoderef = self.autoderef(expr.span, expr_t);
14291428
while let Some((base_t, _)) = autoderef.next() {

‎src/librustc_typeck/check/method/suggest.rs

+92-35
Original file line numberDiff line numberDiff line change
@@ -82,34 +82,63 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
8282
let print_disambiguation_help = |
8383
err: &mut DiagnosticBuilder<'_>,
8484
trait_name: String,
85+
rcvr_ty: Ty<'_>,
86+
kind: ty::AssocKind,
87+
span: Span,
88+
candidate: Option<usize>,
8589
| {
86-
err.help(&format!(
87-
"to disambiguate the method call, write `{}::{}({}{})` instead",
88-
trait_name,
89-
item_name,
90-
if rcvr_ty.is_region_ptr() && args.is_some() {
91-
if rcvr_ty.is_mutable_ptr() {
92-
"&mut "
90+
let mut applicability = Applicability::MachineApplicable;
91+
let sugg_args = if let ty::AssocKind::Method = kind {
92+
format!(
93+
"({}{})",
94+
if rcvr_ty.is_region_ptr() && args.is_some() {
95+
if rcvr_ty.is_mutable_ptr() {
96+
"&mut "
97+
} else {
98+
"&"
99+
}
93100
} else {
94-
"&"
95-
}
96-
} else {
97-
""
98-
},
99-
args.map(|arg| arg
100-
.iter()
101-
.map(|arg| self.tcx.sess.source_map().span_to_snippet(arg.span)
102-
.unwrap_or_else(|_| "...".to_owned()))
103-
.collect::<Vec<_>>()
104-
.join(", ")
105-
).unwrap_or_else(|| "...".to_owned())
106-
));
101+
""
102+
},
103+
args.map(|arg| arg
104+
.iter()
105+
.map(|arg| self.tcx.sess.source_map().span_to_snippet(arg.span)
106+
.unwrap_or_else(|_| {
107+
applicability = Applicability::HasPlaceholders;
108+
"...".to_owned()
109+
}))
110+
.collect::<Vec<_>>()
111+
.join(", ")
112+
).unwrap_or_else(|| {
113+
applicability = Applicability::HasPlaceholders;
114+
"...".to_owned()
115+
}),
116+
)
117+
} else {
118+
String::new()
119+
};
120+
let sugg = format!("{}::{}{}", trait_name, item_name, sugg_args);
121+
err.span_suggestion(
122+
span,
123+
&format!(
124+
"disambiguate the {} for {}",
125+
kind.suggestion_descr(),
126+
if let Some(candidate) = candidate {
127+
format!("candidate #{}", candidate)
128+
} else {
129+
"the candidate".to_string()
130+
},
131+
),
132+
sugg,
133+
applicability,
134+
);
107135
};
108136

109137
let report_candidates = |
110138
span: Span,
111139
err: &mut DiagnosticBuilder<'_>,
112140
mut sources: Vec<CandidateSource>,
141+
sugg_span: Span,
113142
| {
114143
sources.sort();
115144
sources.dedup();
@@ -150,15 +179,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
150179
}
151180
};
152181

153-
let note_str = if sources.len() > 1 {
154-
format!("candidate #{} is defined in an impl{} for the type `{}`",
155-
idx + 1,
156-
insertion,
157-
impl_ty)
182+
let (note_str, idx) = if sources.len() > 1 {
183+
(format!(
184+
"candidate #{} is defined in an impl{} for the type `{}`",
185+
idx + 1,
186+
insertion,
187+
impl_ty,
188+
), Some(idx + 1))
158189
} else {
159-
format!("the candidate is defined in an impl{} for the type `{}`",
160-
insertion,
161-
impl_ty)
190+
(format!(
191+
"the candidate is defined in an impl{} for the type `{}`",
192+
insertion,
193+
impl_ty,
194+
), None)
162195
};
163196
if let Some(note_span) = note_span {
164197
// We have a span pointing to the method. Show note with snippet.
@@ -168,7 +201,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
168201
err.note(&note_str);
169202
}
170203
if let Some(trait_ref) = self.tcx.impl_trait_ref(impl_did) {
171-
print_disambiguation_help(err, self.tcx.def_path_str(trait_ref.def_id));
204+
let path = self.tcx.def_path_str(trait_ref.def_id);
205+
206+
let ty = match item.kind {
207+
ty::AssocKind::Const |
208+
ty::AssocKind::Type |
209+
ty::AssocKind::OpaqueTy => rcvr_ty,
210+
ty::AssocKind::Method => self.tcx.fn_sig(item.def_id)
211+
.inputs()
212+
.skip_binder()
213+
.get(0)
214+
.filter(|ty| ty.is_region_ptr() && !rcvr_ty.is_region_ptr())
215+
.map(|ty| *ty)
216+
.unwrap_or(rcvr_ty),
217+
};
218+
print_disambiguation_help(err, path, ty, item.kind, sugg_span, idx);
172219
}
173220
}
174221
CandidateSource::TraitSource(trait_did) => {
@@ -182,19 +229,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
182229
};
183230
let item_span = self.tcx.sess.source_map()
184231
.def_span(self.tcx.def_span(item.def_id));
185-
if sources.len() > 1 {
232+
let idx = if sources.len() > 1 {
186233
span_note!(err,
187234
item_span,
188235
"candidate #{} is defined in the trait `{}`",
189236
idx + 1,
190237
self.tcx.def_path_str(trait_did));
238+
Some(idx + 1)
191239
} else {
192240
span_note!(err,
193241
item_span,
194242
"the candidate is defined in the trait `{}`",
195243
self.tcx.def_path_str(trait_did));
196-
}
197-
print_disambiguation_help(err, self.tcx.def_path_str(trait_did));
244+
None
245+
};
246+
let path = self.tcx.def_path_str(trait_did);
247+
print_disambiguation_help(err, path, rcvr_ty, item.kind, sugg_span, idx);
198248
}
199249
}
200250
}
@@ -203,6 +253,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
203253
}
204254
};
205255

256+
let sugg_span = if let SelfSource::MethodCall(expr) = source {
257+
// Given `foo.bar(baz)`, `expr` is `bar`, but we want to point to the whole thing.
258+
self.tcx.hir().expect_expr(self.tcx.hir().get_parent_node(expr.hir_id)).span
259+
} else {
260+
span
261+
};
262+
206263
match error {
207264
MethodError::NoMatch(NoMatchData {
208265
static_candidates: static_sources,
@@ -495,9 +552,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
495552
));
496553
}
497554

498-
report_candidates(span, &mut err, static_sources);
555+
report_candidates(span, &mut err, static_sources, sugg_span);
499556
} else if static_sources.len() > 1 {
500-
report_candidates(span, &mut err, static_sources);
557+
report_candidates(span, &mut err, static_sources, sugg_span);
501558
}
502559

503560
if !unsatisfied_predicates.is_empty() {
@@ -584,7 +641,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
584641
"multiple applicable items in scope");
585642
err.span_label(span, format!("multiple `{}` found", item_name));
586643

587-
report_candidates(span, &mut err, sources);
644+
report_candidates(span, &mut err, sources, sugg_span);
588645
err.emit();
589646
}
590647

‎src/test/ui/associated-const/associated-const-ambiguity-report.stderr

+8-2
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `Foo` for the type `i32`
99
|
1010
LL | const ID: i32 = 1;
1111
| ^^^^^^^^^^^^^^^^^^
12-
= help: to disambiguate the method call, write `Foo::ID(...)` instead
1312
note: candidate #2 is defined in an impl of the trait `Bar` for the type `i32`
1413
--> $DIR/associated-const-ambiguity-report.rs:14:5
1514
|
1615
LL | const ID: i32 = 3;
1716
| ^^^^^^^^^^^^^^^^^^
18-
= help: to disambiguate the method call, write `Bar::ID(...)` instead
17+
help: disambiguate the associated constant for candidate #1
18+
|
19+
LL | const X: i32 = Foo::ID;
20+
| ^^^^^^^
21+
help: disambiguate the associated constant for candidate #2
22+
|
23+
LL | const X: i32 = Bar::ID;
24+
| ^^^^^^^
1925

2026
error: aborting due to previous error
2127

‎src/test/ui/error-codes/E0034.stderr

+8-2
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `Trait1` for the type `Tes
99
|
1010
LL | fn foo() {}
1111
| ^^^^^^^^
12-
= help: to disambiguate the method call, write `Trait1::foo(...)` instead
1312
note: candidate #2 is defined in an impl of the trait `Trait2` for the type `Test`
1413
--> $DIR/E0034.rs:16:5
1514
|
1615
LL | fn foo() {}
1716
| ^^^^^^^^
18-
= help: to disambiguate the method call, write `Trait2::foo(...)` instead
17+
help: disambiguate the method call for candidate #1
18+
|
19+
LL | Trait1::foo(...)()
20+
| ^^^^^^^^^^^^^^^^
21+
help: disambiguate the method call for candidate #2
22+
|
23+
LL | Trait2::foo(...)()
24+
| ^^^^^^^^^^^^^^^^
1925

2026
error: aborting due to previous error
2127

‎src/test/ui/inference/inference_unstable_featured.stderr

+8-2
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,15 @@ LL | assert_eq!('x'.ipu_flatten(), 0);
55
| ^^^^^^^^^^^ multiple `ipu_flatten` found
66
|
77
= note: candidate #1 is defined in an impl of the trait `inference_unstable_iterator::IpuIterator` for the type `char`
8-
= help: to disambiguate the method call, write `inference_unstable_iterator::IpuIterator::ipu_flatten('x')` instead
98
= note: candidate #2 is defined in an impl of the trait `inference_unstable_itertools::IpuItertools` for the type `char`
10-
= help: to disambiguate the method call, write `inference_unstable_itertools::IpuItertools::ipu_flatten('x')` instead
9+
help: disambiguate the method call for candidate #1
10+
|
11+
LL | assert_eq!(inference_unstable_iterator::IpuIterator::ipu_flatten(&'x'), 0);
12+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13+
help: disambiguate the method call for candidate #2
14+
|
15+
LL | assert_eq!(inference_unstable_itertools::IpuItertools::ipu_flatten(&'x'), 0);
16+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1117

1218
error: aborting due to previous error
1319

‎src/test/ui/issues/issue-18446.stderr

+4-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ error[E0034]: multiple applicable items in scope
22
--> $DIR/issue-18446.rs:18:7
33
|
44
LL | x.foo();
5-
| ^^^ multiple `foo` found
5+
| --^^^--
6+
| | |
7+
| | multiple `foo` found
8+
| help: disambiguate the method call for candidate #2: `T::foo(&x)`
69
|
710
note: candidate #1 is defined in an impl for the type `dyn T`
811
--> $DIR/issue-18446.rs:9:5
@@ -14,7 +17,6 @@ note: candidate #2 is defined in the trait `T`
1417
|
1518
LL | fn foo(&self);
1619
| ^^^^^^^^^^^^^^
17-
= help: to disambiguate the method call, write `T::foo(&x)` instead
1820

1921
error: aborting due to previous error
2022

‎src/test/ui/issues/issue-3702-2.stderr

+8-2
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `ToPrimitive` for the type
99
|
1010
LL | fn to_int(&self) -> isize { 0 }
1111
| ^^^^^^^^^^^^^^^^^^^^^^^^^
12-
= help: to disambiguate the method call, write `ToPrimitive::to_int(&self)` instead
1312
note: candidate #2 is defined in an impl of the trait `Add` for the type `isize`
1413
--> $DIR/issue-3702-2.rs:14:5
1514
|
1615
LL | fn to_int(&self) -> isize { *self }
1716
| ^^^^^^^^^^^^^^^^^^^^^^^^^
18-
= help: to disambiguate the method call, write `Add::to_int(&self)` instead
17+
help: disambiguate the method call for candidate #1
18+
|
19+
LL | ToPrimitive::to_int(&self) + other.to_int()
20+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
21+
help: disambiguate the method call for candidate #2
22+
|
23+
LL | Add::to_int(&self) + other.to_int()
24+
| ^^^^^^^^^^^^^^^^^^
1925

2026
error: aborting due to previous error
2127

‎src/test/ui/issues/issue-65634-raw-ident-suggestion.stderr

+8-2
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `async` for the type `r#fn
99
|
1010
LL | fn r#struct(&self) {
1111
| ^^^^^^^^^^^^^^^^^^
12-
= help: to disambiguate the method call, write `async::r#struct(r#fn {})` instead
1312
note: candidate #2 is defined in an impl of the trait `await` for the type `r#fn`
1413
--> $DIR/issue-65634-raw-ident-suggestion.rs:10:5
1514
|
1615
LL | fn r#struct(&self) {
1716
| ^^^^^^^^^^^^^^^^^^
18-
= help: to disambiguate the method call, write `await::r#struct(r#fn {})` instead
17+
help: disambiguate the method call for candidate #1
18+
|
19+
LL | async::r#struct(&r#fn {});
20+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
21+
help: disambiguate the method call for candidate #2
22+
|
23+
LL | await::r#struct(&r#fn {});
24+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
1925

2026
error: aborting due to previous error
2127

‎src/test/ui/methods/method-ambig-two-traits-cross-crate.stderr

+8-2
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,15 @@ note: candidate #1 is defined in an impl of the trait `Me2` for the type `usize`
99
|
1010
LL | impl Me2 for usize { fn me(&self) -> usize { *self } }
1111
| ^^^^^^^^^^^^^^^^^^^^^
12-
= help: to disambiguate the method call, write `Me2::me(1_usize)` instead
1312
= note: candidate #2 is defined in an impl of the trait `ambig_impl_2_lib::Me` for the type `usize`
14-
= help: to disambiguate the method call, write `ambig_impl_2_lib::Me::me(1_usize)` instead
13+
help: disambiguate the method call for candidate #1
14+
|
15+
LL | fn main() { Me2::me(&1_usize); }
16+
| ^^^^^^^^^^^^^^^^^
17+
help: disambiguate the method call for candidate #2
18+
|
19+
LL | fn main() { ambig_impl_2_lib::Me::me(&1_usize); }
20+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1521

1622
error: aborting due to previous error
1723

‎src/test/ui/methods/method-ambig-two-traits-from-bounds.stderr

+8-2
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,19 @@ note: candidate #1 is defined in the trait `A`
99
|
1010
LL | trait A { fn foo(&self); }
1111
| ^^^^^^^^^^^^^^
12-
= help: to disambiguate the method call, write `A::foo(t)` instead
1312
note: candidate #2 is defined in the trait `B`
1413
--> $DIR/method-ambig-two-traits-from-bounds.rs:2:11
1514
|
1615
LL | trait B { fn foo(&self); }
1716
| ^^^^^^^^^^^^^^
18-
= help: to disambiguate the method call, write `B::foo(t)` instead
17+
help: disambiguate the method call for candidate #1
18+
|
19+
LL | A::foo(t);
20+
| ^^^^^^^^^
21+
help: disambiguate the method call for candidate #2
22+
|
23+
LL | B::foo(t);
24+
| ^^^^^^^^^
1925

2026
error: aborting due to previous error
2127

0 commit comments

Comments
 (0)