Various rustdoc improvements

doc/rust.css

@@ -101,4 +101,9 @@ td {
 #TOC ul {
     list-style: none;
     padding-left: 0px;
+}
+
+/* Adjust list alignment so rustdoc indexes don't align with blockquotes */
+div.index ul {
+    padding-left: 1em;
 }

src/librustdoc/doc.rs

@@ -135,6 +135,7 @@ pub struct MethodDoc {
 #[deriving(Eq)]
 pub struct ImplDoc {
     item: ItemDoc,
+    bounds_str: Option<~str>,
     trait_types: ~[~str],
     self_ty: Option<~str>,
     methods: ~[MethodDoc]

src/librustdoc/extract.rs

@@ -277,6 +277,7 @@ fn impldoc_from_impl(
 ) -> doc::ImplDoc {
     doc::ImplDoc {
         item: itemdoc,
+        bounds_str: None,
         trait_types: ~[],
         self_ty: None,
         methods: do vec::map(methods) |method| {

src/librustdoc/markdown_index_pass.rs

@@ -143,12 +143,16 @@ fn pandoc_header_id(header: &str) -> ~str {
         let s = str::replace(s, ~":", ~"");
         let s = str::replace(s, ~"&", ~"");
         let s = str::replace(s, ~"^", ~"");
+        let s = str::replace(s, ~",", ~"");
+        let s = str::replace(s, ~"'", ~"");
+        let s = str::replace(s, ~"+", ~"");
         return s;
     }
     fn replace_with_hyphens(s: &str) -> ~str {
         // Collapse sequences of whitespace to a single dash
         // XXX: Hacky implementation here that only covers
         // one or two spaces.
+        let s = str::trim(s);
         let s = str::replace(s, ~"  ", ~"-");
         let s = str::replace(s, ~" ", ~"-");
         return s;
@@ -170,6 +174,17 @@ fn should_remove_punctuation_from_headers() {
         == ~"impl-of-numnum-for-int");
     fail_unless!(pandoc_header_id(~"impl for & condvar")
         == ~"impl-for-condvar");
+    fail_unless!(pandoc_header_id(~"impl of Select<T, U> for (Left, Right)")
+                 == ~"impl-of-selectt-u-for-left-right");
+    fail_unless!(pandoc_header_id(~"impl of Condition<'self, T, U>")
+                 == ~"impl-of-conditionself-t-u");
+    fail_unless!(pandoc_header_id(~"impl of Condition<T: Copy + Clone>")
+                 == ~"impl-of-conditionself-t-copy-clone");
+}
+
+#[test]
+fn should_trim_whitespace_after_removing_punctuation() {
+    fail_unless!(pandoc_header_id("impl foo for ()") == ~"impl-foo-for");
 }
 
 #[test]

src/librustdoc/markdown_pass.rs

@@ -249,6 +249,11 @@ pub fn header_name(doc: doc::ItemTag) -> ~str {
       }
       &doc::ImplTag(ref doc) => {
         fail_unless!(doc.self_ty.is_some());
+          let bounds = if (&doc.bounds_str).is_some() {
+              fmt!(" where %s", (&doc.bounds_str).get())
+          } else {
+              ~""
+          };
         let self_ty = (&doc.self_ty).get();
         let mut trait_part = ~"";
         for doc.trait_types.eachi |i, trait_type| {
@@ -259,7 +264,7 @@ pub fn header_name(doc: doc::ItemTag) -> ~str {
             }
             trait_part += *trait_type;
         }
-        fmt!("%s for %s", trait_part, self_ty)
+        fmt!("%s for %s%s", trait_part, self_ty, bounds)
       }
       _ => {
         doc.name()
@@ -271,11 +276,18 @@ pub fn header_text(doc: doc::ItemTag) -> ~str {
     match &doc {
       &doc::ImplTag(ref ImplDoc) => {
         let header_kind = header_kind(copy doc);
+          let bounds = if (&ImplDoc.bounds_str).is_some() {
+              fmt!(" where `%s`", (&ImplDoc.bounds_str).get())
+          } else {
+              ~""
+          };
         let desc = if ImplDoc.trait_types.is_empty() {
-            fmt!("for `%s`", (&ImplDoc.self_ty).get())
+            fmt!("for `%s`%s", (&ImplDoc.self_ty).get(), bounds)
         } else {
-            fmt!("of `%s` for `%s`", ImplDoc.trait_types[0],
-                 (&ImplDoc.self_ty).get())
+            fmt!("of `%s` for `%s`%s",
+                 ImplDoc.trait_types[0],
+                 (&ImplDoc.self_ty).get(),
+                 bounds)
         };
         return fmt!("%s %s", header_kind, desc);
       }
@@ -424,6 +436,8 @@ fn write_index(ctxt: &Ctxt, index: doc::Index) {
         return;
     }
 
+    ctxt.w.put_line(~"<div class='index'>");
+
     for index.entries.each |entry| {
         let header = header_text_(entry.kind, entry.name);
         let id = copy entry.link;
@@ -434,6 +448,7 @@ fn write_index(ctxt: &Ctxt, index: doc::Index) {
             ctxt.w.put_line(fmt!("* [%s](%s)", header, id));
         }
     }
+    ctxt.w.put_line(~"</div>");
     ctxt.w.put_line(~"");
 }
 
@@ -749,6 +764,12 @@ fn should_write_impl_header() {
     fail_unless!(str::contains(markdown, ~"## Implementation for `int`"));
 }
 
+#[test]
+fn should_write_impl_header_with_bounds() {
+    let markdown = test::render(~"impl <T> int<T> { }");
+    fail_unless!(str::contains(markdown, ~"## Implementation for `int<T>` where `<T>`"));
+}
+
 #[test]
 fn should_write_impl_header_with_trait() {
     let markdown = test::render(~"impl j for int { fn a() { } }");

src/librustdoc/parse.rs

@@ -39,5 +39,5 @@ pub fn from_str_sess(sess: session::Session, source: ~str) -> @ast::crate {
 }
 
 fn cfg(sess: session::Session, input: driver::input) -> ast::crate_cfg {
-    driver::default_configuration(sess, ~"rustdoc", input)
+    driver::build_configuration(sess, ~"rustdoc", input)
 }

src/librustdoc/prune_private_pass.rs

@@ -12,6 +12,9 @@
 
 use core::prelude::*;
 
+use extract;
+use syntax::ast;
+use syntax::ast_map;
 use astsrv;
 use doc;
 use fold::Fold;
@@ -28,12 +31,73 @@ pub fn mk_pass() -> Pass {
 }
 
 pub fn run(srv: astsrv::Srv, doc: doc::Doc) -> doc::Doc {
+    // First strip private methods out of impls
+    let fold = Fold {
+        ctxt: srv.clone(),
+        fold_impl: fold_impl,
+        .. fold::default_any_fold(srv.clone())
+    };
+    let doc = (fold.fold_doc)(&fold, doc);
+
+    // Then strip private items and empty impls
     let fold = Fold {
         ctxt: srv.clone(),
         fold_mod: fold_mod,
         .. fold::default_any_fold(srv)
     };
-    (fold.fold_doc)(&fold, doc)
+    let doc = (fold.fold_doc)(&fold, doc);
+
+    return doc;
+}
+
+fn fold_impl(
+    fold: &fold::Fold<astsrv::Srv>,
+    doc: doc::ImplDoc
+) -> doc::ImplDoc {
+    let doc = fold::default_seq_fold_impl(fold, doc);
+
+    do astsrv::exec(fold.ctxt.clone()) |ctxt| {
+        match ctxt.ast_map.get(&doc.item.id) {
+            ast_map::node_item(item, _) => {
+                match item.node {
+                    ast::item_impl(_, None, _, ref methods) => {
+                        // Associated impls have complex rules for method visibility
+                        strip_priv_methods(copy doc, *methods, item.vis)
+                    }
+                    ast::item_impl(_, Some(_), _ ,_) => {
+                        // Trait impls don't
+                        copy doc
+                    }
+                    _ => fail!()
+                }
+            }
+            _ => fail!()
+        }
+    }
+}
+
+fn strip_priv_methods(
+    doc: doc::ImplDoc,
+    methods: &[@ast::method],
+    item_vis: ast::visibility
+) -> doc::ImplDoc {
+    let methods = do (&doc.methods).filtered |method| {
+        let ast_method = do methods.find |m| {
+            extract::to_str(m.ident) == method.name
+        };
+        fail_unless!(ast_method.is_some());
+        let ast_method = ast_method.unwrap();
+        match ast_method.vis {
+            ast::public => true,
+            ast::private => false,
+            ast::inherited => item_vis == ast::public
+        }
+    };
+
+    doc::ImplDoc {
+        methods: methods,
+        .. doc
+    }
 }
 
 fn fold_mod(
@@ -44,28 +108,40 @@ fn fold_mod(
 
     doc::ModDoc {
         items: doc.items.filtered(|ItemTag| {
-            is_visible(fold.ctxt.clone(), ItemTag.item())
+            match ItemTag {
+                &doc::ImplTag(ref doc) => {
+                    if doc.trait_types.is_empty() {
+                        // This is an associated impl. We have already pruned the
+                        // non-visible methods. If there are any left then
+                        // retain the impl, otherwise throw it away
+                        !doc.methods.is_empty()
+                    } else {
+                        // This is a trait implementation, make it visible
+                        // NOTE: This is not quite right since this could be an impl
+                        // of a private trait. We can't know that without running
+                        // resolve though.
+                        true
+                    }
+                }
+                _ => {
+                    is_visible(fold.ctxt.clone(), ItemTag.item())
+                }
+            }
         }),
         .. doc
     }
 }
 
 fn is_visible(srv: astsrv::Srv, doc: doc::ItemDoc) -> bool {
-    use syntax::ast_map;
-    use syntax::ast;
-
     let id = doc.id;
 
     do astsrv::exec(srv) |ctxt| {
         match ctxt.ast_map.get(&id) {
             ast_map::node_item(item, _) => {
-                match item.node {
-                    ast::item_impl(_, Some(_), _, _) => {
-                        // This is a trait implementation, make it visible
-                        // NOTE: This is not quite right since this could be an impl
-                        // of a private trait. We can't know that without running
-                        // resolve though.
-                        true
+                match &item.node {
+                    &ast::item_impl(*) => {
+                        // Impls handled elsewhere
+                        fail!()
                     }
                     _ => {
                         // Otherwise just look at the visibility
@@ -85,7 +161,8 @@ fn should_prune_items_without_pub_modifier() {
 }
 
 #[test]
-fn unless_they_are_trait_impls() {
+fn should_not_prune_trait_impls() {
+    // Impls are more complicated
     let doc = test::mk_doc(
         ~" \
           trait Foo { } \
@@ -94,16 +171,87 @@ fn unless_they_are_trait_impls() {
     fail_unless!(!doc.cratemod().impls().is_empty());
 }
 
+#[test]
+fn should_prune_associated_methods_without_vis_modifier_on_impls_without_vis_modifier() {
+    let doc = test::mk_doc(
+        ~"impl Foo {\
+          pub fn bar() { }\
+          fn baz() { }\
+          }");
+    fail_unless!(doc.cratemod().impls()[0].methods.len() == 1);
+}
+
+#[test]
+fn should_prune_priv_associated_methods_on_impls_without_vis_modifier() {
+    let doc = test::mk_doc(
+        ~"impl Foo {\
+          pub fn bar() { }\
+          priv fn baz() { }\
+          }");
+    fail_unless!(doc.cratemod().impls()[0].methods.len() == 1);
+}
+
+#[test]
+fn should_prune_priv_associated_methods_on_pub_impls() {
+    let doc = test::mk_doc(
+        ~"pub impl Foo {\
+          fn bar() { }\
+          priv fn baz() { }\
+          }");
+    fail_unless!(doc.cratemod().impls()[0].methods.len() == 1);
+}
+
+#[test]
+fn should_prune_associated_methods_without_vis_modifier_on_priv_impls() {
+    let doc = test::mk_doc(
+        ~"priv impl Foo {\
+          pub fn bar() { }\
+          fn baz() { }\
+          }");
+    fail_unless!(doc.cratemod().impls()[0].methods.len() == 1);
+}
+
+#[test]
+fn should_prune_priv_associated_methods_on_priv_impls() {
+    let doc = test::mk_doc(
+        ~"priv impl Foo {\
+          pub fn bar() { }\
+          priv fn baz() { }\
+          }");
+    fail_unless!(doc.cratemod().impls()[0].methods.len() == 1);
+}
+
+#[test]
+fn should_prune_associated_impls_with_no_pub_methods() {
+    let doc = test::mk_doc(
+        ~"priv impl Foo {\
+          fn baz() { }\
+          }");
+    fail_unless!(doc.cratemod().impls().is_empty());
+}
+
+#[test]
+fn should_not_prune_associated_impls_with_pub_methods() {
+    let doc = test::mk_doc(
+        ~" \
+          impl Foo { pub fn bar() { } } \
+          ");
+    fail_unless!(!doc.cratemod().impls().is_empty());
+}
+
+
 #[cfg(test)]
 pub mod test {
     use astsrv;
     use doc;
     use extract;
+    use tystr_pass;
     use prune_private_pass::run;
 
     pub fn mk_doc(source: ~str) -> doc::Doc {
         do astsrv::from_str(copy source) |srv| {
             let doc = extract::from_srv(srv.clone(), ~"");
+            let doc = tystr_pass::run(srv.clone(), doc);
             run(srv.clone(), doc)
         }
     }