Skip to content

Warn if linking to a private item #72771

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 3 commits into from
Jun 27, 2020
Merged
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
14 changes: 6 additions & 8 deletions src/librustdoc/config.rs
Original file line number Diff line number Diff line change
@@ -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!
@@ -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)
@@ -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 {
@@ -567,8 +565,6 @@ impl Options {
should_test,
test_args,
default_passes,
document_private,
document_hidden,
manual_passes,
display_warnings,
show_coverage,
@@ -597,6 +593,8 @@ impl Options {
markdown_playground_url,
generate_search_filter,
generate_redirect_pages,
document_private,
document_hidden,
},
output_format,
})
15 changes: 8 additions & 7 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
@@ -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> {
@@ -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,
@@ -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());

@@ -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;
}
}

@@ -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);
@@ -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)
})
})
})
2 changes: 1 addition & 1 deletion src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
@@ -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;
}

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

@@ -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 {
6 changes: 6 additions & 0 deletions src/librustdoc/html/render/cache.rs
Original file line number Diff line number Diff line change
@@ -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>,
@@ -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,
@@ -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()),
45 changes: 39 additions & 6 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
@@ -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),
@@ -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) {
@@ -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)
@@ -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 {
@@ -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));
}
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