Skip to content

Properly handle primitive disambiguators in rustdoc #80660

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 7, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 62 additions & 46 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,10 +394,14 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
ns,
impl_,
)
.map(|item| match item.kind {
ty::AssocKind::Fn => "method",
ty::AssocKind::Const => "associatedconstant",
ty::AssocKind::Type => "associatedtype",
.map(|item| {
let kind = item.kind;
self.kind_side_channel.set(Some((kind.as_def_kind(), item.def_id)));
match kind {
ty::AssocKind::Fn => "method",
ty::AssocKind::Const => "associatedconstant",
ty::AssocKind::Type => "associatedtype",
}
})
.map(|out| {
(
Expand Down Expand Up @@ -1142,55 +1146,67 @@ impl LinkCollector<'_, '_> {
callback,
);
};
match res {
Res::Primitive(_) => match disambiguator {
Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => {
Some(ItemLink { link: ori_link.link, link_text, did: None, fragment })
}
Some(other) => {
report_mismatch(other, Disambiguator::Primitive);
None
}
},
Res::Def(kind, id) => {
debug!("intra-doc link to {} resolved to {:?}", path_str, res);

// Disallow e.g. linking to enums with `struct@`
debug!("saw kind {:?} with disambiguator {:?}", kind, disambiguator);
match (self.kind_side_channel.take().map(|(kind, _)| kind).unwrap_or(kind), disambiguator) {
| (DefKind::Const | DefKind::ConstParam | DefKind::AssocConst | DefKind::AnonConst, Some(Disambiguator::Kind(DefKind::Const)))
// NOTE: this allows 'method' to mean both normal functions and associated functions
// This can't cause ambiguity because both are in the same namespace.
| (DefKind::Fn | DefKind::AssocFn, Some(Disambiguator::Kind(DefKind::Fn)))
// These are namespaces; allow anything in the namespace to match
| (_, Some(Disambiguator::Namespace(_)))
// If no disambiguator given, allow anything
| (_, None)
// All of these are valid, so do nothing
=> {}
(actual, Some(Disambiguator::Kind(expected))) if actual == expected => {}
(_, Some(specified @ Disambiguator::Kind(_) | specified @ Disambiguator::Primitive)) => {
report_mismatch(specified, Disambiguator::Kind(kind));
return None;
}

let verify = |kind: DefKind, id: DefId| {
debug!("intra-doc link to {} resolved to {:?}", path_str, res);

// Disallow e.g. linking to enums with `struct@`
debug!("saw kind {:?} with disambiguator {:?}", kind, disambiguator);
match (self.kind_side_channel.take().map(|(kind, _)| kind).unwrap_or(kind), disambiguator) {
| (DefKind::Const | DefKind::ConstParam | DefKind::AssocConst | DefKind::AnonConst, Some(Disambiguator::Kind(DefKind::Const)))
// NOTE: this allows 'method' to mean both normal functions and associated functions
// This can't cause ambiguity because both are in the same namespace.
| (DefKind::Fn | DefKind::AssocFn, Some(Disambiguator::Kind(DefKind::Fn)))
// These are namespaces; allow anything in the namespace to match
| (_, Some(Disambiguator::Namespace(_)))
// If no disambiguator given, allow anything
| (_, None)
// All of these are valid, so do nothing
=> {}
(actual, Some(Disambiguator::Kind(expected))) if actual == expected => {}
(_, Some(specified @ Disambiguator::Kind(_) | specified @ Disambiguator::Primitive)) => {
report_mismatch(specified, Disambiguator::Kind(kind));
return None;
}
}

// item can be non-local e.g. when using #[doc(primitive = "pointer")]
if let Some((src_id, dst_id)) = id
.as_local()
.and_then(|dst_id| item.def_id.as_local().map(|src_id| (src_id, dst_id)))
{
use rustc_hir::def_id::LOCAL_CRATE;

// item can be non-local e.g. when using #[doc(primitive = "pointer")]
if let Some((src_id, dst_id)) = id
.as_local()
.and_then(|dst_id| item.def_id.as_local().map(|src_id| (src_id, dst_id)))
let hir_src = self.cx.tcx.hir().local_def_id_to_hir_id(src_id);
let hir_dst = self.cx.tcx.hir().local_def_id_to_hir_id(dst_id);

if self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_src)
&& !self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_dst)
{
use rustc_hir::def_id::LOCAL_CRATE;
privacy_error(cx, &item, &path_str, dox, &ori_link);
}
}

let hir_src = self.cx.tcx.hir().local_def_id_to_hir_id(src_id);
let hir_dst = self.cx.tcx.hir().local_def_id_to_hir_id(dst_id);
Some((kind, id))
};

if self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_src)
&& !self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_dst)
{
privacy_error(cx, &item, &path_str, dox, &ori_link);
match res {
Res::Primitive(_) => {
if let Some((kind, id)) = self.kind_side_channel.take() {
verify(kind, id)?;
} else {
match disambiguator {
Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => {}
Some(other) => {
report_mismatch(other, Disambiguator::Primitive);
return None;
}
}
}
Some(ItemLink { link: ori_link.link, link_text, did: None, fragment })
}
Res::Def(kind, id) => {
let (kind, id) = verify(kind, id)?;
let id = clean::register_res(cx, rustc_hir::def::Res::Def(kind, id));
Some(ItemLink { link: ori_link.link, link_text, did: Some(id), fragment })
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#![deny(broken_intra_doc_links)]
//! [static@u8::MIN]
//~^ ERROR incompatible link kind
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: incompatible link kind for `u8::MIN`
--> $DIR/incompatible-primitive-disambiguator.rs:2:6
|
LL | //! [static@u8::MIN]
| ^^^^^^^^^^^^^^ help: to link to the associated constant, prefix with `const@`: `const@u8::MIN`
|
note: the lint level is defined here
--> $DIR/incompatible-primitive-disambiguator.rs:1:9
|
LL | #![deny(broken_intra_doc_links)]
| ^^^^^^^^^^^^^^^^^^^^^^
= note: this link resolved to an associated constant, which is not a static

error: aborting due to previous error

4 changes: 4 additions & 0 deletions src/test/rustdoc/intra-doc/primitive-disambiguators.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#![deny(broken_intra_doc_links)]
// @has primitive_disambiguators/index.html
// @has - '//a/@href' 'https://doc.rust-lang.org/nightly/std/primitive.str.html#method.trim'
//! [str::trim()]