Skip to content

Commit edb73eb

Browse files
committed
Return a String from collapsed_doc_value, not an Option
This has almost no effect in practice, since most places were using `unwrap_or_default`, and the rest were doing checks that *should* have been checking for an empty string. The only change in behavior is that the JSON backend no longer distinguishes "undocumented" and "documented with the empty string". This doesn't seem like a particularly useful distinction, but I can add it back for that code only if you think it's important.
1 parent 84f962a commit edb73eb

File tree

10 files changed

+34
-37
lines changed

10 files changed

+34
-37
lines changed

src/librustdoc/clean/types.rs

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ impl Item {
468468

469469
/// Finds all `doc` attributes as NameValues and returns their corresponding values, joined
470470
/// with newlines.
471-
crate fn collapsed_doc_value(&self) -> Option<String> {
471+
crate fn collapsed_doc_value(&self) -> String {
472472
self.attrs.collapsed_doc_value()
473473
}
474474

@@ -956,16 +956,6 @@ fn add_doc_fragment(out: &mut String, frag: &DocFragment) {
956956
}
957957
}
958958

959-
/// Collapse a collection of [`DocFragment`]s into one string,
960-
/// handling indentation and newlines as needed.
961-
crate fn collapse_doc_fragments(doc_strings: &[DocFragment]) -> String {
962-
let mut acc = String::new();
963-
for frag in doc_strings {
964-
add_doc_fragment(&mut acc, frag);
965-
}
966-
acc
967-
}
968-
969959
/// A link that has not yet been rendered.
970960
///
971961
/// This link will be turned into a rendered link by [`Item::links`].
@@ -1104,12 +1094,15 @@ impl Attributes {
11041094

11051095
/// Finds all `doc` attributes as NameValues and returns their corresponding values, joined
11061096
/// with newlines.
1107-
crate fn collapsed_doc_value(&self) -> Option<String> {
1108-
if self.doc_strings.is_empty() {
1109-
None
1110-
} else {
1111-
Some(collapse_doc_fragments(&self.doc_strings))
1112-
}
1097+
///
1098+
/// Unlike `doc_value`, this does not stop processing lines when switching between `#[doc]` and
1099+
/// sugared syntax.
1100+
crate fn collapsed_doc_value(&self) -> String {
1101+
let mut acc = String::new();
1102+
for frag in &self.doc_strings {
1103+
add_doc_fragment(&mut acc, frag);
1104+
}
1105+
acc
11131106
}
11141107

11151108
crate fn get_doc_aliases(&self) -> Box<[Symbol]> {

src/librustdoc/doctest.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,17 +1123,17 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
11231123
}
11241124

11251125
attrs.unindent_doc_comments();
1126-
// The collapse-docs pass won't combine sugared/raw doc attributes, or included files with
1127-
// anything else, this will combine them for us.
1128-
if let Some(doc) = attrs.collapsed_doc_value() {
1126+
// `doc_value()` doesn't combine sugared/raw doc attributes, this will combine them for us.
1127+
let docs = attrs.collapsed_doc_value();
1128+
if !docs.is_empty() {
11291129
// Use the outermost invocation, so that doctest names come from where the docs were written.
11301130
let span = ast_attrs
11311131
.span()
11321132
.map(|span| span.ctxt().outer_expn().expansion_cause().unwrap_or(span))
11331133
.unwrap_or(DUMMY_SP);
11341134
self.collector.set_position(span);
11351135
markdown::find_testable_code(
1136-
&doc,
1136+
&docs,
11371137
self.collector,
11381138
self.codes,
11391139
self.collector.enable_per_target_ignores,

src/librustdoc/html/render/mod.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -567,19 +567,21 @@ fn document_full_inner(
567567
is_collapsible: bool,
568568
heading_offset: HeadingOffset,
569569
) {
570-
if let Some(s) = item.collapsed_doc_value() {
571-
debug!("Doc block: =====\n{}\n=====", s);
570+
// NOTE: "collapsed" here is unrelated to `is_collapsible`, it just combines `#[doc]` and sugared attributes
571+
let docs = item.collapsed_doc_value();
572+
if !docs.is_empty() {
573+
debug!("Doc block: =====\n{}\n=====", docs);
572574
if is_collapsible {
573575
w.write_str(
574576
"<details class=\"rustdoc-toggle top-doc\" open>\
575577
<summary class=\"hideme\">\
576578
<span>Expand description</span>\
577579
</summary>",
578580
);
579-
render_markdown(w, cx, &s, item.links(cx), heading_offset);
581+
render_markdown(w, cx, &docs, item.links(cx), heading_offset);
580582
w.write_str("</details>");
581583
} else {
582-
render_markdown(w, cx, &s, item.links(cx), heading_offset);
584+
render_markdown(w, cx, &docs, item.links(cx), heading_offset);
583585
}
584586
}
585587

@@ -1612,7 +1614,8 @@ fn render_impl(
16121614
write!(w, "</summary>")
16131615
}
16141616

1615-
if let Some(ref dox) = i.impl_item.collapsed_doc_value() {
1617+
let dox = i.impl_item.collapsed_doc_value();
1618+
if !dox.is_empty() {
16161619
let mut ids = cx.id_map.borrow_mut();
16171620
write!(
16181621
w,

src/librustdoc/passes/bare_urls.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ impl<'a, 'tcx> DocVisitor for BareUrlsLinter<'a, 'tcx> {
6868
return;
6969
}
7070
};
71-
let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
71+
let dox = item.attrs.collapsed_doc_value();
7272
if !dox.is_empty() {
7373
let report_diag = |cx: &DocContext<'_>, msg: &str, url: &str, range: Range<usize>| {
7474
let sp = super::source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs)

src/librustdoc/passes/calculate_doc_coverage.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ impl<'a, 'b> DocVisitor for CoverageCalculator<'a, 'b> {
208208
let mut tests = Tests { found_tests: 0 };
209209

210210
find_testable_code(
211-
&i.attrs.collapsed_doc_value().unwrap_or_default(),
211+
&i.attrs.collapsed_doc_value(),
212212
&mut tests,
213213
ErrorCodes::No,
214214
false,

src/librustdoc/passes/check_code_block_syntax.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,8 @@ impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> {
145145

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

src/librustdoc/passes/check_doc_test_visibility.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@ crate fn check_doc_test_visibility(krate: Crate, cx: &mut DocContext<'_>) -> Cra
3535

3636
impl<'a, 'tcx> DocVisitor for DocTestVisibilityLinter<'a, 'tcx> {
3737
fn visit_item(&mut self, item: &Item) {
38-
let dox = item.attrs.collapsed_doc_value().unwrap_or_else(String::new);
39-
40-
look_for_tests(self.cx, &dox, &item);
38+
look_for_tests(self.cx, &item.attrs.collapsed_doc_value(), &item);
4139

4240
self.visit_item_recur(item)
4341
}

src/librustdoc/passes/html_tags.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ impl<'a, 'tcx> DocVisitor for InvalidHtmlTagsLinter<'a, 'tcx> {
176176
return;
177177
}
178178
};
179-
let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
179+
let dox = item.attrs.collapsed_doc_value();
180180
if !dox.is_empty() {
181181
let report_diag = |msg: &str, range: &Range<usize>| {
182182
let sp = match super::source_span_for_markdown_range(tcx, &dox, range, &item.attrs)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
- use rustc's map instead of separate traits in scope
2+
- have a map from def id + link index to resolved DefId
3+
- need to have all 3 namespaces in case the link doesn't resolve

src/rustdoc-json-types/lib.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,8 @@ pub struct Item {
6868
/// By default all documented items are public, but you can tell rustdoc to output private items
6969
/// so this field is needed to differentiate.
7070
pub visibility: Visibility,
71-
/// The full markdown docstring of this item. Absent if there is no documentation at all,
72-
/// Some("") if there is some documentation but it is empty (EG `#[doc = ""]`).
73-
pub docs: Option<String>,
71+
/// The full markdown docstring of this item.
72+
pub docs: String,
7473
/// 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
7574
pub links: HashMap<String, Id>,
7675
/// Stringified versions of the attributes on this item (e.g. `"#[inline]"`)
@@ -511,7 +510,7 @@ pub struct Static {
511510
}
512511

513512
/// rustdoc format-version.
514-
pub const FORMAT_VERSION: u32 = 9;
513+
pub const FORMAT_VERSION: u32 = 10;
515514

516515
#[cfg(test)]
517516
mod tests;

0 commit comments

Comments
 (0)