Skip to content

Commit 33bae12

Browse files
committed
Give a warning if multiple associated items are available
1 parent bdad4f8 commit 33bae12

File tree

1 file changed

+49
-12
lines changed

1 file changed

+49
-12
lines changed

src/librustdoc/passes/collect_intra_doc_links.rs

Lines changed: 49 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ pub fn collect_intra_doc_links(krate: Crate, cx: &DocContext<'_>) -> Crate {
4646
enum ErrorKind {
4747
ResolutionFailure,
4848
AnchorFailure(&'static str),
49+
Ambiguous {
50+
candidates: Vec<Res>,
51+
}
4952
}
5053

5154
struct LinkCollector<'a, 'tcx> {
@@ -291,7 +294,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
291294
let ty = cx.tcx.type_of(did);
292295
// Checks if item_name belongs to `impl SomeItem`
293296
let impls = crate::clean::get_auto_trait_and_blanket_impls(cx, ty, did);
294-
let impl_kind = impls
297+
let candidates: Vec<_> = impls
295298
.flat_map(|impl_outer| {
296299
match impl_outer.inner {
297300
ImplItem(impl_) => {
@@ -306,8 +309,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
306309
// but provided methods come directly from `tcx`.
307310
// Fortunately, we don't need the whole method, we just need to know
308311
// what kind of associated item it is.
309-
assoc.inner.as_assoc_kind()
310-
.expect("inner items for a trait should be associated items")
312+
(
313+
assoc.def_id,
314+
assoc.inner.as_assoc_kind()
315+
.expect("inner items for a trait should be associated items")
316+
)
311317
})
312318
// TODO: this collect seems a shame
313319
.collect()
@@ -322,16 +328,23 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
322328
// TODO: this is wrong, it should look at the trait, not the impl
323329
cx.tcx.associated_items(impl_outer.def_id)
324330
.filter_by_name(cx.tcx, Ident::with_dummy_span(item_name), impl_outer.def_id)
325-
.map(|assoc| assoc.kind)
331+
.map(|assoc| (assoc.def_id, assoc.kind))
326332
// TODO: this collect seems a shame
327333
.collect::<Vec<_>>()
328334
}
329335
}
330336
_ => panic!("get_impls returned something that wasn't an impl"),
331337
}
332338
})
333-
// TODO: give a warning if this is ambiguous
334-
.next();
339+
.chain(cx.tcx.all_impls())
340+
.collect();
341+
if candidates.len() > 1 {
342+
let candidates = candidates.into_iter()
343+
.map(|(def_id, kind)| Res::Def(kind.as_def_kind(), def_id))
344+
.collect();
345+
return Err(ErrorKind::Ambiguous { candidates });
346+
}
347+
let impl_kind = candidates.into_iter().next().map(|(_, kind)| kind);
335348
// TODO: is this necessary? It doesn't look right, and also only works for local items
336349
let trait_kind = self.cx.as_local_hir_id(item.def_id)
337350
.and_then(|item_hir| {
@@ -694,6 +707,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
694707
}
695708

696709
match kind {
710+
// TODO: reduce this duplicate code
697711
Some(ns @ ValueNS) => {
698712
match self.resolve(
699713
path_str,
@@ -716,6 +730,17 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
716730
anchor_failure(cx, &item, &ori_link, &dox, link_range, msg);
717731
continue;
718732
}
733+
Err(ErrorKind::Ambiguous { candidates }) => {
734+
ambiguity_error(
735+
cx,
736+
&item,
737+
path_str,
738+
&dox,
739+
link_range,
740+
candidates.into_iter().map(|res| (res, TypeNS)).collect(),
741+
);
742+
continue;
743+
}
719744
}
720745
}
721746
Some(ns @ TypeNS) => {
@@ -738,6 +763,17 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
738763
anchor_failure(cx, &item, &ori_link, &dox, link_range, msg);
739764
continue;
740765
}
766+
Err(ErrorKind::Ambiguous { candidates }) => {
767+
ambiguity_error(
768+
cx,
769+
&item,
770+
path_str,
771+
&dox,
772+
link_range,
773+
candidates.into_iter().map(|res| (res, TypeNS)).collect(),
774+
);
775+
continue;
776+
}
741777
}
742778
}
743779
None => {
@@ -809,13 +845,18 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
809845
if is_derive_trait_collision(&candidates) {
810846
candidates.macro_ns = None;
811847
}
848+
let candidates = candidates.map(|opt| opt.map(|(res, _)| res));
849+
let candidates = [TypeNS, ValueNS, MacroNS]
850+
.iter()
851+
.filter_map(|&ns| candidates[ns].map(|res| (res, ns)))
852+
.collect();
812853
ambiguity_error(
813854
cx,
814855
&item,
815856
path_str,
816857
&dox,
817858
link_range,
818-
candidates.map(|candidate| candidate.map(|(res, _)| res)),
859+
candidates,
819860
);
820861
continue;
821862
}
@@ -1001,7 +1042,7 @@ fn ambiguity_error(
10011042
path_str: &str,
10021043
dox: &str,
10031044
link_range: Option<Range<usize>>,
1004-
candidates: PerNS<Option<Res>>,
1045+
candidates: Vec<(Res, Namespace)>,
10051046
) {
10061047
let hir_id = match cx.as_local_hir_id(item.def_id) {
10071048
Some(hir_id) => hir_id,
@@ -1020,10 +1061,6 @@ fn ambiguity_error(
10201061
|lint| {
10211062
let mut msg = format!("`{}` is ", path_str);
10221063

1023-
let candidates = [TypeNS, ValueNS, MacroNS]
1024-
.iter()
1025-
.filter_map(|&ns| candidates[ns].map(|res| (res, ns)))
1026-
.collect::<Vec<_>>();
10271064
match candidates.as_slice() {
10281065
[(first_def, _), (second_def, _)] => {
10291066
msg += &format!(

0 commit comments

Comments
 (0)