From e9817385f4798097ebe277aea103bab74f3c7ed6 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 13 Mar 2023 14:31:05 +0100 Subject: [PATCH 1/7] Fix rustdoc intra-doc link invalid ambiguity error message --- .../passes/collect_intra_doc_links.rs | 340 ++++++++++-------- 1 file changed, 198 insertions(+), 142 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 789523c561e57..7e24318dec8a6 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -47,7 +47,18 @@ fn collect_intra_doc_links(krate: Crate, cx: &mut DocContext<'_>) -> Crate { krate } -#[derive(Copy, Clone, Debug, Hash)] +fn filter_assoc_items_by_name_and_namespace<'a>( + tcx: TyCtxt<'a>, + assoc_items_of: DefId, + ident: Ident, + ns: Namespace, +) -> impl Iterator + 'a { + tcx.associated_items(assoc_items_of).filter_by_name_unhygienic(ident.name).filter(move |item| { + item.kind.namespace() == ns && tcx.hygienic_eq(ident, item.ident(tcx), assoc_items_of) + }) +} + +#[derive(Copy, Clone, Debug, Hash, PartialEq)] enum Res { Def(DefKind, DefId), Primitive(PrimitiveType), @@ -317,14 +328,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { prim_ty: PrimitiveType, ns: Namespace, item_name: Symbol, - ) -> Option<(Res, DefId)> { + ) -> Vec<(Res, DefId)> { let tcx = self.cx.tcx; - prim_ty.impls(tcx).find_map(|impl_| { - tcx.associated_items(impl_) - .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, impl_) + prim_ty + .impls(tcx) + .flat_map(|impl_| { + filter_assoc_items_by_name_and_namespace( + tcx, + impl_, + Ident::with_dummy_span(item_name), + ns, + ) .map(|item| (Res::Primitive(prim_ty), item.def_id)) - }) + }) + .collect::>() } fn resolve_self_ty(&self, path_str: &str, ns: Namespace, item_id: DefId) -> Option { @@ -394,14 +412,16 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ns: Namespace, item_id: DefId, module_id: DefId, - ) -> Result<(Res, Option), UnresolvedPath<'path>> { + ) -> Result)>, UnresolvedPath<'path>> { if let Some(res) = self.resolve_path(path_str, ns, item_id, module_id) { return Ok(match res { Res::Def( DefKind::AssocFn | DefKind::AssocConst | DefKind::AssocTy | DefKind::Variant, def_id, - ) => (Res::from_def_id(self.cx.tcx, self.cx.tcx.parent(def_id)), Some(def_id)), - _ => (res, None), + ) => { + vec![(Res::from_def_id(self.cx.tcx, self.cx.tcx.parent(def_id)), Some(def_id))] + } + _ => vec![(res, None)], }); } else if ns == MacroNS { return Err(UnresolvedPath { @@ -436,14 +456,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // links to primitives when `#[doc(primitive)]` is present. It should give an ambiguity // error instead and special case *only* modules with `#[doc(primitive)]`, not all // primitives. - resolve_primitive(&path_root, TypeNS) + match resolve_primitive(&path_root, TypeNS) .or_else(|| self.resolve_path(&path_root, TypeNS, item_id, module_id)) .and_then(|ty_res| { - self.resolve_associated_item(ty_res, item_name, ns, module_id).map(Ok) - }) - .unwrap_or_else(|| { + let candidates = self + .resolve_associated_item(ty_res, item_name, ns, module_id) + .into_iter() + .map(|(res, def_id)| (res, Some(def_id))) + .collect::>(); + if !candidates.is_empty() { Some(candidates) } else { None } + }) { + Some(r) => Ok(r), + None => { if ns == Namespace::ValueNS { self.variant_field(path_str, item_id, module_id) + .map(|(res, def_id)| vec![(res, Some(def_id))]) } else { Err(UnresolvedPath { item_id, @@ -452,8 +479,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { unresolved: path_root.into(), }) } - }) - .map(|(res, def_id)| (res, Some(def_id))) + } + } } /// Convert a DefId to a Res, where possible. @@ -535,24 +562,31 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { item_name: Symbol, ns: Namespace, module_id: DefId, - ) -> Option<(Res, DefId)> { + ) -> Vec<(Res, DefId)> { let tcx = self.cx.tcx; match root_res { Res::Primitive(prim) => { - self.resolve_primitive_associated_item(prim, ns, item_name).or_else(|| { + let items = self.resolve_primitive_associated_item(prim, ns, item_name); + if !items.is_empty() { + items + // Inherent associated items take precedence over items that come from trait impls. + } else { self.primitive_type_to_ty(prim) - .and_then(|ty| { + .map(|ty| { resolve_associated_trait_item(ty, module_id, item_name, ns, self.cx) + .iter() + .map(|item| (root_res, item.def_id)) + .collect::>() }) - .map(|item| (root_res, item.def_id)) - }) + .unwrap_or(Vec::new()) + } } Res::Def(DefKind::TyAlias, did) => { // Resolve the link on the type the alias points to. // FIXME: if the associated item is defined directly on the type alias, // it will show up on its documentation page, we should link there instead. - let res = self.def_id_to_res(did)?; + let Some(res) = self.def_id_to_res(did) else { return Vec::new() }; self.resolve_associated_item(res, item_name, ns, module_id) } Res::Def( @@ -566,7 +600,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty::Adt(adt_def, _) => { for variant in adt_def.variants() { if variant.name == item_name { - return Some((root_res, variant.def_id)); + return vec![(root_res, variant.def_id)]; } } } @@ -575,43 +609,46 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } // Checks if item_name belongs to `impl SomeItem` - let assoc_item = tcx + let mut assoc_items: Vec<_> = tcx .inherent_impls(did) .iter() .flat_map(|&imp| { - tcx.associated_items(imp).find_by_name_and_namespace( + filter_assoc_items_by_name_and_namespace( tcx, + imp, Ident::with_dummy_span(item_name), ns, - imp, ) }) - .copied() - // There should only ever be one associated item that matches from any inherent impl - .next() + .map(|item| (root_res, item.def_id)) + .collect(); + + if assoc_items.is_empty() { // Check if item_name belongs to `impl SomeTrait for SomeItem` // FIXME(#74563): This gives precedence to `impl SomeItem`: // Although having both would be ambiguous, use impl version for compatibility's sake. // To handle that properly resolve() would have to support // something like [`ambi_fn`](::ambi_fn) - .or_else(|| { - resolve_associated_trait_item( - tcx.type_of(did).subst_identity(), - module_id, - item_name, - ns, - self.cx, - ) - }); + assoc_items = resolve_associated_trait_item( + tcx.type_of(did).subst_identity(), + module_id, + item_name, + ns, + self.cx, + ) + .into_iter() + .map(|item| (root_res, item.def_id)) + .collect::>(); + } - debug!("got associated item {:?}", assoc_item); + debug!("got associated item {:?}", assoc_items); - if let Some(item) = assoc_item { - return Some((root_res, item.def_id)); + if !assoc_items.is_empty() { + return assoc_items; } if ns != Namespace::ValueNS { - return None; + return Vec::new(); } debug!("looking for fields named {} for {:?}", item_name, did); // FIXME: this doesn't really belong in `associated_item` (maybe `variant_field` is better?) @@ -631,20 +668,27 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // field syntax) and are handled by the compiler's resolver. let def = match tcx.type_of(did).subst_identity().kind() { ty::Adt(def, _) if !def.is_enum() => def, - _ => return None, + _ => return Vec::new(), }; - let field = - def.non_enum_variant().fields.iter().find(|item| item.name == item_name)?; - Some((root_res, field.did)) + def.non_enum_variant() + .fields + .iter() + .filter(|field| field.name == item_name) + .map(|field| (root_res, field.did)) + .collect::>() } - Res::Def(DefKind::Trait, did) => tcx - .associated_items(did) - .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, did) - .map(|item| { - let res = Res::Def(item.kind.as_def_kind(), item.def_id); - (res, item.def_id) - }), - _ => None, + Res::Def(DefKind::Trait, did) => filter_assoc_items_by_name_and_namespace( + tcx, + did, + Ident::with_dummy_span(item_name), + ns, + ) + .map(|item| { + let res = Res::Def(item.kind.as_def_kind(), item.def_id); + (res, item.def_id) + }) + .collect::>(), + _ => Vec::new(), } } } @@ -664,7 +708,7 @@ fn resolve_associated_trait_item<'a>( item_name: Symbol, ns: Namespace, cx: &mut DocContext<'a>, -) -> Option { +) -> Vec { // FIXME: this should also consider blanket impls (`impl X for T`). Unfortunately // `get_auto_trait_and_blanket_impls` is broken because the caching behavior is wrong. In the // meantime, just don't look for these blanket impls. @@ -672,19 +716,26 @@ fn resolve_associated_trait_item<'a>( // Next consider explicit impls: `impl MyTrait for MyType` // Give precedence to inherent impls. let traits = trait_impls_for(cx, ty, module); + let tcx = cx.tcx; debug!("considering traits {:?}", traits); - let mut candidates = traits.iter().filter_map(|&(impl_, trait_)| { - cx.tcx - .associated_items(trait_) - .find_by_name_and_namespace(cx.tcx, Ident::with_dummy_span(item_name), ns, trait_) - .map(|trait_assoc| { - trait_assoc_to_impl_assoc_item(cx.tcx, impl_, trait_assoc.def_id) + let candidates = traits + .iter() + .flat_map(|&(impl_, trait_)| { + filter_assoc_items_by_name_and_namespace( + cx.tcx, + trait_, + Ident::with_dummy_span(item_name), + ns, + ) + .map(move |trait_assoc| { + trait_assoc_to_impl_assoc_item(tcx, impl_, trait_assoc.def_id) .unwrap_or(*trait_assoc) }) - }); + }) + .collect::>(); // FIXME(#74563): warn about ambiguity - debug!("the candidates were {:?}", candidates.clone().collect::>()); - candidates.next() + debug!("the candidates were {:?}", candidates); + candidates } /// Find the associated item in the impl `impl_id` that corresponds to the @@ -755,20 +806,6 @@ fn trait_impls_for<'a>( iter.collect() } -/// Check for resolve collisions between a trait and its derive. -/// -/// These are common and we should just resolve to the trait in that case. -fn is_derive_trait_collision(ns: &PerNS>>) -> bool { - matches!( - *ns, - PerNS { - type_ns: Ok((Res::Def(DefKind::Trait, _), _)), - macro_ns: Ok((Res::Def(DefKind::Macro(MacroKind::Derive), _), _)), - .. - } - ) -} - impl<'a, 'tcx> DocVisitor for LinkCollector<'a, 'tcx> { fn visit_item(&mut self, item: &Item) { self.resolve_links(item); @@ -978,7 +1015,7 @@ impl LinkCollector<'_, '_> { res = prim; } else { // `[char]` when a `char` module is in scope - let candidates = vec![res, prim]; + let candidates = vec![(res, res.def_id(self.cx.tcx)), (prim, None)]; ambiguity_error(self.cx, diag_info, path_str, candidates); return None; } @@ -986,7 +1023,7 @@ impl LinkCollector<'_, '_> { } match res { - Res::Primitive(prim) => { + Res::Primitive(_) => { if let Some(UrlFragment::Item(id)) = fragment { // We're actually resolving an associated item of a primitive, so we need to // verify the disambiguator (if any) matches the type of the associated item. @@ -1006,15 +1043,6 @@ impl LinkCollector<'_, '_> { item, &diag_info, )?; - - // FIXME: it would be nice to check that the feature gate was enabled in the original crate, not just ignore it altogether. - // However I'm not sure how to check that across crates. - if prim == PrimitiveType::RawPointer - && item.item_id.is_local() - && !self.cx.tcx.features().intra_doc_pointers - { - self.report_rawptr_assoc_feature_gate(dox, ori_link, item); - } } else { match disambiguator { Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => {} @@ -1135,10 +1163,9 @@ impl LinkCollector<'_, '_> { report_diagnostic(self.cx.tcx, BROKEN_INTRA_DOC_LINKS, &msg, diag_info, callback); } - fn report_rawptr_assoc_feature_gate(&self, dox: &str, ori_link: &MarkdownLink, item: &Item) { - let span = - super::source_span_for_markdown_range(self.cx.tcx, dox, &ori_link.range, &item.attrs) - .unwrap_or_else(|| item.attr_span(self.cx.tcx)); + fn report_rawptr_assoc_feature_gate(&self, dox: &str, ori_link: &Range, item: &Item) { + let span = super::source_span_for_markdown_range(self.cx.tcx, dox, ori_link, &item.attrs) + .unwrap_or_else(|| item.attr_span(self.cx.tcx)); rustc_session::parse::feature_err( &self.cx.tcx.sess.parse_sess, sym::intra_doc_pointers, @@ -1163,7 +1190,23 @@ impl LinkCollector<'_, '_> { } } - let res = self.resolve_with_disambiguator(&key, diag.clone()).and_then(|(res, def_id)| { + let mut candidates = self.resolve_with_disambiguator(&key, diag.clone()); + + // FIXME: it would be nice to check that the feature gate was enabled in the original crate, not just ignore it altogether. + // However I'm not sure how to check that across crates. + if let Some(candidate) = candidates.get(0) && + candidate.0 == Res::Primitive(PrimitiveType::RawPointer) && + key.path_str.contains("::") // We only want to check this if this is an associated item. + { + if key.item_id.is_local() && !self.cx.tcx.features().intra_doc_pointers { + self.report_rawptr_assoc_feature_gate(diag.dox, &diag.link_range, diag.item); + return None; + } else { + candidates = vec![candidates[0]]; + } + } + + if let &[(res, def_id)] = candidates.as_slice() { let fragment = match (&key.extra_fragment, def_id) { (Some(_), Some(def_id)) => { report_anchor_conflict(self.cx, diag, def_id); @@ -1173,13 +1216,18 @@ impl LinkCollector<'_, '_> { (None, Some(def_id)) => Some(UrlFragment::Item(def_id)), (None, None) => None, }; - Some((res, fragment)) - }); + let r = Some((res, fragment)); + self.visited_links.insert(key, r.clone()); + return r; + } - if res.is_some() || cache_errors { - self.visited_links.insert(key, res.clone()); + if !candidates.is_empty() { + ambiguity_error(self.cx, diag, &key.path_str, candidates); } - res + if cache_errors { + self.visited_links.insert(key, None); + } + None } /// After parsing the disambiguator, resolve the main part of the link. @@ -1188,7 +1236,7 @@ impl LinkCollector<'_, '_> { &mut self, key: &ResolutionInfo, diag: DiagnosticInfo<'_>, - ) -> Option<(Res, Option)> { + ) -> Vec<(Res, Option)> { let disambiguator = key.dis; let path_str = &key.path_str; let item_id = key.item_id; @@ -1197,7 +1245,7 @@ impl LinkCollector<'_, '_> { match disambiguator.map(Disambiguator::ns) { Some(expected_ns) => { match self.resolve(path_str, expected_ns, item_id, module_id) { - Ok(res) => Some(res), + Ok(candidates) => candidates, Err(err) => { // We only looked in one namespace. Try to give a better error if possible. // FIXME: really it should be `resolution_failure` that does this, not `resolve_with_disambiguator`. @@ -1206,10 +1254,11 @@ impl LinkCollector<'_, '_> { for other_ns in [TypeNS, ValueNS, MacroNS] { if other_ns != expected_ns { if let Ok(res) = - self.resolve(path_str, other_ns, item_id, module_id) + self.resolve(path_str, other_ns, item_id, module_id) && + !res.is_empty() { err = ResolutionFailure::WrongNamespace { - res: full_res(self.cx.tcx, res), + res: full_res(self.cx.tcx, res[0]), expected_ns, }; break; @@ -1230,18 +1279,26 @@ impl LinkCollector<'_, '_> { let candidates = PerNS { macro_ns: candidate(MacroNS), type_ns: candidate(TypeNS), - value_ns: candidate(ValueNS).and_then(|(res, def_id)| { - match res { - // Constructors are picked up in the type namespace. - Res::Def(DefKind::Ctor(..), _) => { - Err(ResolutionFailure::WrongNamespace { res, expected_ns: TypeNS }) + value_ns: candidate(ValueNS).and_then(|v_res| { + for (res, _) in v_res.iter() { + match res { + // Constructors are picked up in the type namespace. + Res::Def(DefKind::Ctor(..), _) => { + return Err(ResolutionFailure::WrongNamespace { + res: *res, + expected_ns: TypeNS, + }); + } + _ => {} } - _ => Ok((res, def_id)), } + Ok(v_res) }), }; - let len = candidates.iter().filter(|res| res.is_ok()).count(); + let len = candidates + .iter() + .fold(0, |acc, res| if let Ok(res) = res { acc + res.len() } else { acc }); if len == 0 { return resolution_failure( @@ -1253,21 +1310,7 @@ impl LinkCollector<'_, '_> { ); } - if len == 1 { - Some(candidates.into_iter().find_map(|res| res.ok()).unwrap()) - } else if len == 2 && is_derive_trait_collision(&candidates) { - Some(candidates.type_ns.unwrap()) - } else { - let ignore_macro = is_derive_trait_collision(&candidates); - // If we're reporting an ambiguity, don't mention the namespaces that failed - let mut candidates = - candidates.map(|candidate| candidate.ok().map(|(res, _)| res)); - if ignore_macro { - candidates.macro_ns = None; - } - ambiguity_error(self.cx, diag, path_str, candidates.present_items().collect()); - None - } + candidates.into_iter().filter_map(|res| res.ok()).flatten().collect::>() } } } @@ -1554,7 +1597,7 @@ fn resolution_failure( path_str: &str, disambiguator: Option, kinds: SmallVec<[ResolutionFailure<'_>; 3]>, -) -> Option<(Res, Option)> { +) -> Vec<(Res, Option)> { let tcx = collector.cx.tcx; let mut recovered_res = None; report_diagnostic( @@ -1613,11 +1656,13 @@ fn resolution_failure( }; name = start; for ns in [TypeNS, ValueNS, MacroNS] { - if let Ok(res) = collector.resolve(start, ns, item_id, module_id) { - debug!("found partial_res={:?}", res); - *partial_res = Some(full_res(collector.cx.tcx, res)); - *unresolved = end.into(); - break 'outer; + if let Ok(v_res) = collector.resolve(start, ns, item_id, module_id) { + debug!("found partial_res={:?}", v_res); + if !v_res.is_empty() { + *partial_res = Some(full_res(collector.cx.tcx, v_res[0])); + *unresolved = end.into(); + break 'outer; + } } } *unresolved = end.into(); @@ -1765,7 +1810,10 @@ fn resolution_failure( }, ); - recovered_res + match recovered_res { + Some(r) => vec![r], + None => Vec::new(), + } } fn report_multiple_anchors(cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>) { @@ -1854,24 +1902,32 @@ fn ambiguity_error( cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>, path_str: &str, - candidates: Vec, + candidates: Vec<(Res, Option)>, ) { let mut msg = format!("`{}` is ", path_str); + let kinds = candidates + .into_iter() + .map( + |(res, def_id)| { + if let Some(def_id) = def_id { Res::from_def_id(cx.tcx, def_id) } else { res } + }, + ) + .collect::>(); - match candidates.as_slice() { - [first_def, second_def] => { + match kinds.as_slice() { + [res1, res2] => { msg += &format!( "both {} {} and {} {}", - first_def.article(), - first_def.descr(), - second_def.article(), - second_def.descr(), + res1.article(), + res1.descr(), + res2.article(), + res2.descr() ); } _ => { - let mut candidates = candidates.iter().peekable(); - while let Some(res) = candidates.next() { - if candidates.peek().is_some() { + let mut kinds = kinds.iter().peekable(); + while let Some(res) = kinds.next() { + if kinds.peek().is_some() { msg += &format!("{} {}, ", res.article(), res.descr()); } else { msg += &format!("and {} {}", res.article(), res.descr()); @@ -1887,7 +1943,7 @@ fn ambiguity_error( diag.note("ambiguous link"); } - for res in candidates { + for res in kinds { suggest_disambiguator(res, diag, path_str, diag_info.ori_link, sp); } }); From e193950c3c087c1acba48e75f6ac27d3dad1586d Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 14 Mar 2023 15:06:27 +0100 Subject: [PATCH 2/7] Special case `ambiguity_error` if all candidates have the same "kind" --- .../passes/collect_intra_doc_links.rs | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 7e24318dec8a6..6df87eb01b2e1 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1016,7 +1016,7 @@ impl LinkCollector<'_, '_> { } else { // `[char]` when a `char` module is in scope let candidates = vec![(res, res.def_id(self.cx.tcx)), (prim, None)]; - ambiguity_error(self.cx, diag_info, path_str, candidates); + ambiguity_error(self.cx, &diag_info, path_str, &candidates); return None; } } @@ -1206,6 +1206,10 @@ impl LinkCollector<'_, '_> { } } + if candidates.len() > 1 && !ambiguity_error(self.cx, &diag, &key.path_str, &candidates) { + candidates = vec![candidates[0]]; + } + if let &[(res, def_id)] = candidates.as_slice() { let fragment = match (&key.extra_fragment, def_id) { (Some(_), Some(def_id)) => { @@ -1221,9 +1225,6 @@ impl LinkCollector<'_, '_> { return r; } - if !candidates.is_empty() { - ambiguity_error(self.cx, diag, &key.path_str, candidates); - } if cache_errors { self.visited_links.insert(key, None); } @@ -1898,21 +1899,30 @@ fn report_malformed_generics( } /// Report an ambiguity error, where there were multiple possible resolutions. +/// +/// If all `candidates` have the same kind, it's not possible to disambiguate so in this case, +/// the function returns `false`. Otherwise, it'll emit the error and return `true`. fn ambiguity_error( cx: &DocContext<'_>, - diag_info: DiagnosticInfo<'_>, + diag_info: &DiagnosticInfo<'_>, path_str: &str, - candidates: Vec<(Res, Option)>, -) { + candidates: &[(Res, Option)], +) -> bool { let mut msg = format!("`{}` is ", path_str); let kinds = candidates - .into_iter() + .iter() .map( |(res, def_id)| { - if let Some(def_id) = def_id { Res::from_def_id(cx.tcx, def_id) } else { res } + if let Some(def_id) = def_id { Res::from_def_id(cx.tcx, *def_id) } else { *res } }, ) .collect::>(); + let descrs = kinds.iter().map(|res| res.descr()).collect::>(); + if descrs.len() == 1 { + // There is no way for users to disambiguate at this point, so better return the first + // candidate and not show a warning. + return false; + } match kinds.as_slice() { [res1, res2] => { @@ -1936,7 +1946,7 @@ fn ambiguity_error( } } - report_diagnostic(cx.tcx, BROKEN_INTRA_DOC_LINKS, &msg, &diag_info, |diag, sp| { + report_diagnostic(cx.tcx, BROKEN_INTRA_DOC_LINKS, &msg, diag_info, |diag, sp| { if let Some(sp) = sp { diag.span_label(sp, "ambiguous link"); } else { @@ -1947,6 +1957,7 @@ fn ambiguity_error( suggest_disambiguator(res, diag, path_str, diag_info.ori_link, sp); } }); + true } /// In case of an ambiguity or mismatched disambiguator, suggest the correct From c78ebab2580b9532ad58facc1f5731a3a1cfa42a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 3 Mar 2023 14:54:56 +0100 Subject: [PATCH 3/7] Add regression tests for #108653 --- tests/rustdoc-ui/intra-doc/ambiguity.stderr | 50 +++++++------- .../issue-108653-associated-items-2.rs | 17 +++++ .../issue-108653-associated-items-2.stderr | 37 ++++++++++ .../issue-108653-associated-items-3.rs | 16 +++++ .../issue-108653-associated-items-3.stderr | 37 ++++++++++ .../issue-108653-associated-items-4.rs | 21 ++++++ .../issue-108653-associated-items-4.stderr | 22 ++++++ .../issue-108653-associated-items-5.rs | 8 +++ .../issue-108653-associated-items-5.stderr | 22 ++++++ .../issue-108653-associated-items-6.rs | 8 +++ .../issue-108653-associated-items-6.stderr | 22 ++++++ .../issue-108653-associated-items-7.rs | 12 ++++ .../issue-108653-associated-items-7.stderr | 22 ++++++ .../issue-108653-associated-items-8.rs | 12 ++++ .../issue-108653-associated-items-8.stderr | 22 ++++++ .../issue-108653-associated-items-9.rs | 11 +++ .../issue-108653-associated-items.rs | 35 ++++++++++ .../issue-108653-associated-items.stderr | 67 +++++++++++++++++++ 18 files changed, 416 insertions(+), 25 deletions(-) create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.rs create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.stderr create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.rs create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.stderr create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.rs create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.stderr create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-5.rs create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-5.stderr create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.rs create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.stderr create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-7.rs create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-7.stderr create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-8.rs create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-8.stderr create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-9.rs create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items.rs create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items.stderr diff --git a/tests/rustdoc-ui/intra-doc/ambiguity.stderr b/tests/rustdoc-ui/intra-doc/ambiguity.stderr index 7974796e47b17..9e12bb5efc3fe 100644 --- a/tests/rustdoc-ui/intra-doc/ambiguity.stderr +++ b/tests/rustdoc-ui/intra-doc/ambiguity.stderr @@ -18,84 +18,84 @@ help: to link to the builtin type, prefix with `prim@` LL | /// [prim@true] | +++++ -error: `ambiguous` is both a struct and a function +error: `ambiguous` is both a function and a struct --> $DIR/ambiguity.rs:27:7 | LL | /// [`ambiguous`] is ambiguous. | ^^^^^^^^^ ambiguous link | -help: to link to the struct, prefix with `struct@` - | -LL | /// [`struct@ambiguous`] is ambiguous. - | +++++++ help: to link to the function, add parentheses | LL | /// [`ambiguous()`] is ambiguous. | ++ +help: to link to the struct, prefix with `struct@` + | +LL | /// [`struct@ambiguous`] is ambiguous. + | +++++++ -error: `ambiguous` is both a struct and a function +error: `ambiguous` is both a function and a struct --> $DIR/ambiguity.rs:29:6 | LL | /// [ambiguous] is ambiguous. | ^^^^^^^^^ ambiguous link | -help: to link to the struct, prefix with `struct@` - | -LL | /// [struct@ambiguous] is ambiguous. - | +++++++ help: to link to the function, add parentheses | LL | /// [ambiguous()] is ambiguous. | ++ +help: to link to the struct, prefix with `struct@` + | +LL | /// [struct@ambiguous] is ambiguous. + | +++++++ -error: `multi_conflict` is a struct, a function, and a macro +error: `multi_conflict` is a function, a struct, and a macro --> $DIR/ambiguity.rs:31:7 | LL | /// [`multi_conflict`] is a three-way conflict. | ^^^^^^^^^^^^^^ ambiguous link | -help: to link to the struct, prefix with `struct@` - | -LL | /// [`struct@multi_conflict`] is a three-way conflict. - | +++++++ help: to link to the function, add parentheses | LL | /// [`multi_conflict()`] is a three-way conflict. | ++ +help: to link to the struct, prefix with `struct@` + | +LL | /// [`struct@multi_conflict`] is a three-way conflict. + | +++++++ help: to link to the macro, add an exclamation mark | LL | /// [`multi_conflict!`] is a three-way conflict. | + -error: `type_and_value` is both a module and a constant +error: `type_and_value` is both a constant and a module --> $DIR/ambiguity.rs:33:16 | LL | /// Ambiguous [type_and_value]. | ^^^^^^^^^^^^^^ ambiguous link | -help: to link to the module, prefix with `mod@` - | -LL | /// Ambiguous [mod@type_and_value]. - | ++++ help: to link to the constant, prefix with `const@` | LL | /// Ambiguous [const@type_and_value]. | ++++++ +help: to link to the module, prefix with `mod@` + | +LL | /// Ambiguous [mod@type_and_value]. + | ++++ -error: `foo::bar` is both an enum and a function +error: `foo::bar` is both a function and an enum --> $DIR/ambiguity.rs:35:43 | LL | /// Ambiguous non-implied shortcut link [`foo::bar`]. | ^^^^^^^^ ambiguous link | -help: to link to the enum, prefix with `enum@` - | -LL | /// Ambiguous non-implied shortcut link [`enum@foo::bar`]. - | +++++ help: to link to the function, add parentheses | LL | /// Ambiguous non-implied shortcut link [`foo::bar()`]. | ++ +help: to link to the enum, prefix with `enum@` + | +LL | /// Ambiguous non-implied shortcut link [`enum@foo::bar`]. + | +++++ error: aborting due to 6 previous errors diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.rs new file mode 100644 index 0000000000000..cbe60f746b682 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.rs @@ -0,0 +1,17 @@ +// This is ensuring that the UI output for associated items is as expected. + +#![deny(rustdoc::broken_intra_doc_links)] + +/// [`Trait::IDENT`] +//~^ ERROR both an associated constant and an associated type +pub trait Trait { + type IDENT; + const IDENT: usize; +} + +/// [`Trait2::IDENT`] +//~^ ERROR both an associated function and an associated type +pub trait Trait2 { + type IDENT; + fn IDENT() {} +} diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.stderr new file mode 100644 index 0000000000000..952392548da8a --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.stderr @@ -0,0 +1,37 @@ +error: `Trait::IDENT` is both an associated constant and an associated type + --> $DIR/issue-108653-associated-items-2.rs:5:7 + | +LL | /// [`Trait::IDENT`] + | ^^^^^^^^^^^^ ambiguous link + | +note: the lint level is defined here + --> $DIR/issue-108653-associated-items-2.rs:3:9 + | +LL | #![deny(rustdoc::broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@Trait::IDENT`] + | ++++++ +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Trait::IDENT`] + | +++++ + +error: `Trait2::IDENT` is both an associated function and an associated type + --> $DIR/issue-108653-associated-items-2.rs:12:7 + | +LL | /// [`Trait2::IDENT`] + | ^^^^^^^^^^^^^ ambiguous link + | +help: to link to the associated function, add parentheses + | +LL | /// [`Trait2::IDENT()`] + | ++ +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Trait2::IDENT`] + | +++++ + +error: aborting due to 2 previous errors + diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.rs new file mode 100644 index 0000000000000..7ffd0a40e7cfe --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.rs @@ -0,0 +1,16 @@ +// This is ensuring that the UI output for associated items works when it's being documented +// from another item. + +#![deny(rustdoc::broken_intra_doc_links)] +#![allow(nonstandard_style)] + +pub trait Trait { + type Trait; + const Trait: usize; +} + +/// [`Trait`] +//~^ ERROR both a constant and a trait +/// [`Trait::Trait`] +//~^ ERROR both an associated constant and an associated type +pub const Trait: usize = 0; diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.stderr new file mode 100644 index 0000000000000..6401dacb57a8e --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.stderr @@ -0,0 +1,37 @@ +error: `Trait` is both a constant and a trait + --> $DIR/issue-108653-associated-items-3.rs:12:7 + | +LL | /// [`Trait`] + | ^^^^^ ambiguous link + | +note: the lint level is defined here + --> $DIR/issue-108653-associated-items-3.rs:4:9 + | +LL | #![deny(rustdoc::broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the constant, prefix with `const@` + | +LL | /// [`const@Trait`] + | ++++++ +help: to link to the trait, prefix with `trait@` + | +LL | /// [`trait@Trait`] + | ++++++ + +error: `Trait::Trait` is both an associated constant and an associated type + --> $DIR/issue-108653-associated-items-3.rs:14:7 + | +LL | /// [`Trait::Trait`] + | ^^^^^^^^^^^^ ambiguous link + | +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@Trait::Trait`] + | ++++++ +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Trait::Trait`] + | +++++ + +error: aborting due to 2 previous errors + diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.rs new file mode 100644 index 0000000000000..537d61364bb1a --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.rs @@ -0,0 +1,21 @@ +// This is ensuring that the UI output for associated items works when it's being documented +// from another item. + +#![deny(rustdoc::broken_intra_doc_links)] +#![allow(nonstandard_style)] + +pub trait Trait { + type Trait; +} + +/// [`Struct::Trait`] +//~^ ERROR both an associated constant and an associated type +pub struct Struct; + +impl Trait for Struct { + type Trait = Struct; +} + +impl Struct { + pub const Trait: usize = 0; +} diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.stderr new file mode 100644 index 0000000000000..a8dc91204c083 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.stderr @@ -0,0 +1,22 @@ +error: `Struct::Trait` is both an associated constant and an associated type + --> $DIR/issue-108653-associated-items-4.rs:11:7 + | +LL | /// [`Struct::Trait`] + | ^^^^^^^^^^^^^ ambiguous link + | +note: the lint level is defined here + --> $DIR/issue-108653-associated-items-4.rs:4:9 + | +LL | #![deny(rustdoc::broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@Struct::Trait`] + | ++++++ +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Struct::Trait`] + | +++++ + +error: aborting due to previous error + diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-5.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-5.rs new file mode 100644 index 0000000000000..bc28bc5442181 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-5.rs @@ -0,0 +1,8 @@ +#![deny(rustdoc::broken_intra_doc_links)] +#![allow(nonstandard_style)] + +/// [`u32::MAX`] +//~^ ERROR both an associated constant and a trait +pub mod u32 { + pub trait MAX {} +} diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-5.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-5.stderr new file mode 100644 index 0000000000000..7430044ac3f1b --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-5.stderr @@ -0,0 +1,22 @@ +error: `u32::MAX` is both an associated constant and a trait + --> $DIR/issue-108653-associated-items-5.rs:4:7 + | +LL | /// [`u32::MAX`] + | ^^^^^^^^ ambiguous link + | +note: the lint level is defined here + --> $DIR/issue-108653-associated-items-5.rs:1:9 + | +LL | #![deny(rustdoc::broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@u32::MAX`] + | ++++++ +help: to link to the trait, prefix with `trait@` + | +LL | /// [`trait@u32::MAX`] + | ++++++ + +error: aborting due to previous error + diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.rs new file mode 100644 index 0000000000000..d5ba259d54b89 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.rs @@ -0,0 +1,8 @@ +#![deny(rustdoc::broken_intra_doc_links)] +#![allow(nonstandard_style)] + +/// [`u32::MAX`] +//~^ ERROR both an associated constant and a builtin type +pub mod u32 { + pub use std::primitive::u32 as MAX; +} diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.stderr new file mode 100644 index 0000000000000..af138a7568279 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.stderr @@ -0,0 +1,22 @@ +error: `u32::MAX` is both an associated constant and a builtin type + --> $DIR/issue-108653-associated-items-6.rs:4:7 + | +LL | /// [`u32::MAX`] + | ^^^^^^^^ ambiguous link + | +note: the lint level is defined here + --> $DIR/issue-108653-associated-items-6.rs:1:9 + | +LL | #![deny(rustdoc::broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@u32::MAX`] + | ++++++ +help: to link to the builtin type, prefix with `prim@` + | +LL | /// [`prim@u32::MAX`] + | +++++ + +error: aborting due to previous error + diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-7.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-7.rs new file mode 100644 index 0000000000000..6e99f4365a78d --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-7.rs @@ -0,0 +1,12 @@ +#![deny(rustdoc::broken_intra_doc_links)] +#![allow(nonstandard_style)] + +pub trait Trait { + type MAX; +} + +/// [`u32::MAX`] +//~^ ERROR both an associated constant and an associated type +impl Trait for u32 { + type MAX = u32; +} diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-7.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-7.stderr new file mode 100644 index 0000000000000..1d302ff42e86f --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-7.stderr @@ -0,0 +1,22 @@ +error: `u32::MAX` is both an associated constant and an associated type + --> $DIR/issue-108653-associated-items-7.rs:8:7 + | +LL | /// [`u32::MAX`] + | ^^^^^^^^ ambiguous link + | +note: the lint level is defined here + --> $DIR/issue-108653-associated-items-7.rs:1:9 + | +LL | #![deny(rustdoc::broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@u32::MAX`] + | ++++++ +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@u32::MAX`] + | +++++ + +error: aborting due to previous error + diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-8.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-8.rs new file mode 100644 index 0000000000000..2f8ee1566bd4e --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-8.rs @@ -0,0 +1,12 @@ +#![deny(rustdoc::broken_intra_doc_links)] +#![allow(nonstandard_style)] + +/// [`u32::MAX`] +//~^ ERROR both an associated constant and an associated type +pub trait T { + type MAX; +} + +impl T for u32 { + type MAX = (); +} diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-8.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-8.stderr new file mode 100644 index 0000000000000..efed0e2ce0ff5 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-8.stderr @@ -0,0 +1,22 @@ +error: `u32::MAX` is both an associated constant and an associated type + --> $DIR/issue-108653-associated-items-8.rs:4:7 + | +LL | /// [`u32::MAX`] + | ^^^^^^^^ ambiguous link + | +note: the lint level is defined here + --> $DIR/issue-108653-associated-items-8.rs:1:9 + | +LL | #![deny(rustdoc::broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@u32::MAX`] + | ++++++ +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@u32::MAX`] + | +++++ + +error: aborting due to previous error + diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-9.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-9.rs new file mode 100644 index 0000000000000..3357ccf2460d8 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-9.rs @@ -0,0 +1,11 @@ +// check-pass + +#![deny(warnings)] + +//! [usize::Item] + +pub trait Foo { type Item; } +pub trait Bar { type Item; } + +impl Foo for usize { type Item = u32; } +impl Bar for usize { type Item = i32; } diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.rs new file mode 100644 index 0000000000000..0a393e26d6a96 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.rs @@ -0,0 +1,35 @@ +// This is ensuring that the UI output for associated items is as expected. + +#![deny(rustdoc::broken_intra_doc_links)] + +pub enum Enum { + IDENT, +} + +/// [`Self::IDENT`] +//~^ ERROR both an associated function and an associated type +pub trait Trait { + type IDENT; + fn IDENT(); +} + +/// [`Self::IDENT`] +//~^ ERROR both an associated function and a variant +impl Trait for Enum { + type IDENT = usize; + fn IDENT() {} +} + +/// [`Self::IDENT2`] +//~^ ERROR both an associated constant and an associated type +pub trait Trait2 { + type IDENT2; + const IDENT2: usize; +} + +/// [`Self::IDENT2`] +//~^ ERROR both an associated constant and an associated type +impl Trait2 for Enum { + type IDENT2 = usize; + const IDENT2: usize = 0; +} diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.stderr new file mode 100644 index 0000000000000..084aefc97c834 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.stderr @@ -0,0 +1,67 @@ +error: `Self::IDENT` is both an associated function and an associated type + --> $DIR/issue-108653-associated-items.rs:9:7 + | +LL | /// [`Self::IDENT`] + | ^^^^^^^^^^^ ambiguous link + | +note: the lint level is defined here + --> $DIR/issue-108653-associated-items.rs:3:9 + | +LL | #![deny(rustdoc::broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the associated function, add parentheses + | +LL | /// [`Self::IDENT()`] + | ++ +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Self::IDENT`] + | +++++ + +error: `Self::IDENT2` is both an associated constant and an associated type + --> $DIR/issue-108653-associated-items.rs:23:7 + | +LL | /// [`Self::IDENT2`] + | ^^^^^^^^^^^^ ambiguous link + | +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@Self::IDENT2`] + | ++++++ +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Self::IDENT2`] + | +++++ + +error: `Self::IDENT2` is both an associated constant and an associated type + --> $DIR/issue-108653-associated-items.rs:30:7 + | +LL | /// [`Self::IDENT2`] + | ^^^^^^^^^^^^ ambiguous link + | +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@Self::IDENT2`] + | ++++++ +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Self::IDENT2`] + | +++++ + +error: `Self::IDENT` is both an associated function and a variant + --> $DIR/issue-108653-associated-items.rs:16:7 + | +LL | /// [`Self::IDENT`] + | ^^^^^^^^^^^ ambiguous link + | +help: to link to the associated function, add parentheses + | +LL | /// [`Self::IDENT()`] + | ++ +help: to link to the variant, prefix with `type@` + | +LL | /// [`type@Self::IDENT`] + | +++++ + +error: aborting due to 4 previous errors + From 95926b2ce59b18ac1e9ec9eeca00cc9023c05395 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 24 Mar 2023 14:47:56 +0100 Subject: [PATCH 4/7] Rename description of primitive from "builtin type" into "primitive type" --- src/librustdoc/passes/collect_intra_doc_links.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 6df87eb01b2e1..795116e255331 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -70,7 +70,7 @@ impl Res { fn descr(self) -> &'static str { match self { Res::Def(kind, id) => ResolveRes::Def(kind, id).descr(), - Res::Primitive(_) => "builtin type", + Res::Primitive(_) => "primitive type", } } From ec43cb3e9c208939e09b4208850aaa34bee0423c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 24 Mar 2023 14:50:12 +0100 Subject: [PATCH 5/7] Update UI tests for primitive type ambiguity error renaming --- tests/rustdoc-ui/intra-doc/ambiguity.rs | 2 +- tests/rustdoc-ui/intra-doc/ambiguity.stderr | 4 ++-- tests/rustdoc-ui/intra-doc/errors.rs | 4 ++-- tests/rustdoc-ui/intra-doc/errors.stderr | 4 ++-- .../intra-doc/issue-108653-associated-items-6.rs | 2 +- .../intra-doc/issue-108653-associated-items-6.stderr | 4 ++-- .../rustdoc-ui/intra-doc/non-path-primitives.stderr | 8 ++++---- tests/rustdoc-ui/intra-doc/prim-conflict.rs | 10 +++++----- tests/rustdoc-ui/intra-doc/prim-conflict.stderr | 12 ++++++------ 9 files changed, 25 insertions(+), 25 deletions(-) diff --git a/tests/rustdoc-ui/intra-doc/ambiguity.rs b/tests/rustdoc-ui/intra-doc/ambiguity.rs index 1f3dc722eff8d..0290b8582042c 100644 --- a/tests/rustdoc-ui/intra-doc/ambiguity.rs +++ b/tests/rustdoc-ui/intra-doc/ambiguity.rs @@ -35,6 +35,6 @@ pub mod foo { /// Ambiguous non-implied shortcut link [`foo::bar`]. //~ERROR `foo::bar` pub struct Docs {} -/// [true] //~ ERROR `true` is both a module and a builtin type +/// [true] //~ ERROR `true` is both a module and a primitive type /// [primitive@true] pub mod r#true {} diff --git a/tests/rustdoc-ui/intra-doc/ambiguity.stderr b/tests/rustdoc-ui/intra-doc/ambiguity.stderr index 9e12bb5efc3fe..47853e0b5899d 100644 --- a/tests/rustdoc-ui/intra-doc/ambiguity.stderr +++ b/tests/rustdoc-ui/intra-doc/ambiguity.stderr @@ -1,4 +1,4 @@ -error: `true` is both a module and a builtin type +error: `true` is both a module and a primitive type --> $DIR/ambiguity.rs:38:6 | LL | /// [true] @@ -13,7 +13,7 @@ help: to link to the module, prefix with `mod@` | LL | /// [mod@true] | ++++ -help: to link to the builtin type, prefix with `prim@` +help: to link to the primitive type, prefix with `prim@` | LL | /// [prim@true] | +++++ diff --git a/tests/rustdoc-ui/intra-doc/errors.rs b/tests/rustdoc-ui/intra-doc/errors.rs index 95dd2b98e037e..f37f49c24ccc5 100644 --- a/tests/rustdoc-ui/intra-doc/errors.rs +++ b/tests/rustdoc-ui/intra-doc/errors.rs @@ -54,11 +54,11 @@ /// [u8::not_found] //~^ ERROR unresolved link -//~| NOTE the builtin type `u8` has no associated item named `not_found` +//~| NOTE the primitive type `u8` has no associated item named `not_found` /// [std::primitive::u8::not_found] //~^ ERROR unresolved link -//~| NOTE the builtin type `u8` has no associated item named `not_found` +//~| NOTE the primitive type `u8` has no associated item named `not_found` /// [type@Vec::into_iter] //~^ ERROR unresolved link diff --git a/tests/rustdoc-ui/intra-doc/errors.stderr b/tests/rustdoc-ui/intra-doc/errors.stderr index 1b2416d7da765..a982bba009591 100644 --- a/tests/rustdoc-ui/intra-doc/errors.stderr +++ b/tests/rustdoc-ui/intra-doc/errors.stderr @@ -80,13 +80,13 @@ error: unresolved link to `u8::not_found` --> $DIR/errors.rs:55:6 | LL | /// [u8::not_found] - | ^^^^^^^^^^^^^ the builtin type `u8` has no associated item named `not_found` + | ^^^^^^^^^^^^^ the primitive type `u8` has no associated item named `not_found` error: unresolved link to `std::primitive::u8::not_found` --> $DIR/errors.rs:59:6 | LL | /// [std::primitive::u8::not_found] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the builtin type `u8` has no associated item named `not_found` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the primitive type `u8` has no associated item named `not_found` error: unresolved link to `Vec::into_iter` --> $DIR/errors.rs:63:6 diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.rs index d5ba259d54b89..8fde74d0ddb47 100644 --- a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.rs +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.rs @@ -2,7 +2,7 @@ #![allow(nonstandard_style)] /// [`u32::MAX`] -//~^ ERROR both an associated constant and a builtin type +//~^ ERROR both an associated constant and a primitive type pub mod u32 { pub use std::primitive::u32 as MAX; } diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.stderr index af138a7568279..fe2d8cafa3026 100644 --- a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.stderr +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-6.stderr @@ -1,4 +1,4 @@ -error: `u32::MAX` is both an associated constant and a builtin type +error: `u32::MAX` is both an associated constant and a primitive type --> $DIR/issue-108653-associated-items-6.rs:4:7 | LL | /// [`u32::MAX`] @@ -13,7 +13,7 @@ help: to link to the associated constant, prefix with `const@` | LL | /// [`const@u32::MAX`] | ++++++ -help: to link to the builtin type, prefix with `prim@` +help: to link to the primitive type, prefix with `prim@` | LL | /// [`prim@u32::MAX`] | +++++ diff --git a/tests/rustdoc-ui/intra-doc/non-path-primitives.stderr b/tests/rustdoc-ui/intra-doc/non-path-primitives.stderr index 8ec894d101b6c..6e08a923963c4 100644 --- a/tests/rustdoc-ui/intra-doc/non-path-primitives.stderr +++ b/tests/rustdoc-ui/intra-doc/non-path-primitives.stderr @@ -39,25 +39,25 @@ error: unresolved link to `unit::eq` --> $DIR/non-path-primitives.rs:28:6 | LL | //! [unit::eq] - | ^^^^^^^^ the builtin type `unit` has no associated item named `eq` + | ^^^^^^^^ the primitive type `unit` has no associated item named `eq` error: unresolved link to `tuple::eq` --> $DIR/non-path-primitives.rs:29:6 | LL | //! [tuple::eq] - | ^^^^^^^^^ the builtin type `tuple` has no associated item named `eq` + | ^^^^^^^^^ the primitive type `tuple` has no associated item named `eq` error: unresolved link to `fn::eq` --> $DIR/non-path-primitives.rs:30:6 | LL | //! [fn::eq] - | ^^^^^^ the builtin type `fn` has no associated item named `eq` + | ^^^^^^ the primitive type `fn` has no associated item named `eq` error: unresolved link to `reference::deref` --> $DIR/non-path-primitives.rs:34:6 | LL | //! [reference::deref] - | ^^^^^^^^^^^^^^^^ the builtin type `reference` has no associated item named `deref` + | ^^^^^^^^^^^^^^^^ the primitive type `reference` has no associated item named `deref` error: aborting due to 8 previous errors diff --git a/tests/rustdoc-ui/intra-doc/prim-conflict.rs b/tests/rustdoc-ui/intra-doc/prim-conflict.rs index 2c1a8b5357aa1..e87ce095cd434 100644 --- a/tests/rustdoc-ui/intra-doc/prim-conflict.rs +++ b/tests/rustdoc-ui/intra-doc/prim-conflict.rs @@ -2,16 +2,16 @@ //~^ NOTE lint level is defined /// [char] -//~^ ERROR both a module and a builtin type +//~^ ERROR both a module and a primitive type //~| NOTE ambiguous link //~| HELP to link to the module -//~| HELP to link to the builtin type +//~| HELP to link to the primitive type /// [type@char] -//~^ ERROR both a module and a builtin type +//~^ ERROR both a module and a primitive type //~| NOTE ambiguous link //~| HELP to link to the module -//~| HELP to link to the builtin type +//~| HELP to link to the primitive type /// [mod@char] // ok /// [prim@char] // ok @@ -26,5 +26,5 @@ pub mod inner { //! [struct@char] //~^ ERROR incompatible link //~| HELP prefix with `prim@` - //~| NOTE resolved to a builtin type + //~| NOTE resolved to a primitive type } diff --git a/tests/rustdoc-ui/intra-doc/prim-conflict.stderr b/tests/rustdoc-ui/intra-doc/prim-conflict.stderr index 6ef3b7eab3baf..03ce8f15f0a5e 100644 --- a/tests/rustdoc-ui/intra-doc/prim-conflict.stderr +++ b/tests/rustdoc-ui/intra-doc/prim-conflict.stderr @@ -1,4 +1,4 @@ -error: `char` is both a module and a builtin type +error: `char` is both a module and a primitive type --> $DIR/prim-conflict.rs:4:6 | LL | /// [char] @@ -13,12 +13,12 @@ help: to link to the module, prefix with `mod@` | LL | /// [mod@char] | ++++ -help: to link to the builtin type, prefix with `prim@` +help: to link to the primitive type, prefix with `prim@` | LL | /// [prim@char] | +++++ -error: `char` is both a module and a builtin type +error: `char` is both a module and a primitive type --> $DIR/prim-conflict.rs:10:6 | LL | /// [type@char] @@ -28,7 +28,7 @@ help: to link to the module, prefix with `mod@` | LL | /// [mod@char] | ~~~~ -help: to link to the builtin type, prefix with `prim@` +help: to link to the primitive type, prefix with `prim@` | LL | /// [prim@char] | ~~~~~ @@ -48,9 +48,9 @@ error: incompatible link kind for `char` --> $DIR/prim-conflict.rs:26:10 | LL | //! [struct@char] - | ^^^^^^^^^^^ this link resolved to a builtin type, which is not a struct + | ^^^^^^^^^^^ this link resolved to a primitive type, which is not a struct | -help: to link to the builtin type, prefix with `prim@` +help: to link to the primitive type, prefix with `prim@` | LL | //! [prim@char] | ~~~~~ From 537fdbd75b13d278645fa223b0e4bee1c0e5dc19 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 24 Mar 2023 15:06:03 +0100 Subject: [PATCH 6/7] Strenghten disambiguation in `ambiguity_error` and improve documentation --- src/librustdoc/passes/collect_intra_doc_links.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 795116e255331..80895893ae86b 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1015,8 +1015,8 @@ impl LinkCollector<'_, '_> { res = prim; } else { // `[char]` when a `char` module is in scope - let candidates = vec![(res, res.def_id(self.cx.tcx)), (prim, None)]; - ambiguity_error(self.cx, &diag_info, path_str, &candidates); + let candidates = &[(res, res.def_id(self.cx.tcx)), (prim, None)]; + ambiguity_error(self.cx, &diag_info, path_str, candidates); return None; } } @@ -1206,6 +1206,10 @@ impl LinkCollector<'_, '_> { } } + // If there are multiple items with the same "kind" (for example, both "associated types") + // and after removing duplicated kinds, only one remains, the `ambiguity_error` function + // won't emit an error. So at this point, we can just take the first candidate as it was + // the first retrieved and use it to generate the link. if candidates.len() > 1 && !ambiguity_error(self.cx, &diag, &key.path_str, &candidates) { candidates = vec![candidates[0]]; } @@ -1901,14 +1905,15 @@ fn report_malformed_generics( /// Report an ambiguity error, where there were multiple possible resolutions. /// /// If all `candidates` have the same kind, it's not possible to disambiguate so in this case, -/// the function returns `false`. Otherwise, it'll emit the error and return `true`. +/// the function won't emit an error and will return `false`. Otherwise, it'll emit the error and +/// return `true`. fn ambiguity_error( cx: &DocContext<'_>, diag_info: &DiagnosticInfo<'_>, path_str: &str, candidates: &[(Res, Option)], ) -> bool { - let mut msg = format!("`{}` is ", path_str); + let mut descrs = FxHashSet::default(); let kinds = candidates .iter() .map( @@ -1916,14 +1921,15 @@ fn ambiguity_error( if let Some(def_id) = def_id { Res::from_def_id(cx.tcx, *def_id) } else { *res } }, ) + .filter(|res| descrs.insert(res.descr())) .collect::>(); - let descrs = kinds.iter().map(|res| res.descr()).collect::>(); if descrs.len() == 1 { // There is no way for users to disambiguate at this point, so better return the first // candidate and not show a warning. return false; } + let mut msg = format!("`{}` is ", path_str); match kinds.as_slice() { [res1, res2] => { msg += &format!( From 415a3ca909084e83441306b21db5232ee8952fc4 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 24 Mar 2023 16:10:31 +0100 Subject: [PATCH 7/7] Put back `is_derive_trait_collision` check --- .../passes/collect_intra_doc_links.rs | 31 +++++++++++++++++-- .../issue-108653-associated-items-10.rs | 22 +++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-10.rs diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 80895893ae86b..0da56e70ed582 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -806,6 +806,20 @@ fn trait_impls_for<'a>( iter.collect() } +/// Check for resolve collisions between a trait and its derive. +/// +/// These are common and we should just resolve to the trait in that case. +fn is_derive_trait_collision(ns: &PerNS, ResolutionFailure<'_>>>) -> bool { + if let (&Ok(ref type_ns), &Ok(ref macro_ns)) = (&ns.type_ns, &ns.macro_ns) { + type_ns.iter().any(|(res, _)| matches!(res, Res::Def(DefKind::Trait, _))) + && macro_ns + .iter() + .any(|(res, _)| matches!(res, Res::Def(DefKind::Macro(MacroKind::Derive), _))) + } else { + false + } +} + impl<'a, 'tcx> DocVisitor for LinkCollector<'a, 'tcx> { fn visit_item(&mut self, item: &Item) { self.resolve_links(item); @@ -1313,9 +1327,22 @@ impl LinkCollector<'_, '_> { disambiguator, candidates.into_iter().filter_map(|res| res.err()).collect(), ); + } else if len == 1 { + candidates.into_iter().filter_map(|res| res.ok()).flatten().collect::>() + } else { + let has_derive_trait_collision = is_derive_trait_collision(&candidates); + if len == 2 && has_derive_trait_collision { + candidates.type_ns.unwrap() + } else { + // If we're reporting an ambiguity, don't mention the namespaces that failed + let mut candidates = candidates.map(|candidate| candidate.ok()); + // If there a collision between a trait and a derive, we ignore the derive. + if has_derive_trait_collision { + candidates.macro_ns = None; + } + candidates.into_iter().filter_map(|res| res).flatten().collect::>() + } } - - candidates.into_iter().filter_map(|res| res.ok()).flatten().collect::>() } } } diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-10.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-10.rs new file mode 100644 index 0000000000000..464c5f0d5439c --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-10.rs @@ -0,0 +1,22 @@ +// This test ensures that this warning doesn't show up: +// warning: `PartialEq` is both a trait and a derive macro +// --> tests/rustdoc-ui/intra-doc/issue-108653-associated-items-10.rs:1:7 +// | +// 1 | //! [`PartialEq`] +// | ^^^^^^^^^ ambiguous link +// | +// = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default +// help: to link to the trait, prefix with `trait@` +// | +// 1 | //! [`trait@PartialEq`] +// | ++++++ +// help: to link to the derive macro, prefix with `derive@` +// | +// 1 | //! [`derive@PartialEq`] +// | +++++++ + +// check-pass + +#![deny(rustdoc::broken_intra_doc_links)] + +//! [`PartialEq`]