From 675319e5586817143fef60243d319395ae858ad1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 26 Oct 2018 00:55:12 +0200 Subject: [PATCH] lint if a private item has doctests --- src/librustc/lint/builtin.rs | 7 +++ src/librustdoc/core.rs | 4 +- .../passes/collect_intra_doc_links.rs | 45 ++------------ src/librustdoc/passes/mod.rs | 60 ++++++++++++++++++- .../passes/private_items_doc_tests.rs | 49 +++++++++++++++ src/test/rustdoc-ui/private-item-doc-test.rs | 20 +++++++ .../rustdoc-ui/private-item-doc-test.stderr | 16 +++++ 7 files changed, 158 insertions(+), 43 deletions(-) create mode 100644 src/librustdoc/passes/private_items_doc_tests.rs create mode 100644 src/test/rustdoc-ui/private-item-doc-test.rs create mode 100644 src/test/rustdoc-ui/private-item-doc-test.stderr diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 01d87bdbf6337..22f2023eefbd8 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -318,6 +318,12 @@ declare_lint! { "warn about missing code example in an item's documentation" } +declare_lint! { + pub PRIVATE_DOC_TESTS, + Allow, + "warn about doc test in private item" +} + declare_lint! { pub WHERE_CLAUSES_OBJECT_SAFETY, Warn, @@ -415,6 +421,7 @@ impl LintPass for HardwiredLints { DUPLICATE_MACRO_EXPORTS, INTRA_DOC_LINK_RESOLUTION_FAILURE, MISSING_DOC_CODE_EXAMPLES, + PRIVATE_DOC_TESTS, WHERE_CLAUSES_OBJECT_SAFETY, PROC_MACRO_DERIVE_RESOLUTION_FALLBACK, MACRO_USE_EXTERN_CRATE, diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 0bd6f6bf8a2f4..aac0f9f94e329 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -351,13 +351,15 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt let warnings_lint_name = lint::builtin::WARNINGS.name; let missing_docs = rustc_lint::builtin::MISSING_DOCS.name; let missing_doc_example = rustc_lint::builtin::MISSING_DOC_CODE_EXAMPLES.name; + let private_doc_tests = rustc_lint::builtin::PRIVATE_DOC_TESTS.name; // In addition to those specific lints, we also need to whitelist those given through // command line, otherwise they'll get ignored and we don't want that. let mut whitelisted_lints = vec![warnings_lint_name.to_owned(), intra_link_resolution_failure_name.to_owned(), missing_docs.to_owned(), - missing_doc_example.to_owned()]; + missing_doc_example.to_owned(), + private_doc_tests.to_owned()]; whitelisted_lints.extend(lint_opts.iter().map(|(lint, _)| lint).cloned()); diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 471ba6345e248..675e3e9be1e62 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -24,9 +24,9 @@ use std::ops::Range; use core::DocContext; use fold::DocFolder; -use html::markdown::{find_testable_code, markdown_links, ErrorCodes, LangString}; +use html::markdown::markdown_links; -use passes::Pass; +use passes::{look_for_tests, Pass}; pub const COLLECT_INTRA_DOC_LINKS: Pass = Pass::early("collect-intra-doc-links", collect_intra_doc_links, @@ -214,43 +214,6 @@ impl<'a, 'tcx, 'rcx, 'cstore> LinkCollector<'a, 'tcx, 'rcx, 'cstore> { } } -fn look_for_tests<'a, 'tcx: 'a, 'rcx: 'a, 'cstore: 'rcx>( - cx: &'a DocContext<'a, 'tcx, 'rcx, 'cstore>, - dox: &str, - item: &Item, -) { - if (item.is_mod() && cx.tcx.hir.as_local_node_id(item.def_id).is_none()) || - cx.as_local_node_id(item.def_id).is_none() { - // If non-local, no need to check anything. - return; - } - - struct Tests { - found_tests: usize, - } - - impl ::test::Tester for Tests { - fn add_test(&mut self, _: String, _: LangString, _: usize) { - self.found_tests += 1; - } - } - - let mut tests = Tests { - found_tests: 0, - }; - - if find_testable_code(&dox, &mut tests, ErrorCodes::No).is_ok() { - if tests.found_tests == 0 { - let mut diag = cx.tcx.struct_span_lint_node( - lint::builtin::MISSING_DOC_CODE_EXAMPLES, - NodeId::from_u32(0), - span_of_attrs(&item.attrs), - "Missing code example in this documentation"); - diag.emit(); - } - } -} - impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstore> { fn fold_item(&mut self, mut item: Item) -> Option { let item_node_id = if item.is_mod() { @@ -313,7 +276,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor let cx = self.cx; let dox = item.attrs.collapsed_doc_value().unwrap_or_else(String::new); - look_for_tests(&cx, &dox, &item); + look_for_tests(&cx, &dox, &item, true); if !self.is_nightly_build { return None; @@ -488,7 +451,7 @@ fn macro_resolve(cx: &DocContext, path_str: &str) -> Option { None } -fn span_of_attrs(attrs: &Attributes) -> syntax_pos::Span { +pub fn span_of_attrs(attrs: &Attributes) -> syntax_pos::Span { if attrs.doc_strings.is_empty() { return DUMMY_SP; } diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index d00eb3257d43c..eee7278e4f0a9 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -12,16 +12,22 @@ //! process. use rustc::hir::def_id::DefId; +use rustc::lint as lint; use rustc::middle::privacy::AccessLevels; use rustc::util::nodemap::DefIdSet; use std::mem; use std::fmt; +use syntax::ast::NodeId; use clean::{self, GetDefId, Item}; -use core::DocContext; +use core::{DocContext, DocAccessLevels}; use fold; use fold::StripItem; +use html::markdown::{find_testable_code, ErrorCodes, LangString}; + +use self::collect_intra_doc_links::span_of_attrs; + mod collapse_docs; pub use self::collapse_docs::COLLAPSE_DOCS; @@ -43,6 +49,9 @@ pub use self::propagate_doc_cfg::PROPAGATE_DOC_CFG; mod collect_intra_doc_links; pub use self::collect_intra_doc_links::COLLECT_INTRA_DOC_LINKS; +mod private_items_doc_tests; +pub use self::private_items_doc_tests::CHECK_PRIVATE_ITEMS_DOC_TESTS; + mod collect_trait_impls; pub use self::collect_trait_impls::COLLECT_TRAIT_IMPLS; @@ -128,6 +137,7 @@ impl Pass { /// The full list of passes. pub const PASSES: &'static [Pass] = &[ + CHECK_PRIVATE_ITEMS_DOC_TESTS, STRIP_HIDDEN, UNINDENT_COMMENTS, COLLAPSE_DOCS, @@ -141,6 +151,7 @@ pub const PASSES: &'static [Pass] = &[ /// The list of passes run by default. pub const DEFAULT_PASSES: &'static [&'static str] = &[ "collect-trait-impls", + "check-private-items-doc-tests", "strip-hidden", "strip-private", "collect-intra-doc-links", @@ -152,6 +163,7 @@ pub const DEFAULT_PASSES: &'static [&'static str] = &[ /// The list of default passes run with `--document-private-items` is passed to rustdoc. pub const DEFAULT_PRIVATE_PASSES: &'static [&'static str] = &[ "collect-trait-impls", + "check-private-items-doc-tests", "strip-priv-imports", "collect-intra-doc-links", "collapse-docs", @@ -348,3 +360,49 @@ impl fold::DocFolder for ImportStripper { } } } + +pub fn look_for_tests<'a, 'tcx: 'a, 'rcx: 'a, 'cstore: 'rcx>( + cx: &'a DocContext<'a, 'tcx, 'rcx, 'cstore>, + dox: &str, + item: &Item, + check_missing_code: bool, +) { + if cx.as_local_node_id(item.def_id).is_none() { + // If non-local, no need to check anything. + return; + } + + struct Tests { + found_tests: usize, + } + + impl ::test::Tester for Tests { + fn add_test(&mut self, _: String, _: LangString, _: usize) { + self.found_tests += 1; + } + } + + let mut tests = Tests { + found_tests: 0, + }; + + if find_testable_code(&dox, &mut tests, ErrorCodes::No).is_ok() { + if check_missing_code == true && tests.found_tests == 0 { + let mut diag = cx.tcx.struct_span_lint_node( + lint::builtin::MISSING_DOC_CODE_EXAMPLES, + NodeId::from_u32(0), + span_of_attrs(&item.attrs), + "Missing code example in this documentation"); + diag.emit(); + } else if check_missing_code == false && + tests.found_tests > 0 && + !cx.renderinfo.borrow().access_levels.is_doc_reachable(item.def_id) { + let mut diag = cx.tcx.struct_span_lint_node( + lint::builtin::PRIVATE_DOC_TESTS, + NodeId::from_u32(0), + span_of_attrs(&item.attrs), + "Documentation test in private item"); + diag.emit(); + } + } +} diff --git a/src/librustdoc/passes/private_items_doc_tests.rs b/src/librustdoc/passes/private_items_doc_tests.rs new file mode 100644 index 0000000000000..7c5ce8894b106 --- /dev/null +++ b/src/librustdoc/passes/private_items_doc_tests.rs @@ -0,0 +1,49 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use clean::*; + +use core::DocContext; +use fold::DocFolder; + +use passes::{look_for_tests, Pass}; + +pub const CHECK_PRIVATE_ITEMS_DOC_TESTS: Pass = + Pass::early("check-private-items-doc-tests", check_private_items_doc_tests, + "check private items doc tests"); + +struct PrivateItemDocTestLinter<'a, 'tcx: 'a, 'rcx: 'a, 'cstore: 'rcx> { + cx: &'a DocContext<'a, 'tcx, 'rcx, 'cstore>, +} + +impl<'a, 'tcx, 'rcx, 'cstore> PrivateItemDocTestLinter<'a, 'tcx, 'rcx, 'cstore> { + fn new(cx: &'a DocContext<'a, 'tcx, 'rcx, 'cstore>) -> Self { + PrivateItemDocTestLinter { + cx, + } + } +} + +pub fn check_private_items_doc_tests(krate: Crate, cx: &DocContext) -> Crate { + let mut coll = PrivateItemDocTestLinter::new(cx); + + coll.fold_crate(krate) +} + +impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for PrivateItemDocTestLinter<'a, 'tcx, 'rcx, 'cstore> { + fn fold_item(&mut self, item: Item) -> Option { + let cx = self.cx; + let dox = item.attrs.collapsed_doc_value().unwrap_or_else(String::new); + + look_for_tests(&cx, &dox, &item, false); + + self.fold_item_recur(item) + } +} diff --git a/src/test/rustdoc-ui/private-item-doc-test.rs b/src/test/rustdoc-ui/private-item-doc-test.rs new file mode 100644 index 0000000000000..5a13fe359f527 --- /dev/null +++ b/src/test/rustdoc-ui/private-item-doc-test.rs @@ -0,0 +1,20 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![deny(private_doc_tests)] + +mod foo { + /// private doc test + /// + /// ``` + /// assert!(false); + /// ``` + fn bar() {} +} diff --git a/src/test/rustdoc-ui/private-item-doc-test.stderr b/src/test/rustdoc-ui/private-item-doc-test.stderr new file mode 100644 index 0000000000000..b43add7ea505f --- /dev/null +++ b/src/test/rustdoc-ui/private-item-doc-test.stderr @@ -0,0 +1,16 @@ +error: Documentation test in private item + --> $DIR/private-item-doc-test.rs:14:5 + | +LL | / /// private doc test +LL | | /// +LL | | /// ``` +LL | | /// assert!(false); +LL | | /// ``` + | |___________^ + | +note: lint level defined here + --> $DIR/private-item-doc-test.rs:11:9 + | +LL | #![deny(private_doc_tests)] + | ^^^^^^^^^^^^^^^^^ +