From 0e0ae47af9843cddb5f67cc060cc163871da6943 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Tue, 15 Dec 2020 19:22:23 +0100 Subject: [PATCH 1/5] Add a new lint to rustdoc for invalid codeblocks This will make rustdoc behave properly when -Dwarnings is given --- compiler/rustc_lint/src/lib.rs | 3 +- compiler/rustc_lint_defs/src/builtin.rs | 13 ++++ src/doc/rustdoc/src/lints.md | 35 +++++++++ src/librustdoc/core.rs | 2 + .../passes/check_code_block_syntax.rs | 71 +++++++++++++------ src/test/rustdoc-ui/invalid-syntax.stderr | 1 + 6 files changed, 102 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 80ef855c3859e..d1a9d518fae87 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -70,7 +70,7 @@ use rustc_middle::ty::TyCtxt; use rustc_session::lint::builtin::{ BARE_TRAIT_OBJECTS, BROKEN_INTRA_DOC_LINKS, ELIDED_LIFETIMES_IN_PATHS, EXPLICIT_OUTLIVES_REQUIREMENTS, INVALID_CODEBLOCK_ATTRIBUTES, INVALID_HTML_TAGS, - MISSING_DOC_CODE_EXAMPLES, NON_AUTOLINKS, PRIVATE_DOC_TESTS, + INVALID_RUST_CODEBLOCK, MISSING_DOC_CODE_EXAMPLES, NON_AUTOLINKS, PRIVATE_DOC_TESTS, }; use rustc_span::symbol::{Ident, Symbol}; use rustc_span::Span; @@ -320,6 +320,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) { BROKEN_INTRA_DOC_LINKS, PRIVATE_INTRA_DOC_LINKS, INVALID_CODEBLOCK_ATTRIBUTES, + INVALID_RUST_CODEBLOCK, MISSING_DOC_CODE_EXAMPLES, PRIVATE_DOC_TESTS, INVALID_HTML_TAGS diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index fa82dce0ae2ed..90c5348c6ea6e 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -1847,6 +1847,18 @@ declare_lint! { "codeblock attribute looks a lot like a known one" } +declare_lint! { + /// The `invalid_rust_codeblock` lint detects Rust code blocks in + /// documentation examples that are invalid (e.g. empty, not parsable as + /// Rust code). This is a `rustdoc` only lint, see the documentation in the + /// [rustdoc book]. + /// + /// [rustdoc book]: ../../../rustdoc/lints.html#invalid_rust_codeblock + pub INVALID_RUST_CODEBLOCK, + Warn, + "codeblock could not be parsed as valid Rust or is empty" +} + declare_lint! { /// The `missing_crate_level_docs` lint detects if documentation is /// missing at the crate root. This is a `rustdoc` only lint, see the @@ -2803,6 +2815,7 @@ declare_lint_pass! { BROKEN_INTRA_DOC_LINKS, PRIVATE_INTRA_DOC_LINKS, INVALID_CODEBLOCK_ATTRIBUTES, + INVALID_RUST_CODEBLOCK, MISSING_CRATE_LEVEL_DOCS, MISSING_DOC_CODE_EXAMPLES, INVALID_HTML_TAGS, diff --git a/src/doc/rustdoc/src/lints.md b/src/doc/rustdoc/src/lints.md index 41292b3d83841..94e0cbe223a95 100644 --- a/src/doc/rustdoc/src/lints.md +++ b/src/doc/rustdoc/src/lints.md @@ -285,6 +285,41 @@ warning: unclosed HTML tag `h1` warning: 2 warnings emitted ``` + +## invalid_rust_codeblock + +This lint **warns by default**. It detects Rust code blocks in documentation +examples that are invalid (e.g. empty, not parsable as Rust). For example: + +```rust +/// Empty code block, with and without Rust: +/// +/// ```rust +/// ``` +/// +/// Unclosed code block: +/// +/// ```rust +fn main() {} +``` + +Which will give: + +```text +warning: Rust code block is empty + --> src/lib.rs:3:5 + | +3 | /// ```rust + | _____^ +4 | | /// ``` + | |_______^ + +warning: Rust code block is empty + --> src/lib.rs:8:5 + | +8 | /// ```rust + | ^^^^^^^ +``` ## non_autolinks diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 9c693c290e7f4..438ac63f0da41 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -322,6 +322,7 @@ crate fn run_core( let private_doc_tests = rustc_lint::builtin::PRIVATE_DOC_TESTS.name; let no_crate_level_docs = rustc_lint::builtin::MISSING_CRATE_LEVEL_DOCS.name; let invalid_codeblock_attributes_name = rustc_lint::builtin::INVALID_CODEBLOCK_ATTRIBUTES.name; + let invalid_rust_codeblock = rustc_lint::builtin::INVALID_RUST_CODEBLOCK.name; let invalid_html_tags = rustc_lint::builtin::INVALID_HTML_TAGS.name; let renamed_and_removed_lints = rustc_lint::builtin::RENAMED_AND_REMOVED_LINTS.name; let non_autolinks = rustc_lint::builtin::NON_AUTOLINKS.name; @@ -337,6 +338,7 @@ crate fn run_core( private_doc_tests.to_owned(), no_crate_level_docs.to_owned(), invalid_codeblock_attributes_name.to_owned(), + invalid_rust_codeblock.to_owned(), invalid_html_tags.to_owned(), renamed_and_removed_lints.to_owned(), unknown_lints.to_owned(), diff --git a/src/librustdoc/passes/check_code_block_syntax.rs b/src/librustdoc/passes/check_code_block_syntax.rs index 0c76dc571beee..323b140847148 100644 --- a/src/librustdoc/passes/check_code_block_syntax.rs +++ b/src/librustdoc/passes/check_code_block_syntax.rs @@ -1,6 +1,8 @@ use rustc_data_structures::sync::{Lock, Lrc}; use rustc_errors::{emitter::Emitter, Applicability, Diagnostic, Handler}; +use rustc_middle::lint::LintDiagnosticBuilder; use rustc_parse::parse_stream_from_source_str; +use rustc_session::lint; use rustc_session::parse::ParseSess; use rustc_span::source_map::{FilePathMapping, SourceMap}; use rustc_span::{FileName, InnerSpan}; @@ -47,51 +49,76 @@ impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> { .unwrap_or(false); let buffer = buffer.borrow(); - if buffer.has_errors || is_empty { - let mut diag = if let Some(sp) = - super::source_span_for_markdown_range(self.cx, &dox, &code_block.range, &item.attrs) - { - let warning_message = if buffer.has_errors { + if !(buffer.has_errors || is_empty) { + // No errors in a non-empty program. + return; + } + + let local_id = match item.def_id.as_local() { + Some(id) => id, + // We don't need to check the syntax for other crates so returning + // without doing anything should not be a problem. + None => return, + }; + + let hir_id = self.cx.tcx.hir().local_def_id_to_hir_id(local_id); + let mark_with_text = code_block.syntax.is_none() && code_block.is_fenced; + + // The span and whether it is precise or not. + let (sp, precise_span) = match super::source_span_for_markdown_range( + self.cx, + &dox, + &code_block.range, + &item.attrs, + ) { + Some(sp) => (sp, true), + None => (super::span_of_attrs(&item.attrs).unwrap_or(item.source.span()), false), + }; + + // lambda that will use the lint to start a new diagnostic and add + // a suggestion to it when needed. + let diag_builder = |lint: LintDiagnosticBuilder<'_>| { + let mut diag = if precise_span { + let msg = if buffer.has_errors { "could not parse code block as Rust code" } else { "Rust code block is empty" }; - let mut diag = self.cx.sess().struct_span_warn(sp, warning_message); - - if code_block.syntax.is_none() && code_block.is_fenced { - let sp = sp.from_inner(InnerSpan::new(0, 3)); + let mut diag = lint.build(msg); + if mark_with_text { diag.span_suggestion( - sp, + sp.from_inner(InnerSpan::new(0, 3)), "mark blocks that do not contain Rust code as text", String::from("```text"), Applicability::MachineApplicable, ); } - diag } else { - // We couldn't calculate the span of the markdown block that had the error, so our - // diagnostics are going to be a bit lacking. - let mut diag = self.cx.sess().struct_span_warn( - super::span_of_attrs(&item.attrs).unwrap_or(item.source.span()), - "doc comment contains an invalid Rust code block", - ); - - if code_block.syntax.is_none() && code_block.is_fenced { + let mut diag = lint.build("doc comment contains an invalid Rust code block"); + if mark_with_text { diag.help("mark blocks that do not contain Rust code as text: ```text"); } diag }; - // FIXME(#67563): Provide more context for these errors by displaying the spans inline. for message in buffer.messages.iter() { diag.note(&message); } - diag.emit(); - } + }; + + // Finally build and emit the completed diagnostic. + // All points of divergence have been handled earlier so this can be + // done the same way whether the span is precise or not. + self.cx.tcx.struct_span_lint_hir( + lint::builtin::INVALID_RUST_CODEBLOCK, + hir_id, + sp, + diag_builder, + ); } } diff --git a/src/test/rustdoc-ui/invalid-syntax.stderr b/src/test/rustdoc-ui/invalid-syntax.stderr index 9a7a4d2101362..a55a0b1d90e24 100644 --- a/src/test/rustdoc-ui/invalid-syntax.stderr +++ b/src/test/rustdoc-ui/invalid-syntax.stderr @@ -7,6 +7,7 @@ LL | | /// \__________pkt->size___________/ \_result->size_/ \__pkt->si LL | | /// ``` | |_______^ | + = note: `#[warn(invalid_rust_codeblock)]` on by default = note: error from rustc: unknown start of token: \ = note: error from rustc: unknown start of token: \ = note: error from rustc: unknown start of token: \ From 41c43300820615aa377b9e6f5711ff0ba1c74fab Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sun, 20 Dec 2020 00:34:49 +0100 Subject: [PATCH 2/5] Make doc-test pass with the new rustdoc --- compiler/rustc_trait_selection/src/opaque_types.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_trait_selection/src/opaque_types.rs b/compiler/rustc_trait_selection/src/opaque_types.rs index f5bc90e6f9621..9cce863ac73ea 100644 --- a/compiler/rustc_trait_selection/src/opaque_types.rs +++ b/compiler/rustc_trait_selection/src/opaque_types.rs @@ -46,6 +46,7 @@ pub struct OpaqueTypeDecl<'tcx> { /// type Foo = impl Baz; /// fn bar() -> Foo { /// // ^^^ This is the span we are looking for! + /// # } /// ``` /// /// In cases where the fn returns `(impl Trait, impl Trait)` or From a6d377d56560350976ccb46f33ae14ad65eb9372 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 22 Dec 2020 16:33:36 -0800 Subject: [PATCH 3/5] Include rustdoc in the compiler docs index. --- src/bootstrap/doc.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index 8cacc2512eff1..30a8229036fd8 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -628,6 +628,8 @@ impl Step for Rustdoc { cargo.arg("-p").arg("rustdoc"); cargo.rustdocflag("--document-private-items"); + cargo.rustdocflag("--enable-index-page"); + cargo.rustdocflag("-Zunstable-options"); builder.run(&mut cargo.into()); } } From d14ef17b4adf96453d72f79b41b6b774bac50585 Mon Sep 17 00:00:00 2001 From: ThePuzzlemaker Date: Sun, 20 Dec 2020 01:34:33 -0600 Subject: [PATCH 4/5] rustdoc: Highlight edition-specific keywords correctly in code blocks, accounting for code block edition modifiers This is a squash of these commits: - Highlight edition-specific keywords correctly in code blocks, accounting for code block edition modifiers - Fix unit tests - Revert changes to rustc_span::symbol to prepare for merge of #80272 - Use new Symbol::is_reserved API from #80272 --- src/librustdoc/html/highlight.rs | 23 ++++++++++++++++------- src/librustdoc/html/highlight/tests.rs | 5 +++-- src/librustdoc/html/markdown.rs | 2 ++ src/librustdoc/html/render/mod.rs | 1 + src/librustdoc/html/sources.rs | 7 ++++--- 5 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 1cbfbf50dd745..c857a54178740 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -11,7 +11,8 @@ use std::fmt::{Display, Write}; use std::iter::Peekable; use rustc_lexer::{LiteralKind, TokenKind}; -use rustc_span::symbol::Ident; +use rustc_span::edition::Edition; +use rustc_span::symbol::Symbol; use rustc_span::with_default_session_globals; /// Highlights `src`, returning the HTML output. @@ -20,6 +21,7 @@ crate fn render_with_highlighting( class: Option<&str>, playground_button: Option<&str>, tooltip: Option<(&str, &str)>, + edition: Edition, ) -> String { debug!("highlighting: ================\n{}\n==============", src); let mut out = String::with_capacity(src.len()); @@ -34,7 +36,7 @@ crate fn render_with_highlighting( } write_header(&mut out, class); - write_code(&mut out, &src); + write_code(&mut out, &src, edition); write_footer(&mut out, playground_button); out @@ -45,10 +47,10 @@ fn write_header(out: &mut String, class: Option<&str>) { .unwrap() } -fn write_code(out: &mut String, src: &str) { +fn write_code(out: &mut String, src: &str, edition: Edition) { // This replace allows to fix how the code source with DOS backline characters is displayed. let src = src.replace("\r\n", "\n"); - Classifier::new(&src).highlight(&mut |highlight| { + Classifier::new(&src, edition).highlight(&mut |highlight| { match highlight { Highlight::Token { text, class } => string(out, Escape(text), class), Highlight::EnterSpan { class } => enter_span(out, class), @@ -139,12 +141,19 @@ struct Classifier<'a> { in_attribute: bool, in_macro: bool, in_macro_nonterminal: bool, + edition: Edition, } impl<'a> Classifier<'a> { - fn new(src: &str) -> Classifier<'_> { + fn new(src: &str, edition: Edition) -> Classifier<'_> { let tokens = TokenIter { src }.peekable(); - Classifier { tokens, in_attribute: false, in_macro: false, in_macro_nonterminal: false } + Classifier { + tokens, + in_attribute: false, + in_macro: false, + in_macro_nonterminal: false, + edition, + } } /// Exhausts the `Classifier` writing the output into `sink`. @@ -296,7 +305,7 @@ impl<'a> Classifier<'a> { "Option" | "Result" => Class::PreludeTy, "Some" | "None" | "Ok" | "Err" => Class::PreludeVal, // Keywords are also included in the identifier set. - _ if Ident::from_str(text).is_reserved() => Class::KeyWord, + _ if Symbol::intern(text).is_reserved(|| self.edition) => Class::KeyWord, _ if self.in_macro_nonterminal => { self.in_macro_nonterminal = false; Class::MacroNonTerminal diff --git a/src/librustdoc/html/highlight/tests.rs b/src/librustdoc/html/highlight/tests.rs index f57f52d6f0875..f97c8a7ab7148 100644 --- a/src/librustdoc/html/highlight/tests.rs +++ b/src/librustdoc/html/highlight/tests.rs @@ -1,5 +1,6 @@ use super::write_code; use expect_test::expect_file; +use rustc_span::edition::Edition; const STYLE: &str = r#"