Add new lint doc_lazy_continuation

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/casts/cast_sign_loss.rs

@@ -255,8 +255,10 @@ fn expr_add_sign(cx: &LateContext<'_>, expr: &Expr<'_>) -> Sign {
 
 /// Peels binary operators such as [`BinOpKind::Mul`], [`BinOpKind::Div`] or [`BinOpKind::Rem`],
 /// where the result depends on:
+///
 /// - the number of negative values in the entire expression, or
 /// - the number of negative values on the left hand side of the expression.
+///
 /// Ignores overflow.
 ///
 ///
@@ -303,8 +305,10 @@ fn exprs_with_muldiv_binop_peeled<'e>(expr: &'e Expr<'_>) -> Vec<&'e Expr<'e>> {
 }
 
 /// Peels binary operators such as [`BinOpKind::Add`], where the result depends on:
+///
 /// - all the expressions being positive, or
 /// - all the expressions being negative.
+///
 /// Ignores overflow.
 ///
 /// Expressions using other operators are preserved, so we can try to evaluate them later.

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;

clippy_lints/src/manual_clamp.rs

@@ -611,15 +611,22 @@ impl<'tcx> BinaryOp<'tcx> {
 
 /// The clamp meta pattern is a pattern shared between many (but not all) patterns.
 /// In summary, this pattern consists of two if statements that meet many criteria,
+///
 /// - binary operators that are one of [`>`, `<`, `>=`, `<=`].
+///
 /// - Both binary statements must have a shared argument
+///
 ///     - Which can appear on the left or right side of either statement
+///
 ///     - The binary operators must define a finite range for the shared argument. To put this in
 ///       the terms of Rust `std` library, the following ranges are acceptable
+///
 ///         - `Range`
 ///         - `RangeInclusive`
+///
 ///       And all other range types are not accepted. For the purposes of `clamp` it's irrelevant
 ///       whether the range is inclusive or not, the output is the same.
+///
 /// - The result of each if statement must be equal to the argument unique to that if statement. The
 ///   result can not be the shared argument in either case.
 fn is_clamp_meta_pattern<'tcx>(

clippy_lints/src/methods/iter_filter.rs

@@ -126,15 +126,15 @@ enum FilterType {
 ///
 /// How this is done:
 /// 1. we know that this is invoked in a method call with `filter` as the method name via `mod.rs`
-/// 2. we check that we are in a trait method. Therefore we are in an
-/// `(x as Iterator).filter({filter_arg})` method call.
+/// 2. we check that we are in a trait method. Therefore we are in an `(x as
+///    Iterator).filter({filter_arg})` method call.
 /// 3. we check that the parent expression is not a map. This is because we don't want to lint
 ///    twice, and we already have a specialized lint for that.
 /// 4. we check that the span of the filter does not contain a comment.
 /// 5. we get the type of the `Item` in the `Iterator`, and compare against the type of Option and
-///   Result.
+///    Result.
 /// 6. we finally check the contents of the filter argument to see if it is a call to `is_some` or
-///   `is_ok`.
+///    `is_ok`.
 /// 7. if all of the above are true, then we return the `FilterType`
 fn expression_type(
     cx: &LateContext<'_>,

clippy_lints/src/methods/iter_kv_map.rs

@@ -12,8 +12,10 @@ use rustc_middle::ty;
 use rustc_span::{sym, Span};
 
 /// lint use of:
+///
 /// - `hashmap.iter().map(|(_, v)| v)`
 /// - `hashmap.into_iter().map(|(_, v)| v)`
+///
 /// on `HashMaps` and `BTreeMaps` in std
 
 pub(super) fn check<'tcx>(

clippy_lints/src/methods/utils.rs

@@ -120,6 +120,7 @@ fn pat_bindings(pat: &Pat<'_>) -> Vec<HirId> {
 /// operations performed on `binding_hir_ids` are:
 /// * to take non-mutable references to them
 /// * to use them as non-mutable `&self` in method calls
+///
 /// If any of `binding_hir_ids` is used in any other way, then `clone_or_copy_needed` will be true
 /// when `CloneOrCopyVisitor` is done visiting.
 struct CloneOrCopyVisitor<'cx, 'tcx> {