From 02e6b861eb04737915636a63eec00ef126f59dd5 Mon Sep 17 00:00:00 2001 From: Roman Date: Tue, 1 Sep 2020 14:19:24 +0200 Subject: [PATCH 1/5] rustdoc: skip allow missing doc in cover. report During the document coverage reporting with ```bash rustdoc something.rs -Z unstable-options --show-coverage ``` the coverage report also includes parts of the code that are marked with `#[allow(missing_docs)]`, which outputs lower numbers in the coverage report even though these parts should be ignored for the calculation. Co-authored-by: Joshua Nelson --- .../passes/calculate_doc_coverage.rs | 21 +++++++++++-- .../rustdoc-ui/coverage/allow_missing_docs.rs | 31 +++++++++++++++++++ .../coverage/allow_missing_docs.stdout | 7 +++++ 3 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 src/test/rustdoc-ui/coverage/allow_missing_docs.rs create mode 100644 src/test/rustdoc-ui/coverage/allow_missing_docs.stdout diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 4bca3996eb4..30dd8def70e 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -5,7 +5,7 @@ use crate::fold::{self, DocFolder}; use crate::html::markdown::{find_testable_code, ErrorCodes}; use crate::passes::doc_test_lints::{should_have_doc_example, Tests}; use crate::passes::Pass; -use rustc_span::symbol::sym; +use rustc_span::symbol::{sym, Ident}; use rustc_span::FileName; use serde::Serialize; @@ -41,8 +41,11 @@ impl ItemCount { has_docs: bool, has_doc_example: bool, should_have_doc_examples: bool, + should_have_docs: bool, ) { - self.total += 1; + if has_docs || should_have_docs { + self.total += 1; + } if has_docs { self.with_docs += 1; @@ -229,6 +232,15 @@ impl fold::DocFolder for CoverageCalculator { } _ => { let has_docs = !i.attrs.doc_strings.is_empty(); + let should_have_docs = !i.attrs.other_attrs.iter().any(|a| { + a.has_name(sym::allow) + && a.meta_item_list().iter().any(|meta_list_item| { + meta_list_item.iter().any(|li| match li.ident() { + Some(ident) => ident == Ident::from_str("missing_docs"), + _ => false, + }) + }) + }); let mut tests = Tests { found_tests: 0 }; find_testable_code( @@ -250,7 +262,12 @@ impl fold::DocFolder for CoverageCalculator { has_docs, has_doc_example, should_have_doc_example(&i.inner), + should_have_docs, ); + + if !should_have_docs { + return Some(i); + } } } diff --git a/src/test/rustdoc-ui/coverage/allow_missing_docs.rs b/src/test/rustdoc-ui/coverage/allow_missing_docs.rs new file mode 100644 index 00000000000..8c076761ede --- /dev/null +++ b/src/test/rustdoc-ui/coverage/allow_missing_docs.rs @@ -0,0 +1,31 @@ +// compile-flags:-Z unstable-options --show-coverage +// check-pass + +//! Make sure to have some docs on your crate root + +#[allow(missing_docs)] +pub mod mod_foo { + pub struct Bar; +} + +/// This is a struct with a `#[allow(missing_docs)]` +pub struct AllowTheMissingDocs { + #[allow(missing_docs)] + pub empty_str: String, + + /// This has + #[allow(missing_docs)] + /// but also has documentation comments + pub hello: usize, + + /// The doc id just to create a boilerplate comment + pub doc_id: Vec, +} + +/// A function that has a documentation +pub fn this_is_func() {} + +#[allow(missing_docs)] +pub struct DemoStruct { + something: usize, +} diff --git a/src/test/rustdoc-ui/coverage/allow_missing_docs.stdout b/src/test/rustdoc-ui/coverage/allow_missing_docs.stdout new file mode 100644 index 00000000000..ea5380e3204 --- /dev/null +++ b/src/test/rustdoc-ui/coverage/allow_missing_docs.stdout @@ -0,0 +1,7 @@ ++-------------------------------------+------------+------------+------------+------------+ +| File | Documented | Percentage | Examples | Percentage | ++-------------------------------------+------------+------------+------------+------------+ +| ...i/coverage/allow_missing_docs.rs | 5 | 100.0% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+ +| Total | 5 | 100.0% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+ From b31f5d05b1c10c1f99b5bc3c14499ff537c7b692 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 8 Oct 2020 18:05:01 +0200 Subject: [PATCH 2/5] Inherit lint level from parents --- .../passes/calculate_doc_coverage.rs | 40 ++++++++----------- .../rustdoc-ui/coverage/allow_missing_docs.rs | 8 ++++ .../coverage/allow_missing_docs.stdout | 4 +- 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 30dd8def70e..093b77d4e94 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -1,11 +1,13 @@ use crate::clean; -use crate::config::OutputFormat; use crate::core::DocContext; use crate::fold::{self, DocFolder}; use crate::html::markdown::{find_testable_code, ErrorCodes}; use crate::passes::doc_test_lints::{should_have_doc_example, Tests}; use crate::passes::Pass; -use rustc_span::symbol::{sym, Ident}; +use rustc_lint::builtin::MISSING_DOCS; +use rustc_middle::lint::LintSource; +use rustc_session::lint; +use rustc_span::symbol::sym; use rustc_span::FileName; use serde::Serialize; @@ -19,10 +21,10 @@ pub const CALCULATE_DOC_COVERAGE: Pass = Pass { }; fn calculate_doc_coverage(krate: clean::Crate, ctx: &DocContext<'_>) -> clean::Crate { - let mut calc = CoverageCalculator::new(); + let mut calc = CoverageCalculator::new(ctx); let krate = calc.fold_crate(krate); - calc.print_results(ctx.renderinfo.borrow().output_format); + calc.print_results(); krate } @@ -97,8 +99,9 @@ impl ops::AddAssign for ItemCount { } } -struct CoverageCalculator { +struct CoverageCalculator<'a, 'b> { items: BTreeMap, + ctx: &'a DocContext<'b>, } fn limit_filename_len(filename: String) -> String { @@ -111,9 +114,9 @@ fn limit_filename_len(filename: String) -> String { } } -impl CoverageCalculator { - fn new() -> CoverageCalculator { - CoverageCalculator { items: Default::default() } +impl<'a, 'b> CoverageCalculator<'a, 'b> { + fn new(ctx: &'a DocContext<'b>) -> CoverageCalculator<'a, 'b> { + CoverageCalculator { items: Default::default(), ctx } } fn to_json(&self) -> String { @@ -127,7 +130,8 @@ impl CoverageCalculator { .expect("failed to convert JSON data to string") } - fn print_results(&self, output_format: Option) { + fn print_results(&self) { + let output_format = self.ctx.renderinfo.borrow().output_format; if output_format.map(|o| o.is_json()).unwrap_or_else(|| false) { println!("{}", self.to_json()); return; @@ -181,7 +185,7 @@ impl CoverageCalculator { } } -impl fold::DocFolder for CoverageCalculator { +impl<'a, 'b> fold::DocFolder for CoverageCalculator<'a, 'b> { fn fold_item(&mut self, i: clean::Item) -> Option { match i.inner { _ if !i.def_id.is_local() => { @@ -232,15 +236,6 @@ impl fold::DocFolder for CoverageCalculator { } _ => { let has_docs = !i.attrs.doc_strings.is_empty(); - let should_have_docs = !i.attrs.other_attrs.iter().any(|a| { - a.has_name(sym::allow) - && a.meta_item_list().iter().any(|meta_list_item| { - meta_list_item.iter().any(|li| match li.ident() { - Some(ident) => ident == Ident::from_str("missing_docs"), - _ => false, - }) - }) - }); let mut tests = Tests { found_tests: 0 }; find_testable_code( @@ -257,6 +252,9 @@ impl fold::DocFolder for CoverageCalculator { ); let has_doc_example = tests.found_tests != 0; + let hir_id = self.ctx.tcx.hir().local_def_id_to_hir_id(i.def_id.expect_local()); + let (level, source) = self.ctx.tcx.lint_level_at_node(MISSING_DOCS, hir_id); + let should_have_docs = level != lint::Level::Allow || !matches!(source, LintSource::Node(..)); debug!("counting {:?} {:?} in {}", i.type_(), i.name, i.source.filename); self.items.entry(i.source.filename.clone()).or_default().count_item( has_docs, @@ -264,10 +262,6 @@ impl fold::DocFolder for CoverageCalculator { should_have_doc_example(&i.inner), should_have_docs, ); - - if !should_have_docs { - return Some(i); - } } } diff --git a/src/test/rustdoc-ui/coverage/allow_missing_docs.rs b/src/test/rustdoc-ui/coverage/allow_missing_docs.rs index 8c076761ede..87af1a45864 100644 --- a/src/test/rustdoc-ui/coverage/allow_missing_docs.rs +++ b/src/test/rustdoc-ui/coverage/allow_missing_docs.rs @@ -29,3 +29,11 @@ pub fn this_is_func() {} pub struct DemoStruct { something: usize, } + +#[allow(missing_docs)] +pub mod bar { + #[warn(missing_docs)] + pub struct Bar { //~ WARN + pub f: u32, //~ WARN + } +} diff --git a/src/test/rustdoc-ui/coverage/allow_missing_docs.stdout b/src/test/rustdoc-ui/coverage/allow_missing_docs.stdout index ea5380e3204..17e8ee9e23d 100644 --- a/src/test/rustdoc-ui/coverage/allow_missing_docs.stdout +++ b/src/test/rustdoc-ui/coverage/allow_missing_docs.stdout @@ -1,7 +1,7 @@ +-------------------------------------+------------+------------+------------+------------+ | File | Documented | Percentage | Examples | Percentage | +-------------------------------------+------------+------------+------------+------------+ -| ...i/coverage/allow_missing_docs.rs | 5 | 100.0% | 0 | 0.0% | +| ...i/coverage/allow_missing_docs.rs | 5 | 71.4% | 0 | 0.0% | +-------------------------------------+------------+------------+------------+------------+ -| Total | 5 | 100.0% | 0 | 0.0% | +| Total | 5 | 71.4% | 0 | 0.0% | +-------------------------------------+------------+------------+------------+------------+ From 22465b35a608487773f960574a798262b23c4abd Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 8 Oct 2020 18:20:00 +0200 Subject: [PATCH 3/5] Apply same treatment to MISSING_DOC_CODE_EXAMPLES --- .../passes/calculate_doc_coverage.rs | 5 +++-- src/librustdoc/passes/doc_test_lints.rs | 15 ++++++++++---- .../coverage/allow_missing_docs.stderr | 20 +++++++++++++++++++ 3 files changed, 34 insertions(+), 6 deletions(-) create mode 100644 src/test/rustdoc-ui/coverage/allow_missing_docs.stderr diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 093b77d4e94..7cdd9a590ba 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -254,12 +254,13 @@ impl<'a, 'b> fold::DocFolder for CoverageCalculator<'a, 'b> { let has_doc_example = tests.found_tests != 0; let hir_id = self.ctx.tcx.hir().local_def_id_to_hir_id(i.def_id.expect_local()); let (level, source) = self.ctx.tcx.lint_level_at_node(MISSING_DOCS, hir_id); - let should_have_docs = level != lint::Level::Allow || !matches!(source, LintSource::Node(..)); + let should_have_docs = + level != lint::Level::Allow || !matches!(source, LintSource::Node(..)); debug!("counting {:?} {:?} in {}", i.type_(), i.name, i.source.filename); self.items.entry(i.source.filename.clone()).or_default().count_item( has_docs, has_doc_example, - should_have_doc_example(&i.inner), + should_have_doc_example(self.ctx, &i), should_have_docs, ); } diff --git a/src/librustdoc/passes/doc_test_lints.rs b/src/librustdoc/passes/doc_test_lints.rs index 78af9f9b856..1787d0a99d9 100644 --- a/src/librustdoc/passes/doc_test_lints.rs +++ b/src/librustdoc/passes/doc_test_lints.rs @@ -9,6 +9,7 @@ use crate::clean::*; use crate::core::DocContext; use crate::fold::DocFolder; use crate::html::markdown::{find_testable_code, ErrorCodes, Ignore, LangString}; +use rustc_middle::lint::LintSource; use rustc_session::lint; pub const CHECK_PRIVATE_ITEMS_DOC_TESTS: Pass = Pass { @@ -56,8 +57,8 @@ impl crate::doctest::Tester for Tests { } } -pub fn should_have_doc_example(item_kind: &clean::ItemEnum) -> bool { - !matches!(item_kind, +pub fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -> bool { + if matches!(item.inner, clean::StructFieldItem(_) | clean::VariantItem(_) | clean::AssocConstItem(_, _) @@ -69,7 +70,13 @@ pub fn should_have_doc_example(item_kind: &clean::ItemEnum) -> bool { | clean::ImportItem(_) | clean::PrimitiveItem(_) | clean::KeywordItem(_) - ) + ) { + return false; + } + let hir_id = cx.tcx.hir().local_def_id_to_hir_id(item.def_id.expect_local()); + let (level, source) = + cx.tcx.lint_level_at_node(lint::builtin::MISSING_DOC_CODE_EXAMPLES, hir_id); + level != lint::Level::Allow || !matches!(source, LintSource::Node(..)) } pub fn look_for_tests<'tcx>(cx: &DocContext<'tcx>, dox: &str, item: &Item) { @@ -88,7 +95,7 @@ pub fn look_for_tests<'tcx>(cx: &DocContext<'tcx>, dox: &str, item: &Item) { if tests.found_tests == 0 && rustc_feature::UnstableFeatures::from_environment().is_nightly_build() { - if should_have_doc_example(&item.inner) { + if should_have_doc_example(cx, &item) { debug!("reporting error for {:?} (hir_id={:?})", item, hir_id); let sp = span_of_attrs(&item.attrs).unwrap_or(item.source.span()); cx.tcx.struct_span_lint_hir( diff --git a/src/test/rustdoc-ui/coverage/allow_missing_docs.stderr b/src/test/rustdoc-ui/coverage/allow_missing_docs.stderr new file mode 100644 index 00000000000..3d5b512d14d --- /dev/null +++ b/src/test/rustdoc-ui/coverage/allow_missing_docs.stderr @@ -0,0 +1,20 @@ +warning: missing documentation for a struct + --> $DIR/allow_missing_docs.rs:36:5 + | +LL | pub struct Bar { + | ^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/allow_missing_docs.rs:35:12 + | +LL | #[warn(missing_docs)] + | ^^^^^^^^^^^^ + +warning: missing documentation for a struct field + --> $DIR/allow_missing_docs.rs:37:9 + | +LL | pub f: u32, + | ^^^^^^^^^^ + +warning: 2 warnings emitted + From 5d20e1aa03c6d86fcf9197ccec16ad5faa94beee Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 12 Oct 2020 13:48:45 +0200 Subject: [PATCH 4/5] Improve lint level handling --- src/librustdoc/passes/calculate_doc_coverage.rs | 4 +++- src/librustdoc/passes/doc_test_lints.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 7cdd9a590ba..ced26fcf5b0 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -254,8 +254,10 @@ impl<'a, 'b> fold::DocFolder for CoverageCalculator<'a, 'b> { let has_doc_example = tests.found_tests != 0; let hir_id = self.ctx.tcx.hir().local_def_id_to_hir_id(i.def_id.expect_local()); let (level, source) = self.ctx.tcx.lint_level_at_node(MISSING_DOCS, hir_id); + // `missing_docs` is allow-by-default, so don't treat this as ignoring the item + // unless the user had an explicit `allow` let should_have_docs = - level != lint::Level::Allow || !matches!(source, LintSource::Node(..)); + level != lint::Level::Allow || matches!(source, LintSource::Default); debug!("counting {:?} {:?} in {}", i.type_(), i.name, i.source.filename); self.items.entry(i.source.filename.clone()).or_default().count_item( has_docs, diff --git a/src/librustdoc/passes/doc_test_lints.rs b/src/librustdoc/passes/doc_test_lints.rs index 1787d0a99d9..686ec51fb06 100644 --- a/src/librustdoc/passes/doc_test_lints.rs +++ b/src/librustdoc/passes/doc_test_lints.rs @@ -76,7 +76,7 @@ pub fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -> bool let hir_id = cx.tcx.hir().local_def_id_to_hir_id(item.def_id.expect_local()); let (level, source) = cx.tcx.lint_level_at_node(lint::builtin::MISSING_DOC_CODE_EXAMPLES, hir_id); - level != lint::Level::Allow || !matches!(source, LintSource::Node(..)) + level != lint::Level::Allow || matches!(source, LintSource::Default) } pub fn look_for_tests<'tcx>(cx: &DocContext<'tcx>, dox: &str, item: &Item) { From 685444008bb2631564b56068bcbf2462bf0cc6af Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 12 Oct 2020 14:32:41 +0200 Subject: [PATCH 5/5] Extend test to ensure that items inherit lint level from the parent --- src/test/rustdoc-ui/coverage/allow_missing_docs.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/rustdoc-ui/coverage/allow_missing_docs.rs b/src/test/rustdoc-ui/coverage/allow_missing_docs.rs index 87af1a45864..c077be31b20 100644 --- a/src/test/rustdoc-ui/coverage/allow_missing_docs.rs +++ b/src/test/rustdoc-ui/coverage/allow_missing_docs.rs @@ -36,4 +36,6 @@ pub mod bar { pub struct Bar { //~ WARN pub f: u32, //~ WARN } + + pub struct NeedsNoDocs; }