Rollup merge of rust-lang#84587 - jyn514:rustdoc-lint-block, r=CraftS…

…pider

rustdoc: Make "rust code block is empty" and "could not parse code block" warnings a lint (`INVALID_RUST_CODEBLOCKS`)

Fixes rust-lang#79792. This already went through FCP in rust-lang#79816, so it only needs final review.

This is mostly a rebase of rust-lang#79816 - thank you `@poliorcetics` for doing most of the work!

compiler/rustc_mir/src/borrow_check/region_infer/mod.rs

@@ -1241,7 +1241,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     /// it. However, it works pretty well in practice. In particular,
     /// this is needed to deal with projection outlives bounds like
     ///
-    /// ```ignore (internal compiler representation so lifetime syntax is invalid)
+    /// ```text
     /// <T as Foo<'0>>::Item: '1
     /// ```
     ///

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

compiler/rustc_typeck/src/check/upvar.rs

@@ -323,7 +323,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     ///
     /// InferBorrowKind results in a structure like this:
     ///
-    /// ```
+    /// ```text
     /// {
     ///       Place(base: hir_id_s, projections: [], ....) -> {
     ///                                                            capture_kind_expr: hir_id_L5,
@@ -348,7 +348,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     /// ```
     ///
     /// After the min capture analysis, we get:
-    /// ```
+    /// ```text
     /// {
     ///       hir_id_s -> [
     ///            Place(base: hir_id_s, projections: [], ....) -> {

src/doc/rustdoc/src/lints.md

@@ -294,6 +294,50 @@ warning: unclosed HTML tag `h1`
 warning: 2 warnings emitted
 ```
 
+## invalid_rust_codeblocks
+
+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 blocks (with and without the `rust` marker):
+///
+/// ```rust
+/// ```
+///
+/// Invalid syntax in code blocks:
+///
+/// ```rust
+/// '<
+/// ```
+pub fn foo() {}
+```
+
+Which will give:
+
+```text
+warning: Rust code block is empty
+ --> lint.rs:3:5
+  |
+3 |   /// ```rust
+  |  _____^
+4 | | /// ```
+  | |_______^
+  |
+  = note: `#[warn(rustdoc::invalid_rust_codeblocks)]` on by default
+
+warning: could not parse code block as Rust code
+  --> lint.rs:8:5
+   |
+8  |   /// ```rust
+   |  _____^
+9  | | /// '<
+10 | | /// ```
+   | |_______^
+   |
+   = note: error from rustc: unterminated character literal
+```
+
 ## bare_urls
 
 This lint is **warn-by-default**. It detects URLs which are not links.

src/librustdoc/lint.rs

@@ -157,13 +157,26 @@ declare_rustdoc_lint! {
     "detects URLs that are not hyperlinks"
 }
 
+declare_rustdoc_lint! {
+   /// The `invalid_rust_codeblocks` 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_codeblocks
+   INVALID_RUST_CODEBLOCKS,
+   Warn,
+   "codeblock could not be parsed as valid Rust or is empty"
+}
+
 crate static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
     vec![
         BROKEN_INTRA_DOC_LINKS,
         PRIVATE_INTRA_DOC_LINKS,
         MISSING_DOC_CODE_EXAMPLES,
         PRIVATE_DOC_TESTS,
         INVALID_CODEBLOCK_ATTRIBUTES,
+        INVALID_RUST_CODEBLOCKS,
         INVALID_HTML_TAGS,
         BARE_URLS,
         MISSING_CRATE_LEVEL_DOCS,

src/librustdoc/passes/check_code_block_syntax.rs

@@ -1,5 +1,6 @@
 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::parse::ParseSess;
 use rustc_span::source_map::{FilePathMapping, SourceMap};
@@ -47,63 +48,86 @@ 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.tcx,
-                &dox,
-                &code_block.range,
-                &item.attrs,
-            ) {
-                let (warning_message, suggest_using_text) = if buffer.has_errors {
-                    ("could not parse code block as Rust code", true)
-                } else {
-                    ("Rust code block is empty", false)
-                };
-
-                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));
-                    diag.span_suggestion(
-                        sp,
-                        "mark blocks that do not contain Rust code as text",
-                        String::from("```text"),
-                        Applicability::MachineApplicable,
+        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 empty_block = code_block.syntax.is_none() && code_block.is_fenced;
+        let is_ignore = code_block.is_ignore;
+
+        // The span and whether it is precise or not.
+        let (sp, precise_span) = match super::source_span_for_markdown_range(
+            self.cx.tcx,
+            &dox,
+            &code_block.range,
+            &item.attrs,
+        ) {
+            Some(sp) => (sp, true),
+            None => (item.attr_span(self.cx.tcx), 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 explanation = if is_ignore {
+                "`ignore` code blocks require valid Rust code for syntax highlighting; \
+                    mark blocks that do not contain Rust code as text"
+            } else {
+                "mark blocks that do not contain Rust code as text"
+            };
+            let msg = if buffer.has_errors {
+                "could not parse code block as Rust code"
+            } else {
+                "Rust code block is empty"
+            };
+            let mut diag = lint.build(msg);
+
+            if precise_span {
+                if is_ignore {
+                    // giving an accurate suggestion is hard because `ignore` might not have come first in the list.
+                    // just give a `help` instead.
+                    diag.span_help(
+                        sp.from_inner(InnerSpan::new(0, 3)),
+                        &format!("{}: ```text", explanation),
                     );
-                } else if suggest_using_text && code_block.is_ignore {
-                    let sp = sp.from_inner(InnerSpan::new(0, 3));
+                } else if empty_block {
                     diag.span_suggestion(
-                        sp,
-                        "`ignore` code blocks require valid Rust code for syntax highlighting. \
-                         Mark blocks that do not contain Rust code as text",
-                        String::from("```text,"),
+                        sp.from_inner(InnerSpan::new(0, 3)),
+                        explanation,
+                        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(
-                    item.attr_span(self.cx.tcx),
-                    "doc comment contains an invalid Rust code block",
-                );
-
-                if code_block.syntax.is_none() && code_block.is_fenced {
-                    diag.help("mark blocks that do not contain Rust code as text: ```text");
-                }
-
-                diag
-            };
+            } else if empty_block || is_ignore {
+                diag.help(&format!("{}: ```text", explanation));
+            }
 
             // 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(
+            crate::lint::INVALID_RUST_CODEBLOCKS,
+            hir_id,
+            sp,
+            diag_builder,
+        );
     }
 }
 

src/test/rustdoc-ui/ignore-block-help.rs

@@ -3,5 +3,8 @@
 /// ```ignore (to-prevent-tidy-error)
 /// let heart = '❤️';
 /// ```
-//~^^^ WARN
+//~^^^ WARNING could not parse code block
+//~| NOTE on by default
+//~| NOTE character literal may only contain one codepoint
+//~| HELP `ignore` code blocks require valid Rust code
 pub struct X;

src/test/rustdoc-ui/ignore-block-help.stderr

@@ -7,11 +7,13 @@ LL | | /// let heart = '❤️';
 LL | | /// ```
    | |_______^
    |
-   = note: error from rustc: character literal may only contain one codepoint
-help: `ignore` code blocks require valid Rust code for syntax highlighting. Mark blocks that do not contain Rust code as text
+   = note: `#[warn(rustdoc::invalid_rust_codeblocks)]` on by default
+help: `ignore` code blocks require valid Rust code for syntax highlighting; mark blocks that do not contain Rust code as text: ```text
+  --> $DIR/ignore-block-help.rs:3:5
    |
-LL | /// ```text,ignore (to-prevent-tidy-error)
-   |     ^^^^^^^^
+LL | /// ```ignore (to-prevent-tidy-error)
+   |     ^^^
+   = note: error from rustc: character literal may only contain one codepoint
 
 warning: 1 warning emitted
 

src/test/rustdoc-ui/invalid-syntax.rs

@@ -71,7 +71,7 @@ pub fn blargh() {}
 /// \_
 #[doc = "```"]
 pub fn crazy_attrs() {}
-//~^^^^ WARNING doc comment contains an invalid Rust code block
+//~^^^^ WARNING could not parse code block
 
 /// ```rust
 /// ```

src/test/rustdoc-ui/invalid-syntax.stderr

@@ -7,6 +7,7 @@ LL | | /// \__________pkt->size___________/          \_result->size_/ \__pkt->si
 LL | | /// ```
    | |_______^
    |
+   = note: `#[warn(rustdoc::invalid_rust_codeblocks)]` 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: \
@@ -90,7 +91,7 @@ LL | | /// ```
    |
    = note: error from rustc: unknown start of token: \
 
-warning: doc comment contains an invalid Rust code block
+warning: could not parse code block as Rust code
   --> $DIR/invalid-syntax.rs:70:1
    |
 LL | / #[doc = "```"]