rustdoc: Handle impl method visibility correctly. rust-lang#5533

Author: brson

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)
         }
     }