From d6385631f4d8d911039287bc4ca5ff2f1f2f0cec Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 18 Sep 2018 00:25:50 +0200 Subject: [PATCH] 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 64056ece987..66cb6f2b52a 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -312,6 +312,12 @@ "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 @@ fn get_lints(&self) -> LintArray { 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 b85604d860b..fdd6929d98a 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 d14275aeb6b..22fa887c358 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -532,8 +532,10 @@ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { } } -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 7b2eb2259d6..f9730035715 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -24,7 +24,8 @@ 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 @@ fn resolve(&self, } } +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 @@ fn fold_item(&mut self, mut 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); + + 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 dbebc3ab393..1a7a3f4478e 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 @@ fn generate_name(&self, line: usize, filename: &FileName) -> String { 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 @@ pub fn add_test(&mut self, test: String, config: LangString, line: usize) { }); } - 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 @@ pub fn get_line(&self) -> 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()) - } - } - - 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. -- 2.44.0