Add lint for markdown lazy paragraph continuations

This is a follow-up for rust-lang#121659,
since most cases of unintended block quotes are lazy continuations.
The lint is designed to be more generally useful than that, though,
because it will also catch unintended list items and unintended
block quotes that didn't coincidentally hit a pulldown-cmark bug.

Author: notriddle

CHANGELOG.md

@@ -5249,6 +5249,7 @@ Released 2018-09-13
 [`disallowed_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_type
 [`disallowed_types`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_types
 [`diverging_sub_expression`]: https://rust-lang.github.io/rust-clippy/master/index.html#diverging_sub_expression
+[`doc_lazy_continuation`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_lazy_continuation
 [`doc_link_with_quotes`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_link_with_quotes
 [`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
 [`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons

clippy_lints/src/declared_lints.rs

@@ -140,6 +140,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::disallowed_names::DISALLOWED_NAMES_INFO,
     crate::disallowed_script_idents::DISALLOWED_SCRIPT_IDENTS_INFO,
     crate::disallowed_types::DISALLOWED_TYPES_INFO,
+    crate::doc::DOC_LAZY_CONTINUATION_INFO,
     crate::doc::DOC_LINK_WITH_QUOTES_INFO,
     crate::doc::DOC_MARKDOWN_INFO,
     crate::doc::EMPTY_DOCS_INFO,

clippy_lints/src/doc/lazy_continuation.rs

@@ -0,0 +1,95 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use itertools::Itertools;
+use rustc_errors::{Applicability, SuggestionStyle};
+use rustc_lint::LateContext;
+use rustc_span::{BytePos, Span};
+use std::ops::Range;
+
+use super::DOC_LAZY_CONTINUATION;
+
+fn map_container_to_text(c: &super::Container) -> &'static str {
+    match c {
+        super::Container::Blockquote => "> ",
+        // numbered list can have up to nine digits, plus the dot, plus four spaces on either side
+        super::Container::List(indent) => &"                  "[0..*indent],
+    }
+}
+
+// TODO: Adjust the parameters as necessary
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    doc: &str,
+    range: Range<usize>,
+    mut span: Span,
+    containers: &[super::Container],
+) {
+    if doc[range.clone()].contains('\t') {
+        // We don't do tab stops correctly.
+        return;
+    }
+
+    let ccount = doc[range.clone()].chars().filter(|c| *c == '>').count();
+    let blockquote_level = containers
+        .iter()
+        .filter(|c| matches!(c, super::Container::Blockquote))
+        .count();
+    let lcount = doc[range.clone()].chars().filter(|c| *c == ' ').count();
+    let list_indentation = containers
+        .iter()
+        .map(|c| {
+            if let super::Container::List(indent) = c {
+                *indent
+            } else {
+                0
+            }
+        })
+        .sum();
+    if ccount < blockquote_level || lcount < list_indentation {
+        let msg = if ccount < blockquote_level {
+            "doc quote missing `>` marker"
+        } else {
+            "doc list item missing indentation"
+        };
+        span_lint_and_then(cx, DOC_LAZY_CONTINUATION, span, msg, |diag| {
+            if ccount == 0 && blockquote_level == 0 {
+                // simpler suggestion style for indentation
+                let indent = list_indentation - lcount;
+                diag.span_suggestion_with_style(
+                    span.shrink_to_hi(),
+                    "indent this line",
+                    std::iter::repeat(" ").take(indent).join(""),
+                    Applicability::MachineApplicable,
+                    SuggestionStyle::ShowAlways,
+                );
+                diag.help("if this is supposed to be its own paragraph, add a blank line");
+                return;
+            }
+            let mut doc_start_range = &doc[range];
+            let mut suggested = String::new();
+            for c in containers {
+                let text = map_container_to_text(c);
+                if doc_start_range.starts_with(text) {
+                    doc_start_range = &doc_start_range[text.len()..];
+                    span = span
+                        .with_lo(span.lo() + BytePos(u32::try_from(text.len()).expect("text is not 2**32 or bigger")));
+                } else if matches!(c, super::Container::Blockquote)
+                    && let Some(i) = doc_start_range.find('>')
+                {
+                    doc_start_range = &doc_start_range[i + 1..];
+                    span =
+                        span.with_lo(span.lo() + BytePos(u32::try_from(i).expect("text is not 2**32 or bigger") + 1));
+                } else {
+                    suggested.push_str(text);
+                }
+            }
+            diag.span_suggestion_with_style(
+                span,
+                "add markers to start of line",
+                suggested,
+                Applicability::MachineApplicable,
+                SuggestionStyle::ShowAlways,
+            );
+            diag.help("if this not intended to be a quote at all, escape it with `\\>`");
+        });
+    }
+}

clippy_lints/src/doc/mod.rs

@@ -1,3 +1,4 @@
+mod lazy_continuation;
 use clippy_utils::attrs::is_doc_hidden;
 use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
 use clippy_utils::macros::{is_panic, root_macro_call_first_node};
@@ -7,7 +8,7 @@ use clippy_utils::{is_entrypoint_fn, is_trait_impl_item, method_chain_args};
 use pulldown_cmark::Event::{
     Code, End, FootnoteReference, HardBreak, Html, Rule, SoftBreak, Start, TaskListMarker, Text,
 };
-use pulldown_cmark::Tag::{BlockQuote, CodeBlock, Heading, Item, Link, Paragraph};
+use pulldown_cmark::Tag::{BlockQuote, CodeBlock, FootnoteDefinition, Heading, Item, Link, Paragraph};
 use pulldown_cmark::{BrokenLink, CodeBlockKind, CowStr, Options};
 use rustc_ast::ast::Attribute;
 use rustc_data_structures::fx::FxHashSet;
@@ -362,6 +363,63 @@ declare_clippy_lint! {
     "docstrings exist but documentation is empty"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// In CommonMark Markdown, the language used to write doc comments, a
+    /// paragraph nested within a list or block quote does not need any line
+    /// after the first one to be indented or marked. The specification calls
+    /// this a "lazy paragraph continuation."
+    ///
+    /// ### Why is this bad?
+    ///
+    /// This is easy to write but hard to read. Lazy continuations makes
+    /// unintended markers hard to see, and make it harder to deduce the
+    /// document's intended structure.
+    ///
+    /// ### Example
+    ///
+    /// This table is probably intended to have two rows,
+    /// but it does not. It has zero rows, and is followed by
+    /// a block quote.
+    /// ```no_run
+    /// /// Range | Description
+    /// /// ----- | -----------
+    /// /// >= 1  | fully opaque
+    /// /// < 1   | partially see-through
+    /// fn set_opacity(opacity: f32) {}
+    /// ```
+    ///
+    /// Fix it by escaping the marker:
+    /// ```no_run
+    /// /// Range | Description
+    /// /// ----- | -----------
+    /// /// \>= 1 | fully opaque
+    /// /// < 1   | partially see-through
+    /// fn set_opacity(opacity: f32) {}
+    /// ```
+    ///
+    /// This example is actually intended to be a list:
+    /// ```no_run
+    /// /// * Do nothing.
+    /// /// * Then do something. Whatever it is needs done,
+    /// /// it should be done right now.
+    /// # fn do_stuff() {}
+    /// ```
+    ///
+    /// Fix it by indenting the list contents:
+    /// ```no_run
+    /// /// * Do nothing.
+    /// /// * Then do something. Whatever it is needs done,
+    /// ///   it should be done right now.
+    /// # fn do_stuff() {}
+    /// ```
+    #[clippy::version = "1.80.0"]
+    pub DOC_LAZY_CONTINUATION,
+    style,
+    "require every line of a paragraph to be indented and marked"
+}
+
 #[derive(Clone)]
 pub struct Documentation {
     valid_idents: FxHashSet<String>,
@@ -388,6 +446,7 @@ impl_lint_pass!(Documentation => [
     UNNECESSARY_SAFETY_DOC,
     SUSPICIOUS_DOC_COMMENTS,
     EMPTY_DOCS,
+    DOC_LAZY_CONTINUATION,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Documentation {
@@ -551,6 +610,7 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
         cx,
         valid_idents,
         parser.into_offset_iter(),
+        &doc,
         Fragments {
             fragments: &fragments,
             doc: &doc,
@@ -560,6 +620,11 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
 
 const RUST_CODE: &[&str] = &["rust", "no_run", "should_panic", "compile_fail"];
 
+enum Container {
+    Blockquote,
+    List(usize),
+}
+
 /// Checks parsed documentation.
 /// This walks the "events" (think sections of markdown) produced by `pulldown_cmark`,
 /// so lints here will generally access that information.
@@ -569,13 +634,15 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
     cx: &LateContext<'_>,
     valid_idents: &FxHashSet<String>,
     events: Events,
+    doc: &str,
     fragments: Fragments<'_>,
 ) -> DocHeaders {
     // true if a safety header was found
     let mut headers = DocHeaders::default();
     let mut in_code = false;
     let mut in_link = None;
     let mut in_heading = false;
+    let mut in_footnote_definition = false;
     let mut is_rust = false;
     let mut no_test = false;
     let mut ignore = false;
@@ -586,7 +653,11 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
     let mut code_level = 0;
     let mut blockquote_level = 0;
 
-    for (event, range) in events {
+    let mut containers = Vec::new();
+
+    let mut events = events.peekable();
+
+    while let Some((event, range)) = events.next() {
         match event {
             Html(tag) => {
                 if tag.starts_with("<code") {
@@ -599,8 +670,14 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
                     blockquote_level -= 1;
                 }
             },
-            Start(BlockQuote) => blockquote_level += 1,
-            End(BlockQuote) => blockquote_level -= 1,
+            Start(BlockQuote) => {
+                blockquote_level += 1;
+                containers.push(Container::Blockquote);
+            },
+            End(BlockQuote) => {
+                blockquote_level -= 1;
+                containers.pop();
+            },
             Start(CodeBlock(ref kind)) => {
                 in_code = true;
                 if let CodeBlockKind::Fenced(lang) = kind {
@@ -633,13 +710,23 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
                 if let Start(Heading(_, _, _)) = event {
                     in_heading = true;
                 }
+                if let Start(Item) = event {
+                    if let Some((_next_event, next_range)) = events.peek() {
+                        containers.push(Container::List(next_range.start - range.start));
+                    } else {
+                        containers.push(Container::List(0));
+                    }
+                }
                 ticks_unbalanced = false;
                 paragraph_range = range;
             },
             End(Heading(_, _, _) | Paragraph | Item) => {
                 if let End(Heading(_, _, _)) = event {
                     in_heading = false;
                 }
+                if let End(Item) = event {
+                    containers.pop();
+                }
                 if ticks_unbalanced && let Some(span) = fragments.span(cx, paragraph_range.clone()) {
                     span_lint_and_help(
                         cx,
@@ -658,8 +745,26 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
                 }
                 text_to_check = Vec::new();
             },
+            Start(FootnoteDefinition(..)) => in_footnote_definition = true,
+            End(FootnoteDefinition(..)) => in_footnote_definition = false,
             Start(_tag) | End(_tag) => (), // We don't care about other tags
-            SoftBreak | HardBreak | TaskListMarker(_) | Code(_) | Rule => (),
+            SoftBreak | HardBreak => {
+                if !containers.is_empty()
+                    && let Some((_next_event, next_range)) = events.peek()
+                    && let Some(next_span) = fragments.span(cx, next_range.clone())
+                    && let Some(span) = fragments.span(cx, range.clone())
+                    && !in_footnote_definition
+                {
+                    lazy_continuation::check(
+                        cx,
+                        doc,
+                        range.end..next_range.start,
+                        Span::new(span.hi(), next_span.lo(), span.ctxt(), span.parent()),
+                        &containers[..],
+                    );
+                }
+            },
+            TaskListMarker(_) | Code(_) | Rule => (),
             FootnoteReference(text) | Text(text) => {
                 paragraph_range.end = range.end;
                 ticks_unbalanced |= text.contains('`') && !in_code;

tests/ui/doc/doc_lazy_blockquote.fixed

@@ -0,0 +1,47 @@
+#![warn(clippy::doc_lazy_continuation)]
+
+/// > blockquote with
+/// > lazy continuation
+//~^ ERROR: doc quote missing `>` marker
+fn first() {}
+
+/// > blockquote with no
+/// > lazy continuation
+fn first_nowarn() {}
+
+/// > blockquote with no
+///
+/// lazy continuation
+fn two_nowarn() {}
+
+/// > nest here
+/// >
+/// > > nest here
+/// > > lazy continuation
+//~^ ERROR: doc quote missing `>` marker
+fn two() {}
+
+/// > nest here
+/// >
+/// > > nest here
+/// > > lazy continuation
+//~^ ERROR: doc quote missing `>` marker
+fn three() {}
+
+/// >   * > nest here
+/// >     > lazy continuation
+//~^ ERROR: doc quote missing `>` marker
+fn four() {}
+
+/// > * > nest here
+/// >   > lazy continuation
+//~^ ERROR: doc quote missing `>` marker
+fn four_point_1() {}
+
+/// * > nest here lazy continuation
+fn five() {}
+
+/// 1. > nest here
+///    > lazy continuation (this results in strange indentation, but still works)
+//~^ ERROR: doc quote missing `>` marker
+fn six() {}