Skip to content

Commit

Permalink
Rollup merge of rust-lang#67875 - dtolnay:hidden, r=GuillaumeGomez
Browse files Browse the repository at this point in the history
Distinguish between private items and hidden items in rustdoc

I believe rustdoc should not be conflating private items (visibility lower than `pub`) and hidden items (attribute `doc(hidden)`). This matters now that Cargo is passing --document-private-items by default for bin crates. In bin crates that rely on macros, intentionally hidden implementation details of the macros can overwhelm the actual useful internal API that one would want to document.

This PR restores the strip-hidden pass when documenting private items, and introduces a separate unstable --document-hidden-items option to skip the strip-hidden pass. The two options are orthogonal to one another.

Fixes rust-lang#67851. Closes rust-lang#60884.
  • Loading branch information
Centril authored Jan 7, 2020
2 parents a7529f7 + 90adafb commit 4b9d279
Show file tree
Hide file tree
Showing 20 changed files with 138 additions and 78 deletions.
43 changes: 26 additions & 17 deletions src/librustdoc/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::html;
use crate::html::markdown::IdMap;
use crate::html::static_files;
use crate::opts;
use crate::passes::{self, DefaultPassOption};
use crate::passes::{self, Condition, DefaultPassOption};
use crate::theme;

/// Configuration options for rustdoc.
Expand Down Expand Up @@ -98,6 +98,10 @@ 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 @@ -146,6 +150,8 @@ 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 @@ -240,22 +246,26 @@ impl Options {
println!("{:>20} - {}", pass.name, pass.description);
}
println!("\nDefault passes for rustdoc:");
for pass in passes::DEFAULT_PASSES {
println!("{:>20}", pass.name);
}
println!("\nPasses run with `--document-private-items`:");
for pass in passes::DEFAULT_PRIVATE_PASSES {
println!("{:>20}", pass.name);
for p in passes::DEFAULT_PASSES {
print!("{:>20}", p.pass.name);
println_condition(p.condition);
}

if nightly_options::is_nightly_build() {
println!("\nPasses run with `--show-coverage`:");
for pass in passes::DEFAULT_COVERAGE_PASSES {
println!("{:>20}", pass.name);
for p in passes::COVERAGE_PASSES {
print!("{:>20}", p.pass.name);
println_condition(p.condition);
}
println!("\nPasses run with `--show-coverage --document-private-items`:");
for pass in passes::PRIVATE_COVERAGE_PASSES {
println!("{:>20}", pass.name);
}

fn println_condition(condition: Condition) {
use Condition::*;
match condition {
Always => println!(),
WhenDocumentPrivate => println!(" (when --document-private-items)"),
WhenNotDocumentPrivate => println!(" (when not --document-private-items)"),
WhenNotDocumentHidden => println!(" (when not --document-hidden-items)"),
}
}

Expand Down Expand Up @@ -444,16 +454,11 @@ impl Options {
});

let show_coverage = matches.opt_present("show-coverage");
let document_private = matches.opt_present("document-private-items");

let default_passes = if matches.opt_present("no-defaults") {
passes::DefaultPassOption::None
} else if show_coverage && document_private {
passes::DefaultPassOption::PrivateCoverage
} else if show_coverage {
passes::DefaultPassOption::Coverage
} else if document_private {
passes::DefaultPassOption::Private
} else {
passes::DefaultPassOption::Default
};
Expand Down Expand Up @@ -492,6 +497,8 @@ impl Options {
let runtool = matches.opt_str("runtool");
let runtool_args = matches.opt_strs("runtool-arg");
let enable_per_target_ignores = matches.opt_present("enable-per-target-ignores");
let document_private = matches.opt_present("document-private-items");
let document_hidden = matches.opt_present("document-hidden-items");

let (lint_opts, describe_lints, lint_cap) = get_cmd_lint_options(matches, error_format);

Expand All @@ -518,6 +525,8 @@ impl Options {
should_test,
test_args,
default_passes,
document_private,
document_hidden,
manual_passes,
display_warnings,
show_coverage,
Expand Down
26 changes: 17 additions & 9 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::clean::{AttributesExt, MAX_DEF_ID};
use crate::config::{Options as RustdocOptions, RenderOptions};
use crate::html::render::RenderInfo;

use crate::passes;
use crate::passes::{self, Condition::*, ConditionalPass};

pub use rustc::session::config::{CodegenOptions, DebuggingOptions, Input, Options};
pub use rustc::session::search_paths::SearchPath;
Expand Down Expand Up @@ -221,6 +221,8 @@ 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 @@ -457,16 +459,14 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
}

if attr.is_word() && name == sym::document_private_items {
if default_passes == passes::DefaultPassOption::Default {
default_passes = passes::DefaultPassOption::Private;
}
document_private = true;
}
}

let passes = passes::defaults(default_passes).iter().chain(
let passes = passes::defaults(default_passes).iter().copied().chain(
manual_passes.into_iter().flat_map(|name| {
if let Some(pass) = passes::find_pass(&name) {
Some(pass)
Some(ConditionalPass::always(pass))
} else {
error!("unknown pass {}, skipping", name);
None
Expand All @@ -476,9 +476,17 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt

info!("Executing passes");

for pass in passes {
debug!("running pass {}", pass.name);
krate = (pass.pass)(krate, &ctxt);
for p in passes {
let run = match p.condition {
Always => true,
WhenDocumentPrivate => document_private,
WhenNotDocumentPrivate => !document_private,
WhenNotDocumentHidden => !document_hidden,
};
if run {
debug!("running pass {}", p.pass.name);
krate = (p.pass.run)(krate, &ctxt);
}
}

ctxt.sess().abort_if_errors();
Expand Down
3 changes: 3 additions & 0 deletions src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ fn opts() -> Vec<RustcOptGroup> {
stable("document-private-items", |o| {
o.optflag("", "document-private-items", "document private items")
}),
unstable("document-hidden-items", |o| {
o.optflag("", "document-hidden-items", "document items that have doc(hidden)")
}),
stable("test", |o| o.optflag("", "test", "run code examples as tests")),
stable("test-args", |o| {
o.optmulti("", "test-args", "arguments to pass to the test runner", "ARGS")
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/calculate_doc_coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::ops;

pub const CALCULATE_DOC_COVERAGE: Pass = Pass {
name: "calculate-doc-coverage",
pass: calculate_doc_coverage,
run: calculate_doc_coverage,
description: "counts the number of items with and without documentation",
};

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/check_code_block_syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::passes::Pass;

pub const CHECK_CODE_BLOCK_SYNTAX: Pass = Pass {
name: "check-code-block-syntax",
pass: check_code_block_syntax,
run: check_code_block_syntax,
description: "validates syntax inside Rust code blocks",
};

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/collapse_docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::mem::take;

pub const COLLAPSE_DOCS: Pass = Pass {
name: "collapse-docs",
pass: collapse_docs,
run: collapse_docs,
description: "concatenates all document attributes into one document attribute",
};

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use super::span_of_attrs;

pub const COLLECT_INTRA_DOC_LINKS: Pass = Pass {
name: "collect-intra-doc-links",
pass: collect_intra_doc_links,
run: collect_intra_doc_links,
description: "reads a crate's documentation to resolve intra-doc-links",
};

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/collect_trait_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_span::symbol::sym;

pub const COLLECT_TRAIT_IMPLS: Pass = Pass {
name: "collect-trait-impls",
pass: collect_trait_impls,
run: collect_trait_impls,
description: "retrieves trait impls for items in the crate",
};

Expand Down
87 changes: 51 additions & 36 deletions src/librustdoc/passes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use rustc_span::{InnerSpan, Span, DUMMY_SP};
use std::mem;
use std::ops::Range;

use self::Condition::*;
use crate::clean::{self, GetDefId, Item};
use crate::core::DocContext;
use crate::fold::{DocFolder, StripItem};
Expand Down Expand Up @@ -52,10 +53,29 @@ pub use self::calculate_doc_coverage::CALCULATE_DOC_COVERAGE;
#[derive(Copy, Clone)]
pub struct Pass {
pub name: &'static str,
pub pass: fn(clean::Crate, &DocContext<'_>) -> clean::Crate,
pub run: fn(clean::Crate, &DocContext<'_>) -> clean::Crate,
pub description: &'static str,
}

/// In a list of passes, a pass that may or may not need to be run depending on options.
#[derive(Copy, Clone)]
pub struct ConditionalPass {
pub pass: Pass,
pub condition: Condition,
}

/// How to decide whether to run a conditional pass.
#[derive(Copy, Clone)]
pub enum Condition {
Always,
/// When `--document-private-items` is passed.
WhenDocumentPrivate,
/// When `--document-private-items` is not passed.
WhenNotDocumentPrivate,
/// When `--document-hidden-items` is not passed.
WhenNotDocumentHidden,
}

/// The full list of passes.
pub const PASSES: &[Pass] = &[
CHECK_PRIVATE_ITEMS_DOC_TESTS,
Expand All @@ -72,63 +92,58 @@ pub const PASSES: &[Pass] = &[
];

/// The list of passes run by default.
pub const DEFAULT_PASSES: &[Pass] = &[
COLLECT_TRAIT_IMPLS,
COLLAPSE_DOCS,
UNINDENT_COMMENTS,
CHECK_PRIVATE_ITEMS_DOC_TESTS,
STRIP_HIDDEN,
STRIP_PRIVATE,
COLLECT_INTRA_DOC_LINKS,
CHECK_CODE_BLOCK_SYNTAX,
PROPAGATE_DOC_CFG,
pub const DEFAULT_PASSES: &[ConditionalPass] = &[
ConditionalPass::always(COLLECT_TRAIT_IMPLS),
ConditionalPass::always(COLLAPSE_DOCS),
ConditionalPass::always(UNINDENT_COMMENTS),
ConditionalPass::always(CHECK_PRIVATE_ITEMS_DOC_TESTS),
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(PROPAGATE_DOC_CFG),
];

/// The list of default passes run with `--document-private-items` is passed to rustdoc.
pub const DEFAULT_PRIVATE_PASSES: &[Pass] = &[
COLLECT_TRAIT_IMPLS,
COLLAPSE_DOCS,
UNINDENT_COMMENTS,
CHECK_PRIVATE_ITEMS_DOC_TESTS,
STRIP_PRIV_IMPORTS,
COLLECT_INTRA_DOC_LINKS,
CHECK_CODE_BLOCK_SYNTAX,
PROPAGATE_DOC_CFG,
/// The list of default passes run when `--doc-coverage` is passed to rustdoc.
pub const COVERAGE_PASSES: &[ConditionalPass] = &[
ConditionalPass::always(COLLECT_TRAIT_IMPLS),
ConditionalPass::new(STRIP_HIDDEN, WhenNotDocumentHidden),
ConditionalPass::new(STRIP_PRIVATE, WhenNotDocumentPrivate),
ConditionalPass::always(CALCULATE_DOC_COVERAGE),
];

/// The list of default passes run when `--doc-coverage` is passed to rustdoc.
pub const DEFAULT_COVERAGE_PASSES: &[Pass] =
&[COLLECT_TRAIT_IMPLS, STRIP_HIDDEN, STRIP_PRIVATE, CALCULATE_DOC_COVERAGE];
impl ConditionalPass {
pub const fn always(pass: Pass) -> Self {
Self::new(pass, Always)
}

/// The list of default passes run when `--doc-coverage --document-private-items` is passed to
/// rustdoc.
pub const PRIVATE_COVERAGE_PASSES: &[Pass] = &[COLLECT_TRAIT_IMPLS, CALCULATE_DOC_COVERAGE];
pub const fn new(pass: Pass, condition: Condition) -> Self {
ConditionalPass { pass, condition }
}
}

/// A shorthand way to refer to which set of passes to use, based on the presence of
/// `--no-defaults` or `--document-private-items`.
/// `--no-defaults` and `--show-coverage`.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub enum DefaultPassOption {
Default,
Private,
Coverage,
PrivateCoverage,
None,
}

/// Returns the given default set of passes.
pub fn defaults(default_set: DefaultPassOption) -> &'static [Pass] {
pub fn defaults(default_set: DefaultPassOption) -> &'static [ConditionalPass] {
match default_set {
DefaultPassOption::Default => DEFAULT_PASSES,
DefaultPassOption::Private => DEFAULT_PRIVATE_PASSES,
DefaultPassOption::Coverage => DEFAULT_COVERAGE_PASSES,
DefaultPassOption::PrivateCoverage => PRIVATE_COVERAGE_PASSES,
DefaultPassOption::Coverage => COVERAGE_PASSES,
DefaultPassOption::None => &[],
}
}

/// If the given name matches a known pass, returns its information.
pub fn find_pass(pass_name: &str) -> Option<&'static Pass> {
PASSES.iter().find(|p| p.name == pass_name)
pub fn find_pass(pass_name: &str) -> Option<Pass> {
PASSES.iter().find(|p| p.name == pass_name).copied()
}

struct Stripper<'a> {
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/private_items_doc_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::passes::{look_for_tests, Pass};

pub const CHECK_PRIVATE_ITEMS_DOC_TESTS: Pass = Pass {
name: "check-private-items-doc-tests",
pass: check_private_items_doc_tests,
run: check_private_items_doc_tests,
description: "check private items doc tests",
};

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/propagate_doc_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::passes::Pass;

pub const PROPAGATE_DOC_CFG: Pass = Pass {
name: "propagate-doc-cfg",
pass: propagate_doc_cfg,
run: propagate_doc_cfg,
description: "propagates `#[doc(cfg(...))]` to child items",
};

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/strip_hidden.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::passes::{ImplStripper, Pass};

pub const STRIP_HIDDEN: Pass = Pass {
name: "strip-hidden",
pass: strip_hidden,
run: strip_hidden,
description: "strips all doc(hidden) items from the output",
};

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/strip_priv_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::passes::{ImportStripper, Pass};

pub const STRIP_PRIV_IMPORTS: Pass = Pass {
name: "strip-priv-imports",
pass: strip_priv_imports,
run: strip_priv_imports,
description: "strips all private import statements (`use`, `extern crate`) from a crate",
};

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/strip_private.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::passes::{ImplStripper, ImportStripper, Pass, Stripper};

pub const STRIP_PRIVATE: Pass = Pass {
name: "strip-private",
pass: strip_private,
run: strip_private,
description: "strips all private items from a crate which cannot be seen externally, \
implies strip-priv-imports",
};
Expand Down
Loading

0 comments on commit 4b9d279

Please sign in to comment.