Address review comments

2 files changed, 43 insertions, 49 deletions

diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs
index eccdd08db8a..a6d6802c0e1 100644
--- a/src/librustdoc/html/markdown.rs
+++ b/src/librustdoc/html/markdown.rs
@@ -35,7 +35,7 @@ use std::fmt;
 use std::slice;
 use std::str;
 
-use html::render::with_unique_id;
+use html::render::derive_id;
 use html::toc::TocBuilder;
 use html::highlight;
 use html::escape::Escape;
@@ -307,17 +307,17 @@ pub fn render(w: &mut fmt::Formatter, s: &str, print_toc: bool) -> fmt::Result {
         let opaque = unsafe { (*data).opaque as *mut hoedown_html_renderer_state };
         let opaque = unsafe { &mut *((*opaque).opaque as *mut MyOpaque) };
 
-        let text = with_unique_id(id, |id| {
-            let sec = opaque.toc_builder.as_mut().map_or("".to_owned(), |builder| {
-                format!("{} ", builder.push(level as u32, s.clone(), id.to_owned()))
-            });
+        let id = derive_id(id);
 
-            // Render the HTML
-            format!("<h{lvl} id='{id}' class='section-header'>\
-                    <a href='#{id}'>{sec}{}</a></h{lvl}>",
-                    s, lvl = level, id = id, sec = sec)
+        let sec = opaque.toc_builder.as_mut().map_or("".to_owned(), |builder| {
+            format!("{} ", builder.push(level as u32, s.clone(), id.clone()))
         });
 
+        // Render the HTML
+        let text = format!("<h{lvl} id='{id}' class='section-header'>\
+                           <a href='#{id}'>{sec}{}</a></h{lvl}>",
+                           s, lvl = level, id = id, sec = sec);
+
         let text = CString::new(text).unwrap();
         unsafe { hoedown_buffer_puts(ob, text.as_ptr()) }
     }
diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs
index 3372c9d4807..aac2a52984a 100644
--- a/src/librustdoc/html/render.rs
+++ b/src/librustdoc/html/render.rs
@@ -372,23 +372,19 @@ pub fn reset_ids() {
     USED_ID_MAP.with(|s| *s.borrow_mut() = init_ids());
 }
 
-pub fn with_unique_id<T, F: FnOnce(&str) -> T>(candidate: String, f: F) -> T {
+pub fn derive_id(candidate: String) -> String {
     USED_ID_MAP.with(|map| {
-        let (id, ret) = match map.borrow_mut().get_mut(&candidate) {
-            None => {
-                let ret = f(&candidate);
-                (candidate, ret)
-            },
+        let id = match map.borrow_mut().get_mut(&candidate) {
+            None => candidate,
             Some(a) => {
                 let id = format!("{}-{}", candidate, *a);
-                let ret = f(&id);
                 *a += 1;
-                (id, ret)
+                id
             }
         };
 
-        map.borrow_mut().insert(id, 1);
-        ret
+        map.borrow_mut().insert(id.clone(), 1);
+        id
     })
 }
 
@@ -1745,10 +1741,9 @@ fn item_module(w: &mut fmt::Formatter, cx: &Context,
                 ItemType::AssociatedType  => ("associated-types", "Associated Types"),
                 ItemType::AssociatedConst => ("associated-consts", "Associated Constants"),
             };
-            try!(with_unique_id(short.to_owned(), |id|
-                write!(w, "<h2 id='{id}' class='section-header'>\
-                          <a href=\"#{id}\">{name}</a></h2>\n<table>",
-                          id = id, name = name)));
+            try!(write!(w, "<h2 id='{id}' class='section-header'>\
+                           <a href=\"#{id}\">{name}</a></h2>\n<table>",
+                           id = derive_id(short.to_owned()), name = name));
         }
 
         match myitem.inner {
@@ -1970,10 +1965,10 @@ fn item_trait(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item,
     fn trait_item(w: &mut fmt::Formatter, cx: &Context, m: &clean::Item)
                   -> fmt::Result {
         let name = m.name.as_ref().unwrap();
-        try!(with_unique_id(format!("{}.{}", shortty(m), name), |id|
-                write!(w, "<h3 id='{id}' class='method stab {stab}'><code>",
+        let id = derive_id(format!("{}.{}", shortty(m), name));
+        try!(write!(w, "<h3 id='{id}' class='method stab {stab}'><code>",
                        id = id,
-                       stab = m.stability_class())));
+                       stab = m.stability_class()));
         try!(render_assoc_item(w, m, AssocItemLink::Anchor));
         try!(write!(w, "</code></h3>"));
         try!(document(w, cx, m));
@@ -2162,12 +2157,11 @@ fn item_struct(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item,
         if fields.peek().is_some() {
             try!(write!(w, "<h2 class='fields'>Fields</h2>\n<table>"));
             for field in fields {
-                let name = field.name.as_ref().unwrap();
-                try!(with_unique_id(format!("structfield.{}", name), |id|
-                    write!(w, "<tr class='stab {}'><td id='{}'><code>{}</code></td><td>",
-                              field.stability_class(),
-                              id,
-                              name)));
+                try!(write!(w, "<tr class='stab {stab}'>
+                                  <td id='structfield.{name}'>\
+                                    <code>{name}</code></td><td>",
+                            stab = field.stability_class(),
+                            name = field.name.as_ref().unwrap()));
                 try!(document(w, cx, field));
                 try!(write!(w, "</td></tr>"));
             }
@@ -2234,9 +2228,8 @@ fn item_enum(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item,
     if !e.variants.is_empty() {
         try!(write!(w, "<h2 class='variants'>Variants</h2>\n<table>"));
         for variant in &e.variants {
-            let name = variant.name.as_ref().unwrap();
-            try!(with_unique_id(format!("variant.{}", name), |id|
-                    write!(w, "<tr><td id='{}'><code>{}</code></td><td>", id, name)));
+            try!(write!(w, "<tr><td id='variant.{name}'><code>{name}</code></td><td>",
+                          name = variant.name.as_ref().unwrap()));
             try!(document(w, cx, variant));
             match variant.inner {
                 clean::VariantItem(ref var) => {
@@ -2254,10 +2247,11 @@ fn item_enum(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item,
                             try!(write!(w, "<h3 class='fields'>Fields</h3>\n
                                               <table>"));
                             for field in fields {
-                                let v = variant.name.as_ref().unwrap();
-                                let f = field.name.as_ref().unwrap();
-                                try!(with_unique_id(format!("variant.{}.field.{}", v, f), |id|
-                                    write!(w, "<tr><td id='{}'><code>{}</code></td><td>", id, f)));
+                                try!(write!(w, "<tr><td \
+                                                  id='variant.{v}.field.{f}'>\
+                                                  <code>{f}</code></td><td>",
+                                              v = variant.name.as_ref().unwrap(),
+                                              f = field.name.as_ref().unwrap()));
                                 try!(document(w, cx, field));
                                 try!(write!(w, "</td></tr>"));
                             }
@@ -2474,33 +2468,33 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi
             clean::MethodItem(..) | clean::TyMethodItem(..) => {
                 // Only render when the method is not static or we allow static methods
                 if !is_static_method(item) || render_static {
-                    try!(with_unique_id(format!("method.{}", name), |id|
-                        write!(w, "<h4 id='{}' class='{}'><code>", id, shortty(item))));
+                    let id = derive_id(format!("method.{}", name));
+                    try!(write!(w, "<h4 id='{}' class='{}'><code>", id, shortty(item)));
                 try!(render_assoc_item(w, item, link));
                     try!(write!(w, "</code></h4>\n"));
                 }
             }
             clean::TypedefItem(ref tydef, _) => {
-                try!(with_unique_id(format!("assoc_type.{}", name), |id|
-                    write!(w, "<h4 id='{}' class='{}'><code>", id, shortty(item))));
+                let id = derive_id(format!("assoc_type.{}", name));
+                try!(write!(w, "<h4 id='{}' class='{}'><code>", id, shortty(item)));
                 try!(write!(w, "type {} = {}", name, tydef.type_));
                 try!(write!(w, "</code></h4>\n"));
             }
             clean::AssociatedConstItem(ref ty, ref default) => {
-                try!(with_unique_id(format!("assoc_const.{}", name), |id|
-                    write!(w, "<h4 id='{}' class='{}'><code>", id, shortty(item))));
+                let id = derive_id(format!("assoc_const.{}", name));
+                try!(write!(w, "<h4 id='{}' class='{}'><code>", id, shortty(item)));
                 try!(assoc_const(w, item, ty, default.as_ref()));
                 try!(write!(w, "</code></h4>\n"));
             }
             clean::ConstantItem(ref c) => {
-                try!(with_unique_id(format!("assoc_const.{}", name), |id|
-                    write!(w, "<h4 id='{}' class='{}'><code>", id, shortty(item))));
+                let id = derive_id(format!("assoc_const.{}", name));
+                try!(write!(w, "<h4 id='{}' class='{}'><code>", id, shortty(item)));
                 try!(assoc_const(w, item, &c.type_, Some(&c.expr)));
                 try!(write!(w, "</code></h4>\n"));
             }
             clean::AssociatedTypeItem(ref bounds, ref default) => {
-                try!(with_unique_id(format!("assoc_type.{}", name), |id|
-                    write!(w, "<h4 id='{}' class='{}'><code>", id, shortty(item))));
+                let id = derive_id(format!("assoc_type.{}", name));
+                try!(write!(w, "<h4 id='{}' class='{}'><code>", id, shortty(item)));
                 try!(assoc_type(w, item, bounds, default));
                 try!(write!(w, "</code></h4>\n"));
             }