Rustdoc links to private types on private items generate a warning

[dennis-hamester]

With #72771 merged (current nightly, e.g. 1.46.0-nightly (0c03aee8b 2020-07-05)) , the following code now produces a warning:

struct Private;

pub struct Public {
    /// A private field with a [`Private`] type.
    private: Private,
}

This triggers intra_doc_link_resolution_failure:

warning: `[Private]` public documentation for `private` links to a private item
 --> src/lib.rs:4:12
  |
4 |     /// A [`Private`] field.
  |            ^^^^^^^^^ this item is private
  |
  = note: `#[warn(intra_doc_link_resolution_failure)]` on by default

But rustdoc never actually generates any links to Private, because it wasn't run with --document-private-items. In my opinion it shouldn't emit warning in this case, because documenting the internals can be very useful to contributors or even future me when --document-private-items is used.

There has already been some discussion at the end of #72771, which is where I asked first about whether this change was intentional.

[jyn514]

@rustbot modify labels: T-rustdoc A-intra-doc-links

[dennis-hamester]

Assuming the consensus is that rustdoc shouldn't warn in this case, would an additional check against the item's visibility be correct? Like:

--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -773,6 +773,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
 
                     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 item_name = item.name.as_deref().unwrap_or("<unknown>");

That certainly gets rid of the warning in my case, but I'm unsure about the other variants of Visibility and all the other kinds of things item can refer to.

[jyn514]

That looks reasonable to me. Could you make a PR with that change and a few test cases?

[dennis-hamester]

Wouldn't the check like I changed it then only catch cases like this:

struct Private;

pub struct Public {
    /// A private field with a [`Private`] type.
    pub public: Private,
}

But that already triggers E0446 anyway:

error[E0446]: private type `Private` in public interface
 --> src/lib.rs:5:5
  |
5 |     pub public: Private,
  |     ^^^^^^^^^^^^^^^^^^^^ can't leak private type

[jyn514]

@dennis-hamester the link is entirely separate from the type of the field. You could have

pub struct Public {
    /// See also [`Private`] type
    pub public: u32,
}

in which case there would be no error but the warning should still be generated.

[dennis-hamester]

@jyn514 Ah, yes of course, thanks for pointing that out :)

I'll open a PR then.