Auto merge of #111512 - petrochenkov:microdoc2, r=GuillaumeGomez

rustdoc: Cleanup doc string collapsing

`doc_value` and (former) `collapsed_doc_value` can be implemented in terms of each other, and `doc_value` doesn't need the `Option`.

This PR doesn't do any semantic changes, only refactoring, although some pre-existing choices between `doc_value` and `collapsed_doc_value` across rustdoc may be questionable.

Author: bors

src/librustdoc/clean/types.rs

@@ -401,12 +401,18 @@ impl Item {
             .unwrap_or_else(|| self.span(tcx).map_or(rustc_span::DUMMY_SP, |span| span.inner()))
     }
 
-    /// Finds the `doc` attribute as a NameValue and returns the corresponding
-    /// value found.
-    pub(crate) fn doc_value(&self) -> Option<String> {
+    /// Combine all doc strings into a single value handling indentation and newlines as needed.
+    pub(crate) fn doc_value(&self) -> String {
         self.attrs.doc_value()
     }
 
+    /// Combine all doc strings into a single value handling indentation and newlines as needed.
+    /// Returns `None` is there's no documentation at all, and `Some("")` if there is some
+    /// documentation but it is empty (e.g. `#[doc = ""]`).
+    pub(crate) fn opt_doc_value(&self) -> Option<String> {
+        self.attrs.opt_doc_value()
+    }
+
     pub(crate) fn from_def_id_and_parts(
         def_id: DefId,
         name: Option<Symbol>,
@@ -443,12 +449,6 @@ impl Item {
         }
     }
 
-    /// Finds all `doc` attributes as NameValues and returns their corresponding values, joined
-    /// with newlines.
-    pub(crate) fn collapsed_doc_value(&self) -> Option<String> {
-        self.attrs.collapsed_doc_value()
-    }
-
     pub(crate) fn links(&self, cx: &Context<'_>) -> Vec<RenderedLink> {
         use crate::html::format::{href, link_tooltip};
 
@@ -1068,17 +1068,6 @@ impl<I: Iterator<Item = ast::NestedMetaItem>> NestedAttributesExt for I {
     }
 }
 
-/// Collapse a collection of [`DocFragment`]s into one string,
-/// handling indentation and newlines as needed.
-pub(crate) fn collapse_doc_fragments(doc_strings: &[DocFragment]) -> String {
-    let mut acc = String::new();
-    for frag in doc_strings {
-        add_doc_fragment(&mut acc, frag);
-    }
-    acc.pop();
-    acc
-}
-
 /// A link that has not yet been rendered.
 ///
 /// This link will be turned into a rendered link by [`Item::links`].
@@ -1163,29 +1152,23 @@ impl Attributes {
         Attributes { doc_strings, other_attrs }
     }
 
-    /// Finds the `doc` attribute as a NameValue and returns the corresponding
-    /// value found.
-    pub(crate) fn doc_value(&self) -> Option<String> {
-        let mut iter = self.doc_strings.iter();
-
-        let ori = iter.next()?;
-        let mut out = String::new();
-        add_doc_fragment(&mut out, ori);
-        for new_frag in iter {
-            add_doc_fragment(&mut out, new_frag);
-        }
-        out.pop();
-        if out.is_empty() { None } else { Some(out) }
+    /// Combine all doc strings into a single value handling indentation and newlines as needed.
+    pub(crate) fn doc_value(&self) -> String {
+        self.opt_doc_value().unwrap_or_default()
     }
 
-    /// Finds all `doc` attributes as NameValues and returns their corresponding values, joined
-    /// with newlines.
-    pub(crate) fn collapsed_doc_value(&self) -> Option<String> {
-        if self.doc_strings.is_empty() {
-            None
-        } else {
-            Some(collapse_doc_fragments(&self.doc_strings))
-        }
+    /// Combine all doc strings into a single value handling indentation and newlines as needed.
+    /// Returns `None` is there's no documentation at all, and `Some("")` if there is some
+    /// documentation but it is empty (e.g. `#[doc = ""]`).
+    pub(crate) fn opt_doc_value(&self) -> Option<String> {
+        (!self.doc_strings.is_empty()).then(|| {
+            let mut res = String::new();
+            for frag in &self.doc_strings {
+                add_doc_fragment(&mut res, frag);
+            }
+            res.pop();
+            res
+        })
     }
 
     pub(crate) fn get_doc_aliases(&self) -> Box<[Symbol]> {

src/librustdoc/clean/types/tests.rs

@@ -1,7 +1,5 @@
 use super::*;
 
-use crate::clean::collapse_doc_fragments;
-
 use rustc_resolve::rustdoc::{unindent_doc_fragments, DocFragment, DocFragmentKind};
 use rustc_span::create_default_session_globals_then;
 use rustc_span::source_map::DUMMY_SP;
@@ -22,7 +20,8 @@ fn run_test(input: &str, expected: &str) {
     create_default_session_globals_then(|| {
         let mut s = create_doc_fragment(input);
         unindent_doc_fragments(&mut s);
-        assert_eq!(collapse_doc_fragments(&s), expected);
+        let attrs = Attributes { doc_strings: s, other_attrs: Default::default() };
+        assert_eq!(attrs.doc_value(), expected);
     });
 }
 

src/librustdoc/core.rs

@@ -367,7 +367,7 @@ pub(crate) fn run_global_ctxt(
 
     let mut krate = tcx.sess.time("clean_crate", || clean::krate(&mut ctxt));
 
-    if krate.module.doc_value().map(|d| d.is_empty()).unwrap_or(true) {
+    if krate.module.doc_value().is_empty() {
         let help = format!(
             "The following guide may be of use:\n\
             {}/rustdoc/how-to-write-documentation.html",

src/librustdoc/doctest.rs

@@ -1237,7 +1237,7 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
         // The collapse-docs pass won't combine sugared/raw doc attributes, or included files with
         // anything else, this will combine them for us.
         let attrs = Attributes::from_ast(ast_attrs);
-        if let Some(doc) = attrs.collapsed_doc_value() {
+        if let Some(doc) = attrs.opt_doc_value() {
             // Use the outermost invocation, so that doctest names come from where the docs were written.
             let span = ast_attrs
                 .iter()

src/librustdoc/formats/cache.rs

@@ -327,9 +327,8 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
                     // which should not be indexed. The crate-item itself is
                     // inserted later on when serializing the search-index.
                     if item.item_id.as_def_id().map_or(false, |idx| !idx.is_crate_root()) {
-                        let desc = item.doc_value().map_or_else(String::new, |x| {
-                            short_markdown_summary(x.as_str(), &item.link_names(self.cache))
-                        });
+                        let desc =
+                            short_markdown_summary(&item.doc_value(), &item.link_names(self.cache));
                         let ty = item.type_();
                         if ty != ItemType::StructField
                             || u16::from_str_radix(s.as_str(), 10).is_err()

src/librustdoc/html/render/context.rs

@@ -184,11 +184,8 @@ impl<'tcx> Context<'tcx> {
         };
         title.push_str(" - Rust");
         let tyname = it.type_();
-        let desc = it
-            .doc_value()
-            .as_ref()
-            .map(|doc| plain_text_summary(doc, &it.link_names(&self.cache())));
-        let desc = if let Some(desc) = desc {
+        let desc = plain_text_summary(&it.doc_value(), &it.link_names(&self.cache()));
+        let desc = if !desc.is_empty() {
             desc
         } else if it.is_crate() {
             format!("API documentation for the Rust `{}` crate.", self.shared.layout.krate)

src/librustdoc/html/render/mod.rs

@@ -468,7 +468,8 @@ fn document_short<'a, 'cx: 'a>(
         if !show_def_docs {
             return Ok(());
         }
-        if let Some(s) = item.doc_value() {
+        let s = item.doc_value();
+        if !s.is_empty() {
             let (mut summary_html, has_more_content) =
                 MarkdownSummaryLine(&s, &item.links(cx)).into_string_with_has_more_content();
 
@@ -511,7 +512,7 @@ fn document_full_inner<'a, 'cx: 'a>(
     heading_offset: HeadingOffset,
 ) -> impl fmt::Display + 'a + Captures<'cx> {
     display_fn(move |f| {
-        if let Some(s) = item.collapsed_doc_value() {
+        if let Some(s) = item.opt_doc_value() {
             debug!("Doc block: =====\n{}\n=====", s);
             if is_collapsible {
                 write!(
@@ -1476,7 +1477,7 @@ fn render_impl(
                     if let Some(it) = t.items.iter().find(|i| i.name == item.name) {
                         // We need the stability of the item from the trait
                         // because impls can't have a stability.
-                        if item.doc_value().is_some() {
+                        if !item.doc_value().is_empty() {
                             document_item_info(cx, it, Some(parent))
                                 .render_into(&mut info_buffer)
                                 .unwrap();
@@ -1747,7 +1748,7 @@ fn render_impl(
             write!(w, "</summary>")
         }
 
-        if let Some(ref dox) = i.impl_item.collapsed_doc_value() {
+        if let Some(ref dox) = i.impl_item.opt_doc_value() {
             if trait_.is_none() && i.inner_impl().items.is_empty() {
                 w.write_str(
                     "<div class=\"item-info\">\

src/librustdoc/html/render/print_item.rs

@@ -420,9 +420,9 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
                     _ => "",
                 };
 
-                let doc_value = myitem.doc_value().unwrap_or_default();
                 w.write_str(ITEM_TABLE_ROW_OPEN);
-                let docs = MarkdownSummaryLine(&doc_value, &myitem.links(cx)).into_string();
+                let docs =
+                    MarkdownSummaryLine(&myitem.doc_value(), &myitem.links(cx)).into_string();
                 let (docs_before, docs_after) = if docs.is_empty() {
                     ("", "")
                 } else {
@@ -1338,7 +1338,7 @@ fn item_enum(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, e: &clean::
                 clean::VariantKind::Tuple(fields) => {
                     // Documentation on tuple variant fields is rare, so to reduce noise we only emit
                     // the section if at least one field is documented.
-                    if fields.iter().any(|f| f.doc_value().is_some()) {
+                    if fields.iter().any(|f| !f.doc_value().is_empty()) {
                         Some(("Tuple Fields", fields))
                     } else {
                         None

src/librustdoc/html/render/search_index.rs

@@ -28,9 +28,7 @@ pub(crate) fn build_index<'tcx>(
     // has since been learned.
     for &OrphanImplItem { parent, ref item, ref impl_generics } in &cache.orphan_impl_items {
         if let Some((fqp, _)) = cache.paths.get(&parent) {
-            let desc = item
-                .doc_value()
-                .map_or_else(String::new, |s| short_markdown_summary(&s, &item.link_names(cache)));
+            let desc = short_markdown_summary(&item.doc_value(), &item.link_names(cache));
             cache.search_index.push(IndexItem {
                 ty: item.type_(),
                 name: item.name.unwrap(),
@@ -45,10 +43,8 @@ pub(crate) fn build_index<'tcx>(
         }
     }
 
-    let crate_doc = krate
-        .module
-        .doc_value()
-        .map_or_else(String::new, |s| short_markdown_summary(&s, &krate.module.link_names(cache)));
+    let crate_doc =
+        short_markdown_summary(&krate.module.doc_value(), &krate.module.link_names(cache));
 
     // Aliases added through `#[doc(alias = "...")]`. Since a few items can have the same alias,
     // we need the alias element to have an array of items.

src/librustdoc/json/conversions.rs

@@ -40,7 +40,7 @@ impl JsonRenderer<'_> {
                 (String::from(&**link), id_from_item_default(id.into(), self.tcx))
             })
             .collect();
-        let docs = item.attrs.collapsed_doc_value();
+        let docs = item.opt_doc_value();
         let attrs = item.attributes(self.tcx, true);
         let span = item.span(self.tcx);
         let visibility = item.visibility(self.tcx);

src/librustdoc/passes/calculate_doc_coverage.rs

@@ -206,13 +206,7 @@ impl<'a, 'b> DocVisitor for CoverageCalculator<'a, 'b> {
                 let has_docs = !i.attrs.doc_strings.is_empty();
                 let mut tests = Tests { found_tests: 0 };
 
-                find_testable_code(
-                    &i.attrs.collapsed_doc_value().unwrap_or_default(),
-                    &mut tests,
-                    ErrorCodes::No,
-                    false,
-                    None,
-                );
+                find_testable_code(&i.doc_value(), &mut tests, ErrorCodes::No, false, None);
 
                 let has_doc_example = tests.found_tests != 0;
                 let hir_id = DocContext::as_local_hir_id(self.ctx.tcx, i.item_id).unwrap();

src/librustdoc/passes/check_doc_test_visibility.rs

@@ -34,9 +34,7 @@ pub(crate) fn check_doc_test_visibility(krate: Crate, cx: &mut DocContext<'_>) -
 
 impl<'a, 'tcx> DocVisitor for DocTestVisibilityLinter<'a, 'tcx> {
     fn visit_item(&mut self, item: &Item) {
-        let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
-
-        look_for_tests(self.cx, &dox, item);
+        look_for_tests(self.cx, &item.doc_value(), item);
 
         self.visit_item_recur(item)
     }

src/librustdoc/passes/lint/bare_urls.rs

@@ -18,7 +18,7 @@ pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item) {
             // If non-local, no need to check anything.
             return;
         };
-    let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
+    let dox = item.doc_value();
     if !dox.is_empty() {
         let report_diag = |cx: &DocContext<'_>, msg: &str, url: &str, range: Range<usize>| {
             let sp = source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs)

src/librustdoc/passes/lint/check_code_block_syntax.rs

@@ -17,7 +17,7 @@ use crate::html::markdown::{self, RustCodeBlock};
 use crate::passes::source_span_for_markdown_range;
 
 pub(crate) fn visit_item(cx: &DocContext<'_>, item: &clean::Item) {
-    if let Some(dox) = &item.attrs.collapsed_doc_value() {
+    if let Some(dox) = &item.opt_doc_value() {
         let sp = item.attr_span(cx.tcx);
         let extra = crate::html::markdown::ExtraInfo::new(cx.tcx, item.item_id.expect_def_id(), sp);
         for code_block in markdown::rust_code_blocks(dox, &extra) {

src/librustdoc/passes/lint/html_tags.rs

@@ -15,7 +15,7 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
     let Some(hir_id) = DocContext::as_local_hir_id(tcx, item.item_id)
     // If non-local, no need to check anything.
     else { return };
-    let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
+    let dox = item.doc_value();
     if !dox.is_empty() {
         let report_diag = |msg: &str, range: &Range<usize>, is_open_tag: bool| {
             let sp = match source_span_for_markdown_range(tcx, &dox, range, &item.attrs) {

src/librustdoc/passes/lint/unescaped_backticks.rs

@@ -16,7 +16,7 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
         return;
     };
 
-    let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
+    let dox = item.doc_value();
     if dox.is_empty() {
         return;
     }

src/librustdoc/passes/stripper.rs

@@ -194,7 +194,7 @@ impl<'a> DocFolder for ImplStripper<'a, '_> {
                     })
                 {
                     return None;
-                } else if imp.items.is_empty() && i.doc_value().is_none() {
+                } else if imp.items.is_empty() && i.doc_value().is_empty() {
                     return None;
                 }
             }