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

Warn if linking to a private item #72771

Merged
merged 3 commits into from
Jun 27, 2020
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
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;
}
jyn514 marked this conversation as resolved.
Show resolved Hide resolved
}
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]
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
//[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