From 810596805b6de2c0fa31061651717a3a50f40044 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 26 Mar 2022 15:29:59 -0500 Subject: [PATCH 1/8] Make `impl Debug for rustdoc::clean::Item` easier to read Before: ``` Item { name: Some("Send"), attrs: Attributes { doc_strings: [DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:15:1: 15:60 (#0), parent_module: None, doc: " Types that can be transferred across thread boundaries.", kind: SugaredDoc, indent: 1 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:16:1: 16:4 (#0), parent_module: None, doc: "", kind: SugaredDoc, indent: 0 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:17:1: 17:78 (#0), parent_module: None, doc: " This trait is automatically implemented when the compiler determines it's", kind: SugaredDoc, indent: 1 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:18:1: 18:17 (#0), parent_module: None, doc: " appropriate.", kind: SugaredDoc, indent: 1 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:19:1: 19:4 (#0), parent_module: None, doc: "", kind: SugaredDoc, indent: 0 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:20:1: 20:70 (#0), parent_module: None, doc: " An example of a non-`Send` type is the reference-counting pointer", kind: SugaredDoc, indent: 1 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:21:1: 21:85 (#0), parent_module: None, doc: " [`rc::Rc`][`Rc`]. If two threads attempt to clone [`Rc`]s that point to the same", kind: SugaredDoc, indent: 1 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:22:1: 22:81 (#0), parent_module: None, doc: " reference-counted value, they might try to update the reference count at the", kind: SugaredDoc, indent: 1 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:23:1: 23:83 (#0), parent_module: None, doc: " same time, which is [undefined behavior][ub] because [`Rc`] doesn't use atomic", kind: SugaredDoc, indent: 1 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:24:1: 24:84 (#0), parent_module: None, doc: " operations. Its cousin [`sync::Arc`][arc] does use atomic operations (incurring", kind: SugaredDoc, indent: 1 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:25:1: 25:39 (#0), parent_module: None, doc: " some overhead) and thus is `Send`.", kind: SugaredDoc, indent: 1 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:26:1: 26:4 (#0), parent_module: None, doc: "", kind: SugaredDoc, indent: 0 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:27:1: 27:74 (#0), parent_module: None, doc: " See [the Nomicon](../../nomicon/send-and-sync.html) for more details.", kind: SugaredDoc, indent: 1 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:28:1: 28:4 (#0), parent_module: None, doc: "", kind: SugaredDoc, indent: 0 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:29:1: 29:40 (#0), parent_module: None, doc: " [`Rc`]: ../../std/rc/struct.Rc.html", kind: SugaredDoc, indent: 1 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:30:1: 30:42 (#0), parent_module: None, doc: " [arc]: ../../std/sync/struct.Arc.html", kind: SugaredDoc, indent: 1 }, DocFragment { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:31:1: 31:61 (#0), parent_module: None, doc: " [ub]: ../../reference/behavior-considered-undefined.html", kind: SugaredDoc, indent: 1 }], other_attrs: [Attribute { kind: Normal(AttrItem { path: Path { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:32:3: 32:9 (#0), segments: [PathSegment { ident: stable#0, id: NodeId(33577), args: None }], tokens: None }, args: Delimited(DelimSpan { open: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:32:9: 32:10 (#0), close: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:32:44: 32:45 (#0) }, Parenthesis, TokenStream([(Token(Token { kind: Ident("feature", false), span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:32:10: 32:17 (#0) }), Alone), (Token(Token { kind: Eq, span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:32:18: 32:19 (#0) }), Alone), (Token(Token { kind: Literal(Lit { kind: Str, symbol: "rust1", suffix: None }), span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:32:20: 32:27 (#0) }), Joint), (Token(Token { kind: Comma, span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:32:27: 32:28 (#0) }), Alone), (Token(Token { kind: Ident("since", false), span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:32:29: 32:34 (#0) }), Alone), (Token(Token { kind: Eq, span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:32:35: 32:36 (#0) }), Alone), (Token(Token { kind: Literal(Lit { kind: Str, symbol: "1.0.0", suffix: None }), span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:32:37: 32:44 (#0) }), Alone)])), tokens: None }, None), id: AttrId(48), style: Outer, span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:32:1: 32:46 (#0) }, Attribute { kind: Normal(AttrItem { path: Path { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:33:23: 33:44 (#0), segments: [PathSegment { ident: rustc_diagnostic_item#0, id: NodeId(33578), args: None }], tokens: None }, args: Eq(/rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:33:45: 33:46 (#0), Token { kind: Literal(Lit { kind: Str, symbol: "Send", suffix: None }), span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:33:47: 33:53 (#0) }), tokens: None }, None), id: AttrId(49), style: Outer, span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:33:23: 33:53 (#0) }, Attribute { kind: Normal(AttrItem { path: Path { span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:34:3: 34:25 (#0), segments: [PathSegment { ident: rustc_on_unimplemented#0, id: NodeId(33580), args: None }], tokens: None }, args: Delimited(DelimSpan { open: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:34:25: 34:26 (#0), close: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:37:1: 37:2 (#0) }, Parenthesis, TokenStream([(Token(Token { kind: Ident("message", false), span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:35:5: 35:12 (#0) }), Alone), (Token(Token { kind: Eq, span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:35:13: 35:14 (#0) }), Alone), (Token(Token { kind: Literal(Lit { kind: Str, symbol: "`{Self}` cannot be sent between threads safely", suffix: None }), span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:35:15: 35:63 (#0) }), Joint), (Token(Token { kind: Comma, span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:35:63: 35:64 (#0) }), Alone), (Token(Token { kind: Ident("label", false), span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:36:5: 36:10 (#0) }), Alone), (Token(Token { kind: Eq, span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:36:11: 36:12 (#0) }), Alone), (Token(Token { kind: Literal(Lit { kind: Str, symbol: "`{Self}` cannot be sent between threads safely", suffix: None }), span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:36:13: 36:61 (#0) }), Alone)])), tokens: None }, None), id: AttrId(50), style: Outer, span: /rustc/37b55c8a0cafdb60b9168da34f904acc70157df8/library/core/src/marker.rs:34:1: 37:3 (#0) }] }, visibility: Public, kind: TraitItem(Trait { unsafety: Unsafe, items: [], generics: Generics { params: [], where_predicates: [] }, bounds: [], is_auto: true }), def_id: DefId(DefId(2:3027 ~ core[7f4a]::marker::Send)), cfg: None } ``` After: ``` Item { name: Some("Send"), visibility: Public, def_id: DefId(DefId(2:3027 ~ core[7f4a]::marker::Send)), kind: Trait, docs: "Types that can be transferred across thread boundaries.\n\nThis trait is automatically implemented when the compiler determines it's\nappropriate.\n\nAn example of a non-`Send` type is the reference-counting pointer\n[`rc::Rc`][`Rc`]. If two threads attempt to clone [`Rc`]s that point to the same\nreference-counted value, they might try to update the reference count at the\nsame time, which is [undefined behavior][ub] because [`Rc`] doesn't use atomic\noperations. Its cousin [`sync::Arc`][arc] does use atomic operations (incurring\nsome overhead) and thus is `Send`.\n\nSee [the Nomicon](../../nomicon/send-and-sync.html) for more details.\n\n[`Rc`]: ../../std/rc/struct.Rc.html\n[arc]: ../../std/sync/struct.Arc.html\n[ub]: ../../reference/behavior-considered-undefined.html" } ``` --- src/librustdoc/clean/types.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 47289eb8978b9..38737c5ee0f9f 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1,3 +1,4 @@ +use core::fmt; use std::cell::RefCell; use std::default::Default; use std::hash::Hash; @@ -351,7 +352,7 @@ crate enum ExternalLocation { /// Anything with a source location and set of attributes and, optionally, a /// name. That is, anything that can be documented. This doesn't correspond /// directly to the AST's concept of an item; it's a strict superset. -#[derive(Clone, Debug)] +#[derive(Clone)] crate struct Item { /// The name of this item. /// Optional because not every item has a name, e.g. impls. @@ -366,6 +367,27 @@ crate struct Item { crate cfg: Option>, } +impl fmt::Debug for Item { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let alternate = f.alternate(); + // hand-picked fields that don't bloat the logs too much + let mut fmt = f.debug_struct("Item"); + fmt.field("name", &self.name) + .field("visibility", &self.visibility) + .field("def_id", &self.def_id); + // allow printing the full item if someone really wants to + if alternate { + fmt.field("attrs", &self.attrs).field("kind", &self.kind).field("cfg", &self.cfg); + } else { + fmt.field("kind", &self.type_()); + if let Some(docs) = self.doc_value() { + fmt.field("docs", &docs); + } + } + fmt.finish() + } +} + // `Item` is used a lot. Make sure it doesn't unintentionally get bigger. #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(Item, 56); From 2ef557026b28f71e1455c20dee5775feb2338f2f Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 11 Feb 2021 20:59:22 -0500 Subject: [PATCH 2/8] 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 --- src/librustdoc/config.rs | 4 +- src/librustdoc/core.rs | 23 +++-- src/librustdoc/html/format.rs | 17 +++- .../passes/collect_intra_doc_links.rs | 95 ++++++++++++++++++- src/librustdoc/passes/mod.rs | 36 +++---- .../rustdoc-ui/assoc-item-not-in-scope.stderr | 19 +++- .../feature-gate-intra-doc-pointers.stderr | 19 +++- 7 files changed, 178 insertions(+), 35 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index cee3dcb416f80..480d72076b773 100644 --- a/src/librustdoc/config.rs +++ b/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); } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index bd64e2b03ce0c..01ebb0e0c930c 100644 --- a/src/librustdoc/core.rs +++ b/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) } diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 5c59609d5b8c6..3b1a5fe453f56 100644 --- a/src/librustdoc/html/format.rs +++ b/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), 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), 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 diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 3d8a62d50e06d..bf05fcd3e632f 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/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 => "".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; diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 5db2d0747c83b..8db0400ba3901 100644 --- a/src/librustdoc/passes/mod.rs +++ b/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 } } diff --git a/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr b/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr index 04594ad414250..40cdeb5a9155f 100644 --- a/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr +++ b/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 diff --git a/src/test/rustdoc-ui/intra-doc/feature-gate-intra-doc-pointers.stderr b/src/test/rustdoc-ui/intra-doc/feature-gate-intra-doc-pointers.stderr index 2c946ed48db17..98b99a5d5b81b 100644 --- a/src/test/rustdoc-ui/intra-doc/feature-gate-intra-doc-pointers.stderr +++ b/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`. From b9085adeae1815a2c556fe6c6ed64fb145c42fc7 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 26 Mar 2022 00:14:39 -0500 Subject: [PATCH 3/8] fix some tests lolzzzzz --- src/librustdoc/clean/mod.rs | 2 ++ src/librustdoc/clean/types.rs | 11 ++++++++--- src/librustdoc/core.rs | 6 ++++-- src/librustdoc/formats/cache.rs | 8 +++++--- src/librustdoc/passes/collect_intra_doc_links.rs | 6 ++++-- src/librustdoc/passes/mod.rs | 1 + src/librustdoc/visit.rs | 7 ++++++- src/librustdoc/visit_ast.rs | 3 +++ src/test/rustdoc-ui/intra-doc/extern-crate-load.rs | 1 + .../intra-doc/feature-gate-intra-doc-pointers.rs | 2 ++ .../intra-doc/feature-gate-intra-doc-pointers.stderr | 4 ++-- 11 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 1e3260ce9ae2b..8dc0ae40f10d7 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1890,6 +1890,7 @@ fn clean_maybe_renamed_item( ) -> Vec { use hir::ItemKind; + debug!("clean HIR item {:?}", item); let def_id = item.def_id.to_def_id(); let mut name = renamed.unwrap_or_else(|| cx.tcx.hir().name(item.hir_id())); cx.with_param_env(def_id, |cx| { @@ -2017,6 +2018,7 @@ fn clean_impl(impl_: &hir::Impl<'_>, hir_id: hir::HirId, cx: &mut DocContext<'_> ret.push(make_item(trait_.clone(), type_alias, items.clone())); } ret.push(make_item(trait_, for_, items)); + debug!("create cleaned impl item {:?}", ret); ret } diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 38737c5ee0f9f..48a0d470a3dc5 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1123,11 +1123,16 @@ impl Attributes { ast::LitKind::Str(s, _) => { aliases.insert(s); } - _ => unreachable!(), + // This can be reached in case of an invalid attribute. + // We test that rustc_passes gives an error in `src/test/rustdoc-ui/check-doc-alias-attr.rs` + _ => {} } } - } else { - aliases.insert(attr.value_str().unwrap()); + // This can be None if the user specified an invalid attribute, e.g. `doc(alias = 0)`. + // The error will be caught by rustc_passes, but we still need to handle it here because + // the Cache is populated before calling `abort_if_err()`. + } else if let Some(value) = attr.value_str() { + aliases.insert(value); } } aliases.into_iter().collect::>().into() diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 01ebb0e0c930c..1ee652b74b686 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -457,13 +457,15 @@ crate fn run_global_ctxt( 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(); + + if tcx.sess.diagnostic().has_errors_or_lint_errors().is_some() { + rustc_errors::FatalError.raise(); + } (krate, ctxt.render_options, ctxt.cache) } diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 53159709586c6..9d360a070b55a 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -210,6 +210,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { .def_id(self.cache) .map_or(false, |d| self.cache.masked_crates.contains(&d.krate)) { + debug!("remove masked impl {:?}", i); return None; } } @@ -461,16 +462,17 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { } } } - let impl_item = Impl { impl_item: item }; + let impl_item = Impl { impl_item: item.clone() }; if impl_item.trait_did().map_or(true, |d| self.cache.traits.contains_key(&d)) { for did in dids { self.cache.impls.entry(did).or_insert_with(Vec::new).push(impl_item.clone()); } } else { let trait_did = impl_item.trait_did().expect("no trait did"); - self.cache.orphan_trait_impls.push((trait_did, dids, impl_item)); + self.cache.orphan_trait_impls.push((trait_did, dids, impl_item.clone())); } - None + // TODO: stripping this from `Module` seems ... not great + Some(impl_item.impl_item) } else { Some(item) }; diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index bf05fcd3e632f..42bc71a098df7 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -41,6 +41,7 @@ crate const COLLECT_INTRA_DOC_LINKS: Pass = Pass { }; fn collect_intra_doc_links(krate: Crate, cx: &mut DocContext<'_>) -> Crate { + debug!("logging works"); let mut collector = LinkCollector { cx, mod_ids: Vec::new(), visited_links: FxHashMap::default() }; collector.visit_crate(&krate); @@ -2344,8 +2345,9 @@ fn missing_docs_for_link( return; } - // shouldn't be possible to resolve private items - assert_ne!(why, HrefError::Private, "{:?}", destination_id); + // NOTE: theoretically it shouldn't be possible to resolve private items, + // but `resolve_primitive_associated_item` is buggy and allows it. + // 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) diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 8db0400ba3901..ca611752d1eed 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -98,6 +98,7 @@ crate const DEFAULT_PASSES: (&[ConditionalPass], &[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), diff --git a/src/librustdoc/visit.rs b/src/librustdoc/visit.rs index b16cab1c646f1..2977218529544 100644 --- a/src/librustdoc/visit.rs +++ b/src/librustdoc/visit.rs @@ -55,7 +55,12 @@ crate trait DocVisitor: Sized { } fn visit_mod(&mut self, m: &Module) { - m.items.iter().for_each(|i| self.visit_item(i)) + m.items.iter().for_each(|i| self.visit_item(i)); + // for i in self.cache.impls { + // if m.def_id() == find_nearest_parent_module(self.tcx, i.def_id) { + // self.visit_item(i); + // } + // } } fn visit_crate(&mut self, c: &Crate) { diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index e793ee75fd2e0..61a39098a599d 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -161,6 +161,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { self.inside_public_path &= self.cx.tcx.visibility(def_id).is_public(); for &i in m.item_ids { let item = self.cx.tcx.hir().item(i); + debug!("{:?}", item); self.visit_item(item, None, &mut om); } self.inside_public_path = orig_inside_public_path; @@ -368,7 +369,9 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { hir::ItemKind::Impl(ref impl_) => { // Don't duplicate impls when inlining or if it's implementing a trait, we'll pick // them up regardless of where they're located. + debug!("handle impl block"); if !self.inlining && impl_.of_trait.is_none() { + debug!("propagate impl block"); om.items.push((item, None)); } } diff --git a/src/test/rustdoc-ui/intra-doc/extern-crate-load.rs b/src/test/rustdoc-ui/intra-doc/extern-crate-load.rs index 438c56aa516a9..eca295a2ee24e 100644 --- a/src/test/rustdoc-ui/intra-doc/extern-crate-load.rs +++ b/src/test/rustdoc-ui/intra-doc/extern-crate-load.rs @@ -4,6 +4,7 @@ // aux-crate:dep3=dep3.rs // aux-crate:dep4=dep4.rs #![deny(rustdoc::broken_intra_doc_links)] +#![allow(rustdoc::private_intra_doc_links)] pub trait Trait { /// [dep1] diff --git a/src/test/rustdoc-ui/intra-doc/feature-gate-intra-doc-pointers.rs b/src/test/rustdoc-ui/intra-doc/feature-gate-intra-doc-pointers.rs index 3cfac942ca85e..b929cb0d3a185 100644 --- a/src/test/rustdoc-ui/intra-doc/feature-gate-intra-doc-pointers.rs +++ b/src/test/rustdoc-ui/intra-doc/feature-gate-intra-doc-pointers.rs @@ -1,6 +1,8 @@ //! [pointer::add] //~^ ERROR: experimental +//~| WARNING will not have documentation generated //! [pointer::wrapping_add] //~^ ERROR: experimental +//~| WARNING will not have documentation generated //! [pointer] // This is explicitly allowed //! [reference] // This is explicitly allowed diff --git a/src/test/rustdoc-ui/intra-doc/feature-gate-intra-doc-pointers.stderr b/src/test/rustdoc-ui/intra-doc/feature-gate-intra-doc-pointers.stderr index 98b99a5d5b81b..754a69088d85a 100644 --- a/src/test/rustdoc-ui/intra-doc/feature-gate-intra-doc-pointers.stderr +++ b/src/test/rustdoc-ui/intra-doc/feature-gate-intra-doc-pointers.stderr @@ -18,7 +18,7 @@ LL | //! [pointer::add] = 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 + --> $DIR/feature-gate-intra-doc-pointers.rs:4:6 | LL | //! [pointer::wrapping_add] | ^^^^^^^^^^^^^^^^^^^^^ this item is will not be documented @@ -26,7 +26,7 @@ LL | //! [pointer::wrapping_add] = 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 + --> $DIR/feature-gate-intra-doc-pointers.rs:4:6 | LL | //! [pointer::wrapping_add] | ^^^^^^^^^^^^^^^^^^^^^ From 53d7f95e206b7076a1a62c7cee5196658f4e9eb7 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 26 Mar 2022 00:35:26 -0500 Subject: [PATCH 4/8] [wip] try to process impl blocks in collect pass aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaahhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh this is so curseeeeeeed currently failing: - rustdoc/doc-cfg-simplification.rs - rustdoc/intra-doc-crate/self.rs --- src/librustdoc/core.rs | 2 ++ src/librustdoc/formats/cache.rs | 19 +++++++++++++++++-- src/librustdoc/formats/mod.rs | 1 + src/librustdoc/visit.rs | 5 ----- .../rustdoc-ui/assoc-item-not-in-scope.stderr | 19 +------------------ src/tools/compiletest/src/runtest.rs | 1 + 6 files changed, 22 insertions(+), 25 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 1ee652b74b686..041f5c75debd5 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -467,6 +467,8 @@ crate fn run_global_ctxt( rustc_errors::FatalError.raise(); } + ctxt.cache.populate_impls(&krate); + (krate, ctxt.render_options, ctxt.cache) } diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 9d360a070b55a..ac48c9e798f43 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -179,9 +179,17 @@ impl Cache { } } } + // god what a mess + *krate.external_traits.borrow_mut() = mem::take(&mut cx.cache.traits); krate } + + /// `external_trats` / `cache.traits` is modified in various passes. + /// Run this separate from the main `populate` call, since `impls` isn't used until later in the HTML formatter. + crate fn populate_impls(&mut self, krate: &clean::Crate) { + self.traits = krate.external_traits.take(); + } } impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { @@ -462,7 +470,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { } } } - let impl_item = Impl { impl_item: item.clone() }; + let impl_item = Impl { impl_item: item }; if impl_item.trait_did().map_or(true, |d| self.cache.traits.contains_key(&d)) { for did in dids { self.cache.impls.entry(did).or_insert_with(Vec::new).push(impl_item.clone()); @@ -472,7 +480,14 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { self.cache.orphan_trait_impls.push((trait_did, dids, impl_item.clone())); } // TODO: stripping this from `Module` seems ... not great - Some(impl_item.impl_item) + // None + let item = impl_item.impl_item; + if item.def_id.is_local() { + debug!("propagating impl {:?}", item); + Some(item) + } else { + None + } } else { Some(item) }; diff --git a/src/librustdoc/formats/mod.rs b/src/librustdoc/formats/mod.rs index 4f0c5a9edee71..123ecdbf5ee8c 100644 --- a/src/librustdoc/formats/mod.rs +++ b/src/librustdoc/formats/mod.rs @@ -25,6 +25,7 @@ crate enum RenderMode { /// Metadata about implementations for a type or trait. #[derive(Clone, Debug)] +// TODO: this should not exist crate struct Impl { crate impl_item: clean::Item, } diff --git a/src/librustdoc/visit.rs b/src/librustdoc/visit.rs index 2977218529544..44cee081be9c3 100644 --- a/src/librustdoc/visit.rs +++ b/src/librustdoc/visit.rs @@ -56,11 +56,6 @@ crate trait DocVisitor: Sized { fn visit_mod(&mut self, m: &Module) { m.items.iter().for_each(|i| self.visit_item(i)); - // for i in self.cache.impls { - // if m.def_id() == find_nearest_parent_module(self.tcx, i.def_id) { - // self.visit_item(i); - // } - // } } fn visit_crate(&mut self, c: &Crate) { diff --git a/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr b/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr index 40cdeb5a9155f..04594ad414250 100644 --- a/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr +++ b/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr @@ -1,20 +1,3 @@ -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 | @@ -27,5 +10,5 @@ note: the lint level is defined here LL | #![deny(rustdoc::broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to previous error; 2 warnings emitted +error: aborting due to previous error diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 6b27d1ecbf550..77c265ca6af60 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1490,6 +1490,7 @@ impl<'test> TestCx<'test> { .arg(out_dir) .arg("--deny") .arg("warnings") + .arg("-Arustdoc::private-intra-doc-links") .arg(&self.testpaths.file) .args(&self.props.compile_flags); From f6419c793e4472748bb2708056720519406491c6 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 26 Mar 2022 16:24:24 -0500 Subject: [PATCH 5/8] fix impl blocks --- src/librustdoc/core.rs | 2 +- src/librustdoc/formats/cache.rs | 33 +++++++++++++++++---------------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 041f5c75debd5..94df9043146c7 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -467,7 +467,7 @@ crate fn run_global_ctxt( rustc_errors::FatalError.raise(); } - ctxt.cache.populate_impls(&krate); + krate = ctxt.cache.populate_impls(krate); (krate, ctxt.render_options, ctxt.cache) } diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index ac48c9e798f43..41f5d571e7b14 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -187,8 +187,20 @@ impl Cache { /// `external_trats` / `cache.traits` is modified in various passes. /// Run this separate from the main `populate` call, since `impls` isn't used until later in the HTML formatter. - crate fn populate_impls(&mut self, krate: &clean::Crate) { + crate fn populate_impls(&mut self, krate: clean::Crate) -> clean::Crate { self.traits = krate.external_traits.take(); + ImplRemover.fold_crate(krate) + } +} + +struct ImplRemover; +impl DocFolder for ImplRemover { + fn fold_item(&mut self, item: clean::Item) -> Option { + let item = self.fold_item_recur(item); + match *item.kind { + clean::ItemKind::ImplItem(_) => None, + _ => Some(item), + } } } @@ -438,7 +450,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { // Once we've recursively found all the generics, hoard off all the // implementations elsewhere. let item = self.fold_item_recur(item); - let ret = if let clean::Item { kind: box clean::ImplItem(ref i), .. } = item { + if let clean::Item { kind: box clean::ImplItem(ref i), .. } = item { // Figure out the id of this impl. This may map to a // primitive rather than always to a struct/enum. // Note: matching twice to restrict the lifetime of the `i` borrow. @@ -470,7 +482,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { } } } - let impl_item = Impl { impl_item: item }; + let impl_item = Impl { impl_item: item.clone() }; if impl_item.trait_did().map_or(true, |d| self.cache.traits.contains_key(&d)) { for did in dids { self.cache.impls.entry(did).or_insert_with(Vec::new).push(impl_item.clone()); @@ -479,18 +491,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { let trait_did = impl_item.trait_did().expect("no trait did"); self.cache.orphan_trait_impls.push((trait_did, dids, impl_item.clone())); } - // TODO: stripping this from `Module` seems ... not great - // None - let item = impl_item.impl_item; - if item.def_id.is_local() { - debug!("propagating impl {:?}", item); - Some(item) - } else { - None - } - } else { - Some(item) - }; + } if pushed { self.cache.stack.pop().expect("stack already empty"); @@ -500,6 +501,6 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { } self.cache.stripped_mod = orig_stripped_mod; self.cache.parent_is_trait_impl = orig_parent_is_trait_impl; - ret + Some(item) } } From 7531753faa1fa2f007b8d69cff7c4404e7b3c427 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 26 Mar 2022 16:27:22 -0500 Subject: [PATCH 6/8] fix tidy so ci is useful again --- src/librustdoc/core.rs | 2 +- src/librustdoc/formats/mod.rs | 1 - src/librustdoc/passes/collect_intra_doc_links.rs | 16 +++++++++++++--- src/librustdoc/passes/mod.rs | 15 +++++++++------ 4 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 94df9043146c7..1c7ccdd06cb01 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -25,7 +25,7 @@ use std::mem; use std::rc::Rc; use crate::clean::inline::build_external_trait; -use crate::clean::{self, ItemId, TraitWithExtraInfo, Crate}; +use crate::clean::{self, Crate, ItemId, TraitWithExtraInfo}; use crate::config::{Options as RustdocOptions, OutputFormat, RenderOptions}; use crate::formats::cache::Cache; use crate::passes::{self, Condition::*, ConditionalPass}; diff --git a/src/librustdoc/formats/mod.rs b/src/librustdoc/formats/mod.rs index 123ecdbf5ee8c..4f0c5a9edee71 100644 --- a/src/librustdoc/formats/mod.rs +++ b/src/librustdoc/formats/mod.rs @@ -25,7 +25,6 @@ crate enum RenderMode { /// Metadata about implementations for a type or trait. #[derive(Clone, Debug)] -// TODO: this should not exist crate struct Impl { crate impl_item: clean::Item, } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 42bc71a098df7..d8a68d8fc8925 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -26,13 +26,21 @@ use std::fmt::Write; use std::mem; use std::ops::Range; -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::{ + clean::{self, utils::find_nearest_parent_module}, + html::{ + format::HrefError, + markdown::{markdown_links, MarkdownLink}, + }, + lint::{BROKEN_INTRA_DOC_LINKS, PRIVATE_INTRA_DOC_LINKS}, + visit::DocVisitor, +}; mod early; -crate use early::early_resolve_intra_doc_links; use super::Pass; +crate use early::early_resolve_intra_doc_links; crate const COLLECT_INTRA_DOC_LINKS: Pass = Pass { name: "collect-intra-doc-links", @@ -1436,7 +1444,9 @@ impl LinkCollector<'_, '_> { 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, &[]) { + 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); } diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index ca611752d1eed..27ffff85b3052 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -110,15 +110,18 @@ crate const DEFAULT_PASSES: (&[ConditionalPass], &[ConditionalPass]) = ( 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], &[ConditionalPass]) = (&[ - ConditionalPass::new(STRIP_HIDDEN, WhenNotDocumentHidden), - ConditionalPass::new(STRIP_PRIVATE, WhenNotDocumentPrivate), - ConditionalPass::always(CALCULATE_DOC_COVERAGE), -], &[]); +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 { From 17f1a53bb7d4d88789b92b80d1fe7b97149b4747 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 26 Mar 2022 16:35:27 -0500 Subject: [PATCH 7/8] it all works!@!!!! --- src/librustdoc/passes/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 27ffff85b3052..03346d2e66a29 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -102,13 +102,13 @@ crate const DEFAULT_PASSES: (&[ConditionalPass], &[ConditionalPass]) = ( ConditionalPass::new(STRIP_HIDDEN, WhenNotDocumentHidden), ConditionalPass::new(STRIP_PRIVATE, WhenNotDocumentPrivate), ConditionalPass::new(STRIP_PRIV_IMPORTS, WhenDocumentPrivate), + ConditionalPass::always(PROPAGATE_DOC_CFG), ], // 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), ], ); From d33d01473649e764f05eacceb7d59a1b66cd2b60 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 26 Mar 2022 17:16:12 -0500 Subject: [PATCH 8/8] add test case and fix one last test --- src/librustdoc/html/format.rs | 2 +- .../passes/collect_intra_doc_links.rs | 16 ++++--- .../rustdoc-ui/intra-doc/warn-undocumented.rs | 42 +++++++++++++++++ .../intra-doc/warn-undocumented.stderr | 47 +++++++++++++++++++ 4 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 src/test/rustdoc-ui/intra-doc/warn-undocumented.rs create mode 100644 src/test/rustdoc-ui/intra-doc/warn-undocumented.stderr diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 3b1a5fe453f56..ba912a7795f53 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -567,7 +567,7 @@ crate fn href_inner( ) -> Result<(String, ItemType, Vec), HrefError> { let def_kind = tcx.def_kind(did); let did = match def_kind { - DefKind::AssocTy | DefKind::AssocFn | DefKind::AssocConst | DefKind::Variant => { + DefKind::AssocTy | DefKind::AssocFn | DefKind::AssocConst | DefKind::Variant | DefKind::Field => { // documented on their parent's page tcx.parent(did).unwrap() } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index d8a68d8fc8925..2f968e5a3c206 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -2324,11 +2324,13 @@ fn missing_docs_for_link( path_str: &str, diag_info: &DiagnosticInfo<'_>, ) { + debug!("missing docs for {:?} because {:?}", item, why); // 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; - } + // if why == HrefError::NotInExternalCache { + // debug!("ignoring NotInExternalCache"); + // return; + // } let item_name = match item.name { Some(name) => name.to_string(), @@ -2355,8 +2357,7 @@ fn missing_docs_for_link( return; } - // NOTE: theoretically it shouldn't be possible to resolve private items, - // but `resolve_primitive_associated_item` is buggy and allows it. + // NOTE: `href_inner` is buggy and thinks that primitive associated items can be unreachable // assert_ne!(why, HrefError::Private, "{:?}", destination_id); if let Some(attr) = @@ -2368,17 +2369,18 @@ fn missing_docs_for_link( 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) { + if let Some(attr) = cx.tcx.get_attrs(parent).lists(sym::doc).get_word_attr(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)]`", + "`{}` is in the {} `{}`, which is marked as `#[doc(hidden)]`", path_str, description, name ), ); + diag.span_label(attr.span(), &format!("`{}` is hidden here", name)); return; } current = parent; diff --git a/src/test/rustdoc-ui/intra-doc/warn-undocumented.rs b/src/test/rustdoc-ui/intra-doc/warn-undocumented.rs new file mode 100644 index 0000000000000..ea2627b411418 --- /dev/null +++ b/src/test/rustdoc-ui/intra-doc/warn-undocumented.rs @@ -0,0 +1,42 @@ +// aux-build:dep1.rs + +#![deny(rustdoc::private_intra_doc_links)] +//~^ NOTE defined here + +//! Link to [dep1] +//~^ ERROR will not have documentation generated +//~| NOTE will not be documented +//~| crate `dep1`, which will not be documented + +//! Link to [S] +//~^ ERROR will not have documentation generated +//~| NOTE will not be documented +//~| NOTE may be in a private module + +//! Link to [T] +//~^ ERROR will not have documentation generated +//~| NOTE will not be documented + +//! Link to [secret::U] +//~^ ERROR will not have documentation generated +//~| NOTE will not be documented + +extern crate dep1; + +#[doc(no_inline)] +pub use inner::S; + +mod inner { + pub struct S; +} + +#[doc(hidden)] +//~^ NOTE doc(hidden) +pub struct T; + +#[doc(hidden)] +//~^ NOTE `secret` is hidden +pub mod secret { +//~^ NOTE doc(hidden) + pub struct U; +} \ No newline at end of file diff --git a/src/test/rustdoc-ui/intra-doc/warn-undocumented.stderr b/src/test/rustdoc-ui/intra-doc/warn-undocumented.stderr new file mode 100644 index 0000000000000..2bc4f220e70b8 --- /dev/null +++ b/src/test/rustdoc-ui/intra-doc/warn-undocumented.stderr @@ -0,0 +1,47 @@ +error: documentation for `warn_undocumented` links to item `dep1` which will not have documentation generated + --> $DIR/warn-undocumented.rs:6:14 + | +LL | //! Link to [dep1] + | ^^^^ this item is will not be documented + | +note: the lint level is defined here + --> $DIR/warn-undocumented.rs:3:9 + | +LL | #![deny(rustdoc::private_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `dep1` is in the crate `dep1`, which will not be documented + +error: documentation for `warn_undocumented` links to item `S` which will not have documentation generated + --> $DIR/warn-undocumented.rs:11:14 + | +LL | //! Link to [S] + | ^ this item is will not be documented + | + = note: `S` may be in a private module with all re-exports marked as `#[doc(no_inline)]` + +error: documentation for `warn_undocumented` links to item `T` which will not have documentation generated + --> $DIR/warn-undocumented.rs:16:14 + | +LL | //! Link to [T] + | ^ this item is will not be documented +... +LL | #[doc(hidden)] + | ------ `T` is marked as `#[doc(hidden)]` + +error: documentation for `warn_undocumented` links to item `secret::U` which will not have documentation generated + --> $DIR/warn-undocumented.rs:20:14 + | +LL | //! Link to [secret::U] + | ^^^^^^^^^ this item is will not be documented +... +LL | #[doc(hidden)] + | ------ `secret` is hidden here +LL | +LL | / pub mod secret { +LL | | +LL | | pub struct U; +LL | | } + | |_- `secret::U` is in the module `secret`, which is marked as `#[doc(hidden)]` + +error: aborting due to 4 previous errors +