Skip to content
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

Properly handle primitive disambiguators in rustdoc #80660

Merged
merged 6 commits into from
Jan 7, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
116 changes: 70 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,75 @@ 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() {
// 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.
// This case should really follow the same flow as the `Res::Def` branch below,
// but attempting to add a call to `clean::register_res` causes an ICE. @jyn514
// thinks `register_res` is only needed for cross-crate re-exports, but Rust
// doesn't allow statements like `use str::trim;`, making this a (hopefully)
// valid omission. See https://github.com/rust-lang/rust/pull/80660#discussion_r551585677
// for discussion on the matter.
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()]