From 4082053b007f5d318adbac8424d3641cd3ce80f9 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 12 May 2023 20:34:24 +0300 Subject: [PATCH] rustdoc: Cleanup doc string collapsing --- src/librustdoc/clean/types.rs | 65 +++++++------------ src/librustdoc/clean/types/tests.rs | 5 +- src/librustdoc/core.rs | 2 +- src/librustdoc/doctest.rs | 2 +- src/librustdoc/formats/cache.rs | 5 +- src/librustdoc/html/render/context.rs | 7 +- src/librustdoc/html/render/mod.rs | 9 +-- src/librustdoc/html/render/print_item.rs | 6 +- src/librustdoc/html/render/search_index.rs | 10 +-- src/librustdoc/json/conversions.rs | 2 +- .../passes/calculate_doc_coverage.rs | 8 +-- .../passes/check_doc_test_visibility.rs | 4 +- src/librustdoc/passes/lint/bare_urls.rs | 2 +- .../passes/lint/check_code_block_syntax.rs | 2 +- src/librustdoc/passes/lint/html_tags.rs | 2 +- .../passes/lint/unescaped_backticks.rs | 2 +- src/librustdoc/passes/stripper.rs | 2 +- 17 files changed, 51 insertions(+), 84 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 38664c3e359a6..e9ccea2cf270f 100644 --- a/src/librustdoc/clean/types.rs +++ b/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 { + /// 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 { + self.attrs.opt_doc_value() + } + pub(crate) fn from_def_id_and_parts( def_id: DefId, name: Option, @@ -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 { - self.attrs.collapsed_doc_value() - } - pub(crate) fn links(&self, cx: &Context<'_>) -> Vec { use crate::html::format::{href, link_tooltip}; @@ -1068,17 +1068,6 @@ impl> 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 { - 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 { - 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 { + (!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]> { diff --git a/src/librustdoc/clean/types/tests.rs b/src/librustdoc/clean/types/tests.rs index d8c91a9680479..394954208a48e 100644 --- a/src/librustdoc/clean/types/tests.rs +++ b/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); }); } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index a6be132337eb6..e10a629777526 100644 --- a/src/librustdoc/core.rs +++ b/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", diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 575d8ee65b7ba..f6631b66f5b65 100644 --- a/src/librustdoc/doctest.rs +++ b/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() diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index c0730e90740eb..c4758fd986655 100644 --- a/src/librustdoc/formats/cache.rs +++ b/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() diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 01a92f6df6a0c..56af257fd5eb9 100644 --- a/src/librustdoc/html/render/context.rs +++ b/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) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 0a2f5f6653cfd..42e27d35a94b1 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/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, "") } - 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( "
\ diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 4cc81e860f09a..76c8e0885a0a2 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/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 diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index a3be6dd526909..846299f02e33c 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/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. diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs index b1cef20b434a6..935bb721f1803 100644 --- a/src/librustdoc/json/conversions.rs +++ b/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); diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index be5286b24d797..6ead0cd961a31 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/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(); diff --git a/src/librustdoc/passes/check_doc_test_visibility.rs b/src/librustdoc/passes/check_doc_test_visibility.rs index 10295cbd189b1..b6cd897d31754 100644 --- a/src/librustdoc/passes/check_doc_test_visibility.rs +++ b/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) } diff --git a/src/librustdoc/passes/lint/bare_urls.rs b/src/librustdoc/passes/lint/bare_urls.rs index 423230cfe381e..a10d5fdb410a9 100644 --- a/src/librustdoc/passes/lint/bare_urls.rs +++ b/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| { let sp = source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs) diff --git a/src/librustdoc/passes/lint/check_code_block_syntax.rs b/src/librustdoc/passes/lint/check_code_block_syntax.rs index 8f873dbe50131..f489f5081daa2 100644 --- a/src/librustdoc/passes/lint/check_code_block_syntax.rs +++ b/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) { diff --git a/src/librustdoc/passes/lint/html_tags.rs b/src/librustdoc/passes/lint/html_tags.rs index 4f72df5a5cda0..f0403647af0da 100644 --- a/src/librustdoc/passes/lint/html_tags.rs +++ b/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, is_open_tag: bool| { let sp = match source_span_for_markdown_range(tcx, &dox, range, &item.attrs) { diff --git a/src/librustdoc/passes/lint/unescaped_backticks.rs b/src/librustdoc/passes/lint/unescaped_backticks.rs index 33cef82a60cbb..683c224c4be5c 100644 --- a/src/librustdoc/passes/lint/unescaped_backticks.rs +++ b/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; } diff --git a/src/librustdoc/passes/stripper.rs b/src/librustdoc/passes/stripper.rs index cba55e5fe655c..73fc26a6b043a 100644 --- a/src/librustdoc/passes/stripper.rs +++ b/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; } }