From d6385631f4d8d911039287bc4ca5ff2f1f2f0cec Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 18 Sep 2018 00:25:50 +0200 Subject: [PATCH 1/3] Add lint for doc without codeblocks --- src/librustc/lint/builtin.rs | 7 +++ src/librustdoc/core.rs | 4 +- src/librustdoc/html/markdown.rs | 6 +- .../passes/collect_intra_doc_links.rs | 46 +++++++++++++- src/librustdoc/test.rs | 60 +++++++++++-------- 5 files changed, 94 insertions(+), 29 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 64056ece98770..66cb6f2b52acf 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -312,6 +312,12 @@ declare_lint! { "warn about documentation intra links resolution failure" } +declare_lint! { + pub MISSING_DOC_ITEM_CODE_EXAMPLE, + Allow, + "warn about missing code example in an item's documentation" +} + declare_lint! { pub WHERE_CLAUSES_OBJECT_SAFETY, Warn, @@ -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, diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index b85604d860be4..fdd6929d98aed 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -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()); diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index d14275aeb6bf5..22fa887c35814 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -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( + doc: &str, + tests: &mut T, + error_codes: ErrorCodes, ) -> Result<(), TestableCodeError> { let mut parser = Parser::new(doc); let mut prev_offset = 0; diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 7b2eb2259d679..f97300357153b 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -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 = @@ -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 { let item_node_id = if item.is_mod() { @@ -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('/') { diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index dbebc3ab39397..1a7a3f4478e74 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -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, @@ -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(); @@ -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; @@ -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. From 2def81a5f1d48625f74ed401329fd9af4e8668f2 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 29 Sep 2018 17:16:06 +0200 Subject: [PATCH 2/3] Store nightly build instead of checking env var every time --- src/librustc/lint/builtin.rs | 2 +- src/librustdoc/core.rs | 2 +- src/librustdoc/passes/collect_intra_doc_links.rs | 6 ++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 66cb6f2b52acf..202a4284f4c82 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -313,7 +313,7 @@ declare_lint! { } declare_lint! { - pub MISSING_DOC_ITEM_CODE_EXAMPLE, + pub MISSING_DOC_CODE_EXAMPLES, Allow, "warn about missing code example in an item's documentation" } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index fdd6929d98aed..76ca408327095 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -348,7 +348,7 @@ 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; + let missing_doc_example = rustc_lint::builtin::MISSING_DOC_CODE_EXAMPLES.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. diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index f97300357153b..a780322e85e86 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -57,6 +57,7 @@ enum PathKind { struct LinkCollector<'a, 'tcx: 'a, 'rcx: 'a, 'cstore: 'rcx> { cx: &'a DocContext<'a, 'tcx, 'rcx, 'cstore>, mod_ids: Vec, + is_nightly_build: bool, } impl<'a, 'tcx, 'rcx, 'cstore> LinkCollector<'a, 'tcx, 'rcx, 'cstore> { @@ -64,6 +65,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> LinkCollector<'a, 'tcx, 'rcx, 'cstore> { LinkCollector { cx, mod_ids: Vec::new(), + is_nightly_build: UnstableFeatures::from_environment().is_nightly_build(), } } @@ -240,7 +242,7 @@ fn look_for_tests<'a, 'tcx: 'a, 'rcx: 'a, 'cstore: 'rcx>( 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, + lint::builtin::MISSING_DOC_CODE_EXAMPLES, NodeId::new(0), span_of_attrs(&item.attrs), "Missing code example in this documentation"); @@ -313,7 +315,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor look_for_tests(&cx, &dox, &item); - if !UnstableFeatures::from_environment().is_nightly_build() { + if !self.is_nightly_build { return None; } From 26479c46371db8a83760890294ac7ad051ec3467 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 9 Oct 2018 16:46:29 +0200 Subject: [PATCH 3/3] Add test for docs without examples --- src/test/rustdoc-ui/doc-without-codeblock.rs | 20 ++++++++++++++ .../rustdoc-ui/doc-without-codeblock.stderr | 26 +++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 src/test/rustdoc-ui/doc-without-codeblock.rs create mode 100644 src/test/rustdoc-ui/doc-without-codeblock.stderr diff --git a/src/test/rustdoc-ui/doc-without-codeblock.rs b/src/test/rustdoc-ui/doc-without-codeblock.rs new file mode 100644 index 0000000000000..e047b272c4160 --- /dev/null +++ b/src/test/rustdoc-ui/doc-without-codeblock.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(missing_doc_code_examples)] + +/// Some docs. +pub struct Foo; + +/// And then, the princess died. +pub mod foo { + /// Or maybe not because she saved herself! + pub fn bar() {} +} diff --git a/src/test/rustdoc-ui/doc-without-codeblock.stderr b/src/test/rustdoc-ui/doc-without-codeblock.stderr new file mode 100644 index 0000000000000..ba5bb7fc0b119 --- /dev/null +++ b/src/test/rustdoc-ui/doc-without-codeblock.stderr @@ -0,0 +1,26 @@ +error: Missing code example in this documentation + | +note: lint level defined here + --> $DIR/doc-without-codeblock.rs:11:9 + | +LL | #![deny(missing_doc_code_examples)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: Missing code example in this documentation + --> $DIR/doc-without-codeblock.rs:13:1 + | +LL | /// Some docs. + | ^^^^^^^^^^^^^^ + +error: Missing code example in this documentation + --> $DIR/doc-without-codeblock.rs:16:1 + | +LL | /// And then, the princess died. + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: Missing code example in this documentation + --> $DIR/doc-without-codeblock.rs:18:5 + | +LL | /// Or maybe not because she saved herself! + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +