From ed53de029cf84651423007ec68305d6e772c819f Mon Sep 17 00:00:00 2001 From: Dennis Hamester Date: Sat, 11 Jul 2020 19:39:02 +0200 Subject: [PATCH 1/2] rustdoc: Always warn when linking from public to private items Change the logic such that linking from a public to a private item always triggers intra_doc_link_resolution_failure. Previously, the warning was not emitted when --document-private-items is passed. Also don't rely anymore on the item's visibility, which would falsely trigger the lint now that the check for --document-private-items is gone. --- .../passes/collect_intra_doc_links.rs | 17 ++++++++++++----- .../intra-links-private.private.stderr | 10 ++++++++++ .../intra-links-private.public.stderr | 2 +- src/test/rustdoc-ui/intra-links-private.rs | 3 +-- src/test/rustdoc-ui/issue-74134.private.stderr | 10 ++++++++++ src/test/rustdoc-ui/issue-74134.rs | 4 ++-- 6 files changed, 36 insertions(+), 10 deletions(-) create mode 100644 src/test/rustdoc-ui/intra-links-private.private.stderr create mode 100644 src/test/rustdoc-ui/issue-74134.private.stderr diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 6f221da1168c6..8e113cfa563c8 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -790,13 +790,20 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { 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()) { + + // item can be non-local e.g. when using #[doc(primitive = "pointer")] + if let Some((src_id, dst_id)) = res + .opt_def_id() + .and_then(|def_id| def_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; - 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) - && (item.visibility == Visibility::Public) - && !self.cx.render_options.document_private + let hir_src = self.cx.tcx.hir().as_local_hir_id(src_id); + let hir_dst = self.cx.tcx.hir().as_local_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) { privacy_error(cx, &item, &path_str, &dox, link_range); continue; diff --git a/src/test/rustdoc-ui/intra-links-private.private.stderr b/src/test/rustdoc-ui/intra-links-private.private.stderr new file mode 100644 index 0000000000000..6a23afb0798a9 --- /dev/null +++ b/src/test/rustdoc-ui/intra-links-private.private.stderr @@ -0,0 +1,10 @@ +warning: public documentation for `DocMe` links to private item `DontDocMe` + --> $DIR/intra-links-private.rs:5:11 + | +LL | /// docs [DontDocMe] + | ^^^^^^^^^ this item is private + | + = note: `#[warn(intra_doc_link_resolution_failure)]` on by default + +warning: 1 warning emitted + diff --git a/src/test/rustdoc-ui/intra-links-private.public.stderr b/src/test/rustdoc-ui/intra-links-private.public.stderr index a124435b08a99..6a23afb0798a9 100644 --- a/src/test/rustdoc-ui/intra-links-private.public.stderr +++ b/src/test/rustdoc-ui/intra-links-private.public.stderr @@ -1,5 +1,5 @@ warning: public documentation for `DocMe` links to private item `DontDocMe` - --> $DIR/intra-links-private.rs:6:11 + --> $DIR/intra-links-private.rs:5:11 | LL | /// docs [DontDocMe] | ^^^^^^^^^ this item is private diff --git a/src/test/rustdoc-ui/intra-links-private.rs b/src/test/rustdoc-ui/intra-links-private.rs index 1b97f6e61bd23..613236d75d2ee 100644 --- a/src/test/rustdoc-ui/intra-links-private.rs +++ b/src/test/rustdoc-ui/intra-links-private.rs @@ -1,10 +1,9 @@ // check-pass // revisions: public private // [private]compile-flags: --document-private-items -#![cfg_attr(private, deny(intra_doc_link_resolution_failure))] /// docs [DontDocMe] -//[public]~^ WARNING public documentation for `DocMe` links to private item `DontDocMe` +//~^ WARNING public documentation for `DocMe` links to private item `DontDocMe` // FIXME: for [private] we should also make sure the link was actually generated pub struct DocMe; struct DontDocMe; diff --git a/src/test/rustdoc-ui/issue-74134.private.stderr b/src/test/rustdoc-ui/issue-74134.private.stderr new file mode 100644 index 0000000000000..3c41f7e63e637 --- /dev/null +++ b/src/test/rustdoc-ui/issue-74134.private.stderr @@ -0,0 +1,10 @@ +warning: public documentation for `public_item` links to private item `PrivateType` + --> $DIR/issue-74134.rs:19:10 + | +LL | /// [`PrivateType`] + | ^^^^^^^^^^^^^ this item is private + | + = note: `#[warn(intra_doc_link_resolution_failure)]` on by default + +warning: 1 warning emitted + diff --git a/src/test/rustdoc-ui/issue-74134.rs b/src/test/rustdoc-ui/issue-74134.rs index fe484b43fb6fc..b1be9123aaf83 100644 --- a/src/test/rustdoc-ui/issue-74134.rs +++ b/src/test/rustdoc-ui/issue-74134.rs @@ -4,7 +4,7 @@ // There are 4 cases here: // 1. public item -> public type: no warning -// 2. public item -> private type: warning, if --document-private-items is not passed +// 2. public item -> private type: warning // 3. private item -> public type: no warning // 4. private item -> private type: no warning // All 4 cases are tested with and without --document-private-items. @@ -17,7 +17,7 @@ pub struct PublicType; pub struct Public { /// [`PublicType`] /// [`PrivateType`] - //[public]~^ WARNING public documentation for `public_item` links to private item `PrivateType` + //~^ WARNING public documentation for `public_item` links to private item `PrivateType` pub public_item: u32, /// [`PublicType`] From c14641a814696f6dffe5a0e7d693af31a4e40d09 Mon Sep 17 00:00:00 2001 From: Dennis Hamester Date: Wed, 22 Jul 2020 19:53:43 +0200 Subject: [PATCH 2/2] rustdoc: Add explanation when linting against public to private item links The additional note helps explaining why the lint was triggered and that --document-private-items directly influences the link resolution. --- src/librustdoc/passes/collect_intra_doc_links.rs | 7 +++++++ src/test/rustdoc-ui/intra-links-private.private.stderr | 1 + src/test/rustdoc-ui/intra-links-private.public.stderr | 1 + src/test/rustdoc-ui/issue-74134.private.stderr | 1 + src/test/rustdoc-ui/issue-74134.public.stderr | 1 + 5 files changed, 11 insertions(+) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 8e113cfa563c8..5187839423d6b 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1076,6 +1076,13 @@ fn privacy_error( if let Some(sp) = sp { diag.span_label(sp, "this item is private"); } + + let note_msg = if cx.render_options.document_private { + "this link resolves only because you passed `--document-private-items`, but will break without" + } else { + "this link will resolve properly if you pass `--document-private-items`" + }; + diag.note(note_msg); }); } diff --git a/src/test/rustdoc-ui/intra-links-private.private.stderr b/src/test/rustdoc-ui/intra-links-private.private.stderr index 6a23afb0798a9..a2148b82f819a 100644 --- a/src/test/rustdoc-ui/intra-links-private.private.stderr +++ b/src/test/rustdoc-ui/intra-links-private.private.stderr @@ -5,6 +5,7 @@ LL | /// docs [DontDocMe] | ^^^^^^^^^ this item is private | = note: `#[warn(intra_doc_link_resolution_failure)]` on by default + = note: this link resolves only because you passed `--document-private-items`, but will break without warning: 1 warning emitted diff --git a/src/test/rustdoc-ui/intra-links-private.public.stderr b/src/test/rustdoc-ui/intra-links-private.public.stderr index 6a23afb0798a9..56742406992fc 100644 --- a/src/test/rustdoc-ui/intra-links-private.public.stderr +++ b/src/test/rustdoc-ui/intra-links-private.public.stderr @@ -5,6 +5,7 @@ LL | /// docs [DontDocMe] | ^^^^^^^^^ this item is private | = note: `#[warn(intra_doc_link_resolution_failure)]` on by default + = note: this link will resolve properly if you pass `--document-private-items` warning: 1 warning emitted diff --git a/src/test/rustdoc-ui/issue-74134.private.stderr b/src/test/rustdoc-ui/issue-74134.private.stderr index 3c41f7e63e637..9c5cdf0117cab 100644 --- a/src/test/rustdoc-ui/issue-74134.private.stderr +++ b/src/test/rustdoc-ui/issue-74134.private.stderr @@ -5,6 +5,7 @@ LL | /// [`PrivateType`] | ^^^^^^^^^^^^^ this item is private | = note: `#[warn(intra_doc_link_resolution_failure)]` on by default + = note: this link resolves only because you passed `--document-private-items`, but will break without warning: 1 warning emitted diff --git a/src/test/rustdoc-ui/issue-74134.public.stderr b/src/test/rustdoc-ui/issue-74134.public.stderr index 3c41f7e63e637..ff2951d864e64 100644 --- a/src/test/rustdoc-ui/issue-74134.public.stderr +++ b/src/test/rustdoc-ui/issue-74134.public.stderr @@ -5,6 +5,7 @@ LL | /// [`PrivateType`] | ^^^^^^^^^^^^^ this item is private | = note: `#[warn(intra_doc_link_resolution_failure)]` on by default + = note: this link will resolve properly if you pass `--document-private-items` warning: 1 warning emitted