Remove fmt::Display impls on Markdown structs

Mark-Simulacrum · Mark-Simulacrum · commit c250b5fd033e · 2019-08-11T10:36:46.000-04:00
These impls prevent ergonomic use of the config (e.g., forcing us to use
RefCell) despite all usecases for these structs only using their Display
impls once.
diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs
@@ -36,7 +36,7 @@ impl ExternalHtml {
                 load_external_files(md_before_content, diag)
                     .map(|m_bc| (ih,
                             format!("{}{}", bc, Markdown(&m_bc, &[], RefCell::new(id_map),
-                                    codes, edition, playground))))
+                                    codes, edition, playground).to_string())))
             )
             .and_then(|(ih, bc)|
                 load_external_files(after_content, diag)
@@ -46,7 +46,7 @@ impl ExternalHtml {
                 load_external_files(md_after_content, diag)
                     .map(|m_ac| (ih, bc,
                             format!("{}{}", ac, Markdown(&m_ac, &[], RefCell::new(id_map),
-                                    codes, edition, playground))))
+                                    codes, edition, playground).to_string())))
             )
             .map(|(ih, bc, ac)|
                 ExternalHtml {
diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs
@@ -1,9 +1,6 @@
 //! Markdown formatting for rustdoc.
 //!
-//! This module implements markdown formatting through the pulldown-cmark
-//! rust-library. This module exposes all of the
-//! functionality through a unit struct, `Markdown`, which has an implementation
-//! of `fmt::Display`. Example usage:
+//! This module implements markdown formatting through the pulldown-cmark library.
 //!
 //! ```
 //! #![feature(rustc_private)]
@@ -16,8 +13,9 @@
 //!
 //! let s = "My *markdown* _text_";
 //! let mut id_map = IdMap::new();
-//! let html = format!("{}", Markdown(s, &[], RefCell::new(&mut id_map),
-//!                                   ErrorCodes::Yes, Edition::Edition2015, None));
+//! let md = Markdown(s, &[], RefCell::new(&mut id_map),
+//!                                   ErrorCodes::Yes, Edition::Edition2015, None);
+//! let html = md.to_string();
 //! // ... something using html
 //! ```
 
@@ -27,7 +25,7 @@ use rustc_data_structures::fx::FxHashMap;
 use std::cell::RefCell;
 use std::collections::VecDeque;
 use std::default::Default;
-use std::fmt::{self, Write};
+use std::fmt::Write;
 use std::borrow::Cow;
 use std::ops::Range;
 use std::str;
@@ -46,9 +44,8 @@ fn opts() -> Options {
     Options::ENABLE_TABLES | Options::ENABLE_FOOTNOTES
 }
 
-/// A tuple struct that has the `fmt::Display` trait implemented.
-/// When formatted, this struct will emit the HTML corresponding to the rendered
-/// version of the contained markdown string.
+/// When `to_string` is called, this struct will emit the HTML corresponding to
+/// the rendered version of the contained markdown string.
 pub struct Markdown<'a>(
     pub &'a str,
     /// A list of link replacements.
@@ -691,13 +688,13 @@ impl LangString {
     }
 }
 
-impl<'a> fmt::Display for Markdown<'a> {
-    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let Markdown(md, links, ref ids, codes, edition, playground) = *self;
+impl Markdown<'_> {
+    pub fn to_string(self) -> String {
+        let Markdown(md, links, ids, codes, edition, playground) = self;
         let mut ids = ids.borrow_mut();
 
         // This is actually common enough to special-case
-        if md.is_empty() { return Ok(()) }
+        if md.is_empty() { return String::new(); }
         let replacer = |_: &str, s: &str| {
             if let Some(&(_, ref replace)) = links.into_iter().find(|link| &*link.0 == s) {
                 Some((replace.clone(), s.to_owned()))
@@ -716,13 +713,13 @@ impl<'a> fmt::Display for Markdown<'a> {
         let p = Footnotes::new(p);
         html::push_html(&mut s, p);
 
-        fmt.write_str(&s)
+        s
     }
 }
 
-impl<'a> fmt::Display for MarkdownWithToc<'a> {
-    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let MarkdownWithToc(md, ref ids, codes, edition, playground) = *self;
+impl MarkdownWithToc<'_> {
+    pub fn to_string(self) -> String {
+        let MarkdownWithToc(md, ref ids, codes, edition, playground) = self;
         let mut ids = ids.borrow_mut();
 
         let p = Parser::new_ext(md, opts());
@@ -738,19 +735,17 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> {
             html::push_html(&mut s, p);
         }
 
-        write!(fmt, "<nav id=\"TOC\">{}</nav>", toc.into_toc())?;
-
-        fmt.write_str(&s)
+        format!("<nav id=\"TOC\">{}</nav>{}", toc.into_toc(), s)
     }
 }
 
-impl<'a> fmt::Display for MarkdownHtml<'a> {
-    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let MarkdownHtml(md, ref ids, codes, edition, playground) = *self;
+impl MarkdownHtml<'_> {
+    pub fn to_string(self) -> String {
+        let MarkdownHtml(md, ref ids, codes, edition, playground) = self;
         let mut ids = ids.borrow_mut();
 
         // This is actually common enough to special-case
-        if md.is_empty() { return Ok(()) }
+        if md.is_empty() { return String::new(); }
         let p = Parser::new_ext(md, opts());
 
         // Treat inline HTML as plain text.
@@ -766,15 +761,15 @@ impl<'a> fmt::Display for MarkdownHtml<'a> {
         let p = Footnotes::new(p);
         html::push_html(&mut s, p);
 
-        fmt.write_str(&s)
+        s
     }
 }
 
-impl<'a> fmt::Display for MarkdownSummaryLine<'a> {
-    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let MarkdownSummaryLine(md, links) = *self;
+impl MarkdownSummaryLine<'_> {
+    pub fn to_string(self) -> String {
+        let MarkdownSummaryLine(md, links) = self;
         // This is actually common enough to special-case
-        if md.is_empty() { return Ok(()) }
+        if md.is_empty() { return String::new(); }
 
         let replacer = |_: &str, s: &str| {
             if let Some(&(_, ref replace)) = links.into_iter().find(|link| &*link.0 == s) {
@@ -790,7 +785,7 @@ impl<'a> fmt::Display for MarkdownSummaryLine<'a> {
 
         html::push_html(&mut s, LinkReplacer::new(SummaryLine::new(p), links));
 
-        fmt.write_str(&s)
+        s
     }
 }
 
diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs
@@ -2596,7 +2596,7 @@ fn render_markdown(w: &mut fmt::Formatter<'_>,
            if is_hidden { " hidden" } else { "" },
            prefix,
            Markdown(md_text, &links, RefCell::new(&mut ids),
-           cx.codes, cx.edition, &cx.playground))
+           cx.codes, cx.edition, &cx.playground).to_string())
 }
 
 fn document_short(
@@ -2866,7 +2866,7 @@ fn item_module(w: &mut fmt::Formatter<'_>, cx: &Context,
                        </tr>",
                        name = *myitem.name.as_ref().unwrap(),
                        stab_tags = stability_tags(myitem),
-                       docs = MarkdownSummaryLine(doc_value, &myitem.links()),
+                       docs = MarkdownSummaryLine(doc_value, &myitem.links()).to_string(),
                        class = myitem.type_(),
                        add = add,
                        stab = stab.unwrap_or_else(|| String::new()),
@@ -2963,7 +2963,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec<String> {
             let mut ids = cx.id_map.borrow_mut();
             let html = MarkdownHtml(
                 &note, RefCell::new(&mut ids), error_codes, cx.edition, &cx.playground);
-            message.push_str(&format!(": {}", html));
+            message.push_str(&format!(": {}", html.to_string()));
         }
         stability.push(format!("<div class='stab deprecated'>{}</div>", message));
     }
@@ -3017,7 +3017,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec<String> {
                     error_codes,
                     cx.edition,
                     &cx.playground,
-                )
+                ).to_string()
             );
         }
 
@@ -4248,7 +4248,7 @@ fn render_impl(w: &mut fmt::Formatter<'_>, cx: &Context, i: &Impl, link: AssocIt
             let mut ids = cx.id_map.borrow_mut();
             write!(w, "<div class='docblock'>{}</div>",
                    Markdown(&*dox, &i.impl_item.links(), RefCell::new(&mut ids),
-                            cx.codes, cx.edition, &cx.playground))?;
+                            cx.codes, cx.edition, &cx.playground).to_string())?;
         }
     }
 
diff --git a/src/tools/error_index_generator/main.rs b/src/tools/error_index_generator/main.rs
@@ -101,7 +101,7 @@ impl Formatter for HTMLFormatter {
                 };
                 write!(output, "{}",
                     Markdown(desc, &[], RefCell::new(&mut id_map),
-                             ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)))?
+                             ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)).to_string())?
             },
             None => write!(output, "<p>No description.</p>\n")?,
         }

Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ impl Formatter for HTMLFormatter {`
`101`	`101`	`};`
`102`	`102`	`write!(output, "{}",`
`103`	`103`	`Markdown(desc, &[], RefCell::new(&mut id_map),`
`104`		`- ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)))?`
	`104`	`+ ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)).to_string())?`
`105`	`105`	`},`
`106`	`106`	`None => write!(output, "<p>No description.</p>\n")?,`
`107`	`107`	`}`