rustdoc: Return a String from collapsed_doc_value, not an Option

src/librustdoc/clean/types.rs

@@ -468,7 +468,7 @@ impl Item {
 
     /// Finds all `doc` attributes as NameValues and returns their corresponding values, joined
     /// with newlines.
-    crate fn collapsed_doc_value(&self) -> Option<String> {
+    crate fn collapsed_doc_value(&self) -> String {
         self.attrs.collapsed_doc_value()
     }
 
@@ -956,16 +956,6 @@ fn add_doc_fragment(out: &mut String, frag: &DocFragment) {
     }
 }
 
-/// Collapse a collection of [`DocFragment`]s into one string,
-/// handling indentation and newlines as needed.
-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
-}
-
 /// A link that has not yet been rendered.
 ///
 /// This link will be turned into a rendered link by [`Item::links`].
@@ -1104,12 +1094,15 @@ impl Attributes {
 
     /// Finds all `doc` attributes as NameValues and returns their corresponding values, joined
     /// with newlines.
-    crate fn collapsed_doc_value(&self) -> Option<String> {
-        if self.doc_strings.is_empty() {
-            None
-        } else {
-            Some(collapse_doc_fragments(&self.doc_strings))
-        }
+    ///
+    /// Unlike `doc_value`, this does not stop processing lines when switching between `#[doc]` and
+    /// sugared syntax.
+    crate fn collapsed_doc_value(&self) -> String {
+        let mut acc = String::new();
+        for frag in &self.doc_strings {
+            add_doc_fragment(&mut acc, frag);
+        }
+        acc
     }
 
     crate fn get_doc_aliases(&self) -> Box<[Symbol]> {

src/librustdoc/doctest.rs

@@ -1123,17 +1123,17 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
         }
 
         attrs.unindent_doc_comments();
-        // The collapse-docs pass won't combine sugared/raw doc attributes, or included files with
-        // anything else, this will combine them for us.
-        if let Some(doc) = attrs.collapsed_doc_value() {
+        // `doc_value()` doesn't combine sugared/raw doc attributes, this will combine them for us.
+        let docs = attrs.collapsed_doc_value();
+        if !docs.is_empty() {
             // Use the outermost invocation, so that doctest names come from where the docs were written.
             let span = ast_attrs
                 .span()
                 .map(|span| span.ctxt().outer_expn().expansion_cause().unwrap_or(span))
                 .unwrap_or(DUMMY_SP);
             self.collector.set_position(span);
             markdown::find_testable_code(
-                &doc,
+                &docs,
                 self.collector,
                 self.codes,
                 self.collector.enable_per_target_ignores,

src/librustdoc/html/render/mod.rs

@@ -567,19 +567,21 @@ fn document_full_inner(
     is_collapsible: bool,
     heading_offset: HeadingOffset,
 ) {
-    if let Some(s) = item.collapsed_doc_value() {
-        debug!("Doc block: =====\n{}\n=====", s);
+    // NOTE: "collapsed" here is unrelated to `is_collapsible`, it just combines `#[doc]` and sugared attributes
+    let docs = item.collapsed_doc_value();
+    if !docs.is_empty() {
+        debug!("Doc block: =====\n{}\n=====", docs);
         if is_collapsible {
             w.write_str(
                 "<details class=\"rustdoc-toggle top-doc\" open>\
                 <summary class=\"hideme\">\
                      <span>Expand description</span>\
                 </summary>",
             );
-            render_markdown(w, cx, &s, item.links(cx), heading_offset);
+            render_markdown(w, cx, &docs, item.links(cx), heading_offset);
             w.write_str("</details>");
         } else {
-            render_markdown(w, cx, &s, item.links(cx), heading_offset);
+            render_markdown(w, cx, &docs, item.links(cx), heading_offset);
         }
     }
 
@@ -1612,7 +1614,8 @@ fn render_impl(
             write!(w, "</summary>")
         }
 
-        if let Some(ref dox) = i.impl_item.collapsed_doc_value() {
+        let dox = i.impl_item.collapsed_doc_value();
+        if !dox.is_empty() {
             let mut ids = cx.id_map.borrow_mut();
             write!(
                 w,

src/librustdoc/passes/bare_urls.rs

@@ -68,7 +68,7 @@ impl<'a, 'tcx> DocVisitor for BareUrlsLinter<'a, 'tcx> {
                 return;
             }
         };
-        let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
+        let dox = item.attrs.collapsed_doc_value();
         if !dox.is_empty() {
             let report_diag = |cx: &DocContext<'_>, msg: &str, url: &str, range: Range<usize>| {
                 let sp = super::source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs)

src/librustdoc/passes/calculate_doc_coverage.rs

@@ -208,7 +208,7 @@ impl<'a, 'b> DocVisitor for CoverageCalculator<'a, 'b> {
                 let mut tests = Tests { found_tests: 0 };
 
                 find_testable_code(
-                    &i.attrs.collapsed_doc_value().unwrap_or_default(),
+                    &i.attrs.collapsed_doc_value(),
                     &mut tests,
                     ErrorCodes::No,
                     false,

src/librustdoc/passes/check_code_block_syntax.rs

@@ -145,7 +145,8 @@ impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> {
 
 impl<'a, 'tcx> DocVisitor for SyntaxChecker<'a, 'tcx> {
     fn visit_item(&mut self, item: &clean::Item) {
-        if let Some(dox) = &item.attrs.collapsed_doc_value() {
+        let dox = &item.attrs.collapsed_doc_value();
+        if !dox.is_empty() {
             let sp = item.attr_span(self.cx.tcx);
             let extra = crate::html::markdown::ExtraInfo::new_did(
                 self.cx.tcx,

src/librustdoc/passes/check_doc_test_visibility.rs

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

src/librustdoc/passes/html_tags.rs

@@ -176,7 +176,7 @@ impl<'a, 'tcx> DocVisitor for InvalidHtmlTagsLinter<'a, 'tcx> {
                 return;
             }
         };
-        let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
+        let dox = item.attrs.collapsed_doc_value();
         if !dox.is_empty() {
             let report_diag = |msg: &str, range: &Range<usize>| {
                 let sp = match super::source_span_for_markdown_range(tcx, &dox, range, &item.attrs)

src/librustdoc/passes/petrochenkov-suggestions.txt

@@ -0,0 +1,3 @@
+- use rustc's map instead of separate traits in scope
+- have a map from def id + link index to resolved DefId
+- need to have all 3 namespaces in case the link doesn't resolve

src/rustdoc-json-types/lib.rs

@@ -68,9 +68,12 @@ pub struct Item {
     /// By default all documented items are public, but you can tell rustdoc to output private items
     /// so this field is needed to differentiate.
     pub visibility: Visibility,
-    /// The full markdown docstring of this item. Absent if there is no documentation at all,
-    /// Some("") if there is some documentation but it is empty (EG `#[doc = ""]`).
-    pub docs: Option<String>,
+    /// The full markdown docstring of this item.
+    ///
+    /// This field has already been pre-parsed, including combining lines and de-indenting
+    /// whitespace. Each line is delimited by a newline, but the docs will not end with a trailing
+    /// newline (unless added explicitly by the user with `#[doc = "\n"]`).
+    pub docs: String,
     /// This mapping resolves [intra-doc links](https://github.com/rust-lang/rfcs/blob/master/text/1946-intra-rustdoc-links.md) from the docstring to their IDs
     pub links: HashMap<String, Id>,
     /// Stringified versions of the attributes on this item (e.g. `"#[inline]"`)
@@ -511,7 +514,7 @@ pub struct Static {
 }
 
 /// rustdoc format-version.
-pub const FORMAT_VERSION: u32 = 9;
+pub const FORMAT_VERSION: u32 = 10;
 
 #[cfg(test)]
 mod tests;

src/test/rustdoc-json/empty-lines.rs

@@ -0,0 +1,12 @@
+// @has empty_lines.json "$.index[*][?(@.name == 'foo')]"
+// @has - "$.index[*][?(@.name == 'foo')].docs" \"\\n\\n\"
+///
+///
+///
+// ^ note that the above line does *not* include a trailing new line in the docs
+pub fn foo() {}
+
+// @has empty_lines.json "$.index[*][?(@.name == 'bar')].docs" "\"first line\\nsecond line \""
+#[doc = "\n first line"]
+#[doc = "\n second line \n"]
+pub fn bar() {}

src/tools/compiletest/src/runtest.rs

@@ -2401,8 +2401,6 @@ impl<'test> TestCx<'test> {
         }
 
         let root = self.config.find_rust_src_root().unwrap();
-        let mut json_out = out_dir.join(self.testpaths.file.file_stem().unwrap());
-        json_out.set_extension("json");
         let res = self.cmd2procres(
             Command::new(self.config.jsondocck_path.as_ref().unwrap())
                 .arg("--doc-dir")
@@ -2415,7 +2413,9 @@ impl<'test> TestCx<'test> {
             self.fatal_proc_rec("jsondocck failed!", &res)
         }
 
-        let mut json_out = out_dir.join(self.testpaths.file.file_stem().unwrap());
+        let crate_name =
+            self.testpaths.file.file_stem().unwrap().to_str().unwrap().replace('-', "_");
+        let mut json_out = out_dir.join(crate_name);
         json_out.set_extension("json");
         let res = self.cmd2procres(
             Command::new(&self.config.docck_python)

src/tools/jsondocck/src/main.rs

@@ -62,7 +62,10 @@ impl CommandKind {
         };
 
         if !count {
-            print_err(&format!("Incorrect number of arguments to `@{}`", self), lineno);
+            print_err(
+                &format!("Incorrect number of arguments to `@{}` (got {})", self, args.len()),
+                lineno,
+            );
             return false;
         }
 
@@ -317,6 +320,8 @@ fn string_to_value<'a>(s: &str, cache: &'a Cache) -> Cow<'a, Value> {
             panic!("No variable: `{}`. Current state: `{:?}`", &s[1..], cache.variables)
         }))
     } else {
-        Cow::Owned(serde_json::from_str(s).unwrap())
+        Cow::Owned(serde_json::from_str(s).unwrap_or_else(|err| {
+            panic!("failed to parse {:?} as a string: {}", s, err);
+        }))
     }
 }