From 009c91a294e56878e3359d58903d5dc8d3d0f35b Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Wed, 30 Jan 2019 14:04:56 -0600 Subject: [PATCH 01/14] add option to calculate documentation coverage --- src/doc/rustdoc/src/unstable-features.md | 21 ++++ src/librustdoc/config.rs | 12 +++ src/librustdoc/core.rs | 11 +- src/librustdoc/lib.rs | 12 +++ .../passes/calculate_doc_coverage.rs | 101 ++++++++++++++++++ src/librustdoc/passes/mod.rs | 14 +++ 6 files changed, 167 insertions(+), 4 deletions(-) create mode 100644 src/librustdoc/passes/calculate_doc_coverage.rs diff --git a/src/doc/rustdoc/src/unstable-features.md b/src/doc/rustdoc/src/unstable-features.md index 3463cdb126cc6..cb741d1bf99ff 100644 --- a/src/doc/rustdoc/src/unstable-features.md +++ b/src/doc/rustdoc/src/unstable-features.md @@ -428,3 +428,24 @@ $ rustdoc src/lib.rs --test -Z unstable-options --persist-doctests target/rustdo This flag allows you to keep doctest executables around after they're compiled or run. Usually, rustdoc will immediately discard a compiled doctest after it's been tested, but with this option, you can keep those binaries around for farther testing. + +### `--show-coverage`: calculate the percentage of items with documentation + +Using this flag looks like this: + +```bash +$ rustdoc src/lib.rs -Z unstable-options --show-coverage +``` + +If you want to determine how many items in your crate are documented, pass this flag to rustdoc. +When it receives this flag, it will count the public items in your crate that have documentation, +and print out the counts and a percentage instead of generating docs. + +Some methodology notes about what rustdoc counts in this metric: + +* Rustdoc will only count items from your crate (i.e. items re-exported from other crates don't + count). +* Since trait implementations can inherit documentation from their trait, it will count trait impl + blocks separately, and show totals both with and without trait impls included. +* Inherent impl blocks are not counted, even though their doc comments are displayed, because the + common pattern in Rust code is to write all inherent methods into the same impl block. diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index b1c53ea92b300..d7c6b197164dc 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -85,6 +85,9 @@ pub struct Options { /// Whether to display warnings during doc generation or while gathering doctests. By default, /// all non-rustdoc-specific lints are allowed when generating docs. pub display_warnings: bool, + /// Whether to run the `calculate-doc-coverage` pass, which counts the number of public items + /// with and without documentation. + pub show_coverage: bool, // Options that alter generated documentation pages @@ -128,6 +131,7 @@ impl fmt::Debug for Options { .field("default_passes", &self.default_passes) .field("manual_passes", &self.manual_passes) .field("display_warnings", &self.display_warnings) + .field("show_coverage", &self.show_coverage) .field("crate_version", &self.crate_version) .field("render_options", &self.render_options) .finish() @@ -224,6 +228,10 @@ impl Options { for &name in passes::DEFAULT_PRIVATE_PASSES { println!("{:>20}", name); } + println!("\nPasses run with `--show-coverage`:"); + for &name in passes::DEFAULT_COVERAGE_PASSES { + println!("{:>20}", name); + } return Err(0); } @@ -415,12 +423,15 @@ impl Options { let default_passes = if matches.opt_present("no-defaults") { passes::DefaultPassOption::None + } else if matches.opt_present("show-coverage") { + passes::DefaultPassOption::Coverage } else if matches.opt_present("document-private-items") { passes::DefaultPassOption::Private } else { passes::DefaultPassOption::Default }; let manual_passes = matches.opt_strs("passes"); + let show_coverage = matches.opt_present("show-coverage"); let crate_name = matches.opt_str("crate-name"); let playground_url = matches.opt_str("playground-url"); @@ -463,6 +474,7 @@ impl Options { default_passes, manual_passes, display_warnings, + show_coverage, crate_version, persist_doctests, render_options: RenderOptions { diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 4dce4e86cc449..76c3bca7c14c8 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -605,10 +605,13 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt info!("Executing passes"); - for pass in &passes { - match passes::find_pass(pass).map(|p| p.pass) { - Some(pass) => krate = pass(krate, &ctxt), - None => error!("unknown pass {}, skipping", *pass), + for pass_name in &passes { + match passes::find_pass(pass_name).map(|p| p.pass) { + Some(pass) => { + debug!("running pass {}", pass_name); + krate = pass(krate, &ctxt); + } + None => error!("unknown pass {}, skipping", *pass_name), } } diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 39e504951d1c6..625e3d05c2997 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -347,6 +347,11 @@ fn opts() -> Vec { "generate-redirect-pages", "Generate extra pages to support legacy URLs and tool links") }), + unstable("show-coverage", |o| { + o.optflag("", + "show-coverage", + "calculate percentage of public items with documentation") + }), ] } @@ -391,7 +396,14 @@ fn main_args(args: &[String]) -> isize { let diag_opts = (options.error_format, options.debugging_options.treat_err_as_bug, options.debugging_options.ui_testing); + let show_coverage = options.show_coverage; rust_input(options, move |out| { + if show_coverage { + // if we ran coverage, bail early, we don't need to also generate docs at this point + // (also we didn't load in any of the useful passes) + return rustc_driver::EXIT_SUCCESS; + } + let Output { krate, passes, renderinfo, renderopts } = out; info!("going to format"); let (error_format, treat_err_as_bug, ui_testing) = diag_opts; diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs new file mode 100644 index 0000000000000..b812415d6775f --- /dev/null +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -0,0 +1,101 @@ +use crate::clean; +use crate::core::DocContext; +use crate::fold::{self, DocFolder}; +use crate::passes::Pass; + +use syntax::attr; + +pub const CALCULATE_DOC_COVERAGE: Pass = Pass { + name: "calculate-doc-coverage", + pass: calculate_doc_coverage, + description: "counts the number of items with and without documentation", +}; + +fn calculate_doc_coverage(krate: clean::Crate, _: &DocContext<'_, '_, '_>) -> clean::Crate { + let mut calc = CoverageCalculator::default(); + let krate = calc.fold_crate(krate); + + let total_minus_traits = calc.total - calc.total_trait_impls; + let docs_minus_traits = calc.with_docs - calc.trait_impls_with_docs; + + print!("Rustdoc found {}/{} items with documentation", calc.with_docs, calc.total); + println!(" ({}/{} not counting trait impls)", docs_minus_traits, total_minus_traits); + + if calc.total > 0 { + let percentage = (calc.with_docs as f64 * 100.0) / calc.total as f64; + let percentage_minus_traits = + (docs_minus_traits as f64 * 100.0) / total_minus_traits as f64; + println!(" Score: {:.1}% ({:.1}% not counting trait impls)", + percentage, percentage_minus_traits); + } + + krate +} + +#[derive(Default)] +struct CoverageCalculator { + total: usize, + with_docs: usize, + total_trait_impls: usize, + trait_impls_with_docs: usize, +} + +impl fold::DocFolder for CoverageCalculator { + fn fold_item(&mut self, i: clean::Item) -> Option { + match i.inner { + clean::StrippedItem(..) => {} + clean::ImplItem(ref impl_) + if attr::contains_name(&i.attrs.other_attrs, "automatically_derived") + || impl_.synthetic || impl_.blanket_impl.is_some() => + { + // skip counting anything inside these impl blocks + // FIXME(misdreavus): need to also find items that came out of a derive macro + return Some(i); + } + // non-local items are skipped because they can be out of the users control, especially + // in the case of trait impls, which rustdoc eagerly inlines + _ => if i.def_id.is_local() { + let has_docs = !i.attrs.doc_strings.is_empty(); + + if let clean::ImplItem(ref i) = i.inner { + if let Some(ref tr) = i.trait_ { + debug!("counting impl {:#} for {:#}", tr, i.for_); + + self.total += 1; + if has_docs { + self.with_docs += 1; + } + + // trait impls inherit their docs from the trait definition, so documenting + // them can be considered optional + + self.total_trait_impls += 1; + if has_docs { + self.trait_impls_with_docs += 1; + } + + for it in &i.items { + self.total_trait_impls += 1; + if !it.attrs.doc_strings.is_empty() { + self.trait_impls_with_docs += 1; + } + } + } else { + // inherent impls *can* be documented, and those docs show up, but in most + // cases it doesn't make sense, as all methods on a type are in one single + // impl block + debug!("not counting impl {:#}", i.for_); + } + } else { + debug!("counting {} {:?}", i.type_(), i.name); + self.total += 1; + if has_docs { + self.with_docs += 1; + } + } + } + } + + self.fold_item_recur(i) + } +} diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 3a9d8ef20ce84..bda63ae18fd7b 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -45,6 +45,9 @@ pub use self::collect_trait_impls::COLLECT_TRAIT_IMPLS; mod check_code_block_syntax; pub use self::check_code_block_syntax::CHECK_CODE_BLOCK_SYNTAX; +mod calculate_doc_coverage; +pub use self::calculate_doc_coverage::CALCULATE_DOC_COVERAGE; + /// A single pass over the cleaned documentation. /// /// Runs in the compiler context, so it has access to types and traits and the like. @@ -67,6 +70,7 @@ pub const PASSES: &'static [Pass] = &[ COLLECT_INTRA_DOC_LINKS, CHECK_CODE_BLOCK_SYNTAX, COLLECT_TRAIT_IMPLS, + CALCULATE_DOC_COVERAGE, ]; /// The list of passes run by default. @@ -94,12 +98,21 @@ pub const DEFAULT_PRIVATE_PASSES: &[&str] = &[ "propagate-doc-cfg", ]; +/// The list of default passes run when `--doc-coverage` is passed to rustdoc. +pub const DEFAULT_COVERAGE_PASSES: &'static [&'static str] = &[ + "collect-trait-impls", + "strip-hidden", + "strip-private", + "calculate-doc-coverage", +]; + /// A shorthand way to refer to which set of passes to use, based on the presence of /// `--no-defaults` or `--document-private-items`. #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum DefaultPassOption { Default, Private, + Coverage, None, } @@ -108,6 +121,7 @@ pub fn defaults(default_set: DefaultPassOption) -> &'static [&'static str] { match default_set { DefaultPassOption::Default => DEFAULT_PASSES, DefaultPassOption::Private => DEFAULT_PRIVATE_PASSES, + DefaultPassOption::Coverage => DEFAULT_COVERAGE_PASSES, DefaultPassOption::None => &[], } } From 9e98a25b9520861a6b443a1d28c04a9b1854e24e Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Wed, 30 Jan 2019 14:15:09 -0600 Subject: [PATCH 02/14] tabs -> spaces --- src/doc/rustdoc/src/unstable-features.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/doc/rustdoc/src/unstable-features.md b/src/doc/rustdoc/src/unstable-features.md index cb741d1bf99ff..d838f9d95132f 100644 --- a/src/doc/rustdoc/src/unstable-features.md +++ b/src/doc/rustdoc/src/unstable-features.md @@ -53,7 +53,7 @@ For example, in the following code: ```rust /// Does the thing. pub fn do_the_thing(_: SomeType) { - println!("Let's do the thing!"); + println!("Let's do the thing!"); } /// Token you use to [`do_the_thing`]. @@ -66,15 +66,15 @@ target out also works: ```rust pub mod some_module { - /// Token you use to do the thing. - pub struct SomeStruct; + /// Token you use to do the thing. + pub struct SomeStruct; } /// Does the thing. Requires one [`SomeStruct`] for the thing to work. /// /// [`SomeStruct`]: some_module::SomeStruct pub fn do_the_thing(_: some_module::SomeStruct) { - println!("Let's do the thing!"); + println!("Let's do the thing!"); } ``` From fc9459351c0136715089f9e9d96f57fed2c80a52 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Wed, 20 Feb 2019 15:15:13 -0600 Subject: [PATCH 03/14] count fewer items in calculate-doc-coverage --- src/librustdoc/passes/calculate_doc_coverage.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index b812415d6775f..57ac75bf4d418 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -43,7 +43,11 @@ struct CoverageCalculator { impl fold::DocFolder for CoverageCalculator { fn fold_item(&mut self, i: clean::Item) -> Option { match i.inner { - clean::StrippedItem(..) => {} + clean::StrippedItem(..) => { + // don't count items in stripped modules + return Some(i); + } + clean::ImportItem(..) | clean::ExternCrateItem(..) => {} clean::ImplItem(ref impl_) if attr::contains_name(&i.attrs.other_attrs, "automatically_derived") || impl_.synthetic || impl_.blanket_impl.is_some() => From 95500c078bec0082fe0e4f3e20d962d436a0e099 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Wed, 20 Feb 2019 15:26:35 -0600 Subject: [PATCH 04/14] refactor: combine item count numbers into a new struct --- .../passes/calculate_doc_coverage.rs | 86 ++++++++++++------- 1 file changed, 57 insertions(+), 29 deletions(-) diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 57ac75bf4d418..cb6d180fbd3b4 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -5,6 +5,9 @@ use crate::passes::Pass; use syntax::attr; +use std::ops::Sub; +use std::fmt; + pub const CALCULATE_DOC_COVERAGE: Pass = Pass { name: "calculate-doc-coverage", pass: calculate_doc_coverage, @@ -15,29 +18,66 @@ fn calculate_doc_coverage(krate: clean::Crate, _: &DocContext<'_, '_, '_>) -> cl let mut calc = CoverageCalculator::default(); let krate = calc.fold_crate(krate); - let total_minus_traits = calc.total - calc.total_trait_impls; - let docs_minus_traits = calc.with_docs - calc.trait_impls_with_docs; + let non_traits = calc.items - calc.trait_impl_items; - print!("Rustdoc found {}/{} items with documentation", calc.with_docs, calc.total); - println!(" ({}/{} not counting trait impls)", docs_minus_traits, total_minus_traits); + print!("Rustdoc found {} items with documentation", calc.items); + println!(" ({} not counting trait impls)", non_traits); - if calc.total > 0 { - let percentage = (calc.with_docs as f64 * 100.0) / calc.total as f64; - let percentage_minus_traits = - (docs_minus_traits as f64 * 100.0) / total_minus_traits as f64; + if let (Some(percentage), Some(percentage_non_traits)) = + (calc.items.percentage(), non_traits.percentage()) + { println!(" Score: {:.1}% ({:.1}% not counting trait impls)", - percentage, percentage_minus_traits); + percentage, percentage_non_traits); } krate } +#[derive(Default, Copy, Clone)] +struct ItemCount { + total: u64, + with_docs: u64, +} + +impl ItemCount { + fn count_item(&mut self, has_docs: bool) { + self.total += 1; + + if has_docs { + self.with_docs += 1; + } + } + + fn percentage(&self) -> Option { + if self.total > 0 { + Some((self.with_docs as f64 * 100.0) / self.total as f64) + } else { + None + } + } +} + +impl Sub for ItemCount { + type Output = Self; + + fn sub(self, rhs: Self) -> Self { + ItemCount { + total: self.total - rhs.total, + with_docs: self.with_docs - rhs.with_docs, + } + } +} + +impl fmt::Display for ItemCount { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}/{}", self.with_docs, self.total) + } +} + #[derive(Default)] struct CoverageCalculator { - total: usize, - with_docs: usize, - total_trait_impls: usize, - trait_impls_with_docs: usize, + items: ItemCount, + trait_impl_items: ItemCount, } impl fold::DocFolder for CoverageCalculator { @@ -65,24 +105,15 @@ impl fold::DocFolder for CoverageCalculator { if let Some(ref tr) = i.trait_ { debug!("counting impl {:#} for {:#}", tr, i.for_); - self.total += 1; - if has_docs { - self.with_docs += 1; - } + self.items.count_item(has_docs); // trait impls inherit their docs from the trait definition, so documenting // them can be considered optional - self.total_trait_impls += 1; - if has_docs { - self.trait_impls_with_docs += 1; - } + self.trait_impl_items.count_item(has_docs); for it in &i.items { - self.total_trait_impls += 1; - if !it.attrs.doc_strings.is_empty() { - self.trait_impls_with_docs += 1; - } + self.trait_impl_items.count_item(!it.attrs.doc_strings.is_empty()); } } else { // inherent impls *can* be documented, and those docs show up, but in most @@ -92,10 +123,7 @@ impl fold::DocFolder for CoverageCalculator { } } else { debug!("counting {} {:?}", i.type_(), i.name); - self.total += 1; - if has_docs { - self.with_docs += 1; - } + self.items.count_item(has_docs); } } } From 5eb1ab5265402cf0a40b71a5236cfe6289e7647d Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Feb 2019 10:58:40 -0600 Subject: [PATCH 05/14] print doc coverage as a table of individual item types --- src/librustdoc/html/item_type.rs | 2 +- .../passes/calculate_doc_coverage.rs | 206 ++++++++++++++---- 2 files changed, 166 insertions(+), 42 deletions(-) diff --git a/src/librustdoc/html/item_type.rs b/src/librustdoc/html/item_type.rs index 353fa4ae8c999..366e60b3ad920 100644 --- a/src/librustdoc/html/item_type.rs +++ b/src/librustdoc/html/item_type.rs @@ -15,7 +15,7 @@ use crate::clean; /// module headings. If you are adding to this enum and want to ensure that the sidebar also prints /// a heading, edit the listing in `html/render.rs`, function `sidebar_module`. This uses an /// ordering based on a helper function inside `item_module`, in the same file. -#[derive(Copy, PartialEq, Clone, Debug)] +#[derive(Copy, PartialEq, Eq, Clone, Debug, PartialOrd, Ord)] pub enum ItemType { Module = 0, ExternCrate = 1, diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index cb6d180fbd3b4..06f9a604ec83f 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -1,12 +1,14 @@ use crate::clean; use crate::core::DocContext; +use crate::html::item_type::ItemType; use crate::fold::{self, DocFolder}; use crate::passes::Pass; use syntax::attr; -use std::ops::Sub; +use std::collections::BTreeMap; use std::fmt; +use std::ops; pub const CALCULATE_DOC_COVERAGE: Pass = Pass { name: "calculate-doc-coverage", @@ -18,17 +20,7 @@ fn calculate_doc_coverage(krate: clean::Crate, _: &DocContext<'_, '_, '_>) -> cl let mut calc = CoverageCalculator::default(); let krate = calc.fold_crate(krate); - let non_traits = calc.items - calc.trait_impl_items; - - print!("Rustdoc found {} items with documentation", calc.items); - println!(" ({} not counting trait impls)", non_traits); - - if let (Some(percentage), Some(percentage_non_traits)) = - (calc.items.percentage(), non_traits.percentage()) - { - println!(" Score: {:.1}% ({:.1}% not counting trait impls)", - percentage, percentage_non_traits); - } + calc.print_results(); krate } @@ -57,7 +49,7 @@ impl ItemCount { } } -impl Sub for ItemCount { +impl ops::Sub for ItemCount { type Output = Self; fn sub(self, rhs: Self) -> Self { @@ -68,6 +60,13 @@ impl Sub for ItemCount { } } +impl ops::AddAssign for ItemCount { + fn add_assign(&mut self, rhs: Self) { + self.total += rhs.total; + self.with_docs += rhs.with_docs; + } +} + impl fmt::Display for ItemCount { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}/{}", self.with_docs, self.total) @@ -76,58 +75,183 @@ impl fmt::Display for ItemCount { #[derive(Default)] struct CoverageCalculator { - items: ItemCount, - trait_impl_items: ItemCount, + items: BTreeMap, +} + +impl CoverageCalculator { + fn print_results(&self) { + use crate::html::item_type::ItemType::*; + + let mut total = ItemCount::default(); + + let main_types = [ + Module, Function, + Struct, StructField, + Enum, Variant, + Union, + Method, + Trait, TyMethod, + AssociatedType, AssociatedConst, + Macro, + Static, Constant, + ForeignType, Existential, + Typedef, TraitAlias, + Primitive, Keyword, + ]; + + println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + println!("| {:<25} | {:>10} | {:>10} | {:>10} |", + "Item Type", "Documented", "Total", "Percentage"); + println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + + for item_type in &main_types { + let count = self.items.get(item_type).cloned().unwrap_or_default(); + + if let Some(percentage) = count.percentage() { + println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", + table_name(item_type), count.with_docs, count.total, percentage); + + total += count; + } + } + + println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + + if let Some(count) = self.items.get(&Impl) { + if let Some(percentage) = count.percentage() { + if let Some(percentage) = total.percentage() { + println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", + "Total (non trait impls)", total.with_docs, total.total, percentage); + } + + println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + + println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", + table_name(&Impl), count.with_docs, count.total, percentage); + + println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + + total += *count; + } + } + + println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", + "Total", total.with_docs, total.total, total.percentage().unwrap_or(0.0)); + println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + } } impl fold::DocFolder for CoverageCalculator { - fn fold_item(&mut self, i: clean::Item) -> Option { + fn fold_item(&mut self, mut i: clean::Item) -> Option { + let has_docs = !i.attrs.doc_strings.is_empty(); + match i.inner { + _ if !i.def_id.is_local() => { + // non-local items are skipped because they can be out of the users control, + // especially in the case of trait impls, which rustdoc eagerly inlines + return Some(i); + } clean::StrippedItem(..) => { // don't count items in stripped modules return Some(i); } - clean::ImportItem(..) | clean::ExternCrateItem(..) => {} + clean::ImportItem(..) | clean::ExternCrateItem(..) => { + // docs on `use` and `extern crate` statements are not displayed, so they're not + // worth counting + return Some(i); + } clean::ImplItem(ref impl_) if attr::contains_name(&i.attrs.other_attrs, "automatically_derived") || impl_.synthetic || impl_.blanket_impl.is_some() => { - // skip counting anything inside these impl blocks + // built-in derives get the `#[automatically_derived]` attribute, and + // synthetic/blanket impls are made up by rustdoc and can't be documented // FIXME(misdreavus): need to also find items that came out of a derive macro return Some(i); } - // non-local items are skipped because they can be out of the users control, especially - // in the case of trait impls, which rustdoc eagerly inlines - _ => if i.def_id.is_local() { - let has_docs = !i.attrs.doc_strings.is_empty(); - - if let clean::ImplItem(ref i) = i.inner { - if let Some(ref tr) = i.trait_ { - debug!("counting impl {:#} for {:#}", tr, i.for_); + clean::ImplItem(ref impl_) => { + if let Some(ref tr) = impl_.trait_ { + debug!("counting impl {:#} for {:#}", tr, impl_.for_); - self.items.count_item(has_docs); + // trait impls inherit their docs from the trait definition, so documenting + // them can be considered optional + self.items.entry(ItemType::Impl).or_default().count_item(has_docs); - // trait impls inherit their docs from the trait definition, so documenting - // them can be considered optional + for it in &impl_.items { + let has_docs = !it.attrs.doc_strings.is_empty(); + self.items.entry(ItemType::Impl).or_default().count_item(has_docs); + } - self.trait_impl_items.count_item(has_docs); + // now skip recursing, so that we don't double-count this impl's items + return Some(i); + } else { + // inherent impls *can* be documented, and those docs show up, but in most + // cases it doesn't make sense, as all methods on a type are in one single + // impl block + debug!("not counting impl {:#}", impl_.for_); + } + } + clean::MacroItem(..) | clean::ProcMacroItem(..) => { + // combine `macro_rules!` macros and proc-macros in the same count + debug!("counting macro {:?}", i.name); + self.items.entry(ItemType::Macro).or_default().count_item(has_docs); + } + clean::TraitItem(ref mut trait_) => { + // because both trait methods with a default impl and struct methods are + // ItemType::Method, we need to properly tag trait methods as TyMethod instead + debug!("counting trait {:?}", i.name); + self.items.entry(ItemType::Trait).or_default().count_item(has_docs); - for it in &i.items { - self.trait_impl_items.count_item(!it.attrs.doc_strings.is_empty()); - } + // since we're not going on to document the crate, it doesn't matter if we discard + // the item after counting it + trait_.items.retain(|it| { + if it.type_() == ItemType::Method { + let has_docs = !it.attrs.doc_strings.is_empty(); + self.items.entry(ItemType::TyMethod).or_default().count_item(has_docs); + false } else { - // inherent impls *can* be documented, and those docs show up, but in most - // cases it doesn't make sense, as all methods on a type are in one single - // impl block - debug!("not counting impl {:#}", i.for_); + true } - } else { - debug!("counting {} {:?}", i.type_(), i.name); - self.items.count_item(has_docs); - } + }); + } + _ => { + debug!("counting {} {:?}", i.type_(), i.name); + self.items.entry(i.type_()).or_default().count_item(has_docs); } } self.fold_item_recur(i) } } + +fn table_name(type_: &ItemType) -> &'static str { + match *type_ { + ItemType::Module => "Modules", + ItemType::Struct => "Structs", + ItemType::Union => "Unions", + ItemType::Enum => "Enums", + ItemType::Function => "Functions", + ItemType::Typedef => "Type Aliases", + ItemType::Static => "Statics", + ItemType::Trait => "Traits", + // inherent impls aren't counted, and trait impls get all their items thrown into this + // counter + ItemType::Impl => "Trait Impl Items", + // even though trait methods with a default impl get cleaned as Method, we convert them + // to TyMethod when counting + ItemType::TyMethod => "Trait Methods", + ItemType::Method => "Methods", + ItemType::StructField => "Struct Fields", + ItemType::Variant => "Enum Variants", + ItemType::Macro => "Macros", + ItemType::Primitive => "Primitives", + ItemType::AssociatedType => "Associated Types", + ItemType::Constant => "Constants", + ItemType::AssociatedConst => "Associated Constants", + ItemType::ForeignType => "Foreign Types", + ItemType::Keyword => "Keywords", + ItemType::Existential => "Existential Types", + ItemType::TraitAlias => "Trait Aliases", + _ => panic!("unanticipated ItemType: {}", type_), + } +} From a3a255990e714827601623634bbc5fd59f689f71 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Feb 2019 11:10:12 -0600 Subject: [PATCH 06/14] add a coverage mode for private items --- src/librustdoc/config.rs | 14 +++++++++++--- src/librustdoc/passes/mod.rs | 9 +++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index d7c6b197164dc..5cbcc2433ba57 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -232,6 +232,10 @@ impl Options { for &name in passes::DEFAULT_COVERAGE_PASSES { println!("{:>20}", name); } + println!("\nPasses run with `--show-coverage --document-private-items`:"); + for &name in passes::PRIVATE_COVERAGE_PASSES { + println!("{:>20}", name); + } return Err(0); } @@ -421,17 +425,21 @@ 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 matches.opt_present("show-coverage") { + } else if show_coverage && document_private { + passes::DefaultPassOption::PrivateCoverage + } else if show_coverage { passes::DefaultPassOption::Coverage - } else if matches.opt_present("document-private-items") { + } else if document_private { passes::DefaultPassOption::Private } else { passes::DefaultPassOption::Default }; let manual_passes = matches.opt_strs("passes"); - let show_coverage = matches.opt_present("show-coverage"); let crate_name = matches.opt_str("crate-name"); let playground_url = matches.opt_str("playground-url"); diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index bda63ae18fd7b..e36a029f97523 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -106,6 +106,13 @@ pub const DEFAULT_COVERAGE_PASSES: &'static [&'static str] = &[ "calculate-doc-coverage", ]; +/// The list of default passes run when `--doc-coverage --document-private-items` is passed to +/// rustdoc. +pub const PRIVATE_COVERAGE_PASSES: &'static [&'static str] = &[ + "collect-trait-impls", + "calculate-doc-coverage", +]; + /// A shorthand way to refer to which set of passes to use, based on the presence of /// `--no-defaults` or `--document-private-items`. #[derive(Copy, Clone, PartialEq, Eq, Debug)] @@ -113,6 +120,7 @@ pub enum DefaultPassOption { Default, Private, Coverage, + PrivateCoverage, None, } @@ -122,6 +130,7 @@ pub fn defaults(default_set: DefaultPassOption) -> &'static [&'static str] { DefaultPassOption::Default => DEFAULT_PASSES, DefaultPassOption::Private => DEFAULT_PRIVATE_PASSES, DefaultPassOption::Coverage => DEFAULT_COVERAGE_PASSES, + DefaultPassOption::PrivateCoverage => PRIVATE_COVERAGE_PASSES, DefaultPassOption::None => &[], } } From 3ce19b4a2c062a7a269bd2783a4a441d515b64c8 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Feb 2019 16:02:21 -0600 Subject: [PATCH 07/14] tweak wording of extern types --- src/librustdoc/passes/calculate_doc_coverage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 06f9a604ec83f..f70de1ce9510e 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -248,7 +248,7 @@ fn table_name(type_: &ItemType) -> &'static str { ItemType::AssociatedType => "Associated Types", ItemType::Constant => "Constants", ItemType::AssociatedConst => "Associated Constants", - ItemType::ForeignType => "Foreign Types", + ItemType::ForeignType => "Extern Types", ItemType::Keyword => "Keywords", ItemType::Existential => "Existential Types", ItemType::TraitAlias => "Trait Aliases", From 63bdd29ef4e744d6cacc2373b3b317eeebdf2a07 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Feb 2019 16:02:56 -0600 Subject: [PATCH 08/14] add tests for doc coverage --- src/test/rustdoc-ui/coverage/basic.rs | 50 +++++++++++++++++++ src/test/rustdoc-ui/coverage/basic.stdout | 15 ++++++ src/test/rustdoc-ui/coverage/empty.rs | 4 ++ src/test/rustdoc-ui/coverage/empty.stdout | 7 +++ src/test/rustdoc-ui/coverage/enums.rs | 22 ++++++++ src/test/rustdoc-ui/coverage/enums.stdout | 10 ++++ src/test/rustdoc-ui/coverage/exotic.rs | 15 ++++++ src/test/rustdoc-ui/coverage/exotic.stdout | 9 ++++ src/test/rustdoc-ui/coverage/private.rs | 21 ++++++++ src/test/rustdoc-ui/coverage/private.stdout | 10 ++++ .../rustdoc-ui/coverage/statics-consts.rs | 23 +++++++++ .../rustdoc-ui/coverage/statics-consts.stdout | 12 +++++ src/test/rustdoc-ui/coverage/traits.rs | 37 ++++++++++++++ src/test/rustdoc-ui/coverage/traits.stdout | 16 ++++++ 14 files changed, 251 insertions(+) create mode 100644 src/test/rustdoc-ui/coverage/basic.rs create mode 100644 src/test/rustdoc-ui/coverage/basic.stdout create mode 100644 src/test/rustdoc-ui/coverage/empty.rs create mode 100644 src/test/rustdoc-ui/coverage/empty.stdout create mode 100644 src/test/rustdoc-ui/coverage/enums.rs create mode 100644 src/test/rustdoc-ui/coverage/enums.stdout create mode 100644 src/test/rustdoc-ui/coverage/exotic.rs create mode 100644 src/test/rustdoc-ui/coverage/exotic.stdout create mode 100644 src/test/rustdoc-ui/coverage/private.rs create mode 100644 src/test/rustdoc-ui/coverage/private.stdout create mode 100644 src/test/rustdoc-ui/coverage/statics-consts.rs create mode 100644 src/test/rustdoc-ui/coverage/statics-consts.stdout create mode 100644 src/test/rustdoc-ui/coverage/traits.rs create mode 100644 src/test/rustdoc-ui/coverage/traits.stdout diff --git a/src/test/rustdoc-ui/coverage/basic.rs b/src/test/rustdoc-ui/coverage/basic.rs new file mode 100644 index 0000000000000..4247fdf989556 --- /dev/null +++ b/src/test/rustdoc-ui/coverage/basic.rs @@ -0,0 +1,50 @@ +// compile-flags:-Z unstable-options --show-coverage +// compile-pass + +#![feature(extern_types)] + +//! Make sure to have some docs on your crate root + +/// This struct is documented, but its fields are not. +/// +/// However, one field is private, so it shouldn't show in the total. +pub struct SomeStruct { + pub some_field: usize, + other_field: usize, +} + +impl SomeStruct { + /// Method with docs + pub fn this_fn(&self) {} + + // Method without docs + pub fn other_method(&self) {} +} + +// struct without docs +pub struct OtherStruct; + +// function with no docs +pub fn some_fn() {} + +/// Function with docs +pub fn other_fn() {} + +pub enum SomeEnum { + /// Some of these variants are documented... + VarOne, + /// ...but some of them are not. + VarTwo, + // (like this one) + VarThree, +} + +/// There's a macro here, too +#[macro_export] +macro_rules! some_macro { + () => {}; +} + +extern { + pub type ExternType; +} diff --git a/src/test/rustdoc-ui/coverage/basic.stdout b/src/test/rustdoc-ui/coverage/basic.stdout new file mode 100644 index 0000000000000..089ab6017d194 --- /dev/null +++ b/src/test/rustdoc-ui/coverage/basic.stdout @@ -0,0 +1,15 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 1 | 1 | 100.0% | +| Functions | 1 | 2 | 50.0% | +| Structs | 1 | 2 | 50.0% | +| Struct Fields | 0 | 1 | 0.0% | +| Enums | 0 | 1 | 0.0% | +| Enum Variants | 2 | 3 | 66.7% | +| Methods | 1 | 2 | 50.0% | +| Macros | 1 | 1 | 100.0% | +| Extern Types | 0 | 1 | 0.0% | ++---------------------------+------------+------------+------------+ +| Total | 7 | 14 | 50.0% | ++---------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/empty.rs b/src/test/rustdoc-ui/coverage/empty.rs new file mode 100644 index 0000000000000..463617a1143df --- /dev/null +++ b/src/test/rustdoc-ui/coverage/empty.rs @@ -0,0 +1,4 @@ +// compile-flags:-Z unstable-options --show-coverage +// compile-pass + +// an empty crate still has one item to document: the crate root diff --git a/src/test/rustdoc-ui/coverage/empty.stdout b/src/test/rustdoc-ui/coverage/empty.stdout new file mode 100644 index 0000000000000..df68205bbaa3f --- /dev/null +++ b/src/test/rustdoc-ui/coverage/empty.stdout @@ -0,0 +1,7 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 0 | 1 | 0.0% | ++---------------------------+------------+------------+------------+ +| Total | 0 | 1 | 0.0% | ++---------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/enums.rs b/src/test/rustdoc-ui/coverage/enums.rs new file mode 100644 index 0000000000000..5cd7f490d1a9a --- /dev/null +++ b/src/test/rustdoc-ui/coverage/enums.rs @@ -0,0 +1,22 @@ +// compile-flags:-Z unstable-options --show-coverage +// compile-pass + +//! (remember the crate root is still a module) + +/// so check out this enum here +pub enum ThisEnum { + /// this variant has some weird stuff going on + VarOne { + /// like, it has some named fields inside + field_one: usize, + // (these show up as struct fields) + field_two: usize, + }, + /// here's another variant for you + VarTwo(String), + // but not all of them need to be documented as thoroughly + VarThree, +} + +/// uninhabited enums? sure, let's throw one of those around +pub enum OtherEnum {} diff --git a/src/test/rustdoc-ui/coverage/enums.stdout b/src/test/rustdoc-ui/coverage/enums.stdout new file mode 100644 index 0000000000000..651ea0953102f --- /dev/null +++ b/src/test/rustdoc-ui/coverage/enums.stdout @@ -0,0 +1,10 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 1 | 1 | 100.0% | +| Struct Fields | 1 | 2 | 50.0% | +| Enums | 2 | 2 | 100.0% | +| Enum Variants | 2 | 3 | 66.7% | ++---------------------------+------------+------------+------------+ +| Total | 6 | 8 | 75.0% | ++---------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/exotic.rs b/src/test/rustdoc-ui/coverage/exotic.rs new file mode 100644 index 0000000000000..b4adf45b90b8a --- /dev/null +++ b/src/test/rustdoc-ui/coverage/exotic.rs @@ -0,0 +1,15 @@ +// compile-flags:-Z unstable-options --show-coverage +// compile-pass + +#![feature(doc_keyword)] + +//! the features only used in std also have entries in the table, so make sure those get pulled out +//! properly as well + +/// woo, check it out, we can write our own primitive docs lol +#[doc(primitive="unit")] +mod prim_unit {} + +/// keywords? sure, pile them on +#[doc(keyword="where")] +mod where_keyword {} diff --git a/src/test/rustdoc-ui/coverage/exotic.stdout b/src/test/rustdoc-ui/coverage/exotic.stdout new file mode 100644 index 0000000000000..97eab50a55a48 --- /dev/null +++ b/src/test/rustdoc-ui/coverage/exotic.stdout @@ -0,0 +1,9 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 1 | 1 | 100.0% | +| Primitives | 1 | 1 | 100.0% | +| Keywords | 1 | 1 | 100.0% | ++---------------------------+------------+------------+------------+ +| Total | 3 | 3 | 100.0% | ++---------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/private.rs b/src/test/rustdoc-ui/coverage/private.rs new file mode 100644 index 0000000000000..9024185856daa --- /dev/null +++ b/src/test/rustdoc-ui/coverage/private.rs @@ -0,0 +1,21 @@ +// compile-flags:-Z unstable-options --show-coverage --document-private-items +// compile-pass + +#![allow(unused)] + +//! when `--document-private-items` is passed, nothing is safe. everything must have docs or your +//! score will suffer the consequences + +mod this_mod { + fn private_fn() {} +} + +/// See, our public items have docs! +pub struct SomeStruct { + /// Look, all perfectly documented! + pub field: usize, + other: usize, +} + +/// Nothing shady going on here. Just a bunch of well-documented code. (cough) +pub fn public_fn() {} diff --git a/src/test/rustdoc-ui/coverage/private.stdout b/src/test/rustdoc-ui/coverage/private.stdout new file mode 100644 index 0000000000000..f1a5461b836cf --- /dev/null +++ b/src/test/rustdoc-ui/coverage/private.stdout @@ -0,0 +1,10 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 1 | 2 | 50.0% | +| Functions | 1 | 2 | 50.0% | +| Structs | 1 | 1 | 100.0% | +| Struct Fields | 1 | 2 | 50.0% | ++---------------------------+------------+------------+------------+ +| Total | 4 | 7 | 57.1% | ++---------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/statics-consts.rs b/src/test/rustdoc-ui/coverage/statics-consts.rs new file mode 100644 index 0000000000000..3c1dd35dfe1ab --- /dev/null +++ b/src/test/rustdoc-ui/coverage/statics-consts.rs @@ -0,0 +1,23 @@ +// compile-flags:-Z unstable-options --show-coverage +// compile-pass + +//! gotta make sure we can count statics and consts correctly, too + +/// static like electricity, right? +pub static THIS_STATIC: usize = 0; + +/// (it's not electricity, is it) +pub const THIS_CONST: usize = 1; + +/// associated consts show up separately, but let's throw them in as well +pub trait SomeTrait { + /// just like that, yeah + const ASSOC_CONST: usize; +} + +pub struct SomeStruct; + +impl SomeStruct { + /// wait, structs can have them too, can't forget those + pub const ASSOC_CONST: usize = 100; +} diff --git a/src/test/rustdoc-ui/coverage/statics-consts.stdout b/src/test/rustdoc-ui/coverage/statics-consts.stdout new file mode 100644 index 0000000000000..54516fe613fb4 --- /dev/null +++ b/src/test/rustdoc-ui/coverage/statics-consts.stdout @@ -0,0 +1,12 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 1 | 1 | 100.0% | +| Structs | 0 | 1 | 0.0% | +| Traits | 1 | 1 | 100.0% | +| Associated Constants | 2 | 2 | 100.0% | +| Statics | 1 | 1 | 100.0% | +| Constants | 1 | 1 | 100.0% | ++---------------------------+------------+------------+------------+ +| Total | 6 | 7 | 85.7% | ++---------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/traits.rs b/src/test/rustdoc-ui/coverage/traits.rs new file mode 100644 index 0000000000000..0a6defa17f85b --- /dev/null +++ b/src/test/rustdoc-ui/coverage/traits.rs @@ -0,0 +1,37 @@ +// compile-flags:-Z unstable-options --show-coverage +// compile-pass + +#![feature(trait_alias)] + +/// look at this trait right here +pub trait ThisTrait { + /// that's a trait all right + fn right_here(&self); + + /// even the provided functions show up as trait methods + fn aww_yeah(&self) {} + + /// gotta check those associated types, they're slippery + type SomeType; +} + +/// so what happens if we take some struct... +pub struct SomeStruct; + +/// ...and slap this trait on it? +impl ThisTrait for SomeStruct { + /// what we get is a perfect combo! + fn right_here(&self) {} + + type SomeType = String; +} + +/// but what about those aliases? i hear they're pretty exotic +pub trait MyAlias = ThisTrait + Send + Sync; + +// FIXME(58624): once rustdoc can process existential types, we need to make sure they're counted +// /// woah, getting all existential in here +// pub existential type ThisExists: ThisTrait; +// +// /// why don't we get a little more concrete +// pub fn defines() -> ThisExists { SomeStruct {} } diff --git a/src/test/rustdoc-ui/coverage/traits.stdout b/src/test/rustdoc-ui/coverage/traits.stdout new file mode 100644 index 0000000000000..6f5db8729efb3 --- /dev/null +++ b/src/test/rustdoc-ui/coverage/traits.stdout @@ -0,0 +1,16 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 0 | 1 | 0.0% | +| Structs | 1 | 1 | 100.0% | +| Traits | 1 | 1 | 100.0% | +| Trait Methods | 2 | 2 | 100.0% | +| Associated Types | 1 | 1 | 100.0% | +| Trait Aliases | 1 | 1 | 100.0% | ++---------------------------+------------+------------+------------+ +| Total (non trait impls) | 6 | 7 | 85.7% | ++---------------------------+------------+------------+------------+ +| Trait Impl Items | 2 | 3 | 66.7% | ++---------------------------+------------+------------+------------+ +| Total | 8 | 10 | 80.0% | ++---------------------------+------------+------------+------------+ From 80b49191bbf8cf7b418fc828f944bf4580121db3 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Feb 2019 16:04:56 -0600 Subject: [PATCH 09/14] update docs for doc coverage --- src/doc/rustdoc/src/unstable-features.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/doc/rustdoc/src/unstable-features.md b/src/doc/rustdoc/src/unstable-features.md index d838f9d95132f..22bfa0bd553b3 100644 --- a/src/doc/rustdoc/src/unstable-features.md +++ b/src/doc/rustdoc/src/unstable-features.md @@ -445,7 +445,9 @@ Some methodology notes about what rustdoc counts in this metric: * Rustdoc will only count items from your crate (i.e. items re-exported from other crates don't count). -* Since trait implementations can inherit documentation from their trait, it will count trait impl - blocks separately, and show totals both with and without trait impls included. +* Since trait implementations can inherit documentation from their trait, separate totals are given + both with and without trait implementations. * Inherent impl blocks are not counted, even though their doc comments are displayed, because the common pattern in Rust code is to write all inherent methods into the same impl block. +* By default, only public items are counted. To count private items as well, pass + `--document-private-items` at the same time. From 1b63543dc62d2df0143b8d003017e29eca097063 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 28 Feb 2019 15:27:42 -0600 Subject: [PATCH 10/14] track items per-file instead of per-type --- .../passes/calculate_doc_coverage.rs | 150 ++++-------------- 1 file changed, 33 insertions(+), 117 deletions(-) diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index f70de1ce9510e..6e0238b7a4d5e 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -1,10 +1,10 @@ use crate::clean; use crate::core::DocContext; -use crate::html::item_type::ItemType; use crate::fold::{self, DocFolder}; use crate::passes::Pass; use syntax::attr; +use syntax_pos::FileName; use std::collections::BTreeMap; use std::fmt; @@ -75,74 +75,51 @@ impl fmt::Display for ItemCount { #[derive(Default)] struct CoverageCalculator { - items: BTreeMap, + items: BTreeMap, } impl CoverageCalculator { fn print_results(&self) { - use crate::html::item_type::ItemType::*; - let mut total = ItemCount::default(); - let main_types = [ - Module, Function, - Struct, StructField, - Enum, Variant, - Union, - Method, - Trait, TyMethod, - AssociatedType, AssociatedConst, - Macro, - Static, Constant, - ForeignType, Existential, - Typedef, TraitAlias, - Primitive, Keyword, - ]; - - println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); - println!("| {:<25} | {:>10} | {:>10} | {:>10} |", - "Item Type", "Documented", "Total", "Percentage"); - println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); - - for item_type in &main_types { - let count = self.items.get(item_type).cloned().unwrap_or_default(); - - if let Some(percentage) = count.percentage() { - println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", - table_name(item_type), count.with_docs, count.total, percentage); + fn print_table_line() { + println!("+-{0:->35}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + } - total += count; - } + fn print_table_record(name: &str, count: ItemCount, percentage: f64) { + println!("| {:<35} | {:>10} | {:>10} | {:>9.1}% |", + name, count.with_docs, count.total, percentage); } - println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + print_table_line(); + println!("| {:<35} | {:>10} | {:>10} | {:>10} |", + "File", "Documented", "Total", "Percentage"); + print_table_line(); - if let Some(count) = self.items.get(&Impl) { + for (file, &count) in &self.items { if let Some(percentage) = count.percentage() { - if let Some(percentage) = total.percentage() { - println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", - "Total (non trait impls)", total.with_docs, total.total, percentage); + let mut name = file.to_string(); + // if a filename is too long, shorten it so we don't blow out the table + // FIXME(misdreavus): this needs to count graphemes, and probably also track + // double-wide characters... + if name.len() > 35 { + name = "...".to_string() + &name[name.len()-32..]; } - println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); - - println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", - table_name(&Impl), count.with_docs, count.total, percentage); - - println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + print_table_record(&name, count, percentage); - total += *count; + total += count; } } - println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", - "Total", total.with_docs, total.total, total.percentage().unwrap_or(0.0)); - println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + print_table_line(); + print_table_record("Total", total, total.percentage().unwrap_or(0.0)); + print_table_line(); } } impl fold::DocFolder for CoverageCalculator { - fn fold_item(&mut self, mut i: clean::Item) -> Option { + fn fold_item(&mut self, i: clean::Item) -> Option { let has_docs = !i.attrs.doc_strings.is_empty(); match i.inner { @@ -171,87 +148,26 @@ impl fold::DocFolder for CoverageCalculator { } clean::ImplItem(ref impl_) => { if let Some(ref tr) = impl_.trait_ { - debug!("counting impl {:#} for {:#}", tr, impl_.for_); - - // trait impls inherit their docs from the trait definition, so documenting - // them can be considered optional - self.items.entry(ItemType::Impl).or_default().count_item(has_docs); - - for it in &impl_.items { - let has_docs = !it.attrs.doc_strings.is_empty(); - self.items.entry(ItemType::Impl).or_default().count_item(has_docs); - } + debug!("impl {:#} for {:#} in {}", tr, impl_.for_, i.source.filename); - // now skip recursing, so that we don't double-count this impl's items + // don't count trait impls, the missing-docs lint doesn't so we shouldn't + // either return Some(i); } else { // inherent impls *can* be documented, and those docs show up, but in most // cases it doesn't make sense, as all methods on a type are in one single // impl block - debug!("not counting impl {:#}", impl_.for_); + debug!("impl {:#} in {}", impl_.for_, i.source.filename); } } - clean::MacroItem(..) | clean::ProcMacroItem(..) => { - // combine `macro_rules!` macros and proc-macros in the same count - debug!("counting macro {:?}", i.name); - self.items.entry(ItemType::Macro).or_default().count_item(has_docs); - } - clean::TraitItem(ref mut trait_) => { - // because both trait methods with a default impl and struct methods are - // ItemType::Method, we need to properly tag trait methods as TyMethod instead - debug!("counting trait {:?}", i.name); - self.items.entry(ItemType::Trait).or_default().count_item(has_docs); - - // since we're not going on to document the crate, it doesn't matter if we discard - // the item after counting it - trait_.items.retain(|it| { - if it.type_() == ItemType::Method { - let has_docs = !it.attrs.doc_strings.is_empty(); - self.items.entry(ItemType::TyMethod).or_default().count_item(has_docs); - false - } else { - true - } - }); - } _ => { - debug!("counting {} {:?}", i.type_(), i.name); - self.items.entry(i.type_()).or_default().count_item(has_docs); + debug!("counting {} {:?} in {}", i.type_(), i.name, i.source.filename); + self.items.entry(i.source.filename.clone()) + .or_default() + .count_item(has_docs); } } self.fold_item_recur(i) } } - -fn table_name(type_: &ItemType) -> &'static str { - match *type_ { - ItemType::Module => "Modules", - ItemType::Struct => "Structs", - ItemType::Union => "Unions", - ItemType::Enum => "Enums", - ItemType::Function => "Functions", - ItemType::Typedef => "Type Aliases", - ItemType::Static => "Statics", - ItemType::Trait => "Traits", - // inherent impls aren't counted, and trait impls get all their items thrown into this - // counter - ItemType::Impl => "Trait Impl Items", - // even though trait methods with a default impl get cleaned as Method, we convert them - // to TyMethod when counting - ItemType::TyMethod => "Trait Methods", - ItemType::Method => "Methods", - ItemType::StructField => "Struct Fields", - ItemType::Variant => "Enum Variants", - ItemType::Macro => "Macros", - ItemType::Primitive => "Primitives", - ItemType::AssociatedType => "Associated Types", - ItemType::Constant => "Constants", - ItemType::AssociatedConst => "Associated Constants", - ItemType::ForeignType => "Extern Types", - ItemType::Keyword => "Keywords", - ItemType::Existential => "Existential Types", - ItemType::TraitAlias => "Trait Aliases", - _ => panic!("unanticipated ItemType: {}", type_), - } -} From 74cf1adfd640ac81f01238d6d5b2a5befb707e6f Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 28 Feb 2019 15:31:39 -0600 Subject: [PATCH 11/14] tweak docs for rustdoc's `--show-coverage` --- src/doc/rustdoc/src/unstable-features.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/doc/rustdoc/src/unstable-features.md b/src/doc/rustdoc/src/unstable-features.md index 22bfa0bd553b3..3938df1a68267 100644 --- a/src/doc/rustdoc/src/unstable-features.md +++ b/src/doc/rustdoc/src/unstable-features.md @@ -445,9 +445,13 @@ Some methodology notes about what rustdoc counts in this metric: * Rustdoc will only count items from your crate (i.e. items re-exported from other crates don't count). -* Since trait implementations can inherit documentation from their trait, separate totals are given - both with and without trait implementations. -* Inherent impl blocks are not counted, even though their doc comments are displayed, because the - common pattern in Rust code is to write all inherent methods into the same impl block. +* Docs written directly onto inherent impl blocks are not counted, even though their doc comments + are displayed, because the common pattern in Rust code is to write all inherent methods into the + same impl block. +* Items in a trait implementation are not counted, as those impls will inherit any docs from the + trait itself. * By default, only public items are counted. To count private items as well, pass `--document-private-items` at the same time. + +Public items that are not documented can be seen with the built-in `missing_docs` lint. Private +items that are not documented can be seen with Clippy's `missing_docs_in_private_items` lint. From 515dbe73abb3ee20582011f9bcc2cb20ced098d3 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 28 Feb 2019 16:24:38 -0600 Subject: [PATCH 12/14] update rustdoc coverage tests with new table layout --- src/test/rustdoc-ui/coverage/basic.stdout | 22 ++++++------------ src/test/rustdoc-ui/coverage/empty.stdout | 14 +++++------ src/test/rustdoc-ui/coverage/enums.stdout | 17 ++++++-------- src/test/rustdoc-ui/coverage/exotic.stdout | 17 +++++++------- src/test/rustdoc-ui/coverage/private.stdout | 17 ++++++-------- .../rustdoc-ui/coverage/statics-consts.stdout | 19 ++++++--------- src/test/rustdoc-ui/coverage/traits.rs | 2 +- src/test/rustdoc-ui/coverage/traits.stdout | 23 ++++++------------- 8 files changed, 51 insertions(+), 80 deletions(-) diff --git a/src/test/rustdoc-ui/coverage/basic.stdout b/src/test/rustdoc-ui/coverage/basic.stdout index 089ab6017d194..3e91660631626 100644 --- a/src/test/rustdoc-ui/coverage/basic.stdout +++ b/src/test/rustdoc-ui/coverage/basic.stdout @@ -1,15 +1,7 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 1 | 1 | 100.0% | -| Functions | 1 | 2 | 50.0% | -| Structs | 1 | 2 | 50.0% | -| Struct Fields | 0 | 1 | 0.0% | -| Enums | 0 | 1 | 0.0% | -| Enum Variants | 2 | 3 | 66.7% | -| Methods | 1 | 2 | 50.0% | -| Macros | 1 | 1 | 100.0% | -| Extern Types | 0 | 1 | 0.0% | -+---------------------------+------------+------------+------------+ -| Total | 7 | 14 | 50.0% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...est/rustdoc-ui/coverage/basic.rs | 7 | 14 | 50.0% | ++-------------------------------------+------------+------------+------------+ +| Total | 7 | 14 | 50.0% | ++-------------------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/empty.stdout b/src/test/rustdoc-ui/coverage/empty.stdout index df68205bbaa3f..11b514fbfeaef 100644 --- a/src/test/rustdoc-ui/coverage/empty.stdout +++ b/src/test/rustdoc-ui/coverage/empty.stdout @@ -1,7 +1,7 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 0 | 1 | 0.0% | -+---------------------------+------------+------------+------------+ -| Total | 0 | 1 | 0.0% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...est/rustdoc-ui/coverage/empty.rs | 0 | 1 | 0.0% | ++-------------------------------------+------------+------------+------------+ +| Total | 0 | 1 | 0.0% | ++-------------------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/enums.stdout b/src/test/rustdoc-ui/coverage/enums.stdout index 651ea0953102f..87e2ad9f20df6 100644 --- a/src/test/rustdoc-ui/coverage/enums.stdout +++ b/src/test/rustdoc-ui/coverage/enums.stdout @@ -1,10 +1,7 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 1 | 1 | 100.0% | -| Struct Fields | 1 | 2 | 50.0% | -| Enums | 2 | 2 | 100.0% | -| Enum Variants | 2 | 3 | 66.7% | -+---------------------------+------------+------------+------------+ -| Total | 6 | 8 | 75.0% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...est/rustdoc-ui/coverage/enums.rs | 6 | 8 | 75.0% | ++-------------------------------------+------------+------------+------------+ +| Total | 6 | 8 | 75.0% | ++-------------------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/exotic.stdout b/src/test/rustdoc-ui/coverage/exotic.stdout index 97eab50a55a48..2bacfcfcecabe 100644 --- a/src/test/rustdoc-ui/coverage/exotic.stdout +++ b/src/test/rustdoc-ui/coverage/exotic.stdout @@ -1,9 +1,8 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 1 | 1 | 100.0% | -| Primitives | 1 | 1 | 100.0% | -| Keywords | 1 | 1 | 100.0% | -+---------------------------+------------+------------+------------+ -| Total | 3 | 3 | 100.0% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...st/rustdoc-ui/coverage/exotic.rs | 1 | 1 | 100.0% | +| | 2 | 2 | 100.0% | ++-------------------------------------+------------+------------+------------+ +| Total | 3 | 3 | 100.0% | ++-------------------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/private.stdout b/src/test/rustdoc-ui/coverage/private.stdout index f1a5461b836cf..0d4c7c68fd05e 100644 --- a/src/test/rustdoc-ui/coverage/private.stdout +++ b/src/test/rustdoc-ui/coverage/private.stdout @@ -1,10 +1,7 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 1 | 2 | 50.0% | -| Functions | 1 | 2 | 50.0% | -| Structs | 1 | 1 | 100.0% | -| Struct Fields | 1 | 2 | 50.0% | -+---------------------------+------------+------------+------------+ -| Total | 4 | 7 | 57.1% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...t/rustdoc-ui/coverage/private.rs | 4 | 7 | 57.1% | ++-------------------------------------+------------+------------+------------+ +| Total | 4 | 7 | 57.1% | ++-------------------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/statics-consts.stdout b/src/test/rustdoc-ui/coverage/statics-consts.stdout index 54516fe613fb4..8459f90ae7b31 100644 --- a/src/test/rustdoc-ui/coverage/statics-consts.stdout +++ b/src/test/rustdoc-ui/coverage/statics-consts.stdout @@ -1,12 +1,7 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 1 | 1 | 100.0% | -| Structs | 0 | 1 | 0.0% | -| Traits | 1 | 1 | 100.0% | -| Associated Constants | 2 | 2 | 100.0% | -| Statics | 1 | 1 | 100.0% | -| Constants | 1 | 1 | 100.0% | -+---------------------------+------------+------------+------------+ -| Total | 6 | 7 | 85.7% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...oc-ui/coverage/statics-consts.rs | 6 | 7 | 85.7% | ++-------------------------------------+------------+------------+------------+ +| Total | 6 | 7 | 85.7% | ++-------------------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/traits.rs b/src/test/rustdoc-ui/coverage/traits.rs index 0a6defa17f85b..5f32d5b0cccc7 100644 --- a/src/test/rustdoc-ui/coverage/traits.rs +++ b/src/test/rustdoc-ui/coverage/traits.rs @@ -20,7 +20,7 @@ pub struct SomeStruct; /// ...and slap this trait on it? impl ThisTrait for SomeStruct { - /// what we get is a perfect combo! + /// nothing! trait impls are totally ignored in this calculation, sorry. fn right_here(&self) {} type SomeType = String; diff --git a/src/test/rustdoc-ui/coverage/traits.stdout b/src/test/rustdoc-ui/coverage/traits.stdout index 6f5db8729efb3..e347a4da0b978 100644 --- a/src/test/rustdoc-ui/coverage/traits.stdout +++ b/src/test/rustdoc-ui/coverage/traits.stdout @@ -1,16 +1,7 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 0 | 1 | 0.0% | -| Structs | 1 | 1 | 100.0% | -| Traits | 1 | 1 | 100.0% | -| Trait Methods | 2 | 2 | 100.0% | -| Associated Types | 1 | 1 | 100.0% | -| Trait Aliases | 1 | 1 | 100.0% | -+---------------------------+------------+------------+------------+ -| Total (non trait impls) | 6 | 7 | 85.7% | -+---------------------------+------------+------------+------------+ -| Trait Impl Items | 2 | 3 | 66.7% | -+---------------------------+------------+------------+------------+ -| Total | 8 | 10 | 80.0% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...st/rustdoc-ui/coverage/traits.rs | 6 | 7 | 85.7% | ++-------------------------------------+------------+------------+------------+ +| Total | 6 | 7 | 85.7% | ++-------------------------------------+------------+------------+------------+ From e28cf7416207111308bdd0a91196d99b009f031c Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 5 Mar 2019 14:18:26 -0600 Subject: [PATCH 13/14] remove unused Display impl --- src/librustdoc/passes/calculate_doc_coverage.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 6e0238b7a4d5e..04f403888c1fb 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -7,7 +7,6 @@ use syntax::attr; use syntax_pos::FileName; use std::collections::BTreeMap; -use std::fmt; use std::ops; pub const CALCULATE_DOC_COVERAGE: Pass = Pass { @@ -67,12 +66,6 @@ impl ops::AddAssign for ItemCount { } } -impl fmt::Display for ItemCount { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}/{}", self.with_docs, self.total) - } -} - #[derive(Default)] struct CoverageCalculator { items: BTreeMap, From 3df0b895c1ba705bd4ecf110e3f6bdac7fe20953 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 5 Mar 2019 14:23:37 -0600 Subject: [PATCH 14/14] only print coverage pass lists if running on nightly --- src/librustdoc/config.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 5cbcc2433ba57..aeff78350d37c 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -228,14 +228,18 @@ impl Options { for &name in passes::DEFAULT_PRIVATE_PASSES { println!("{:>20}", name); } - println!("\nPasses run with `--show-coverage`:"); - for &name in passes::DEFAULT_COVERAGE_PASSES { - println!("{:>20}", name); - } - println!("\nPasses run with `--show-coverage --document-private-items`:"); - for &name in passes::PRIVATE_COVERAGE_PASSES { - println!("{:>20}", name); + + if nightly_options::is_nightly_build() { + println!("\nPasses run with `--show-coverage`:"); + for &name in passes::DEFAULT_COVERAGE_PASSES { + println!("{:>20}", name); + } + println!("\nPasses run with `--show-coverage --document-private-items`:"); + for &name in passes::PRIVATE_COVERAGE_PASSES { + println!("{:>20}", name); + } } + return Err(0); }