Skip to content

[rustdoc] Add lint for doc without codeblocks #54349

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

Merged
merged 3 commits into from
Oct 18, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Add lint for doc without codeblocks
  • Loading branch information
GuillaumeGomez committed Oct 9, 2018
commit d6385631f4d8d911039287bc4ca5ff2f1f2f0cec
7 changes: 7 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,12 @@ declare_lint! {
"warn about documentation intra links resolution failure"
}

declare_lint! {
pub MISSING_DOC_ITEM_CODE_EXAMPLE,
Copy link
Member

Choose a reason for hiding this comment

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

This may work better as MISSING_DOC_CODE_EXAMPLES - users don't need to care about the fact that docs go on items, and making it plural fits in better with other lints.

Copy link
Member Author

Choose a reason for hiding this comment

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

I indeed prefer the new proposition. :)

Allow,
"warn about missing code example in an item's documentation"
}

declare_lint! {
pub WHERE_CLAUSES_OBJECT_SAFETY,
Warn,
Expand Down Expand Up @@ -408,6 +414,7 @@ impl LintPass for HardwiredLints {
DUPLICATE_ASSOCIATED_TYPE_BINDINGS,
DUPLICATE_MACRO_EXPORTS,
INTRA_DOC_LINK_RESOLUTION_FAILURE,
MISSING_DOC_CODE_EXAMPLES,
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 @@ -348,12 +348,14 @@ pub fn run_core(search_paths: SearchPaths,
let intra_link_resolution_failure_name = lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE.name;
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_ITEM_CODE_EXAMPLE.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_docs.to_owned(),
missing_doc_example.to_owned()];

whitelisted_lints.extend(cmd_lints.iter().map(|(lint, _)| lint).cloned());

Expand Down
6 changes: 4 additions & 2 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,8 +532,10 @@ impl fmt::Display for TestableCodeError {
}
}

pub fn find_testable_code(
doc: &str, tests: &mut test::Collector, error_codes: ErrorCodes,
pub fn find_testable_code<T: test::Tester>(
doc: &str,
tests: &mut T,
error_codes: ErrorCodes,
) -> Result<(), TestableCodeError> {
let mut parser = Parser::new(doc);
let mut prev_offset = 0;
Expand Down
46 changes: 45 additions & 1 deletion src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ use std::ops::Range;

use core::DocContext;
use fold::DocFolder;
use html::markdown::markdown_links;
use html::markdown::{find_testable_code, markdown_links, ErrorCodes, LangString};

use passes::Pass;

pub const COLLECT_INTRA_DOC_LINKS: Pass =
Expand Down Expand Up @@ -211,6 +212,43 @@ 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_ITEM_CODE_EXAMPLE,
NodeId::new(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 @@ -273,6 +311,12 @@ 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);

if !UnstableFeatures::from_environment().is_nightly_build() {
return None;
}

for (ori_link, link_range) in markdown_links(&dox) {
// bail early for real links
if ori_link.contains('/') {
Expand Down
60 changes: 35 additions & 25 deletions src/librustdoc/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,14 @@ fn partition_source(s: &str) -> (String, String) {
(before, after)
}

pub trait Tester {
fn add_test(&mut self, test: String, config: LangString, line: usize);
fn get_line(&self) -> usize {
0
}
fn register_header(&mut self, _name: &str, _level: u32) {}
}

pub struct Collector {
pub tests: Vec<testing::TestDescAndFn>,

Expand Down Expand Up @@ -534,7 +542,31 @@ impl Collector {
format!("{} - {} (line {})", filename, self.names.join("::"), line)
}

pub fn add_test(&mut self, test: String, config: LangString, line: usize) {
pub fn set_position(&mut self, position: Span) {
self.position = position;
}

fn get_filename(&self) -> FileName {
if let Some(ref source_map) = self.source_map {
let filename = source_map.span_to_filename(self.position);
if let FileName::Real(ref filename) = filename {
if let Ok(cur_dir) = env::current_dir() {
if let Ok(path) = filename.strip_prefix(&cur_dir) {
return path.to_owned().into();
}
}
}
filename
} else if let Some(ref filename) = self.filename {
filename.clone().into()
} else {
FileName::Custom("input".to_owned())
}
}
}

impl Tester for Collector {
fn add_test(&mut self, test: String, config: LangString, line: usize) {
let filename = self.get_filename();
let name = self.generate_name(line, &filename);
let cfgs = self.cfgs.clone();
Expand Down Expand Up @@ -588,7 +620,7 @@ impl Collector {
});
}

pub fn get_line(&self) -> usize {
fn get_line(&self) -> usize {
if let Some(ref source_map) = self.source_map {
let line = self.position.lo().to_usize();
let line = source_map.lookup_char_pos(BytePos(line as u32)).line;
Expand All @@ -598,29 +630,7 @@ impl Collector {
}
}

pub fn set_position(&mut self, position: Span) {
self.position = position;
}

fn get_filename(&self) -> FileName {
if let Some(ref source_map) = self.source_map {
let filename = source_map.span_to_filename(self.position);
if let FileName::Real(ref filename) = filename {
if let Ok(cur_dir) = env::current_dir() {
if let Ok(path) = filename.strip_prefix(&cur_dir) {
return path.to_owned().into();
}
}
}
filename
} else if let Some(ref filename) = self.filename {
filename.clone().into()
} else {
FileName::Custom("input".to_owned())
}
}

pub fn register_header(&mut self, name: &str, level: u32) {
fn register_header(&mut self, name: &str, level: u32) {
if self.use_headers {
// we use these headings as test names, so it's good if
// they're valid identifiers.
Expand Down