Skip to content

Commit 2e9788b

Browse files
authored
Rollup merge of #80660 - max-heller:issue-80559-fix, r=jyn514
Properly handle primitive disambiguators in rustdoc Fixes #80559 r? ``@jyn514`` Is there a way to test that the generated intra-doc link is what I expect?
2 parents d02b31c + 2bdbb0d commit 2e9788b

File tree

4 files changed

+92
-46
lines changed

4 files changed

+92
-46
lines changed

src/librustdoc/passes/collect_intra_doc_links.rs

+70-46
Original file line numberDiff line numberDiff line change
@@ -394,10 +394,14 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
394394
ns,
395395
impl_,
396396
)
397-
.map(|item| match item.kind {
398-
ty::AssocKind::Fn => "method",
399-
ty::AssocKind::Const => "associatedconstant",
400-
ty::AssocKind::Type => "associatedtype",
397+
.map(|item| {
398+
let kind = item.kind;
399+
self.kind_side_channel.set(Some((kind.as_def_kind(), item.def_id)));
400+
match kind {
401+
ty::AssocKind::Fn => "method",
402+
ty::AssocKind::Const => "associatedconstant",
403+
ty::AssocKind::Type => "associatedtype",
404+
}
401405
})
402406
.map(|out| {
403407
(
@@ -1142,55 +1146,75 @@ impl LinkCollector<'_, '_> {
11421146
callback,
11431147
);
11441148
};
1145-
match res {
1146-
Res::Primitive(_) => match disambiguator {
1147-
Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => {
1148-
Some(ItemLink { link: ori_link.link, link_text, did: None, fragment })
1149-
}
1150-
Some(other) => {
1151-
report_mismatch(other, Disambiguator::Primitive);
1152-
None
1153-
}
1154-
},
1155-
Res::Def(kind, id) => {
1156-
debug!("intra-doc link to {} resolved to {:?}", path_str, res);
1157-
1158-
// Disallow e.g. linking to enums with `struct@`
1159-
debug!("saw kind {:?} with disambiguator {:?}", kind, disambiguator);
1160-
match (self.kind_side_channel.take().map(|(kind, _)| kind).unwrap_or(kind), disambiguator) {
1161-
| (DefKind::Const | DefKind::ConstParam | DefKind::AssocConst | DefKind::AnonConst, Some(Disambiguator::Kind(DefKind::Const)))
1162-
// NOTE: this allows 'method' to mean both normal functions and associated functions
1163-
// This can't cause ambiguity because both are in the same namespace.
1164-
| (DefKind::Fn | DefKind::AssocFn, Some(Disambiguator::Kind(DefKind::Fn)))
1165-
// These are namespaces; allow anything in the namespace to match
1166-
| (_, Some(Disambiguator::Namespace(_)))
1167-
// If no disambiguator given, allow anything
1168-
| (_, None)
1169-
// All of these are valid, so do nothing
1170-
=> {}
1171-
(actual, Some(Disambiguator::Kind(expected))) if actual == expected => {}
1172-
(_, Some(specified @ Disambiguator::Kind(_) | specified @ Disambiguator::Primitive)) => {
1173-
report_mismatch(specified, Disambiguator::Kind(kind));
1174-
return None;
1175-
}
1149+
1150+
let verify = |kind: DefKind, id: DefId| {
1151+
debug!("intra-doc link to {} resolved to {:?}", path_str, res);
1152+
1153+
// Disallow e.g. linking to enums with `struct@`
1154+
debug!("saw kind {:?} with disambiguator {:?}", kind, disambiguator);
1155+
match (self.kind_side_channel.take().map(|(kind, _)| kind).unwrap_or(kind), disambiguator) {
1156+
| (DefKind::Const | DefKind::ConstParam | DefKind::AssocConst | DefKind::AnonConst, Some(Disambiguator::Kind(DefKind::Const)))
1157+
// NOTE: this allows 'method' to mean both normal functions and associated functions
1158+
// This can't cause ambiguity because both are in the same namespace.
1159+
| (DefKind::Fn | DefKind::AssocFn, Some(Disambiguator::Kind(DefKind::Fn)))
1160+
// These are namespaces; allow anything in the namespace to match
1161+
| (_, Some(Disambiguator::Namespace(_)))
1162+
// If no disambiguator given, allow anything
1163+
| (_, None)
1164+
// All of these are valid, so do nothing
1165+
=> {}
1166+
(actual, Some(Disambiguator::Kind(expected))) if actual == expected => {}
1167+
(_, Some(specified @ Disambiguator::Kind(_) | specified @ Disambiguator::Primitive)) => {
1168+
report_mismatch(specified, Disambiguator::Kind(kind));
1169+
return None;
11761170
}
1171+
}
1172+
1173+
// item can be non-local e.g. when using #[doc(primitive = "pointer")]
1174+
if let Some((src_id, dst_id)) = id
1175+
.as_local()
1176+
.and_then(|dst_id| item.def_id.as_local().map(|src_id| (src_id, dst_id)))
1177+
{
1178+
use rustc_hir::def_id::LOCAL_CRATE;
11771179

1178-
// item can be non-local e.g. when using #[doc(primitive = "pointer")]
1179-
if let Some((src_id, dst_id)) = id
1180-
.as_local()
1181-
.and_then(|dst_id| item.def_id.as_local().map(|src_id| (src_id, dst_id)))
1180+
let hir_src = self.cx.tcx.hir().local_def_id_to_hir_id(src_id);
1181+
let hir_dst = self.cx.tcx.hir().local_def_id_to_hir_id(dst_id);
1182+
1183+
if self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_src)
1184+
&& !self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_dst)
11821185
{
1183-
use rustc_hir::def_id::LOCAL_CRATE;
1186+
privacy_error(cx, &item, &path_str, dox, &ori_link);
1187+
}
1188+
}
11841189

1185-
let hir_src = self.cx.tcx.hir().local_def_id_to_hir_id(src_id);
1186-
let hir_dst = self.cx.tcx.hir().local_def_id_to_hir_id(dst_id);
1190+
Some((kind, id))
1191+
};
11871192

1188-
if self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_src)
1189-
&& !self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_dst)
1190-
{
1191-
privacy_error(cx, &item, &path_str, dox, &ori_link);
1193+
match res {
1194+
Res::Primitive(_) => {
1195+
if let Some((kind, id)) = self.kind_side_channel.take() {
1196+
// We're actually resolving an associated item of a primitive, so we need to
1197+
// verify the disambiguator (if any) matches the type of the associated item.
1198+
// This case should really follow the same flow as the `Res::Def` branch below,
1199+
// but attempting to add a call to `clean::register_res` causes an ICE. @jyn514
1200+
// thinks `register_res` is only needed for cross-crate re-exports, but Rust
1201+
// doesn't allow statements like `use str::trim;`, making this a (hopefully)
1202+
// valid omission. See https://github.com/rust-lang/rust/pull/80660#discussion_r551585677
1203+
// for discussion on the matter.
1204+
verify(kind, id)?;
1205+
} else {
1206+
match disambiguator {
1207+
Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => {}
1208+
Some(other) => {
1209+
report_mismatch(other, Disambiguator::Primitive);
1210+
return None;
1211+
}
11921212
}
11931213
}
1214+
Some(ItemLink { link: ori_link.link, link_text, did: None, fragment })
1215+
}
1216+
Res::Def(kind, id) => {
1217+
let (kind, id) = verify(kind, id)?;
11941218
let id = clean::register_res(cx, rustc_hir::def::Res::Def(kind, id));
11951219
Some(ItemLink { link: ori_link.link, link_text, did: Some(id), fragment })
11961220
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#![deny(broken_intra_doc_links)]
2+
//! [static@u8::MIN]
3+
//~^ ERROR incompatible link kind
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: incompatible link kind for `u8::MIN`
2+
--> $DIR/incompatible-primitive-disambiguator.rs:2:6
3+
|
4+
LL | //! [static@u8::MIN]
5+
| ^^^^^^^^^^^^^^ help: to link to the associated constant, prefix with `const@`: `const@u8::MIN`
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/incompatible-primitive-disambiguator.rs:1:9
9+
|
10+
LL | #![deny(broken_intra_doc_links)]
11+
| ^^^^^^^^^^^^^^^^^^^^^^
12+
= note: this link resolved to an associated constant, which is not a static
13+
14+
error: aborting due to previous error
15+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#![deny(broken_intra_doc_links)]
2+
// @has primitive_disambiguators/index.html
3+
// @has - '//a/@href' 'https://doc.rust-lang.org/nightly/std/primitive.str.html#method.trim'
4+
//! [str::trim()]

0 commit comments

Comments
 (0)