From 1f7eb4f9aae43cb1177e2cf67a799a63b911ea78 Mon Sep 17 00:00:00 2001 From: David Creswick Date: Mon, 11 Nov 2013 22:17:47 -0600 Subject: [PATCH] make missing_doc lint respect the visibility rules Previously, the `exported_items` set created by the privacy pass was incomplete. Specifically, it did not include items that had been defined at a private path but then `pub use`d at a public path. This commit finds all crate exports during the privacy pass. Consequently, some code in the reachable pass and in rustdoc is no longer necessary. This commit then removes the separate `MissingDocLintVisitor` lint pass, opting to check missing_doc lint in the same pass as the other lint checkers using the visibility result computed by the privacy pass. Fixes #9777. --- src/librustc/driver/driver.rs | 5 +- src/librustc/middle/lint.rs | 243 +++++++++++----------- src/librustc/middle/privacy.rs | 78 +++++-- src/librustc/middle/reachable.rs | 38 +--- src/librustdoc/core.rs | 16 +- src/librustdoc/passes.rs | 39 +--- src/libsyntax/visit.rs | 32 +-- src/test/compile-fail/lint-missing-doc.rs | 44 +++- 8 files changed, 252 insertions(+), 243 deletions(-) diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs index 34c03ac8e42..772b322a0e5 100644 --- a/src/librustc/driver/driver.rs +++ b/src/librustc/driver/driver.rs @@ -305,11 +305,10 @@ pub fn phase_3_run_analysis_passes(sess: Session, let reachable_map = time(time_passes, "reachability checking", (), |_| - reachable::find_reachable(ty_cx, method_map, exp_map2, - &exported_items)); + reachable::find_reachable(ty_cx, method_map, &exported_items)); time(time_passes, "lint checking", (), |_| - lint::check_crate(ty_cx, crate)); + lint::check_crate(ty_cx, &exported_items, crate)); CrateAnalysis { exp_map2: exp_map2, diff --git a/src/librustc/middle/lint.rs b/src/librustc/middle/lint.rs index 4517e19f48e..64438906dae 100644 --- a/src/librustc/middle/lint.rs +++ b/src/librustc/middle/lint.rs @@ -34,6 +34,7 @@ //! Context itself, span_lint should be used instead of add_lint. use driver::session; +use middle::privacy; use middle::trans::adt; // for `adt::is_ffi_safe` use middle::ty; use middle::pat_util; @@ -317,13 +318,20 @@ pub fn get_lint_dict() -> LintDict { return map; } -struct Context { +struct Context<'self> { // All known lint modes (string versions) dict: @LintDict, // Current levels of each lint warning cur: SmallIntMap<(level, LintSource)>, // context we're checking in (used to access fields like sess) tcx: ty::ctxt, + // Items exported by the crate; used by the missing_doc lint. + exported_items: &'self privacy::ExportedItems, + // The id of the current `ast::struct_def` being walked. + cur_struct_def_id: ast::NodeId, + // Whether some ancestor of the current node was marked + // #[doc(hidden)]. + is_doc_hidden: bool, // When recursing into an attributed node of the ast which modifies lint // levels, this stack keeps track of the previous lint levels of whatever @@ -331,7 +339,7 @@ struct Context { lint_stack: ~[(lint, level, LintSource)], } -impl Context { +impl<'self> Context<'self> { fn get_level(&self, lint: lint) -> level { match self.cur.find(&(lint as uint)) { Some(&(lvl, _)) => lvl, @@ -440,9 +448,16 @@ impl Context { true }; + let old_is_doc_hidden = self.is_doc_hidden; + self.is_doc_hidden = self.is_doc_hidden || + attrs.iter().any(|attr| ("doc" == attr.name() && match attr.meta_item_list() + { None => false, + Some(l) => attr::contains_name(l, "hidden") })); + f(self); // rollback + self.is_doc_hidden = old_is_doc_hidden; do pushed.times { let (lint, lvl, src) = self.lint_stack.pop(); self.set_level(lint, lvl, src); @@ -870,123 +885,81 @@ fn check_unnecessary_allocation(cx: &Context, e: &ast::Expr) { } } -struct MissingDocLintVisitor(ty::ctxt); +fn check_missing_doc_attrs(cx: &Context, + id: ast::NodeId, + attrs: &[ast::Attribute], + sp: Span, + desc: &'static str) { + // If we're building a test harness, then warning about + // documentation is probably not really relevant right now. + if cx.tcx.sess.opts.test { return } -impl MissingDocLintVisitor { - fn check_attrs(&self, attrs: &[ast::Attribute], id: ast::NodeId, - sp: Span, msg: ~str) { - if !attrs.iter().any(|a| a.node.is_sugared_doc) { - self.sess.add_lint(missing_doc, id, sp, msg); - } - } + // `#[doc(hidden)]` disables missing_doc check. + if cx.is_doc_hidden { return } - fn check_struct(&self, sdef: &ast::struct_def) { - for field in sdef.fields.iter() { - match field.node.kind { - ast::named_field(_, vis) if vis != ast::private => { - self.check_attrs(field.node.attrs, field.node.id, field.span, - ~"missing documentation for a field"); - } - ast::unnamed_field | ast::named_field(*) => {} - } - } - } + // Only check publicly-visible items, using the result from the + // privacy pass. + if !cx.exported_items.contains(&id) { return } - fn doc_hidden(&self, attrs: &[ast::Attribute]) -> bool { - do attrs.iter().any |attr| { - "doc" == attr.name() && - match attr.meta_item_list() { - Some(l) => attr::contains_name(l, "hidden"), - None => false // not of the form #[doc(...)] - } - } + if !attrs.iter().any(|a| a.node.is_sugared_doc) { + cx.span_lint(missing_doc, sp, + format!("missing documentation for {}", desc)); } } -impl Visitor<()> for MissingDocLintVisitor { - fn visit_ty_method(&mut self, m:&ast::TypeMethod, _: ()) { - if self.doc_hidden(m.attrs) { return } +fn check_missing_doc_item(cx: &mut Context, it: &ast::item) { // XXX doesn't need to be mut + let desc = match it.node { + ast::item_fn(*) => "a function", + ast::item_mod(*) => "a module", + ast::item_enum(*) => "an enum", + ast::item_struct(*) => "a struct", + ast::item_trait(*) => "a trait", + _ => return + }; + check_missing_doc_attrs(cx, it.id, it.attrs, it.span, desc); +} - // All ty_method objects are linted about because they're part of a - // trait (no visibility) - self.check_attrs(m.attrs, m.id, m.span, - ~"missing documentation for a method"); - visit::walk_ty_method(self, m, ()); - } - - fn visit_fn(&mut self, fk: &visit::fn_kind, d: &ast::fn_decl, - b: &ast::Block, sp: Span, id: ast::NodeId, _: ()) { - // Only warn about explicitly public methods. - match *fk { - visit::fk_method(_, _, m) => { - if self.doc_hidden(m.attrs) { - return; - } - // If we're in a trait implementation, no need to duplicate - // documentation - if m.vis == ast::public { - self.check_attrs(m.attrs, id, sp, - ~"missing documentation for a method"); - } - } - _ => {} - } - visit::walk_fn(self, fk, d, b, sp, id, ()); - } - - fn visit_item(&mut self, it: @ast::item, _: ()) { - // If we're building a test harness, then warning about documentation is - // probably not really relevant right now - if self.sess.opts.test { return } - if self.doc_hidden(it.attrs) { return } - - match it.node { - ast::item_struct(sdef, _) if it.vis == ast::public => { - self.check_attrs(it.attrs, it.id, it.span, - ~"missing documentation for a struct"); - self.check_struct(sdef); - } - - // Skip implementations because they inherit documentation from the - // trait (which was already linted) - ast::item_impl(_, Some(*), _, _) => return, - - ast::item_trait(*) if it.vis != ast::public => return, - ast::item_trait(*) => self.check_attrs(it.attrs, it.id, it.span, - ~"missing documentation for a trait"), - - ast::item_fn(*) if it.vis == ast::public => { - self.check_attrs(it.attrs, it.id, it.span, - ~"missing documentation for a function"); - } - - ast::item_mod(*) if it.vis == ast::public => { - self.check_attrs(it.attrs, it.id, it.span, - ~"missing documentation for a module"); - } - - ast::item_enum(ref edef, _) if it.vis == ast::public => { - self.check_attrs(it.attrs, it.id, it.span, - ~"missing documentation for an enum"); - for variant in edef.variants.iter() { - if variant.node.vis == ast::private { continue; } - - self.check_attrs(variant.node.attrs, variant.node.id, - variant.span, - ~"missing documentation for a variant"); - match variant.node.kind { - ast::struct_variant_kind(sdef) => { - self.check_struct(sdef); - } - _ => () +fn check_missing_doc_method(cx: &Context, m: &ast::method) { + let did = ast::DefId { + crate: ast::LOCAL_CRATE, + node: m.id + }; + match cx.tcx.methods.find(&did) { + None => cx.tcx.sess.span_bug(m.span, "missing method descriptor?!"), + Some(md) => { + match md.container { + // Always check default methods defined on traits. + ty::TraitContainer(*) => {} + // For methods defined on impls, it depends on whether + // it is an implementation for a trait or is a plain + // impl. + ty::ImplContainer(cid) => { + match ty::impl_trait_ref(cx.tcx, cid) { + Some(*) => return, // impl for trait: don't doc + None => {} // plain impl: doc according to privacy } } } - - _ => {} } - visit::walk_item(self, it, ()); } + check_missing_doc_attrs(cx, m.id, m.attrs, m.span, "a method"); +} + +fn check_missing_doc_ty_method(cx: &Context, tm: &ast::TypeMethod) { + check_missing_doc_attrs(cx, tm.id, tm.attrs, tm.span, "a type method"); +} + +fn check_missing_doc_struct_field(cx: &Context, sf: &ast::struct_field) { + match sf.node.kind { + ast::named_field(_, vis) if vis != ast::private => + check_missing_doc_attrs(cx, cx.cur_struct_def_id, sf.node.attrs, + sf.span, "a struct field"), + _ => {} + } +} + +fn check_missing_doc_variant(cx: &Context, v: &ast::variant) { + check_missing_doc_attrs(cx, v.node.id, v.node.attrs, v.span, "a variant"); } /// Checks for use of items with #[deprecated], #[experimental] and @@ -1062,13 +1035,14 @@ fn check_stability(cx: &Context, e: &ast::Expr) { cx.span_lint(lint, e.span, msg); } -impl Visitor<()> for Context { +impl<'self> Visitor<()> for Context<'self> { fn visit_item(&mut self, it: @ast::item, _: ()) { do self.with_lint_attrs(it.attrs) |cx| { check_item_ctypes(cx, it); check_item_non_camel_case_types(cx, it); check_item_non_uppercase_statics(cx, it); check_heap_item(cx, it); + check_missing_doc_item(cx, it); do cx.visit_ids |v| { v.visit_item(it, ()); @@ -1111,6 +1085,8 @@ impl Visitor<()> for Context { match *fk { visit::fk_method(_, _, m) => { do self.with_lint_attrs(m.attrs) |cx| { + check_missing_doc_method(cx, m); + do cx.visit_ids |v| { v.visit_fn(fk, decl, body, span, id, ()); } @@ -1120,9 +1096,45 @@ impl Visitor<()> for Context { _ => recurse(self), } } + + fn visit_ty_method(&mut self, t: &ast::TypeMethod, _: ()) { + do self.with_lint_attrs(t.attrs) |cx| { + check_missing_doc_ty_method(cx, t); + + visit::walk_ty_method(cx, t, ()); + } + } + + fn visit_struct_def(&mut self, + s: @ast::struct_def, + i: ast::Ident, + g: &ast::Generics, + id: ast::NodeId, + _: ()) { + let old_id = self.cur_struct_def_id; + self.cur_struct_def_id = id; + visit::walk_struct_def(self, s, i, g, id, ()); + self.cur_struct_def_id = old_id; + } + + fn visit_struct_field(&mut self, s: @ast::struct_field, _: ()) { + do self.with_lint_attrs(s.node.attrs) |cx| { + check_missing_doc_struct_field(cx, s); + + visit::walk_struct_field(cx, s, ()); + } + } + + fn visit_variant(&mut self, v: &ast::variant, g: &ast::Generics, _: ()) { + do self.with_lint_attrs(v.node.attrs) |cx| { + check_missing_doc_variant(cx, v); + + visit::walk_variant(cx, v, g, ()); + } + } } -impl ast_util::IdVisitingOperation for Context { +impl<'self> ast_util::IdVisitingOperation for Context<'self> { fn visit_id(&self, id: ast::NodeId) { match self.tcx.sess.lints.pop(&id) { None => {} @@ -1135,17 +1147,16 @@ impl ast_util::IdVisitingOperation for Context { } } -pub fn check_crate(tcx: ty::ctxt, crate: &ast::Crate) { - // This visitor contains more state than is currently maintained in Context, - // and there's no reason for the Context to keep track of this information - // really - let mut dox = MissingDocLintVisitor(tcx); - visit::walk_crate(&mut dox, crate, ()); - +pub fn check_crate(tcx: ty::ctxt, + exported_items: &privacy::ExportedItems, + crate: &ast::Crate) { let mut cx = Context { dict: @get_lint_dict(), cur: SmallIntMap::new(), tcx: tcx, + exported_items: exported_items, + cur_struct_def_id: -1, + is_doc_hidden: false, lint_stack: ~[], }; diff --git a/src/librustc/middle/privacy.rs b/src/librustc/middle/privacy.rs index eeedd25adac..d516483dec3 100644 --- a/src/librustc/middle/privacy.rs +++ b/src/librustc/middle/privacy.rs @@ -1,4 +1,4 @@ -// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// Copyright 2012-2013 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -31,8 +31,7 @@ use syntax::visit::Visitor; type Context<'self> = (&'self method_map, &'self resolve::ExportMap2); -// A set of all nodes in the ast which can be considered "publicly exported" in -// the sense that they are accessible from anywhere in any hierarchy. +/// A set of AST nodes exported by the crate. pub type ExportedItems = HashSet; // This visitor is used to determine the parent of all nodes in question when it @@ -141,11 +140,28 @@ impl<'self> Visitor<()> for ParentVisitor<'self> { // This visitor is used to determine which items of the ast are embargoed, // otherwise known as not exported. struct EmbargoVisitor<'self> { + tcx: ty::ctxt, + // A set of all nodes in the ast which can be considered "publicly + // exported" in the sense that they are accessible from anywhere + // in any hierarchy. They are public items whose ancestors are all + // public. + path_all_public_items: &'self mut ExportedItems, + // A set of all nodes in the ast that can be reached via a public + // path. This includes everything in `path_all_public_items` as + // well as re-exported private nodes (`pub use`ing a private + // path). exported_items: &'self mut ExportedItems, exp_map2: &'self resolve::ExportMap2, path_all_public: bool, } +impl<'self> EmbargoVisitor<'self> { + fn add_path_all_public_item(&mut self, id: ast::NodeId) { + self.path_all_public_items.insert(id); + self.exported_items.insert(id); + } +} + impl<'self> Visitor<()> for EmbargoVisitor<'self> { fn visit_item(&mut self, item: @ast::item, _: ()) { let orig_all_pub = self.path_all_public; @@ -162,7 +178,7 @@ impl<'self> Visitor<()> for EmbargoVisitor<'self> { } if self.path_all_public { - self.exported_items.insert(item.id); + self.add_path_all_public_item(item.id); } match item.node { @@ -171,7 +187,7 @@ impl<'self> Visitor<()> for EmbargoVisitor<'self> { ast::item_enum(ref def, _) if self.path_all_public => { for variant in def.variants.iter() { if variant.node.vis != ast::private { - self.exported_items.insert(variant.node.id); + self.add_path_all_public_item(variant.node.id); } } } @@ -184,7 +200,7 @@ impl<'self> Visitor<()> for EmbargoVisitor<'self> { _ => true, } && method.vis == ast::public; if public { - self.exported_items.insert(method.id); + self.add_path_all_public_item(method.id); } } } @@ -193,7 +209,7 @@ impl<'self> Visitor<()> for EmbargoVisitor<'self> { ast::item_impl(_, Some(*), _, ref methods) => { for method in methods.iter() { debug!("exporting: {}", method.id); - self.exported_items.insert(method.id); + self.add_path_all_public_item(method.id); } } @@ -204,21 +220,20 @@ impl<'self> Visitor<()> for EmbargoVisitor<'self> { match *method { ast::provided(ref m) => { debug!("provided {}", m.id); - self.exported_items.insert(m.id); + self.add_path_all_public_item(m.id); } ast::required(ref m) => { debug!("required {}", m.id); - self.exported_items.insert(m.id); + self.add_path_all_public_item(m.id); } } } } - // Default methods on traits are all public so long as the trait is - // public + // Struct constructors are public if the struct is all public. ast::item_struct(ref def, _) if self.path_all_public => { match def.ctor_id { - Some(id) => { self.exported_items.insert(id); } + Some(id) => { self.add_path_all_public_item(id); } None => {} } } @@ -233,17 +248,36 @@ impl<'self> Visitor<()> for EmbargoVisitor<'self> { fn visit_foreign_item(&mut self, a: @ast::foreign_item, _: ()) { if self.path_all_public && a.vis == ast::public { - self.exported_items.insert(a.id); + self.add_path_all_public_item(a.id); } } + + fn visit_mod(&mut self, m: &ast::_mod, sp: Span, id: ast::NodeId, _: ()) { + // This code is here instead of in visit_item so that the + // crate module gets processed as well. + if self.path_all_public { + match self.exp_map2.find(&id) { + Some(exports) => { + for export in exports.iter() { + if is_local(export.def_id) && export.reexport { + self.exported_items.insert(export.def_id.node); + } + } + } + None => self.tcx.sess.span_bug(sp, "missing exp_map2 entry \ + for module"), + } + } + visit::walk_mod(self, m, ()) + } } struct PrivacyVisitor<'self> { tcx: ty::ctxt, curitem: ast::NodeId, - // Results of previous analyses necessary for privacy checking. - exported_items: &'self ExportedItems, + // See comments on the same field in `EmbargoVisitor`. + path_all_public_items: &'self ExportedItems, method_map: &'self method_map, parents: &'self HashMap, external_exports: resolve::ExternalExports, @@ -303,7 +337,7 @@ impl<'self> PrivacyVisitor<'self> { ExternallyDenied } }; - } else if self.exported_items.contains(&did.node) { + } else if self.path_all_public_items.contains(&did.node) { debug!("privacy - exported item {}", self.nodestr(did.node)); return Allowable; } @@ -842,6 +876,7 @@ pub fn check_crate(tcx: ty::ctxt, last_private_map: resolve::LastPrivateMap, crate: &ast::Crate) -> ExportedItems { let mut parents = HashMap::new(); + let mut path_all_public_items = HashSet::new(); let mut exported_items = HashSet::new(); // First, figure out who everyone's parent is @@ -855,14 +890,16 @@ pub fn check_crate(tcx: ty::ctxt, // Next, build up the list of all exported items from this crate { - // Initialize the exported items with resolve's id for the "root crate" - // to resolve references to `super` leading to the root and such. - exported_items.insert(ast::CRATE_NODE_ID); let mut visitor = EmbargoVisitor { + tcx: tcx, + path_all_public_items: &mut path_all_public_items, exported_items: &mut exported_items, exp_map2: exp_map2, path_all_public: true, // start out as public }; + // Initialize the exported items with resolve's id for the "root crate" + // to resolve references to `super` leading to the root and such. + visitor.add_path_all_public_item(ast::CRATE_NODE_ID); visit::walk_crate(&mut visitor, crate, ()); } @@ -871,7 +908,7 @@ pub fn check_crate(tcx: ty::ctxt, let mut visitor = PrivacyVisitor { curitem: ast::DUMMY_NODE_ID, tcx: tcx, - exported_items: &exported_items, + path_all_public_items: &path_all_public_items, parents: &parents, method_map: method_map, external_exports: external_exports, @@ -879,5 +916,6 @@ pub fn check_crate(tcx: ty::ctxt, }; visit::walk_crate(&mut visitor, crate, ()); } + return exported_items; } diff --git a/src/librustc/middle/reachable.rs b/src/librustc/middle/reachable.rs index 840e3081656..09b3e8163fd 100644 --- a/src/librustc/middle/reachable.rs +++ b/src/librustc/middle/reachable.rs @@ -1,4 +1,4 @@ -// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// Copyright 2012-2013 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -18,7 +18,6 @@ use middle::ty; use middle::typeck; use middle::privacy; -use middle::resolve; use std::hashmap::HashSet; use syntax::ast; @@ -105,8 +104,6 @@ struct ReachableContext { // A worklist of item IDs. Each item ID in this worklist will be inlined // and will be scanned for further references. worklist: @mut ~[ast::NodeId], - // Known reexports of modules - exp_map2: resolve::ExportMap2, } struct MarkSymbolVisitor { @@ -173,14 +170,12 @@ impl Visitor<()> for MarkSymbolVisitor { impl ReachableContext { // Creates a new reachability computation context. - fn new(tcx: ty::ctxt, method_map: typeck::method_map, - exp_map2: resolve::ExportMap2) -> ReachableContext { + fn new(tcx: ty::ctxt, method_map: typeck::method_map) -> ReachableContext { ReachableContext { tcx: tcx, method_map: method_map, reachable_symbols: @mut HashSet::new(), worklist: @mut ~[], - exp_map2: exp_map2, } } @@ -255,19 +250,6 @@ impl ReachableContext { } } - fn propagate_mod(&self, id: ast::NodeId) { - match self.exp_map2.find(&id) { - Some(l) => { - for reexport in l.iter() { - if reexport.reexport && is_local(reexport.def_id) { - self.worklist.push(reexport.def_id.node); - } - } - } - None => {} - } - } - // Step 2: Mark all symbols that the symbols on the worklist touch. fn propagate(&self) { let mut visitor = self.init_visitor(); @@ -292,13 +274,6 @@ impl ReachableContext { } } - // Our recursion into modules involves looking up their - // public reexports and the destinations of those - // exports. Privacy will put them in the worklist, but - // we won't find them in the ast_map, so this is where - // we deal with publicly re-exported items instead. - ast::item_mod(*) => self.propagate_mod(item.id), - // Implementations of exported structs/enums need to get // added to the worklist (as all their methods should be // accessible) @@ -339,7 +314,7 @@ impl ReachableContext { // inherently and their children are already in the // worklist ast::item_static(*) | ast::item_ty(*) | - ast::item_foreign_mod(*) => {} + ast::item_mod(*) | ast::item_foreign_mod(*) => {} _ => { self.tcx.sess.span_bug(item.span, @@ -376,9 +351,7 @@ impl ReachableContext { worklist: {}", desc)) } - None if search_item == ast::CRATE_NODE_ID => { - self.propagate_mod(search_item); - } + None if search_item == ast::CRATE_NODE_ID => {} None => { self.tcx.sess.bug(format!("found unmapped ID in worklist: \ {}", @@ -404,10 +377,9 @@ impl ReachableContext { pub fn find_reachable(tcx: ty::ctxt, method_map: typeck::method_map, - exp_map2: resolve::ExportMap2, exported_items: &privacy::ExportedItems) -> @mut HashSet { - let reachable_context = ReachableContext::new(tcx, method_map, exp_map2); + let reachable_context = ReachableContext::new(tcx, method_map); // Step 1: Seed the worklist with all nodes which were found to be public as // a result of the privacy pass diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 0a30978db2b..c802d41bcee 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -13,14 +13,13 @@ use rustc::{driver, middle}; use rustc::middle::privacy; use syntax::ast; -use syntax::ast_util::is_local; use syntax::diagnostic; use syntax::parse; use syntax; use std::os; use std::local_data; -use std::hashmap::{HashMap,HashSet}; +use std::hashmap::{HashSet}; use visit_ast::RustdocVisitor; use clean; @@ -34,7 +33,6 @@ pub struct DocContext { pub struct CrateAnalysis { exported_items: privacy::ExportedItems, - reexports: HashMap, } /// Parses, resolves, and typechecks the given crate @@ -73,20 +71,12 @@ fn get_ast_and_resolve(cpath: &Path, let mut crate = phase_1_parse_input(sess, cfg.clone(), &input); crate = phase_2_configure_and_expand(sess, cfg, crate); let driver::driver::CrateAnalysis { - exported_items, ty_cx, exp_map2, _ + exported_items, ty_cx, _ } = phase_3_run_analysis_passes(sess, &crate); - let mut reexports = HashMap::new(); - for (&module, nodes) in exp_map2.iter() { - reexports.insert(module, nodes.iter() - .filter(|e| e.reexport && is_local(e.def_id)) - .map(|e| e.def_id.node) - .to_owned_vec()); - } - debug!("crate: {:?}", crate); return (DocContext { crate: crate, tycx: ty_cx, sess: sess }, - CrateAnalysis { reexports: reexports, exported_items: exported_items }); + CrateAnalysis { exported_items: exported_items }); } pub fn run_core (libs: HashSet, path: &Path) -> (clean::Crate, CrateAnalysis) { diff --git a/src/librustdoc/passes.rs b/src/librustdoc/passes.rs index 1e7c42455f2..72e2b1d12a4 100644 --- a/src/librustdoc/passes.rs +++ b/src/librustdoc/passes.rs @@ -16,7 +16,6 @@ use std::local_data; use syntax::ast; -use core; use clean; use clean::Item; use plugins; @@ -59,17 +58,7 @@ pub fn strip_private(crate: clean::Crate) -> plugins::PluginResult { let mut retained = HashSet::new(); let crate = Cell::new(crate); let exported_items = do local_data::get(super::analysiskey) |analysis| { - let analysis = analysis.unwrap(); - let mut exported_items = analysis.exported_items.clone(); - { - let mut finder = ExportedItemsFinder { - exported_items: &mut exported_items, - analysis: analysis, - }; - let c = finder.fold_crate(crate.take()); - crate.put_back(c); - } - exported_items + analysis.unwrap().exported_items.clone() }; let mut crate = crate.take(); @@ -90,32 +79,6 @@ pub fn strip_private(crate: clean::Crate) -> plugins::PluginResult { (crate, None) } -struct ExportedItemsFinder<'self> { - exported_items: &'self mut HashSet, - analysis: &'self core::CrateAnalysis, -} - -impl<'self> fold::DocFolder for ExportedItemsFinder<'self> { - fn fold_item(&mut self, i: Item) -> Option { - match i.inner { - clean::ModuleItem(*) => { - if self.analysis.exported_items.contains(&i.id) { - match self.analysis.reexports.find(&i.id) { - Some(l) => { - for &id in l.iter() { - self.exported_items.insert(id); - } - } - None => {} - } - } - } - _ => {} - } - return self.fold_item_recur(i); - } -} - struct Stripper<'self> { retained: &'self mut HashSet, exported_items: &'self HashSet, diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index 60127be87b6..a195bfb7717 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -90,6 +90,7 @@ pub trait Visitor { walk_struct_def(self, s, i, g, n, e) } fn visit_struct_field(&mut self, s:@struct_field, e:E) { walk_struct_field(self, s, e) } + fn visit_variant(&mut self, v:&variant, g:&Generics, e:E) { walk_variant(self, v, g, e) } fn visit_opt_lifetime_ref(&mut self, _span: Span, opt_lifetime: &Option, @@ -234,20 +235,27 @@ pub fn walk_enum_def>(visitor: &mut V, generics: &Generics, env: E) { for variant in enum_definition.variants.iter() { - match variant.node.kind { - tuple_variant_kind(ref variant_arguments) => { - for variant_argument in variant_arguments.iter() { - visitor.visit_ty(&variant_argument.ty, env.clone()) - } - } - struct_variant_kind(struct_definition) => { - visitor.visit_struct_def(struct_definition, - variant.node.name, - generics, - variant.node.id, - env.clone()) + visitor.visit_variant(variant, generics, env.clone()); + } +} + +pub fn walk_variant>(visitor:&mut V, + variant: &variant, + generics: &Generics, + env: E) { + match variant.node.kind { + tuple_variant_kind(ref variant_arguments) => { + for variant_argument in variant_arguments.iter() { + visitor.visit_ty(&variant_argument.ty, env.clone()) } } + struct_variant_kind(struct_definition) => { + visitor.visit_struct_def(struct_definition, + variant.node.name, + generics, + variant.node.id, + env.clone()) + } } } diff --git a/src/test/compile-fail/lint-missing-doc.rs b/src/test/compile-fail/lint-missing-doc.rs index 463fd352c5a..c866462a3e9 100644 --- a/src/test/compile-fail/lint-missing-doc.rs +++ b/src/test/compile-fail/lint-missing-doc.rs @@ -11,6 +11,7 @@ // When denying at the crate level, be sure to not get random warnings from the // injected intrinsics by the compiler. #[feature(struct_variant)]; +#[feature(globs)]; #[deny(missing_doc)]; struct Foo { @@ -39,16 +40,21 @@ fn foo3() {} #[allow(missing_doc)] pub fn foo4() {} /// dox -pub trait A {} -trait B {} -pub trait C {} //~ ERROR: missing documentation -#[allow(missing_doc)] pub trait D {} - -trait Bar { +pub trait A { + /// dox fn foo(); - fn foo_with_impl() { - } + /// dox + fn foo_with_impl() {} } +trait B { + fn foo(); + fn foo_with_impl() {} +} +pub trait C { //~ ERROR: missing documentation + fn foo(); //~ ERROR: missing documentation + fn foo_with_impl() {} //~ ERROR: missing documentation +} +#[allow(missing_doc)] pub trait D {} impl Foo { pub fn foo() {} //~ ERROR: missing documentation @@ -120,4 +126,26 @@ pub enum PubBaz3 { #[doc(hidden)] pub fn baz() {} +mod internal_impl { + /// dox + pub fn documented() {} + pub fn undocumented1() {} //~ ERROR: missing documentation + pub fn undocumented2() {} //~ ERROR: missing documentation + fn undocumented3() {} + /// dox + pub mod globbed { + /// dox + pub fn also_documented() {} + pub fn also_undocumented1() {} //~ ERROR: missing documentation + fn also_undocumented2() {} + } +} +/// dox +pub mod public_interface { + pub use foo = internal_impl::documented; + pub use bar = internal_impl::undocumented1; + pub use internal_impl::{documented, undocumented2}; + pub use internal_impl::globbed::*; +} + fn main() {}