simplify deprecation and stability rendering

euclio · euclio · commit 5f3431691dd2 · 2018-12-14T11:15:25.000-05:00
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
@@ -478,7 +478,7 @@ impl Item {
                 classes.push("unstable");
             }
 
-            if !s.deprecated_since.is_empty() {
+            if s.deprecation.is_some() {
                 classes.push("deprecated");
             }
 
@@ -503,6 +503,15 @@ impl Item {
     pub fn type_(&self) -> ItemType {
         ItemType::from(self)
     }
+
+    /// Returns the info in the item's `#[deprecated]` or `#[rustc_deprecated]` attributes.
+    ///
+    /// If the item is not deprecated, returns `None`.
+    pub fn deprecation(&self) -> Option<&Deprecation> {
+        self.deprecation
+            .as_ref()
+            .or_else(|| self.stability.as_ref().and_then(|s| s.deprecation.as_ref()))
+    }
 }
 
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
@@ -3838,40 +3847,37 @@ impl Clean<Item> for doctree::ProcMacro {
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
 pub struct Stability {
     pub level: stability::StabilityLevel,
-    pub feature: String,
+    pub feature: Option<String>,
     pub since: String,
-    pub deprecated_since: String,
-    pub deprecated_reason: String,
-    pub unstable_reason: String,
-    pub issue: Option<u32>
+    pub deprecation: Option<Deprecation>,
+    pub unstable_reason: Option<String>,
+    pub issue: Option<u32>,
 }
 
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
 pub struct Deprecation {
-    pub since: String,
-    pub note: String,
+    pub since: Option<String>,
+    pub note: Option<String>,
 }
 
 impl Clean<Stability> for attr::Stability {
     fn clean(&self, _: &DocContext) -> Stability {
         Stability {
             level: stability::StabilityLevel::from_attr_level(&self.level),
-            feature: self.feature.to_string(),
+            feature: Some(self.feature.to_string()).filter(|f| !f.is_empty()),
             since: match self.level {
                 attr::Stable {ref since} => since.to_string(),
                 _ => String::new(),
             },
-            deprecated_since: match self.rustc_depr {
-                Some(attr::RustcDeprecation {ref since, ..}) => since.to_string(),
-                _=> String::new(),
-            },
-            deprecated_reason: match self.rustc_depr {
-                Some(ref depr) => depr.reason.to_string(),
-                _ => String::new(),
-            },
+            deprecation: self.rustc_depr.as_ref().map(|d| {
+                Deprecation {
+                    note: Some(d.reason.to_string()).filter(|r| !r.is_empty()),
+                    since: Some(d.since.to_string()).filter(|d| !d.is_empty()),
+                }
+            }),
             unstable_reason: match self.level {
-                attr::Unstable { reason: Some(ref reason), .. } => reason.to_string(),
-                _ => String::new(),
+                attr::Unstable { reason: Some(ref reason), .. } => Some(reason.to_string()),
+                _ => None,
             },
             issue: match self.level {
                 attr::Unstable {issue, ..} => Some(issue),
@@ -3890,8 +3896,8 @@ impl<'a> Clean<Stability> for &'a attr::Stability {
 impl Clean<Deprecation> for attr::Deprecation {
     fn clean(&self, _: &DocContext) -> Deprecation {
         Deprecation {
-            since: self.since.as_ref().map_or(String::new(), |s| s.to_string()),
-            note: self.note.as_ref().map_or(String::new(), |s| s.to_string()),
+            since: self.since.map(|s| s.to_string()).filter(|s| !s.is_empty()),
+            note: self.note.map(|n| n.to_string()).filter(|n| !n.is_empty()),
         }
     }
 }
diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs
@@ -65,7 +65,7 @@ use rustc::hir;
 use rustc::util::nodemap::{FxHashMap, FxHashSet};
 use rustc_data_structures::flock;
 
-use clean::{self, AttributesExt, GetDefId, SelfTy, Mutability};
+use clean::{self, AttributesExt, Deprecation, GetDefId, SelfTy, Mutability};
 use config::RenderOptions;
 use doctree;
 use fold::DocFolder;
@@ -2449,7 +2449,7 @@ fn document_full(w: &mut fmt::Formatter, item: &clean::Item,
 
 fn document_stability(w: &mut fmt::Formatter, cx: &Context, item: &clean::Item,
                       is_hidden: bool) -> fmt::Result {
-    let stabilities = short_stability(item, cx, true);
+    let stabilities = short_stability(item, cx);
     if !stabilities.is_empty() {
         write!(w, "<div class='stability{}'>", if is_hidden { " hidden" } else { "" })?;
         for stability in stabilities {
@@ -2642,18 +2642,6 @@ fn item_module(w: &mut fmt::Formatter, cx: &Context,
             _ => {
                 if myitem.name.is_none() { continue }
 
-                let stabilities = short_stability(myitem, cx, false);
-
-                let stab_docs = if !stabilities.is_empty() {
-                    stabilities.iter()
-                               .map(|s| format!("[{}]", s))
-                               .collect::<Vec<_>>()
-                               .as_slice()
-                               .join(" ")
-                } else {
-                    String::new()
-                };
-
                 let unsafety_flag = match myitem.inner {
                     clean::FunctionItem(ref func) | clean::ForeignFunctionItem(ref func)
                     if func.header.unsafety == hir::Unsafety::Unsafe => {
@@ -2674,11 +2662,11 @@ fn item_module(w: &mut fmt::Formatter, cx: &Context,
                        <tr class='{stab}{add}module-item'>\
                            <td><a class=\"{class}\" href=\"{href}\" \
                                   title='{title}'>{name}</a>{unsafety_flag}</td>\
-                           <td class='docblock-short'>{stab_docs}{docs}\
+                           <td class='docblock-short'>{stab_tags}{docs}\
                            </td>\
                        </tr>",
                        name = *myitem.name.as_ref().unwrap(),
-                       stab_docs = stab_docs,
+                       stab_tags = stability_tags(myitem),
                        docs = MarkdownSummaryLine(doc_value, &myitem.links()),
                        class = myitem.type_(),
                        add = add,
@@ -2705,101 +2693,99 @@ fn item_module(w: &mut fmt::Formatter, cx: &Context,
     Ok(())
 }
 
-fn short_stability(item: &clean::Item, cx: &Context, show_reason: bool) -> Vec<String> {
+/// Render the stability and deprecation tags that are displayed in the item's summary at the
+/// module level.
+fn stability_tags(item: &clean::Item) -> String {
+    let mut tags = String::new();
+
+    // The trailing space after each tag is to space it properly against the rest of the docs.
+    if item.deprecation().is_some() {
+        tags.push_str("[<div class='stab deprecated'>Deprecated</div>] ");
+    }
+
+    if item
+        .stability
+        .as_ref()
+        .filter(|s| s.level == stability::Unstable)
+        .is_some()
+    {
+        tags.push_str("[<div class='stab unstable'>Experimental</div>] ");
+    }
+
+    if let Some(ref cfg) = item.attrs.cfg {
+        tags.push_str(&format!(
+            "[<div class='stab portability'>{}</div>] ",
+            cfg.render_short_html()
+        ));
+    }
+
+    tags
+}
+
+/// Render the stability and/or deprecation warning that is displayed at the top of the item's
+/// documentation.
+fn short_stability(item: &clean::Item, cx: &Context) -> Vec<String> {
     let mut stability = vec![];
     let error_codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build());
 
-    if let Some(stab) = item.stability.as_ref() {
-        let deprecated_reason = if show_reason && !stab.deprecated_reason.is_empty() {
-            format!(": {}", stab.deprecated_reason)
+    if let Some(Deprecation { since, note }) = &item.deprecation() {
+        let mut message = if let Some(since) = since {
+            if stability::deprecation_in_effect(since) {
+                format!("Deprecated since {}", Escape(since))
+            } else {
+                format!("Deprecating in {}", Escape(since))
+            }
         } else {
-            String::new()
+            String::from("Deprecated")
         };
-        if !stab.deprecated_since.is_empty() {
-            let since = if show_reason {
-                format!(" since {}", Escape(&stab.deprecated_since))
-            } else {
-                String::new()
-            };
+
+        if let Some(note) = note {
             let mut ids = cx.id_map.borrow_mut();
-            let html = MarkdownHtml(&deprecated_reason, RefCell::new(&mut ids), error_codes);
-            let text = if stability::deprecation_in_effect(&stab.deprecated_since) {
-                format!("Deprecated{}{}", since, html)
-            } else {
-                format!("Deprecating in {}{}", Escape(&stab.deprecated_since), html)
-            };
-            stability.push(format!("<div class='stab deprecated'>{}</div>", text))
-        };
+            let html = MarkdownHtml(&note, RefCell::new(&mut ids), error_codes);
+            message.push_str(&format!(": {}", html));
+        }
+        stability.push(format!("<div class='stab deprecated'>{}</div>", message));
+    }
 
-        if stab.level == stability::Unstable {
-            if show_reason {
-                let unstable_extra = match (!stab.feature.is_empty(),
-                                            &cx.shared.issue_tracker_base_url,
-                                            stab.issue) {
-                    (true, &Some(ref tracker_url), Some(issue_no)) if issue_no > 0 =>
-                        format!(" (<code>{} </code><a href=\"{}{}\">#{}</a>)",
-                                Escape(&stab.feature), tracker_url, issue_no, issue_no),
-                    (false, &Some(ref tracker_url), Some(issue_no)) if issue_no > 0 =>
-                        format!(" (<a href=\"{}{}\">#{}</a>)", Escape(&tracker_url), issue_no,
-                                issue_no),
-                    (true, ..) =>
-                        format!(" (<code>{}</code>)", Escape(&stab.feature)),
-                    _ => String::new(),
-                };
-                if stab.unstable_reason.is_empty() {
-                    stability.push(format!("<div class='stab unstable'>\
-                                            <span class=microscope>🔬</span> \
-                                            This is a nightly-only experimental API. {}\
-                                            </div>",
-                                           unstable_extra));
-                } else {
-                    let mut ids = cx.id_map.borrow_mut();
-                    let text = format!("<summary><span class=microscope>🔬</span> \
-                                        This is a nightly-only experimental API. {}\
-                                        </summary>{}",
-                                       unstable_extra,
-                                       MarkdownHtml(
-                                           &stab.unstable_reason,
-                                           RefCell::new(&mut ids),
-                                           error_codes));
-                    stability.push(format!("<div class='stab unstable'><details>{}</details></div>",
-                                   text));
-                }
-            } else {
-                stability.push("<div class='stab unstable'>Experimental</div>".to_string())
+    if let Some(stab) = item
+        .stability
+        .as_ref()
+        .filter(|stab| stab.level == stability::Unstable)
+    {
+        let mut message = String::from(
+            "<span class=microscope>🔬</span> This is a nightly-only experimental API.",
+        );
+
+        if let Some(feature) = stab.feature.as_ref() {
+            let mut feature = format!("<code>{}</code>", Escape(&feature));
+            if let (Some(url), Some(issue)) = (&cx.shared.issue_tracker_base_url, stab.issue) {
+                feature.push_str(&format!(
+                    "&nbsp;<a href=\"{url}{issue}\">#{issue}</a>",
+                    url = url,
+                    issue = issue
+                ));
             }
-        };
-    } else if let Some(depr) = item.deprecation.as_ref() {
-        let note = if show_reason && !depr.note.is_empty() {
-            format!(": {}", depr.note)
-        } else {
-            String::new()
-        };
-        let since = if show_reason && !depr.since.is_empty() {
-            format!(" since {}", Escape(&depr.since))
-        } else {
-            String::new()
-        };
 
-        let mut ids = cx.id_map.borrow_mut();
-        let text = if stability::deprecation_in_effect(&depr.since) {
-            format!("Deprecated{}{}",
-                    since,
-                    MarkdownHtml(&note, RefCell::new(&mut ids), error_codes))
-        } else {
-            format!("Deprecating in {}{}",
-                    Escape(&depr.since),
-                    MarkdownHtml(&note, RefCell::new(&mut ids), error_codes))
-        };
-        stability.push(format!("<div class='stab deprecated'>{}</div>", text))
+            message.push_str(&format!(" ({})", feature));
+        }
+
+        if let Some(unstable_reason) = &stab.unstable_reason {
+            let mut ids = cx.id_map.borrow_mut();
+            message = format!(
+                "<details><summary>{}</summary>{}</details>",
+                message,
+                MarkdownHtml(&unstable_reason, RefCell::new(&mut ids), error_codes)
+            );
+        }
+
+        stability.push(format!("<div class='stab unstable'>{}</div>", message))
     }
 
     if let Some(ref cfg) = item.attrs.cfg {
-        stability.push(format!("<div class='stab portability'>{}</div>", if show_reason {
+        stability.push(format!(
+            "<div class='stab portability'>{}</div>",
             cfg.render_long_html()
-        } else {
-            cfg.render_short_html()
-        }));
+        ));
     }
 
     stability
diff --git a/src/test/rustdoc/deprecated.rs b/src/test/rustdoc/deprecated.rs
@@ -10,7 +10,29 @@
 
 #![feature(deprecated)]
 
+// @matches deprecated/index.html '//*[@class="docblock-short"]' \
+//      '^\[Deprecated\] Deprecated docs'
 // @has deprecated/struct.S.html '//*[@class="stab deprecated"]' \
 //      'Deprecated since 1.0.0: text'
+/// Deprecated docs
 #[deprecated(since = "1.0.0", note = "text")]
 pub struct S;
+
+// @matches deprecated/index.html '//*[@class="docblock-short"]' '^Docs'
+/// Docs
+pub struct T;
+
+// @matches deprecated/struct.U.html '//*[@class="stab deprecated"]' \
+//      'Deprecated since 1.0.0$'
+#[deprecated(since = "1.0.0")]
+pub struct U;
+
+// @matches deprecated/struct.V.html '//*[@class="stab deprecated"]' \
+//      'Deprecated: text$'
+#[deprecated(note = "text")]
+pub struct V;
+
+// @matches deprecated/struct.W.html '//*[@class="stab deprecated"]' \
+//      'Deprecated$'
+#[deprecated]
+pub struct W;
diff --git a/src/test/rustdoc/issue-27759.rs b/src/test/rustdoc/issue-27759.rs
@@ -14,13 +14,11 @@
 #![unstable(feature="test", issue="27759")]
 
 // @has issue_27759/unstable/index.html
-// @has - '<code>test </code>'
-// @has - '<a href="http://issue_url/27759">#27759</a>'
+// @has - '<code>test</code>&nbsp;<a href="http://issue_url/27759">#27759</a>'
 #[unstable(feature="test", issue="27759")]
 pub mod unstable {
     // @has issue_27759/unstable/fn.issue.html
-    // @has - '<code>test_function </code>'
-    // @has - '<a href="http://issue_url/1234567890">#1234567890</a>'
-    #[unstable(feature="test_function", issue="1234567890")]
+    // @has - '<code>test_function</code>&nbsp;<a href="http://issue_url/12345">#12345</a>'
+    #[unstable(feature="test_function", issue="12345")]
     pub fn issue() {}
 }
diff --git a/src/test/rustdoc/issue-32374.rs b/src/test/rustdoc/issue-32374.rs
@@ -13,15 +13,15 @@
 
 #![unstable(feature="test", issue = "32374")]
 
-// @has issue_32374/index.html '//*[@class="docblock-short"]' \
-//      '[Deprecated] [Experimental]'
+// @matches issue_32374/index.html '//*[@class="docblock-short"]' \
+//      '^\[Deprecated\] \[Experimental\] Docs'
 
 // @has issue_32374/struct.T.html '//*[@class="stab deprecated"]' \
 //      'Deprecated since 1.0.0: text'
-// @has - '<code>test </code>'
-// @has - '<a href="http://issue_url/32374">#32374</a>'
+// @has - '<code>test</code>&nbsp;<a href="http://issue_url/32374">#32374</a>'
 // @matches issue_32374/struct.T.html '//*[@class="stab unstable"]' \
-//      '🔬 This is a nightly-only experimental API.  \(test #32374\)$'
+//      '🔬 This is a nightly-only experimental API. \(test #32374\)$'
+/// Docs
 #[rustc_deprecated(since = "1.0.0", reason = "text")]
 #[unstable(feature = "test", issue = "32374")]
 pub struct T;
