From 4b30625094eec6b339c83bb41b92eb206df7280d Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 13 Feb 2021 21:44:42 +0100 Subject: [PATCH 1/6] Don't warn for `missing_doc_examples` when item is #[doc(hidden)] --- src/librustdoc/passes/doc_test_lints.rs | 11 ++++++++++- src/librustdoc/visit_ast.rs | 26 ++++++++++++------------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/passes/doc_test_lints.rs b/src/librustdoc/passes/doc_test_lints.rs index 81104236314d9..19815c7cd5f75 100644 --- a/src/librustdoc/passes/doc_test_lints.rs +++ b/src/librustdoc/passes/doc_test_lints.rs @@ -9,8 +9,10 @@ use crate::clean::*; use crate::core::DocContext; use crate::fold::DocFolder; use crate::html::markdown::{find_testable_code, ErrorCodes, Ignore, LangString}; +use crate::visit_ast::inherits_doc_hidden; use rustc_middle::lint::LintLevelSource; use rustc_session::lint; +use rustc_span::symbol::sym; crate const CHECK_PRIVATE_ITEMS_DOC_TESTS: Pass = Pass { name: "check-private-items-doc-tests", @@ -68,6 +70,11 @@ crate fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -> boo return false; } let hir_id = cx.tcx.hir().local_def_id_to_hir_id(item.def_id.expect_local()); + if cx.tcx.hir().attrs(hir_id).lists(sym::doc).has_word(sym::hidden) + || inherits_doc_hidden(cx, hir_id) + { + return false; + } let (level, source) = cx.tcx.lint_level_at_node(crate::lint::MISSING_DOC_CODE_EXAMPLES, hir_id); level != lint::Level::Allow || matches!(source, LintLevelSource::Default) } @@ -86,7 +93,9 @@ crate fn look_for_tests<'tcx>(cx: &DocContext<'tcx>, dox: &str, item: &Item) { find_testable_code(&dox, &mut tests, ErrorCodes::No, false, None); if tests.found_tests == 0 && cx.tcx.sess.is_nightly_build() { - if should_have_doc_example(cx, &item) { + if cx.renderinfo.borrow().access_levels.is_public(item.def_id) + && 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/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 5da7d2f1e9b84..b9b2be1db802e 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -29,6 +29,19 @@ fn def_id_to_path(tcx: TyCtxt<'_>, did: DefId) -> Vec { std::iter::once(crate_name).chain(relative).collect() } +crate fn inherits_doc_hidden(cx: &core::DocContext<'_>, mut node: hir::HirId) -> bool { + while let Some(id) = cx.tcx.hir().get_enclosing_scope(node) { + node = id; + if cx.tcx.hir().attrs(node).lists(sym::doc).has_word(sym::hidden) { + return true; + } + if node == hir::CRATE_HIR_ID { + break; + } + } + false +} + // Also, is there some reason that this doesn't use the 'visit' // framework from syntax?. @@ -158,19 +171,6 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { om: &mut Module<'tcx>, please_inline: bool, ) -> bool { - fn inherits_doc_hidden(cx: &core::DocContext<'_>, mut node: hir::HirId) -> bool { - while let Some(id) = cx.tcx.hir().get_enclosing_scope(node) { - node = id; - if cx.tcx.hir().attrs(node).lists(sym::doc).has_word(sym::hidden) { - return true; - } - if node == hir::CRATE_HIR_ID { - break; - } - } - false - } - debug!("maybe_inline_local res: {:?}", res); let tcx = self.cx.tcx; From 91095b1be5b1befb545dc3a303e029f408aba4ee Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 13 Feb 2021 21:45:15 +0100 Subject: [PATCH 2/6] Update missing code example test --- .../lint-missing-doc-code-example.rs | 21 +++++++++++++++---- .../lint-missing-doc-code-example.stderr | 2 +- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/test/rustdoc-ui/lint-missing-doc-code-example.rs b/src/test/rustdoc-ui/lint-missing-doc-code-example.rs index 8d727b0d0b550..41e8847792694 100644 --- a/src/test/rustdoc-ui/lint-missing-doc-code-example.rs +++ b/src/test/rustdoc-ui/lint-missing-doc-code-example.rs @@ -12,16 +12,16 @@ /// ``` /// println!("hello"); /// ``` -fn test() { +pub fn test() { } #[allow(missing_docs)] -mod module1 { //~ ERROR +pub mod module1 { //~ ERROR } #[allow(rustdoc::missing_doc_code_examples)] /// doc -mod module2 { +pub mod module2 { /// doc pub fn test() {} @@ -63,9 +63,22 @@ pub enum Enum { /// Doc //~^ ERROR #[repr(C)] -union Union { +pub union Union { /// Doc, but no code example and it's fine! a: i32, /// Doc, but no code example and it's fine! b: f32, } + + +#[doc(hidden)] +pub mod foo { + pub fn bar() {} +} + +fn babar() {} + + +mod fofoo { + pub fn tadam() {} +} diff --git a/src/test/rustdoc-ui/lint-missing-doc-code-example.stderr b/src/test/rustdoc-ui/lint-missing-doc-code-example.stderr index 370c577f85d8f..3715797854281 100644 --- a/src/test/rustdoc-ui/lint-missing-doc-code-example.stderr +++ b/src/test/rustdoc-ui/lint-missing-doc-code-example.stderr @@ -1,7 +1,7 @@ error: missing code example in this documentation --> $DIR/lint-missing-doc-code-example.rs:19:1 | -LL | / mod module1 { +LL | / pub mod module1 { LL | | } | |_^ | From 186f13914a825039081795d382eb75c6a9f981ae Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 15 Feb 2021 14:17:43 +0100 Subject: [PATCH 3/6] Move visibility check inside the should_have_doc_example function --- src/librustdoc/passes/doc_test_lints.rs | 34 ++++++++++++------------- src/librustdoc/visit_ast.rs | 3 --- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/librustdoc/passes/doc_test_lints.rs b/src/librustdoc/passes/doc_test_lints.rs index 19815c7cd5f75..cf8966fadf60b 100644 --- a/src/librustdoc/passes/doc_test_lints.rs +++ b/src/librustdoc/passes/doc_test_lints.rs @@ -53,20 +53,22 @@ impl crate::doctest::Tester for Tests { } crate fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -> bool { - if matches!( - *item.kind, - clean::StructFieldItem(_) - | clean::VariantItem(_) - | clean::AssocConstItem(_, _) - | clean::AssocTypeItem(_, _) - | clean::TypedefItem(_, _) - | clean::StaticItem(_) - | clean::ConstantItem(_) - | clean::ExternCrateItem(_, _) - | clean::ImportItem(_) - | clean::PrimitiveItem(_) - | clean::KeywordItem(_) - ) { + if !cx.renderinfo.borrow().access_levels.is_public(item.def_id) + || matches!( + *item.kind, + clean::StructFieldItem(_) + | clean::VariantItem(_) + | clean::AssocConstItem(_, _) + | clean::AssocTypeItem(_, _) + | clean::TypedefItem(_, _) + | clean::StaticItem(_) + | clean::ConstantItem(_) + | clean::ExternCrateItem(_, _) + | 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()); @@ -93,9 +95,7 @@ crate fn look_for_tests<'tcx>(cx: &DocContext<'tcx>, dox: &str, item: &Item) { find_testable_code(&dox, &mut tests, ErrorCodes::No, false, None); if tests.found_tests == 0 && cx.tcx.sess.is_nightly_build() { - if cx.renderinfo.borrow().access_levels.is_public(item.def_id) - && should_have_doc_example(cx, &item) - { + 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/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index b9b2be1db802e..875be028e848d 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -35,9 +35,6 @@ crate fn inherits_doc_hidden(cx: &core::DocContext<'_>, mut node: hir::HirId) -> if cx.tcx.hir().attrs(node).lists(sym::doc).has_word(sym::hidden) { return true; } - if node == hir::CRATE_HIR_ID { - break; - } } false } From ad30c39918ca4191cb4b0191cfb56b97d9bef330 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 23 Feb 2021 15:19:13 +0100 Subject: [PATCH 4/6] Pass TyCtxt directly instead of DocContext in librustdoc::visit_ast::inherits_doc_hidden --- src/librustdoc/passes/doc_test_lints.rs | 2 +- src/librustdoc/visit_ast.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/passes/doc_test_lints.rs b/src/librustdoc/passes/doc_test_lints.rs index cf8966fadf60b..2ce43dd9ff818 100644 --- a/src/librustdoc/passes/doc_test_lints.rs +++ b/src/librustdoc/passes/doc_test_lints.rs @@ -73,7 +73,7 @@ crate fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -> boo } let hir_id = cx.tcx.hir().local_def_id_to_hir_id(item.def_id.expect_local()); if cx.tcx.hir().attrs(hir_id).lists(sym::doc).has_word(sym::hidden) - || inherits_doc_hidden(cx, hir_id) + || inherits_doc_hidden(cx.tcx, hir_id) { return false; } diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 875be028e848d..ba6bb359b9135 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -29,10 +29,10 @@ fn def_id_to_path(tcx: TyCtxt<'_>, did: DefId) -> Vec { std::iter::once(crate_name).chain(relative).collect() } -crate fn inherits_doc_hidden(cx: &core::DocContext<'_>, mut node: hir::HirId) -> bool { - while let Some(id) = cx.tcx.hir().get_enclosing_scope(node) { +crate fn inherits_doc_hidden(tcx: TyCtxt<'_>, mut node: hir::HirId) -> bool { + while let Some(id) = tcx.hir().get_enclosing_scope(node) { node = id; - if cx.tcx.hir().attrs(node).lists(sym::doc).has_word(sym::hidden) { + if tcx.hir().attrs(node).lists(sym::doc).has_word(sym::hidden) { return true; } } @@ -209,7 +209,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { }; let is_private = !self.cx.cache.access_levels.is_public(res_did); - let is_hidden = inherits_doc_hidden(self.cx, res_hir_id); + let is_hidden = inherits_doc_hidden(self.cx.tcx, res_hir_id); // Only inline if requested or if the item would otherwise be stripped. if (!please_inline && !is_private && !is_hidden) || is_no_inline { From e4287992a7b37ed0ae9a9158b1d7bfb4509d2164 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 23 Feb 2021 21:53:28 +0100 Subject: [PATCH 5/6] No more need for borrow call --- src/librustdoc/passes/doc_test_lints.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/passes/doc_test_lints.rs b/src/librustdoc/passes/doc_test_lints.rs index 2ce43dd9ff818..c97684c7e01b9 100644 --- a/src/librustdoc/passes/doc_test_lints.rs +++ b/src/librustdoc/passes/doc_test_lints.rs @@ -53,7 +53,7 @@ impl crate::doctest::Tester for Tests { } crate fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -> bool { - if !cx.renderinfo.borrow().access_levels.is_public(item.def_id) + if !cx.renderinfo.access_levels.is_public(item.def_id) || matches!( *item.kind, clean::StructFieldItem(_) From 1683cb12e44e433bc75ab3ffa2cfc65c6d0bdd24 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 2 Mar 2021 11:59:37 +0100 Subject: [PATCH 6/6] Use cache access levels --- src/librustdoc/passes/doc_test_lints.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/passes/doc_test_lints.rs b/src/librustdoc/passes/doc_test_lints.rs index c97684c7e01b9..ede9e18a511a8 100644 --- a/src/librustdoc/passes/doc_test_lints.rs +++ b/src/librustdoc/passes/doc_test_lints.rs @@ -53,7 +53,7 @@ impl crate::doctest::Tester for Tests { } crate fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -> bool { - if !cx.renderinfo.access_levels.is_public(item.def_id) + if !cx.cache.access_levels.is_public(item.def_id) || matches!( *item.kind, clean::StructFieldItem(_)