doc_refdef_list_item: new lint for suspicious list syntax

This is more likely to be intended as an intra-doc link than it is to be intended as a refdef. If a refdef is intended, it does not need to be nested within a list item. ```markdown - [`LONG_INTRA_DOC_LINK`]: this looks like an intra-doc link, but is actually a refdef. The first line will seem to disappear when rendered as HTML. ```
rust-lang · Nov 19, 2024 · eb228e7 · eb228e7
1 parent 10677c3
commit eb228e7
Show file tree

Hide file tree

Showing 6 changed files with 373 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5444,6 +5444,7 @@ Released 2018-09-13
 [`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
+[`doc_refdef_list_item`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_refdef_list_item
 [`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons
 [`double_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_must_use
 [`double_neg`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_neg

diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -138,6 +138,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::doc::DOC_LAZY_CONTINUATION_INFO,
     crate::doc::DOC_LINK_WITH_QUOTES_INFO,
     crate::doc::DOC_MARKDOWN_INFO,
+    crate::doc::DOC_REFDEF_LIST_ITEM_INFO,
     crate::doc::EMPTY_DOCS_INFO,
     crate::doc::EMPTY_LINE_AFTER_DOC_COMMENTS_INFO,
     crate::doc::EMPTY_LINE_AFTER_OUTER_ATTR_INFO,

diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs
@@ -3,7 +3,7 @@ mod too_long_first_doc_paragraph;
 
 use clippy_config::Conf;
 use clippy_utils::attrs::is_doc_hidden;
-use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
+use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_then};
 use clippy_utils::macros::{is_panic, root_macro_call_first_node};
 use clippy_utils::ty::is_type_diagnostic_item;
 use clippy_utils::visitors::Visitable;
@@ -16,6 +16,7 @@ use pulldown_cmark::Tag::{BlockQuote, CodeBlock, FootnoteDefinition, Heading, It
 use pulldown_cmark::{BrokenLink, CodeBlockKind, CowStr, Options, TagEnd};
 use rustc_ast::ast::Attribute;
 use rustc_data_structures::fx::FxHashSet;
+use rustc_errors::Applicability;
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_hir::{AnonConst, Expr, ImplItemKind, ItemKind, Node, Safety, TraitItemKind};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
@@ -532,6 +533,32 @@ declare_clippy_lint! {
     "empty line after doc comments"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Warns if a link reference definition appears at the start of a
+    /// list item.
+    ///
+    /// ### Why is this bad?
+    /// This is probably intended as an intra-doc link. If it is really
+    /// supposed to be a reference definition, it can be written outside
+    /// of the list item.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// //! - [link]: description
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// //! - [link][]: description (for intra-doc link)
+    /// //!
+    /// //! [link]: destination (for link reference definition)
+    /// ```
+    #[clippy::version = "1.84.0"]
+    pub DOC_REFDEF_LIST_ITEM,
+    suspicious,
+    "link reference defined in list item"
+}
+
 pub struct Documentation {
     valid_idents: FxHashSet<String>,
     check_private_items: bool,
@@ -549,6 +576,7 @@ impl Documentation {
 impl_lint_pass!(Documentation => [
     DOC_LINK_WITH_QUOTES,
     DOC_MARKDOWN,
+    DOC_REFDEF_LIST_ITEM,
     MISSING_SAFETY_DOC,
     MISSING_ERRORS_DOC,
     MISSING_PANICS_DOC,
@@ -836,11 +864,37 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
                     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));
+                    let indent = if let Some((next_event, next_range)) = events.peek() {
+                        let next_start = match next_event {
+                            End(TagEnd::Item) => next_range.end,
+                            _ => next_range.start,
+                        };
+                        if let Some(refdefrange) = looks_like_refdef(doc, range.start..next_start) &&
+                            let Some(refdefspan) = fragments.span(cx, refdefrange.clone())
+                        {
+                            span_lint_and_then(
+                                cx,
+                                DOC_REFDEF_LIST_ITEM,
+                                refdefspan,
+                                "link reference defined in list item",
+                                |diag| {
+                                    diag.span_suggestion_short(
+                                        refdefspan.shrink_to_hi(),
+                                        "for an intra-doc link, add `[]` between the label and the colon",
+                                        "[]",
+                                        Applicability::MaybeIncorrect,
+                                    );
+                                    diag.help("link definitions are not shown in rendered documentation");
+                                }
+                            );
+                            refdefrange.start - range.start
+                        } else {
+                            next_range.start - range.start
+                        }
                     } else {
-                        containers.push(Container::List(0));
-                    }
+                        0
+                    };
+                    containers.push(Container::List(indent));
                 }
                 ticks_unbalanced = false;
                 paragraph_range = range;
@@ -1012,3 +1066,25 @@ impl<'tcx> Visitor<'tcx> for FindPanicUnwrap<'_, 'tcx> {
         self.cx.tcx.hir()
     }
 }
+
+#[allow(clippy::range_plus_one)] // inclusive ranges aren't the same type
+fn looks_like_refdef(doc: &str, range: Range<usize>) -> Option<Range<usize>> {
+    let offset = range.start;
+    let mut iterator = doc.as_bytes()[range].iter().copied().enumerate();
+    let mut start = None;
+    while let Some((i, byte)) = iterator.next() {
+        if byte == b'\\' {
+            iterator.next();
+            continue;
+        }
+        if byte == b'[' {
+            start = Some(i + offset);
+        }
+        if let Some(start) = start
+            && byte == b']'
+        {
+            return Some(start..i + offset + 1);
+        }
+    }
+    None
+}
diff --git a/tests/ui/doc/doc_refdef_list_item.fixed b/tests/ui/doc/doc_refdef_list_item.fixed
@@ -0,0 +1,71 @@
+// https://github.com/rust-lang/rust/issues/133150
+#![warn(clippy::doc_refdef_list_item)]
+#[rustfmt::skip]
+/// - [link][]: def
+//~^ ERROR: link reference defined in list item
+///
+/// - [link][]: def (title)
+//~^ ERROR: link reference defined in list item
+///
+/// - [link][]: def "title"
+//~^ ERROR: link reference defined in list item
+///
+/// - [link]: not def
+///
+/// - [link][]: notdef
+///
+/// - [link]\: notdef
+pub struct Empty;
+
+#[rustfmt::skip]
+/// - [link][]: def
+//~^ ERROR: link reference defined in list item
+/// - [link][]: def (title)
+//~^ ERROR: link reference defined in list item
+/// - [link][]: def "title"
+//~^ ERROR: link reference defined in list item
+/// - [link]: not def
+/// - [link][]: notdef
+/// - [link]\: notdef
+pub struct EmptyTight;
+
+#[rustfmt::skip]
+/// - [link][]: def
+//~^ ERROR: link reference defined in list item
+///   inner text
+///
+/// - [link][]: def (title)
+//~^ ERROR: link reference defined in list item
+///   inner text
+///
+/// - [link][]: def "title"
+//~^ ERROR: link reference defined in list item
+///   inner text
+///
+/// - [link]: not def
+///   inner text
+///
+/// - [link][]: notdef
+///   inner text
+///
+/// - [link]\: notdef
+///   inner text
+pub struct NotEmpty;
+
+#[rustfmt::skip]
+/// - [link][]: def
+//~^ ERROR: link reference defined in list item
+///   inner text
+/// - [link][]: def (title)
+//~^ ERROR: link reference defined in list item
+///   inner text
+/// - [link][]: def "title"
+//~^ ERROR: link reference defined in list item
+///   inner text
+/// - [link]: not def
+///   inner text
+/// - [link][]: notdef
+///   inner text
+/// - [link]\: notdef
+///   inner text
+pub struct NotEmptyTight;
diff --git a/tests/ui/doc/doc_refdef_list_item.rs b/tests/ui/doc/doc_refdef_list_item.rs
@@ -0,0 +1,71 @@
+// https://github.com/rust-lang/rust/issues/133150
+#![warn(clippy::doc_refdef_list_item)]
+#[rustfmt::skip]
+/// - [link]: def
+//~^ ERROR: link reference defined in list item
+///
+/// - [link]: def (title)
+//~^ ERROR: link reference defined in list item
+///
+/// - [link]: def "title"
+//~^ ERROR: link reference defined in list item
+///
+/// - [link]: not def
+///
+/// - [link][]: notdef
+///
+/// - [link]\: notdef
+pub struct Empty;
+
+#[rustfmt::skip]
+/// - [link]: def
+//~^ ERROR: link reference defined in list item
+/// - [link]: def (title)
+//~^ ERROR: link reference defined in list item
+/// - [link]: def "title"
+//~^ ERROR: link reference defined in list item
+/// - [link]: not def
+/// - [link][]: notdef
+/// - [link]\: notdef
+pub struct EmptyTight;
+
+#[rustfmt::skip]
+/// - [link]: def
+//~^ ERROR: link reference defined in list item
+///   inner text
+///
+/// - [link]: def (title)
+//~^ ERROR: link reference defined in list item
+///   inner text
+///
+/// - [link]: def "title"
+//~^ ERROR: link reference defined in list item
+///   inner text
+///
+/// - [link]: not def
+///   inner text
+///
+/// - [link][]: notdef
+///   inner text
+///
+/// - [link]\: notdef
+///   inner text
+pub struct NotEmpty;
+
+#[rustfmt::skip]
+/// - [link]: def
+//~^ ERROR: link reference defined in list item
+///   inner text
+/// - [link]: def (title)
+//~^ ERROR: link reference defined in list item
+///   inner text
+/// - [link]: def "title"
+//~^ ERROR: link reference defined in list item
+///   inner text
+/// - [link]: not def
+///   inner text
+/// - [link][]: notdef
+///   inner text
+/// - [link]\: notdef
+///   inner text
+pub struct NotEmptyTight;