Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lint if a private item has doctests #55367

Merged
merged 1 commit into from
Nov 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down
45 changes: 4 additions & 41 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<Item> {
let item_node_id = if item.is_mod() {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -488,7 +451,7 @@ fn macro_resolve(cx: &DocContext, path_str: &str) -> Option<Def> {
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;
}
Expand Down
60 changes: 59 additions & 1 deletion src/librustdoc/passes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we revisit this function, i'd like to replace this bool parameter with an enum that represents which check is occurring. It will make the callsite much easier to understand, IMO.

) {
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();
}
}
}
49 changes: 49 additions & 0 deletions src/librustdoc/passes/private_items_doc_tests.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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<Item> {
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)
}
}
20 changes: 20 additions & 0 deletions src/test/rustdoc-ui/private-item-doc-test.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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() {}
}
16 changes: 16 additions & 0 deletions src/test/rustdoc-ui/private-item-doc-test.stderr
Original file line number Diff line number Diff line change
@@ -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)]
| ^^^^^^^^^^^^^^^^^