Skip to content

Commit

Permalink
Rollup merge of #72771 - jyn514:rustdoc, r=Manishearth
Browse files Browse the repository at this point in the history
Warn if linking to a private item

Closes #72769

r? @GuillaumeGomez
  • Loading branch information
Manishearth authored Jun 26, 2020
2 parents 7750c3d + 6742382 commit 8adc781
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 23 deletions.
14 changes: 6 additions & 8 deletions src/librustdoc/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,6 @@ pub struct Options {
///
/// Be aware: This option can come both from the CLI and from crate attributes!
pub default_passes: DefaultPassOption,
/// Document items that have lower than `pub` visibility.
pub document_private: bool,
/// Document items that have `doc(hidden)`.
pub document_hidden: bool,
/// Any passes manually selected by the user.
///
/// Be aware: This option can come both from the CLI and from crate attributes!
Expand Down Expand Up @@ -177,8 +173,6 @@ impl fmt::Debug for Options {
.field("test_args", &self.test_args)
.field("persist_doctests", &self.persist_doctests)
.field("default_passes", &self.default_passes)
.field("document_private", &self.document_private)
.field("document_hidden", &self.document_hidden)
.field("manual_passes", &self.manual_passes)
.field("display_warnings", &self.display_warnings)
.field("show_coverage", &self.show_coverage)
Expand Down Expand Up @@ -250,6 +244,10 @@ pub struct RenderOptions {
pub generate_search_filter: bool,
/// Option (disabled by default) to generate files used by RLS and some other tools.
pub generate_redirect_pages: bool,
/// Document items that have lower than `pub` visibility.
pub document_private: bool,
/// Document items that have `doc(hidden)`.
pub document_hidden: bool,
}

impl Options {
Expand Down Expand Up @@ -567,8 +565,6 @@ impl Options {
should_test,
test_args,
default_passes,
document_private,
document_hidden,
manual_passes,
display_warnings,
show_coverage,
Expand Down Expand Up @@ -597,6 +593,8 @@ impl Options {
markdown_playground_url,
generate_search_filter,
generate_redirect_pages,
document_private,
document_hidden,
},
output_format,
})
Expand Down
15 changes: 8 additions & 7 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ pub struct DocContext<'tcx> {
// FIXME(eddyb) make this a `ty::TraitRef<'tcx>` set.
pub generated_synthetics: RefCell<FxHashSet<(Ty<'tcx>, DefId)>>,
pub auto_traits: Vec<DefId>,
/// The options given to rustdoc that could be relevant to a pass.
pub render_options: RenderOptions,
}

impl<'tcx> DocContext<'tcx> {
Expand Down Expand Up @@ -281,8 +283,6 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
describe_lints,
lint_cap,
mut default_passes,
mut document_private,
document_hidden,
mut manual_passes,
display_warnings,
render_options,
Expand Down Expand Up @@ -448,6 +448,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
.cloned()
.filter(|trait_def_id| tcx.trait_is_auto(*trait_def_id))
.collect(),
render_options,
};
debug!("crate: {:?}", tcx.hir().krate());

Expand Down Expand Up @@ -524,7 +525,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
}

if attr.is_word() && name == sym::document_private_items {
document_private = true;
ctxt.render_options.document_private = true;
}
}

Expand All @@ -544,9 +545,9 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
for p in passes {
let run = match p.condition {
Always => true,
WhenDocumentPrivate => document_private,
WhenNotDocumentPrivate => !document_private,
WhenNotDocumentHidden => !document_hidden,
WhenDocumentPrivate => ctxt.render_options.document_private,
WhenNotDocumentPrivate => !ctxt.render_options.document_private,
WhenNotDocumentHidden => !ctxt.render_options.document_hidden,
};
if run {
debug!("running pass {}", p.pass.name);
Expand All @@ -556,7 +557,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt

ctxt.sess().abort_if_errors();

(krate, ctxt.renderinfo.into_inner(), render_options)
(krate, ctxt.renderinfo.into_inner(), ctxt.render_options)
})
})
})
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ impl clean::Path {

pub fn href(did: DefId) -> Option<(String, ItemType, Vec<String>)> {
let cache = cache();
if !did.is_local() && !cache.access_levels.is_public(did) {
if !did.is_local() && !cache.access_levels.is_public(did) && !cache.document_private {
return None;
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/html/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ pub fn run(
static_root_path,
generate_search_filter,
generate_redirect_pages,
document_private,
..
} = options;

Expand Down Expand Up @@ -546,7 +547,7 @@ pub fn run(
scx.ensure_dir(&dst)?;
krate = sources::render(&dst, &mut scx, krate)?;
let (new_crate, index, cache) =
Cache::from_krate(renderinfo, &extern_html_root_urls, &dst, krate);
Cache::from_krate(renderinfo, document_private, &extern_html_root_urls, &dst, krate);
krate = new_crate;
let cache = Arc::new(cache);
let mut cx = Context {
Expand Down
6 changes: 6 additions & 0 deletions src/librustdoc/html/render/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ crate struct Cache {
/// The version of the crate being documented, if given from the `--crate-version` flag.
pub crate_version: Option<String>,

/// Whether to document private items.
/// This is stored in `Cache` so it doesn't need to be passed through all rustdoc functions.
pub document_private: bool,

// Private fields only used when initially crawling a crate to build a cache
stack: Vec<String>,
parent_stack: Vec<DefId>,
Expand Down Expand Up @@ -126,6 +130,7 @@ crate struct Cache {
impl Cache {
pub fn from_krate(
renderinfo: RenderInfo,
document_private: bool,
extern_html_root_urls: &BTreeMap<String, String>,
dst: &Path,
mut krate: clean::Crate,
Expand Down Expand Up @@ -160,6 +165,7 @@ impl Cache {
stripped_mod: false,
access_levels,
crate_version: krate.version.take(),
document_private,
orphan_impl_items: Vec::new(),
orphan_trait_impls: Vec::new(),
traits: krate.external_traits.replace(Default::default()),
Expand Down
45 changes: 39 additions & 6 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
let result = cx.enter_resolver(|resolver| {
resolver.resolve_str_path_error(DUMMY_SP, &path_str, ns, module_id)
});
debug!("{} resolved to {:?} in namespace {:?}", path_str, result, ns);
let result = match result {
Ok((_, Res::Err)) => Err(ErrorKind::ResolutionFailure),
_ => result.map_err(|_| ErrorKind::ResolutionFailure),
Expand All @@ -202,7 +203,13 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
}
return Ok((res, Some(path_str.to_owned())));
}
_ => return Ok((res, extra_fragment.clone())),
other => {
debug!(
"failed to resolve {} in namespace {:?} (got {:?})",
path_str, ns, other
);
return Ok((res, extra_fragment.clone()));
}
};

if value != (ns == ValueNS) {
Expand Down Expand Up @@ -555,12 +562,13 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
} else {
(parts[0].to_owned(), None)
};
let resolved_self;
let mut path_str;
let (res, fragment) = {
let mut kind = None;
let mut path_str = if let Some(prefix) =
["struct@", "enum@", "type@", "trait@", "union@"]
.iter()
.find(|p| link.starts_with(**p))
path_str = if let Some(prefix) = ["struct@", "enum@", "type@", "trait@", "union@"]
.iter()
.find(|p| link.starts_with(**p))
{
kind = Some(TypeNS);
link.trim_start_matches(prefix)
Expand Down Expand Up @@ -614,7 +622,6 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
let base_node =
if item.is_mod() && item.attrs.inner_docs { None } else { parent_node };

let resolved_self;
// replace `Self` with suitable item's parent name
if path_str.starts_with("Self::") {
if let Some(ref name) = parent_name {
Expand Down Expand Up @@ -760,6 +767,32 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
if let Res::PrimTy(_) = res {
item.attrs.links.push((ori_link, None, fragment));
} else {
debug!("intra-doc link to {} resolved to {:?}", path_str, res);
if let Some(local) = res.opt_def_id().and_then(|def_id| def_id.as_local()) {
use rustc_hir::def_id::LOCAL_CRATE;

let hir_id = self.cx.tcx.hir().as_local_hir_id(local);
if !self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_id)
&& !self.cx.render_options.document_private
{
let item_name = item.name.as_deref().unwrap_or("<unknown>");
let err_msg = format!(
"public documentation for `{}` links to a private item",
item_name
);
build_diagnostic(
cx,
&item,
path_str,
&dox,
link_range,
&err_msg,
"this item is private",
None,
);
continue;
}
}
let id = register_res(cx, res);
item.attrs.links.push((ori_link, Some(id), fragment));
}
Expand Down
10 changes: 10 additions & 0 deletions src/test/rustdoc-ui/intra-links-private.public.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
warning: `[DontDocMe]` public documentation for `DocMe` links to a private item
--> $DIR/intra-links-private.rs:6:11
|
LL | /// docs [DontDocMe]
| ^^^^^^^^^ this item is private
|
= note: `#[warn(intra_doc_link_resolution_failure)]` on by default

warning: 1 warning emitted

10 changes: 10 additions & 0 deletions src/test/rustdoc-ui/intra-links-private.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// check-pass
// revisions: public private
// [private]compile-flags: --document-private-items
#![cfg_attr(private, deny(intra_doc_resolution_failure))]

/// docs [DontDocMe]
//[public]~^ WARNING `[DontDocMe]` public documentation for `DocMe` links to a private item
// FIXME: for [private] we should also make sure the link was actually generated
pub struct DocMe;
struct DontDocMe;
6 changes: 6 additions & 0 deletions src/test/rustdoc-ui/reference-link-has-one-warning.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// ignore-test
// check-pass

/// docs [label][with#anchor#error]
//~^ WARNING has an issue with the link anchor
pub struct S;
10 changes: 10 additions & 0 deletions src/test/rustdoc-ui/reference-link-has-one-warning.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
warning: `[with#anchor#error]` has an issue with the link anchor.
--> $DIR/reference-link-has-one-warning.rs:3:18
|
LL | /// docs [label][with#anchor#error]
| ^^^^^^^^^^^^^^^^^ only one `#` is allowed in a link
|
= note: `#[warn(intra_doc_link_resolution_failure)]` on by default

warning: 1 warning emitted

0 comments on commit 8adc781

Please sign in to comment.