From c4739bc920a192b852ceb48fb500831594d5ff96 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 19 Dec 2020 14:21:00 +0100 Subject: [PATCH 1/6] Rework DocFragment --- src/librustdoc/clean/types.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index d0d37046e59..5c5ad5d7ccf 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -481,7 +481,7 @@ fn get_word_attr(mut self, word: Symbol) -> (Option, bool) RawDoc, /// A doc fragment created from a `#[doc(include="filename")]` attribute. Contains both the /// given filename and the file contents. - Include { filename: String }, + Include { filename: Symbol }, } impl<'a> FromIterator<&'a DocFragment> for String { @@ -565,7 +565,7 @@ impl Attributes { /// Reads a `MetaItem` from within an attribute, looks for whether it is a /// `#[doc(include="file")]`, and returns the filename and contents of the file as loaded from /// its expansion. - crate fn extract_include(mi: &ast::MetaItem) -> Option<(String, String)> { + crate fn extract_include(mi: &ast::MetaItem) -> Option<(Symbol, String)> { mi.meta_item_list().and_then(|list| { for meta in list { if meta.has_name(sym::include) { @@ -573,13 +573,13 @@ impl Attributes { // `#[doc(include(file="filename", contents="file contents")]` so we need to // look for that instead return meta.meta_item_list().and_then(|list| { - let mut filename: Option = None; + let mut filename: Option = None; let mut contents: Option = None; for it in list { if it.has_name(sym::file) { if let Some(name) = it.value_str() { - filename = Some(name.to_string()); + filename = Some(name); } } else if it.has_name(sym::contents) { if let Some(docs) = it.value_str() { From 122b141f58fd12deea53fb73efcaaaa56fb87bd1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 19 Dec 2020 15:04:42 +0100 Subject: [PATCH 2/6] End of rework of Attributes struct --- src/librustdoc/clean/types.rs | 116 +++++++++++++++--- src/librustdoc/core.rs | 2 +- src/librustdoc/doctest.rs | 1 - src/librustdoc/formats/cache.rs | 7 +- src/librustdoc/html/render/cache.rs | 4 +- src/librustdoc/html/render/mod.rs | 15 ++- .../passes/calculate_doc_coverage.rs | 2 +- src/librustdoc/passes/collapse_docs.rs | 62 +--------- .../passes/collect_intra_doc_links.rs | 27 +--- src/librustdoc/passes/unindent_comments.rs | 18 +-- .../passes/unindent_comments/tests.rs | 27 +++- 11 files changed, 145 insertions(+), 136 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 5c5ad5d7ccf..48d720fa77f 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -122,7 +122,7 @@ impl Item { /// Finds the `doc` attribute as a NameValue and returns the corresponding /// value found. - crate fn doc_value(&self) -> Option<&str> { + crate fn doc_value(&self) -> Option { self.attrs.doc_value() } @@ -469,11 +469,13 @@ fn get_word_attr(mut self, word: Symbol) -> (Option, bool) /// This allows distinguishing between the original documentation and a pub re-export. /// If it is `None`, the item was not re-exported. crate parent_module: Option, - crate doc: String, + crate doc: Symbol, crate kind: DocFragmentKind, + crate need_backline: bool, + crate indent: usize, } -#[derive(Clone, PartialEq, Eq, Debug, Hash)] +#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)] crate enum DocFragmentKind { /// A doc fragment created from a `///` or `//!` doc comment. SugaredDoc, @@ -484,16 +486,42 @@ fn get_word_attr(mut self, word: Symbol) -> (Option, bool) Include { filename: Symbol }, } +fn add_doc_fragment(out: &mut String, frag: &DocFragment) { + let s = frag.doc.as_str(); + let mut iter = s.lines().peekable(); + while let Some(line) = iter.next() { + if line.chars().any(|c| !c.is_whitespace()) { + assert!(line.len() >= frag.indent); + out.push_str(&line[frag.indent..]); + } else { + out.push_str(line); + } + if iter.peek().is_some() { + out.push('\n'); + } + } + if frag.need_backline { + out.push('\n'); + } +} + impl<'a> FromIterator<&'a DocFragment> for String { fn from_iter(iter: T) -> Self where T: IntoIterator, { + let mut prev_kind: Option = None; iter.into_iter().fold(String::new(), |mut acc, frag| { - if !acc.is_empty() { + if !acc.is_empty() + && prev_kind + .take() + .map(|p| matches!(p, DocFragmentKind::Include { .. }) && p != frag.kind) + .unwrap_or(false) + { acc.push('\n'); } - acc.push_str(&frag.doc); + add_doc_fragment(&mut acc, &frag); + prev_kind = Some(frag.kind); acc }) } @@ -565,7 +593,7 @@ impl Attributes { /// Reads a `MetaItem` from within an attribute, looks for whether it is a /// `#[doc(include="file")]`, and returns the filename and contents of the file as loaded from /// its expansion. - crate fn extract_include(mi: &ast::MetaItem) -> Option<(Symbol, String)> { + crate fn extract_include(mi: &ast::MetaItem) -> Option<(Symbol, Symbol)> { mi.meta_item_list().and_then(|list| { for meta in list { if meta.has_name(sym::include) { @@ -574,7 +602,7 @@ impl Attributes { // look for that instead return meta.meta_item_list().and_then(|list| { let mut filename: Option = None; - let mut contents: Option = None; + let mut contents: Option = None; for it in list { if it.has_name(sym::file) { @@ -583,7 +611,7 @@ impl Attributes { } } else if it.has_name(sym::contents) { if let Some(docs) = it.value_str() { - contents = Some(docs.to_string()); + contents = Some(docs); } } } @@ -622,15 +650,30 @@ impl Attributes { attrs: &[ast::Attribute], additional_attrs: Option<(&[ast::Attribute], DefId)>, ) -> Attributes { - let mut doc_strings = vec![]; + let mut doc_strings: Vec = vec![]; let mut sp = None; let mut cfg = Cfg::True; let mut doc_line = 0; + fn update_need_backline(doc_strings: &mut Vec, frag: &DocFragment) { + if let Some(prev) = doc_strings.last_mut() { + if matches!(prev.kind, DocFragmentKind::Include { .. }) + || prev.kind != frag.kind + || prev.parent_module != frag.parent_module + { + // add a newline for extra padding between segments + prev.need_backline = prev.kind == DocFragmentKind::SugaredDoc + || prev.kind == DocFragmentKind::RawDoc + } else { + prev.need_backline = true; + } + } + } + let clean_attr = |(attr, parent_module): (&ast::Attribute, _)| { if let Some(value) = attr.doc_str() { trace!("got doc_str={:?}", value); - let value = beautify_doc_string(value).to_string(); + let value = beautify_doc_string(value); let kind = if attr.is_doc_comment() { DocFragmentKind::SugaredDoc } else { @@ -638,14 +681,20 @@ impl Attributes { }; let line = doc_line; - doc_line += value.lines().count(); - doc_strings.push(DocFragment { + doc_line += value.as_str().lines().count(); + let frag = DocFragment { line, span: attr.span, doc: value, kind, parent_module, - }); + need_backline: false, + indent: 0, + }; + + update_need_backline(&mut doc_strings, &frag); + + doc_strings.push(frag); if sp.is_none() { sp = Some(attr.span); @@ -663,14 +712,18 @@ impl Attributes { } else if let Some((filename, contents)) = Attributes::extract_include(&mi) { let line = doc_line; - doc_line += contents.lines().count(); - doc_strings.push(DocFragment { + doc_line += contents.as_str().lines().count(); + let frag = DocFragment { line, span: attr.span, doc: contents, kind: DocFragmentKind::Include { filename }, parent_module, - }); + need_backline: false, + indent: 0, + }; + update_need_backline(&mut doc_strings, &frag); + doc_strings.push(frag); } } } @@ -721,14 +774,39 @@ impl Attributes { /// Finds the `doc` attribute as a NameValue and returns the corresponding /// value found. - crate fn doc_value(&self) -> Option<&str> { - self.doc_strings.first().map(|s| s.doc.as_str()) + 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); + while let Some(new_frag) = iter.next() { + if matches!(ori.kind, DocFragmentKind::Include { .. }) + || new_frag.kind != ori.kind + || new_frag.parent_module != ori.parent_module + { + break; + } + add_doc_fragment(&mut out, &new_frag); + } + if out.is_empty() { None } else { Some(out) } + } + + crate fn collapsed_doc_value_by_module_level(&self) -> FxHashMap, String> { + let mut ret = FxHashMap::default(); + + for new_frag in self.doc_strings.iter() { + let out = ret.entry(new_frag.parent_module).or_insert_with(|| String::new()); + add_doc_fragment(out, &new_frag); + } + ret } /// Finds all `doc` attributes as NameValues and returns their corresponding values, joined /// with newlines. crate fn collapsed_doc_value(&self) -> Option { - if !self.doc_strings.is_empty() { Some(self.doc_strings.iter().collect()) } else { None } + let s: String = self.doc_strings.iter().collect(); + if s.is_empty() { None } else { Some(s) } } /// Gets links as a vector diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 7e85342ac7d..053eaac0169 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -525,7 +525,7 @@ pub(crate) fn init_lints( let mut krate = tcx.sess.time("clean_crate", || clean::krate(&mut ctxt)); if let Some(ref m) = krate.module { - if let None | Some("") = m.doc_value() { + if m.doc_value().map(|d| d.is_empty()).unwrap_or(true) { let help = "The following guide may be of use:\n\ https://doc.rust-lang.org/nightly/rustdoc/how-to-write-documentation.html"; tcx.struct_lint_node( diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 0edb9babef4..09627be9701 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -987,7 +987,6 @@ fn visit_testable( self.collector.names.push(name); } - attrs.collapse_doc_comments(); 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. diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 899d61d8e43..df892d15b7b 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -314,9 +314,10 @@ fn fold_item(&mut self, item: clean::Item) -> Option { ty: item.type_(), name: s.to_string(), path: path.join("::"), - desc: item - .doc_value() - .map_or_else(|| String::new(), short_markdown_summary), + desc: item.doc_value().map_or_else( + || String::new(), + |x| short_markdown_summary(&x.as_str()), + ), parent, parent_idx: None, search_type: get_index_search_type(&item), diff --git a/src/librustdoc/html/render/cache.rs b/src/librustdoc/html/render/cache.rs index c408e576639..497cbbb4250 100644 --- a/src/librustdoc/html/render/cache.rs +++ b/src/librustdoc/html/render/cache.rs @@ -78,7 +78,7 @@ ty: item.type_(), name: item.name.unwrap().to_string(), path: fqp[..fqp.len() - 1].join("::"), - desc: item.doc_value().map_or_else(|| String::new(), short_markdown_summary), + desc: item.doc_value().map_or_else(String::new, |s| short_markdown_summary(&s)), parent: Some(did), parent_idx: None, search_type: get_index_search_type(&item), @@ -127,7 +127,7 @@ let crate_doc = krate .module .as_ref() - .map(|module| module.doc_value().map_or_else(|| String::new(), short_markdown_summary)) + .map(|module| module.doc_value().map_or_else(String::new, |s| short_markdown_summary(&s))) .unwrap_or_default(); #[derive(Serialize)] diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 2f5ee4f238d..4339fa077e5 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -30,7 +30,6 @@ #[cfg(test)] mod tests; -use std::borrow::Cow; use std::cell::{Cell, RefCell}; use std::cmp::Ordering; use std::collections::{BTreeMap, VecDeque}; @@ -198,11 +197,11 @@ impl SharedContext<'_> { /// Based on whether the `collapse-docs` pass was run, return either the `doc_value` or the /// `collapsed_doc_value` of the given item. - crate fn maybe_collapsed_doc_value<'a>(&self, item: &'a clean::Item) -> Option> { + crate fn maybe_collapsed_doc_value<'a>(&self, item: &'a clean::Item) -> Option { if self.collapsed { - item.collapsed_doc_value().map(|s| s.into()) + item.collapsed_doc_value().map(|s| s.to_string()) } else { - item.doc_value().map(|s| s.into()) + item.doc_value().map(|s| s.to_string()) } } } @@ -1622,7 +1621,7 @@ fn build_sidebar_items(&self, m: &clean::Module) -> BTreeMapRead more"#, naive_assoc_href(item, link)); @@ -2197,7 +2196,7 @@ fn cmp( let stab = myitem.stability_class(cx.tcx()); let add = if stab.is_some() { " " } else { "" }; - let doc_value = myitem.doc_value().unwrap_or(""); + let doc_value = myitem.doc_value().unwrap_or_else(String::new); write!( w, "\ @@ -2207,7 +2206,7 @@ fn cmp( ", name = *myitem.name.as_ref().unwrap(), stab_tags = extra_info_tags(myitem, item, cx.tcx()), - docs = MarkdownSummaryLine(doc_value, &myitem.links()).into_string(), + docs = MarkdownSummaryLine(&doc_value, &myitem.links()).into_string(), class = myitem.type_(), add = add, stab = stab.unwrap_or_else(String::new), diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 0a00323a317..c8c72545669 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -238,7 +238,7 @@ fn fold_item(&mut self, i: clean::Item) -> Option { &i.attrs .doc_strings .iter() - .map(|d| d.doc.as_str()) + .map(|d| d.doc.to_string()) .collect::>() .join("\n"), &mut tests, diff --git a/src/librustdoc/passes/collapse_docs.rs b/src/librustdoc/passes/collapse_docs.rs index e1ba75baa0f..4994765dfe5 100644 --- a/src/librustdoc/passes/collapse_docs.rs +++ b/src/librustdoc/passes/collapse_docs.rs @@ -1,72 +1,14 @@ -use crate::clean::{self, DocFragment, DocFragmentKind, Item}; +use crate::clean; use crate::core::DocContext; -use crate::fold; -use crate::fold::DocFolder; use crate::passes::Pass; -use std::mem::take; - crate const COLLAPSE_DOCS: Pass = Pass { name: "collapse-docs", run: collapse_docs, description: "concatenates all document attributes into one document attribute", }; -crate fn collapse_docs(krate: clean::Crate, _: &DocContext<'_>) -> clean::Crate { - let mut krate = Collapser.fold_crate(krate); +crate fn collapse_docs(mut krate: clean::Crate, _: &DocContext<'_>) -> clean::Crate { krate.collapsed = true; krate } - -struct Collapser; - -impl fold::DocFolder for Collapser { - fn fold_item(&mut self, mut i: Item) -> Option { - i.attrs.collapse_doc_comments(); - Some(self.fold_item_recur(i)) - } -} - -fn collapse(doc_strings: &mut Vec) { - let mut docs = vec![]; - let mut last_frag: Option = None; - - for frag in take(doc_strings) { - if let Some(mut curr_frag) = last_frag.take() { - let curr_kind = &curr_frag.kind; - let new_kind = &frag.kind; - - if matches!(*curr_kind, DocFragmentKind::Include { .. }) - || curr_kind != new_kind - || curr_frag.parent_module != frag.parent_module - { - if *curr_kind == DocFragmentKind::SugaredDoc - || *curr_kind == DocFragmentKind::RawDoc - { - // add a newline for extra padding between segments - curr_frag.doc.push('\n'); - } - docs.push(curr_frag); - last_frag = Some(frag); - } else { - curr_frag.doc.push('\n'); - curr_frag.doc.push_str(&frag.doc); - curr_frag.span = curr_frag.span.to(frag.span); - last_frag = Some(curr_frag); - } - } else { - last_frag = Some(frag); - } - } - - if let Some(frag) = last_frag.take() { - docs.push(frag); - } - *doc_strings = docs; -} - -impl clean::Attributes { - crate fn collapse_doc_comments(&mut self) { - collapse(&mut self.doc_strings); - } -} diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 1719b35a362..d539c14b12a 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -891,37 +891,20 @@ fn fold_item(&mut self, mut item: Item) -> Option { // In the presence of re-exports, this is not the same as the module of the item. // Rather than merging all documentation into one, resolve it one attribute at a time // so we know which module it came from. - let mut attrs = item.attrs.doc_strings.iter().peekable(); - while let Some(attr) = attrs.next() { - // `collapse_docs` does not have the behavior we want: - // we want `///` and `#[doc]` to count as the same attribute, - // but currently it will treat them as separate. - // As a workaround, combine all attributes with the same parent module into the same attribute. - let mut combined_docs = attr.doc.clone(); - loop { - match attrs.peek() { - Some(next) if next.parent_module == attr.parent_module => { - combined_docs.push('\n'); - combined_docs.push_str(&attrs.next().unwrap().doc); - } - _ => break, - } - } - debug!("combined_docs={}", combined_docs); + for (parent_module, doc) in item.attrs.collapsed_doc_value_by_module_level() { + debug!("combined_docs={}", doc); - let (krate, parent_node) = if let Some(id) = attr.parent_module { - trace!("docs {:?} came from {:?}", attr.doc, id); + let (krate, parent_node) = if let Some(id) = parent_module { (id.krate, Some(id)) } else { - trace!("no parent found for {:?}", attr.doc); (item.def_id.krate, parent_node) }; // NOTE: if there are links that start in one crate and end in another, this will not resolve them. // This is a degenerate case and it's not supported by rustdoc. - for (ori_link, link_range) in markdown_links(&combined_docs) { + for (ori_link, link_range) in markdown_links(&doc) { let link = self.resolve_link( &item, - &combined_docs, + &doc, &self_name, parent_node, krate, diff --git a/src/librustdoc/passes/unindent_comments.rs b/src/librustdoc/passes/unindent_comments.rs index d0345d1e48c..1cad480d4e8 100644 --- a/src/librustdoc/passes/unindent_comments.rs +++ b/src/librustdoc/passes/unindent_comments.rs @@ -68,7 +68,7 @@ fn unindent_fragments(docs: &mut Vec) { let min_indent = match docs .iter() .map(|fragment| { - fragment.doc.lines().fold(usize::MAX, |min_indent, line| { + fragment.doc.as_str().lines().fold(usize::MAX, |min_indent, line| { if line.chars().all(|c| c.is_whitespace()) { min_indent } else { @@ -87,7 +87,7 @@ fn unindent_fragments(docs: &mut Vec) { }; for fragment in docs { - if fragment.doc.lines().count() == 0 { + if fragment.doc.as_str().lines().count() == 0 { continue; } @@ -97,18 +97,6 @@ fn unindent_fragments(docs: &mut Vec) { min_indent }; - fragment.doc = fragment - .doc - .lines() - .map(|line| { - if line.chars().all(|c| c.is_whitespace()) { - line.to_string() - } else { - assert!(line.len() >= min_indent); - line[min_indent..].to_string() - } - }) - .collect::>() - .join("\n"); + fragment.indent = min_indent; } } diff --git a/src/librustdoc/passes/unindent_comments/tests.rs b/src/librustdoc/passes/unindent_comments/tests.rs index 9dec71f7683..ca8f6921fdc 100644 --- a/src/librustdoc/passes/unindent_comments/tests.rs +++ b/src/librustdoc/passes/unindent_comments/tests.rs @@ -1,21 +1,40 @@ use super::*; use rustc_span::source_map::DUMMY_SP; +use rustc_span::symbol::Symbol; +use rustc_span::with_default_session_globals; fn create_doc_fragment(s: &str) -> Vec { vec![DocFragment { line: 0, span: DUMMY_SP, parent_module: None, - doc: s.to_string(), + doc: Symbol::intern(s), kind: DocFragmentKind::SugaredDoc, + need_backline: false, + indent: 0, }] } #[track_caller] fn run_test(input: &str, expected: &str) { - let mut s = create_doc_fragment(input); - unindent_fragments(&mut s); - assert_eq!(s[0].doc, expected); + with_default_session_globals(|| { + let mut s = create_doc_fragment(input); + unindent_fragments(&mut s); + assert_eq!( + &s[0] + .doc + .as_str() + .lines() + .map(|l| if l.len() > s[0].indent { + l[s[0].indent..].to_string() + } else { + String::new() + }) + .collect::>() + .join("\n"), + expected + ); + }); } #[test] From 0ab6c90699e5fc70ee0a9bfe369bc356f05127f9 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 21 Dec 2020 21:03:19 +0100 Subject: [PATCH 3/6] Update rustdoc-ui test output --- src/test/rustdoc-ui/invalid-syntax.stderr | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/rustdoc-ui/invalid-syntax.stderr b/src/test/rustdoc-ui/invalid-syntax.stderr index 9a7a4d21013..75acdc5ab5f 100644 --- a/src/test/rustdoc-ui/invalid-syntax.stderr +++ b/src/test/rustdoc-ui/invalid-syntax.stderr @@ -127,8 +127,10 @@ LL | /// ```text warning: could not parse code block as Rust code --> $DIR/invalid-syntax.rs:92:9 | -LL | /// \____/ - | ^^^^^^ +LL | /// \____/ + | _________^ +LL | | /// + | |_ | = note: error from rustc: unknown start of token: \ From 4ba1928fa2aa73abeb541dad7d72bda94509d113 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 26 Dec 2020 14:03:33 +0100 Subject: [PATCH 4/6] Improve code for DocFragment rework --- src/librustdoc/clean/types.rs | 15 ++++++++++++--- src/librustdoc/formats/cache.rs | 7 +++---- src/librustdoc/html/render/mod.rs | 8 ++------ src/librustdoc/passes/calculate_doc_coverage.rs | 7 +------ 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 48d720fa77f..8d5ef1ed28c 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -486,6 +486,13 @@ fn get_word_attr(mut self, word: Symbol) -> (Option, bool) Include { filename: Symbol }, } +// The goal of this function is to apply the `DocFragment` transformations that are required when +// transforming into the final markdown. So the transformations in here are: +// +// * Applying the computed indent to each lines in each doc fragment (a `DocFragment` can contain +// multiple lines in case of `#[doc = ""]`). +// * Adding backlines between `DocFragment`s and adding an extra one if required (stored in the +// `need_backline` field). fn add_doc_fragment(out: &mut String, frag: &DocFragment) { let s = frag.doc.as_str(); let mut iter = s.lines().peekable(); @@ -792,11 +799,14 @@ fn update_need_backline(doc_strings: &mut Vec, frag: &DocFragment) if out.is_empty() { None } else { Some(out) } } + /// Return the doc-comments on this item, grouped by the module they came from. + /// + /// The module can be different if this is a re-export with added documentation. crate fn collapsed_doc_value_by_module_level(&self) -> FxHashMap, String> { let mut ret = FxHashMap::default(); for new_frag in self.doc_strings.iter() { - let out = ret.entry(new_frag.parent_module).or_insert_with(|| String::new()); + let out = ret.entry(new_frag.parent_module).or_default(); add_doc_fragment(out, &new_frag); } ret @@ -805,8 +815,7 @@ fn update_need_backline(doc_strings: &mut Vec, frag: &DocFragment) /// Finds all `doc` attributes as NameValues and returns their corresponding values, joined /// with newlines. crate fn collapsed_doc_value(&self) -> Option { - let s: String = self.doc_strings.iter().collect(); - if s.is_empty() { None } else { Some(s) } + if self.doc_strings.is_empty() { None } else { Some(self.doc_strings.iter().collect()) } } /// Gets links as a vector diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index df892d15b7b..c1f8b12cac4 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -314,10 +314,9 @@ fn fold_item(&mut self, item: clean::Item) -> Option { ty: item.type_(), name: s.to_string(), path: path.join("::"), - desc: item.doc_value().map_or_else( - || String::new(), - |x| short_markdown_summary(&x.as_str()), - ), + desc: item + .doc_value() + .map_or_else(String::new, |x| short_markdown_summary(&x.as_str())), parent, parent_idx: None, search_type: get_index_search_type(&item), diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 4339fa077e5..1c713df9276 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -198,11 +198,7 @@ impl SharedContext<'_> { /// Based on whether the `collapse-docs` pass was run, return either the `doc_value` or the /// `collapsed_doc_value` of the given item. crate fn maybe_collapsed_doc_value<'a>(&self, item: &'a clean::Item) -> Option { - if self.collapsed { - item.collapsed_doc_value().map(|s| s.to_string()) - } else { - item.doc_value().map(|s| s.to_string()) - } + if self.collapsed { item.collapsed_doc_value() } else { item.doc_value() } } } @@ -2196,7 +2192,7 @@ fn cmp( let stab = myitem.stability_class(cx.tcx()); let add = if stab.is_some() { " " } else { "" }; - let doc_value = myitem.doc_value().unwrap_or_else(String::new); + let doc_value = myitem.doc_value().unwrap_or_default(); write!( w, "\ diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index c8c72545669..05a3a15adac 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -235,12 +235,7 @@ fn fold_item(&mut self, i: clean::Item) -> Option { let mut tests = Tests { found_tests: 0 }; find_testable_code( - &i.attrs - .doc_strings - .iter() - .map(|d| d.doc.to_string()) - .collect::>() - .join("\n"), + &i.attrs.collapsed_doc_value().unwrap_or_default(), &mut tests, ErrorCodes::No, false, From d0e7523a3246a36d6647f60e5bd75d9e3c71280e Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 26 Dec 2020 14:22:08 +0100 Subject: [PATCH 5/6] Remove unused collapse pass --- src/librustdoc/core.rs | 3 +++ src/librustdoc/passes/collapse_docs.rs | 14 -------------- src/librustdoc/passes/mod.rs | 5 ----- 3 files changed, 3 insertions(+), 19 deletions(-) delete mode 100644 src/librustdoc/passes/collapse_docs.rs diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 053eaac0169..4aeca0faea7 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -623,6 +623,9 @@ fn report_deprecated_attr(name: &str, diag: &rustc_errors::Handler) { ctxt.sess().abort_if_errors(); + // The main crate doc comments are always collapsed. + krate.collapsed = true; + (krate, ctxt.renderinfo.into_inner(), ctxt.render_options) } diff --git a/src/librustdoc/passes/collapse_docs.rs b/src/librustdoc/passes/collapse_docs.rs deleted file mode 100644 index 4994765dfe5..00000000000 --- a/src/librustdoc/passes/collapse_docs.rs +++ /dev/null @@ -1,14 +0,0 @@ -use crate::clean; -use crate::core::DocContext; -use crate::passes::Pass; - -crate const COLLAPSE_DOCS: Pass = Pass { - name: "collapse-docs", - run: collapse_docs, - description: "concatenates all document attributes into one document attribute", -}; - -crate fn collapse_docs(mut krate: clean::Crate, _: &DocContext<'_>) -> clean::Crate { - krate.collapsed = true; - krate -} diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 51818d7faf0..7ac42c75992 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -14,9 +14,6 @@ mod non_autolinks; crate use self::non_autolinks::CHECK_NON_AUTOLINKS; -mod collapse_docs; -crate use self::collapse_docs::COLLAPSE_DOCS; - mod strip_hidden; crate use self::strip_hidden::STRIP_HIDDEN; @@ -84,7 +81,6 @@ CHECK_PRIVATE_ITEMS_DOC_TESTS, STRIP_HIDDEN, UNINDENT_COMMENTS, - COLLAPSE_DOCS, STRIP_PRIVATE, STRIP_PRIV_IMPORTS, PROPAGATE_DOC_CFG, @@ -99,7 +95,6 @@ /// The list of passes run by default. crate const DEFAULT_PASSES: &[ConditionalPass] = &[ ConditionalPass::always(COLLECT_TRAIT_IMPLS), - ConditionalPass::always(COLLAPSE_DOCS), ConditionalPass::always(UNINDENT_COMMENTS), ConditionalPass::always(CHECK_PRIVATE_ITEMS_DOC_TESTS), ConditionalPass::new(STRIP_HIDDEN, WhenNotDocumentHidden), From df2df14424b6fa3b2b2a8a8cdd6cb2307ddbb474 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 27 Dec 2020 23:57:45 +0100 Subject: [PATCH 6/6] Simplify docfragment transformation in unindent tests --- src/librustdoc/passes/unindent_comments/tests.rs | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/librustdoc/passes/unindent_comments/tests.rs b/src/librustdoc/passes/unindent_comments/tests.rs index ca8f6921fdc..9c9924841b9 100644 --- a/src/librustdoc/passes/unindent_comments/tests.rs +++ b/src/librustdoc/passes/unindent_comments/tests.rs @@ -20,20 +20,7 @@ fn run_test(input: &str, expected: &str) { with_default_session_globals(|| { let mut s = create_doc_fragment(input); unindent_fragments(&mut s); - assert_eq!( - &s[0] - .doc - .as_str() - .lines() - .map(|l| if l.len() > s[0].indent { - l[s[0].indent..].to_string() - } else { - String::new() - }) - .collect::>() - .join("\n"), - expected - ); + assert_eq!(&s.iter().collect::(), expected); }); }