Intra-doc links side of warning about undocumented items

Unfortunately, this currently breaks because the cache is not populated
until later, so rustdoc thinks some items won't be documented when they
will be.

- Track why links are missing
- Populate cache before running pass

Author: jyn514

src/librustdoc/config.rs

@@ -339,14 +339,14 @@ impl Options {
                 println!("{:>20} - {}", pass.name, pass.description);
             }
             println!("\nDefault passes for rustdoc:");
-            for p in passes::DEFAULT_PASSES {
+            for &p in passes::DEFAULT_PASSES.0.iter().chain(passes::DEFAULT_PASSES.1) {
                 print!("{:>20}", p.pass.name);
                 println_condition(p.condition);
             }
 
             if nightly_options::match_is_nightly_build(matches) {
                 println!("\nPasses run with `--show-coverage`:");
-                for p in passes::COVERAGE_PASSES {
+                for &p in passes::COVERAGE_PASSES.0.iter().chain(passes::COVERAGE_PASSES.1) {
                     print!("{:>20}", p.pass.name);
                     println_condition(p.condition);
                 }

src/librustdoc/core.rs

@@ -25,10 +25,10 @@ use std::mem;
 use std::rc::Rc;
 
 use crate::clean::inline::build_external_trait;
-use crate::clean::{self, ItemId, TraitWithExtraInfo};
+use crate::clean::{self, ItemId, TraitWithExtraInfo, Crate};
 use crate::config::{Options as RustdocOptions, OutputFormat, RenderOptions};
 use crate::formats::cache::Cache;
-use crate::passes::{self, Condition::*};
+use crate::passes::{self, Condition::*, ConditionalPass};
 
 crate use rustc_session::config::{DebuggingOptions, Input, Options};
 
@@ -438,8 +438,7 @@ crate fn run_global_ctxt(
     }
 
     info!("Executing passes");
-
-    for p in passes::defaults(show_coverage) {
+    let run_pass = |p: ConditionalPass, krate: Crate, ctxt: &mut DocContext<'_>| {
         let run = match p.condition {
             Always => true,
             WhenDocumentPrivate => ctxt.render_options.document_private,
@@ -448,15 +447,23 @@ crate fn run_global_ctxt(
         };
         if run {
             debug!("running pass {}", p.pass.name);
-            krate = tcx.sess.time(p.pass.name, || (p.pass.run)(krate, &mut ctxt));
+            tcx.sess.time(p.pass.name, || (p.pass.run)(krate, ctxt))
+        } else {
+            krate
         }
-    }
+    };
 
-    if tcx.sess.diagnostic().has_errors_or_lint_errors().is_some() {
-        rustc_errors::FatalError.raise();
+    let (passes_before_cache, passes_after_cache) = passes::defaults(show_coverage);
+    for &p in passes_before_cache {
+        krate = run_pass(p, krate, &mut ctxt);
     }
+    ctxt.sess().abort_if_errors();
 
     krate = tcx.sess.time("create_format_cache", || Cache::populate(&mut ctxt, krate));
+    for &p in passes_after_cache {
+        krate = run_pass(p, krate, &mut ctxt);
+    }
+    ctxt.sess().abort_if_errors();
 
     (krate, ctxt.render_options, ctxt.cache)
 }

src/librustdoc/html/format.rs

@@ -26,6 +26,7 @@ use crate::clean::{
     self, types::ExternalLocation, utils::find_nearest_parent_module, ExternalCrate, ItemId,
     PrimitiveType,
 };
+use crate::formats::cache::Cache;
 use crate::formats::item_type::ItemType;
 use crate::html::escape::Escape;
 use crate::html::render::Context;
@@ -523,6 +524,7 @@ impl clean::GenericArgs {
 }
 
 // Possible errors when computing href link source for a `DefId`
+#[derive(Debug, PartialEq, Eq)]
 crate enum HrefError {
     /// This item is known to rustdoc, but from a crate that does not have documentation generated.
     ///
@@ -551,6 +553,18 @@ crate fn href_with_root_path(
     root_path: Option<&str>,
 ) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
     let tcx = cx.tcx();
+    let cache = cx.cache();
+    let relative_to = &cx.current;
+    href_inner(did, tcx, cache, root_path, relative_to)
+}
+
+crate fn href_inner(
+    did: DefId,
+    tcx: TyCtxt<'_>,
+    cache: &Cache,
+    root_path: Option<&str>,
+    relative_to: &[Symbol],
+) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
     let def_kind = tcx.def_kind(did);
     let did = match def_kind {
         DefKind::AssocTy | DefKind::AssocFn | DefKind::AssocConst | DefKind::Variant => {
@@ -559,12 +573,11 @@ crate fn href_with_root_path(
         }
         _ => did,
     };
-    let cache = cx.cache();
-    let relative_to = &cx.current;
     fn to_module_fqp(shortty: ItemType, fqp: &[Symbol]) -> &[Symbol] {
         if shortty == ItemType::Module { fqp } else { &fqp[..fqp.len() - 1] }
     }
 
+    trace!("calculating href for {:?}", did);
     if !did.is_local()
         && !cache.access_levels.is_public(did)
         && !cache.document_private

src/librustdoc/passes/collect_intra_doc_links.rs

@@ -26,16 +26,13 @@ use std::fmt::Write;
 use std::mem;
 use std::ops::Range;
 
-use crate::clean::{self, utils::find_nearest_parent_module};
+use crate::{clean::{self, utils::find_nearest_parent_module}, html::{format::HrefError, markdown::{MarkdownLink, markdown_links}}, lint::{PRIVATE_INTRA_DOC_LINKS, BROKEN_INTRA_DOC_LINKS}, visit::DocVisitor};
 use crate::clean::{Crate, Item, ItemId, ItemLink, PrimitiveType};
 use crate::core::DocContext;
-use crate::html::markdown::{markdown_links, MarkdownLink};
-use crate::lint::{BROKEN_INTRA_DOC_LINKS, PRIVATE_INTRA_DOC_LINKS};
-use crate::passes::Pass;
-use crate::visit::DocVisitor;
 
 mod early;
 crate use early::early_resolve_intra_doc_links;
+use super::Pass;
 
 crate const COLLECT_INTRA_DOC_LINKS: Pass = Pass {
     name: "collect-intra-doc-links",
@@ -1413,6 +1410,7 @@ impl LinkCollector<'_, '_> {
                 }
             }
 
+        let mut had_error = false;
         // item can be non-local e.g. when using #[doc(primitive = "pointer")]
         if let Some((src_id, dst_id)) = id
             .as_local()
@@ -1426,9 +1424,21 @@ impl LinkCollector<'_, '_> {
                 && !self.cx.tcx.privacy_access_levels(()).is_exported(dst_id)
             {
                 privacy_error(self.cx, diag_info, path_str);
+                had_error = true;
             }
         }
 
+        // Even if the item exists and is public, it may not have documentation generated
+        // (e.g. if it's marked with `doc(hidden)`, or in a dependency with `--no-deps`).
+        // Give a warning if so.
+        if had_error {
+            return Some(());
+        }
+        // `relative_to` is only used for the actual path of the link, not whether it's resolved or not.
+        if let Err(missing) = crate::html::format::href_inner(id, self.cx.tcx, &self.cx.cache, None, &[]) {
+            missing_docs_for_link(self.cx, &item, id, missing, &path_str, &diag_info);
+        }
+
         Some(())
     }
 
@@ -2294,6 +2304,81 @@ fn suggest_disambiguator(
     }
 }
 
+/// Report a link to an item that will not have documentation generated.
+fn missing_docs_for_link(
+    cx: &DocContext<'_>,
+    item: &Item,
+    destination_id: DefId,
+    why: HrefError,
+    path_str: &str,
+    diag_info: &DiagnosticInfo<'_>,
+) {
+    // FIXME: this is a bug, rustdoc should load all items into the cache before doing this check.
+    //   Otherwise, there will be false negatives where rustdoc doesn't warn but should.
+    if why == HrefError::NotInExternalCache {
+        return;
+    }
+
+    let item_name = match item.name {
+        Some(name) => name.to_string(),
+        None => "<unknown>".into(),
+    };
+    let msg = format!(
+        "documentation for `{}` links to item `{}` which will not have documentation generated",
+        item_name, path_str
+    );
+
+    report_diagnostic(cx.tcx, PRIVATE_INTRA_DOC_LINKS, &msg, diag_info, |diag, sp| {
+        use crate::clean::types::{AttributesExt, NestedAttributesExt};
+
+        if let Some(sp) = sp {
+            diag.span_label(sp, "this item is will not be documented");
+        }
+
+        if why == HrefError::DocumentationNotBuilt {
+            diag.note(&format!(
+                "`{}` is in the crate `{}`, which will not be documented",
+                path_str,
+                cx.tcx.crate_name(destination_id.krate)
+            ));
+            return;
+        }
+
+        // shouldn't be possible to resolve private items
+        assert_ne!(why, HrefError::Private, "{:?}", destination_id);
+
+        if let Some(attr) =
+            cx.tcx.get_attrs(destination_id).lists(sym::doc).get_word_attr(sym::hidden)
+        {
+            diag.span_label(attr.span(), &format!("`{}` is marked as `#[doc(hidden)]`", path_str));
+            return;
+        }
+
+        let mut current = destination_id;
+        while let Some(parent) = cx.tcx.parent(current) {
+            if cx.tcx.get_attrs(parent).lists(sym::doc).has_word(sym::hidden) {
+                let name = cx.tcx.item_name(parent);
+                let (_, description) = cx.tcx.article_and_description(parent);
+                let span = cx.tcx.def_span(parent);
+                diag.span_label(
+                    span,
+                    &format!(
+                        "{} is in the {} `{}`, which is marked as `#[doc(hidden)]`",
+                        path_str, description, name
+                    ),
+                );
+                return;
+            }
+            current = parent;
+        }
+
+        diag.note(&format!(
+            "`{}` may be in a private module with all re-exports marked as `#[doc(no_inline)]`",
+            path_str
+        ));
+    });
+}
+
 /// Report a link from a public item to a private one.
 fn privacy_error(cx: &DocContext<'_>, diag_info: &DiagnosticInfo<'_>, path_str: &str) {
     let sym;

src/librustdoc/passes/mod.rs

@@ -94,26 +94,30 @@ crate const PASSES: &[Pass] = &[
 ];
 
 /// The list of passes run by default.
-crate const DEFAULT_PASSES: &[ConditionalPass] = &[
-    ConditionalPass::always(COLLECT_TRAIT_IMPLS),
-    ConditionalPass::always(UNINDENT_COMMENTS),
-    ConditionalPass::always(CHECK_DOC_TEST_VISIBILITY),
-    ConditionalPass::new(STRIP_HIDDEN, WhenNotDocumentHidden),
-    ConditionalPass::new(STRIP_PRIVATE, WhenNotDocumentPrivate),
-    ConditionalPass::new(STRIP_PRIV_IMPORTS, WhenDocumentPrivate),
-    ConditionalPass::always(COLLECT_INTRA_DOC_LINKS),
-    ConditionalPass::always(CHECK_CODE_BLOCK_SYNTAX),
-    ConditionalPass::always(CHECK_INVALID_HTML_TAGS),
-    ConditionalPass::always(PROPAGATE_DOC_CFG),
-    ConditionalPass::always(CHECK_BARE_URLS),
-];
+crate const DEFAULT_PASSES: (&[ConditionalPass], &[ConditionalPass]) = (
+    &[
+        ConditionalPass::always(COLLECT_TRAIT_IMPLS),
+        ConditionalPass::always(UNINDENT_COMMENTS),
+        ConditionalPass::new(STRIP_HIDDEN, WhenNotDocumentHidden),
+        ConditionalPass::new(STRIP_PRIVATE, WhenNotDocumentPrivate),
+        ConditionalPass::new(STRIP_PRIV_IMPORTS, WhenDocumentPrivate),
+    ],
+    // populate cache
+    &[
+        ConditionalPass::always(COLLECT_INTRA_DOC_LINKS),
+        ConditionalPass::always(CHECK_CODE_BLOCK_SYNTAX),
+        ConditionalPass::always(CHECK_INVALID_HTML_TAGS),
+        ConditionalPass::always(PROPAGATE_DOC_CFG),
+        ConditionalPass::always(CHECK_BARE_URLS),
+    ]
+);
 
 /// The list of default passes run when `--doc-coverage` is passed to rustdoc.
-crate const COVERAGE_PASSES: &[ConditionalPass] = &[
+crate const COVERAGE_PASSES: (&[ConditionalPass], &[ConditionalPass]) = (&[
     ConditionalPass::new(STRIP_HIDDEN, WhenNotDocumentHidden),
     ConditionalPass::new(STRIP_PRIVATE, WhenNotDocumentPrivate),
     ConditionalPass::always(CALCULATE_DOC_COVERAGE),
-];
+], &[]);
 
 impl ConditionalPass {
     crate const fn always(pass: Pass) -> Self {
@@ -126,7 +130,7 @@ impl ConditionalPass {
 }
 
 /// Returns the given default set of passes.
-crate fn defaults(show_coverage: bool) -> &'static [ConditionalPass] {
+crate fn defaults(show_coverage: bool) -> (&'static [ConditionalPass], &'static [ConditionalPass]) {
     if show_coverage { COVERAGE_PASSES } else { DEFAULT_PASSES }
 }
 

src/test/rustdoc-ui/assoc-item-not-in-scope.stderr

@@ -1,3 +1,20 @@
+warning: documentation for `f` links to item `S::fmt` which will not have documentation generated
+  --> $DIR/assoc-item-not-in-scope.rs:12:18
+   |
+LL |     /// Link to [`S::fmt`]
+   |                  ^^^^^^^^ this item is will not be documented
+   |
+   = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default
+   = note: `S::fmt` may be in a private module with all re-exports marked as `#[doc(no_inline)]`
+
+warning: documentation for `f` links to item `S::fmt` which will not have documentation generated
+  --> $DIR/assoc-item-not-in-scope.rs:20:18
+   |
+LL |     /// Link to [`S::fmt`]
+   |                  ^^^^^^^^ this item is will not be documented
+   |
+   = note: `S::fmt` may be in a private module with all re-exports marked as `#[doc(no_inline)]`
+
 error: unresolved link to `S::fmt`
   --> $DIR/assoc-item-not-in-scope.rs:4:15
    |
@@ -10,5 +27,5 @@ note: the lint level is defined here
 LL | #![deny(rustdoc::broken_intra_doc_links)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to previous error
+error: aborting due to previous error; 2 warnings emitted
 

src/test/rustdoc-ui/intra-doc/feature-gate-intra-doc-pointers.stderr

@@ -1,3 +1,12 @@
+warning: documentation for `feature_gate_intra_doc_pointers` links to item `pointer::add` which will not have documentation generated
+  --> $DIR/feature-gate-intra-doc-pointers.rs:1:6
+   |
+LL | //! [pointer::add]
+   |      ^^^^^^^^^^^^ this item is will not be documented
+   |
+   = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default
+   = note: `pointer::add` may be in a private module with all re-exports marked as `#[doc(no_inline)]`
+
 error[E0658]: linking to associated items of raw pointers is experimental
   --> $DIR/feature-gate-intra-doc-pointers.rs:1:6
    |
@@ -8,6 +17,14 @@ LL | //! [pointer::add]
    = help: add `#![feature(intra_doc_pointers)]` to the crate attributes to enable
    = note: rustdoc does not allow disambiguating between `*const` and `*mut`, and pointers are unstable until it does
 
+warning: documentation for `feature_gate_intra_doc_pointers` links to item `pointer::wrapping_add` which will not have documentation generated
+  --> $DIR/feature-gate-intra-doc-pointers.rs:3:6
+   |
+LL | //! [pointer::wrapping_add]
+   |      ^^^^^^^^^^^^^^^^^^^^^ this item is will not be documented
+   |
+   = note: `pointer::wrapping_add` may be in a private module with all re-exports marked as `#[doc(no_inline)]`
+
 error[E0658]: linking to associated items of raw pointers is experimental
   --> $DIR/feature-gate-intra-doc-pointers.rs:3:6
    |
@@ -18,6 +35,6 @@ LL | //! [pointer::wrapping_add]
    = help: add `#![feature(intra_doc_pointers)]` to the crate attributes to enable
    = note: rustdoc does not allow disambiguating between `*const` and `*mut`, and pointers are unstable until it does
 
-error: aborting due to 2 previous errors
+error: aborting due to 2 previous errors; 2 warnings emitted
 
 For more information about this error, try `rustc --explain E0658`.