From b1543a1aac0c9443e509e00ed6cddcc9b9400ac3 Mon Sep 17 00:00:00 2001 From: mitaa Date: Thu, 31 Mar 2016 18:15:54 +0200 Subject: [PATCH 1/3] Make the rendering process less pass-aware Instead of hardcoding knowledge about the strip-private pass into the rendering process we represent (some) stripped items as `ItemEnum::StrippedItem`. Rustdoc will, for example, generate redirect pages for public items contained in private modules which have been re-exported to somewhere externally reachable - this will now not only work for the `strip-private` pass, but for other passes as well, such as the `strip-hidden` pass. --- src/librustdoc/clean/mod.rs | 29 ++-- src/librustdoc/fold.rs | 56 ++++++-- src/librustdoc/html/item_type.rs | 8 +- src/librustdoc/html/render.rs | 190 ++++++++++++--------------- src/librustdoc/passes.rs | 40 +++--- src/librustdoc/test.rs | 4 +- src/test/auxiliary/reexp_stripped.rs | 21 +++ src/test/rustdoc/redirect.rs | 48 +++++++ 8 files changed, 247 insertions(+), 149 deletions(-) create mode 100644 src/test/auxiliary/reexp_stripped.rs create mode 100644 src/test/rustdoc/redirect.rs diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index b26e56008ac..9c60b90a1fc 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -53,6 +53,7 @@ use std::env::current_dir; use core::DocContext; use doctree; use visit_ast; +use html::item_type::ItemType; /// A stable identifier to the particular version of JSON output. /// Increment this when the `Crate` and related structures change. @@ -273,36 +274,40 @@ impl Item { } pub fn is_crate(&self) -> bool { match self.inner { - ModuleItem(Module { items: _, is_crate: true }) => true, - _ => false + StrippedItem(box ModuleItem(Module { is_crate: true, ..})) | + ModuleItem(Module { is_crate: true, ..}) => true, + _ => false, } } pub fn is_mod(&self) -> bool { - match self.inner { ModuleItem(..) => true, _ => false } + ItemType::from_item(self) == ItemType::Module } pub fn is_trait(&self) -> bool { - match self.inner { TraitItem(..) => true, _ => false } + ItemType::from_item(self) == ItemType::Trait } pub fn is_struct(&self) -> bool { - match self.inner { StructItem(..) => true, _ => false } + ItemType::from_item(self) == ItemType::Struct } pub fn is_enum(&self) -> bool { - match self.inner { EnumItem(..) => true, _ => false } + ItemType::from_item(self) == ItemType::Module } pub fn is_fn(&self) -> bool { - match self.inner { FunctionItem(..) => true, _ => false } + ItemType::from_item(self) == ItemType::Function } pub fn is_associated_type(&self) -> bool { - match self.inner { AssociatedTypeItem(..) => true, _ => false } + ItemType::from_item(self) == ItemType::AssociatedType } pub fn is_associated_const(&self) -> bool { - match self.inner { AssociatedConstItem(..) => true, _ => false } + ItemType::from_item(self) == ItemType::AssociatedConst } pub fn is_method(&self) -> bool { - match self.inner { MethodItem(..) => true, _ => false } + ItemType::from_item(self) == ItemType::Method } pub fn is_ty_method(&self) -> bool { - match self.inner { TyMethodItem(..) => true, _ => false } + ItemType::from_item(self) == ItemType::TyMethod + } + pub fn is_stripped(&self) -> bool { + match self.inner { StrippedItem(..) => true, _ => false } } pub fn stability_class(&self) -> String { @@ -352,6 +357,8 @@ pub enum ItemEnum { AssociatedConstItem(Type, Option), AssociatedTypeItem(Vec, Option), DefaultImplItem(DefaultImpl), + /// An item that has been stripped by a rustdoc pass + StrippedItem(Box), } #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] diff --git a/src/librustdoc/fold.rs b/src/librustdoc/fold.rs index ceec80402c0..5595c749256 100644 --- a/src/librustdoc/fold.rs +++ b/src/librustdoc/fold.rs @@ -10,28 +10,50 @@ use clean::*; +pub enum FoldItem { + Retain(Item), + Strip(Item), + Erase, +} + +impl FoldItem { + pub fn fold(self) -> Option { + match self { + FoldItem::Erase => None, + FoldItem::Retain(i) => Some(i), + FoldItem::Strip(item@ Item { inner: StrippedItem(..), .. } ) => Some(item), + FoldItem::Strip(mut i) => { + i.inner = StrippedItem(box i.inner); + Some(i) + } + } + } +} + pub trait DocFolder : Sized { fn fold_item(&mut self, item: Item) -> Option { self.fold_item_recur(item) } /// don't override! - fn fold_item_recur(&mut self, item: Item) -> Option { - let Item { attrs, name, source, visibility, def_id, inner, stability, deprecation } = item; - let inner = match inner { + fn fold_inner_recur(&mut self, inner: ItemEnum) -> ItemEnum { + match inner { + StrippedItem(..) => unreachable!(), + ModuleItem(i) => { + ModuleItem(self.fold_mod(i)) + }, StructItem(mut i) => { let num_fields = i.fields.len(); i.fields = i.fields.into_iter().filter_map(|x| self.fold_item(x)).collect(); - i.fields_stripped |= num_fields != i.fields.len(); + i.fields_stripped |= num_fields != i.fields.len() || + i.fields.iter().any(|f| f.is_stripped()); StructItem(i) }, - ModuleItem(i) => { - ModuleItem(self.fold_mod(i)) - }, EnumItem(mut i) => { let num_variants = i.variants.len(); i.variants = i.variants.into_iter().filter_map(|x| self.fold_item(x)).collect(); - i.variants_stripped |= num_variants != i.variants.len(); + i.variants_stripped |= num_variants != i.variants.len() || + i.variants.iter().any(|f| f.is_stripped()); EnumItem(i) }, TraitItem(mut i) => { @@ -48,13 +70,24 @@ pub trait DocFolder : Sized { StructVariant(mut j) => { let num_fields = j.fields.len(); j.fields = j.fields.into_iter().filter_map(|x| self.fold_item(x)).collect(); - j.fields_stripped |= num_fields != j.fields.len(); + j.fields_stripped |= num_fields != j.fields.len() || + j.fields.iter().any(|f| f.is_stripped()); VariantItem(Variant {kind: StructVariant(j), ..i2}) }, _ => VariantItem(i2) } }, x => x + } + } + + /// don't override! + fn fold_item_recur(&mut self, item: Item) -> Option { + let Item { attrs, name, source, visibility, def_id, inner, stability, deprecation } = item; + + let inner = match inner { + StrippedItem(box i) => StrippedItem(box self.fold_inner_recur(i)), + _ => self.fold_inner_recur(inner), }; Some(Item { attrs: attrs, name: name, source: source, inner: inner, @@ -70,9 +103,8 @@ pub trait DocFolder : Sized { } fn fold_crate(&mut self, mut c: Crate) -> Crate { - c.module = c.module.and_then(|module| { - self.fold_item(module) - }); + c.module = c.module.and_then(|module| self.fold_item(module)); + c.external_traits = c.external_traits.into_iter().map(|(k, mut v)| { v.items = v.items.into_iter().filter_map(|i| self.fold_item(i)).collect(); (k, v) diff --git a/src/librustdoc/html/item_type.rs b/src/librustdoc/html/item_type.rs index afc93f41172..74f7b099044 100644 --- a/src/librustdoc/html/item_type.rs +++ b/src/librustdoc/html/item_type.rs @@ -44,7 +44,12 @@ pub enum ItemType { impl ItemType { pub fn from_item(item: &clean::Item) -> ItemType { - match item.inner { + let inner = match item.inner { + clean::StrippedItem(box ref item) => item, + ref inner@_ => inner, + }; + + match *inner { clean::ModuleItem(..) => ItemType::Module, clean::ExternCrateItem(..) => ItemType::ExternCrate, clean::ImportItem(..) => ItemType::Import, @@ -67,6 +72,7 @@ impl ItemType { clean::AssociatedConstItem(..) => ItemType::AssociatedConst, clean::AssociatedTypeItem(..) => ItemType::AssociatedType, clean::DefaultImplItem(..) => ItemType::Impl, + clean::StrippedItem(..) => unreachable!(), } } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 0f436efd70b..782b3cc52e8 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -245,8 +245,7 @@ pub struct Cache { parent_stack: Vec, parent_is_trait_impl: bool, search_index: Vec, - privmod: bool, - remove_priv: bool, + stripped_mod: bool, access_levels: AccessLevels, deref_trait_did: Option, @@ -492,8 +491,7 @@ pub fn run(mut krate: clean::Crate, parent_is_trait_impl: false, extern_locations: HashMap::new(), primitive_locations: HashMap::new(), - remove_priv: cx.passes.contains("strip-private"), - privmod: false, + stripped_mod: false, access_levels: access_levels, orphan_methods: Vec::new(), traits: mem::replace(&mut krate.external_traits, HashMap::new()), @@ -874,7 +872,6 @@ impl<'a> DocFolder for SourceCollector<'a> { } }; } - self.fold_item_recur(item) } } @@ -938,14 +935,15 @@ impl<'a> SourceCollector<'a> { impl DocFolder for Cache { fn fold_item(&mut self, item: clean::Item) -> Option { - // If this is a private module, we don't want it in the search index. - let orig_privmod = match item.inner { - clean::ModuleItem(..) => { - let prev = self.privmod; - self.privmod = prev || (self.remove_priv && item.visibility != Some(hir::Public)); + // If this is a stripped module, + // we don't want it or its children in the search index. + let orig_stripped_mod = match item.inner { + clean::StrippedItem(box clean::ModuleItem(..)) => { + let prev = self.stripped_mod; + self.stripped_mod = true; prev } - _ => self.privmod, + _ => self.stripped_mod, }; // Register any generics to their corresponding string. This is used @@ -983,6 +981,7 @@ impl DocFolder for Cache { // Index this method for searching later on if let Some(ref s) = item.name { let (parent, is_method) = match item.inner { + clean::StrippedItem(..) => ((None, None), false), clean::AssociatedConstItem(..) | clean::TypedefItem(_, true) if self.parent_is_trait_impl => { // skip associated items in trait impls @@ -1027,7 +1026,7 @@ impl DocFolder for Cache { }; match parent { - (parent, Some(path)) if is_method || (!self.privmod && !hidden_field) => { + (parent, Some(path)) if is_method || (!self.stripped_mod && !hidden_field) => { // Needed to determine `self` type. let parent_basename = self.parent_stack.first().and_then(|parent| { match self.paths.get(parent) { @@ -1035,6 +1034,7 @@ impl DocFolder for Cache { _ => None } }); + debug_assert!(!item.is_stripped()); // A crate has a module at its root, containing all items, // which should not be indexed. The crate-item itself is @@ -1051,7 +1051,7 @@ impl DocFolder for Cache { }); } } - (Some(parent), None) if is_method || (!self.privmod && !hidden_field)=> { + (Some(parent), None) if is_method || (!self.stripped_mod && !hidden_field)=> { if parent.is_local() { // We have a parent, but we don't know where they're // defined yet. Wait for later to index this item. @@ -1075,7 +1075,7 @@ impl DocFolder for Cache { clean::StructItem(..) | clean::EnumItem(..) | clean::TypedefItem(..) | clean::TraitItem(..) | clean::FunctionItem(..) | clean::ModuleItem(..) | - clean::ForeignFunctionItem(..) if !self.privmod => { + clean::ForeignFunctionItem(..) if !self.stripped_mod => { // Reexported items mean that the same id can show up twice // in the rustdoc ast that we're looking at. We know, // however, that a reexported item doesn't show up in the @@ -1093,7 +1093,7 @@ impl DocFolder for Cache { } // link variants to their parent enum because pages aren't emitted // for each variant - clean::VariantItem(..) if !self.privmod => { + clean::VariantItem(..) if !self.stripped_mod => { let mut stack = self.stack.clone(); stack.pop(); self.paths.insert(item.def_id, (stack, ItemType::Enum)); @@ -1176,7 +1176,7 @@ impl DocFolder for Cache { if pushed { self.stack.pop().unwrap(); } if parent_pushed { self.parent_stack.pop().unwrap(); } - self.privmod = orig_privmod; + self.stripped_mod = orig_stripped_mod; self.parent_is_trait_impl = orig_parent_is_trait_impl; return ret; } @@ -1233,15 +1233,12 @@ impl Context { // render the crate documentation let mut work = vec!((self, item)); - loop { - match work.pop() { - Some((mut cx, item)) => cx.item(item, |cx, item| { - work.push((cx.clone(), item)); - })?, - None => break, - } - } + while let Some((mut cx, item)) = work.pop() { + cx.item(item, |cx, item| { + work.push((cx.clone(), item)) + })? + } Ok(()) } @@ -1296,79 +1293,72 @@ impl Context { layout::render(&mut writer, &cx.layout, &page, &Sidebar{ cx: cx, item: it }, &Item{ cx: cx, item: it })?; + } else { let mut url = repeat("../").take(cx.current.len()) .collect::(); - match cache().paths.get(&it.def_id) { - Some(&(ref names, _)) => { - for name in &names[..names.len() - 1] { - url.push_str(name); - url.push_str("/"); - } - url.push_str(&item_path(it)); - layout::redirect(&mut writer, &url)?; + if let Some(&(ref names, _)) = cache().paths.get(&it.def_id) { + for name in &names[..names.len() - 1] { + url.push_str(name); + url.push_str("/"); } - None => {} + url.push_str(&item_path(it)); + layout::redirect(&mut writer, &url)?; } } writer.flush() } - // Private modules may survive the strip-private pass if they - // contain impls for public types. These modules can also + // Stripped modules survive the rustdoc passes (i.e. `strip-private`) + // if they contain impls for public types. These modules can also // contain items such as publicly reexported structures. // // External crates will provide links to these structures, so - // these modules are recursed into, but not rendered normally (a - // flag on the context). + // these modules are recursed into, but not rendered normally + // (a flag on the context). if !self.render_redirect_pages { - self.render_redirect_pages = self.ignore_private_item(&item); + self.render_redirect_pages = self.maybe_ignore_item(&item); } - match item.inner { + if item.is_mod() { // modules are special because they add a namespace. We also need to // recurse into the items of the module as well. - clean::ModuleItem(..) => { - let name = item.name.as_ref().unwrap().to_string(); - let mut item = Some(item); - self.recurse(name, |this| { - let item = item.take().unwrap(); - let joint_dst = this.dst.join("index.html"); - let dst = try_err!(File::create(&joint_dst), &joint_dst); - try_err!(render(dst, this, &item, false), &joint_dst); - - let m = match item.inner { - clean::ModuleItem(m) => m, - _ => unreachable!() - }; - - // render sidebar-items.js used throughout this module - { - let items = this.build_sidebar_items(&m); - let js_dst = this.dst.join("sidebar-items.js"); - let mut js_out = BufWriter::new(try_err!(File::create(&js_dst), &js_dst)); - try_err!(write!(&mut js_out, "initSidebarItems({});", - as_json(&items)), &js_dst); - } - - for item in m.items { - f(this,item); - } - Ok(()) - }) - } - - // Things which don't have names (like impls) don't get special - // pages dedicated to them. - _ if item.name.is_some() => { - let joint_dst = self.dst.join(&item_path(&item)); - + let name = item.name.as_ref().unwrap().to_string(); + let mut item = Some(item); + self.recurse(name, |this| { + let item = item.take().unwrap(); + let joint_dst = this.dst.join("index.html"); let dst = try_err!(File::create(&joint_dst), &joint_dst); - try_err!(render(dst, self, &item, true), &joint_dst); - Ok(()) - } + try_err!(render(dst, this, &item, false), &joint_dst); - _ => Ok(()) + let m = match item.inner { + clean::StrippedItem(box clean::ModuleItem(m)) | + clean::ModuleItem(m) => m, + _ => unreachable!() + }; + + // render sidebar-items.js used throughout this module + { + let items = this.build_sidebar_items(&m); + let js_dst = this.dst.join("sidebar-items.js"); + let mut js_out = BufWriter::new(try_err!(File::create(&js_dst), &js_dst)); + try_err!(write!(&mut js_out, "initSidebarItems({});", + as_json(&items)), &js_dst); + } + + for item in m.items { + f(this,item); + } + Ok(()) + }) + } else if item.name.is_some() { + let joint_dst = self.dst.join(&item_path(&item)); + + let dst = try_err!(File::create(&joint_dst), &joint_dst); + try_err!(render(dst, self, &item, true), &joint_dst); + Ok(()) + } else { + Ok(()) } } @@ -1376,7 +1366,7 @@ impl Context { // BTreeMap instead of HashMap to get a sorted output let mut map = BTreeMap::new(); for item in &m.items { - if self.ignore_private_item(item) { continue } + if self.maybe_ignore_item(item) { continue } let short = shortty(item).to_static_str(); let myname = match item.name { @@ -1394,27 +1384,18 @@ impl Context { return map; } - fn ignore_private_item(&self, it: &clean::Item) -> bool { + fn maybe_ignore_item(&self, it: &clean::Item) -> bool { match it.inner { + clean::StrippedItem(..) => true, clean::ModuleItem(ref m) => { - (m.items.is_empty() && - it.doc_value().is_none() && - it.visibility != Some(hir::Public)) || - (self.passes.contains("strip-private") && it.visibility != Some(hir::Public)) - } - clean::PrimitiveItem(..) => it.visibility != Some(hir::Public), + it.doc_value().is_none() && m.items.is_empty() && it.visibility != Some(hir::Public) + }, _ => false, } } } impl<'a> Item<'a> { - fn ismodule(&self) -> bool { - match self.item.inner { - clean::ModuleItem(..) => true, _ => false - } - } - /// Generate a url appropriate for an `href` attribute back to the source of /// this item. /// @@ -1495,6 +1476,7 @@ impl<'a> Item<'a> { impl<'a> fmt::Display for Item<'a> { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + debug_assert!(!self.item.is_stripped()); // Write the breadcrumb trail header for the top write!(fmt, "\n

")?; match self.item.inner { @@ -1516,7 +1498,7 @@ impl<'a> fmt::Display for Item<'a> { }; if !is_primitive { let cur = &self.cx.current; - let amt = if self.ismodule() { cur.len() - 1 } else { cur.len() }; + let amt = if self.item.is_mod() { cur.len() - 1 } else { cur.len() }; for (i, component) in cur.iter().enumerate().take(amt) { write!(fmt, "{}::", repeat("../").take(cur.len() - i - 1) @@ -1575,15 +1557,12 @@ impl<'a> fmt::Display for Item<'a> { } fn item_path(item: &clean::Item) -> String { - match item.inner { - clean::ModuleItem(..) => { - format!("{}/index.html", item.name.as_ref().unwrap()) - } - _ => { - format!("{}.{}.html", - shortty(item).to_static_str(), - *item.name.as_ref().unwrap()) - } + if item.is_mod() { + format!("{}/index.html", item.name.as_ref().unwrap()) + } else { + format!("{}.{}.html", + shortty(item).to_static_str(), + *item.name.as_ref().unwrap()) } } @@ -1626,7 +1605,7 @@ fn item_module(w: &mut fmt::Formatter, cx: &Context, document(w, cx, item)?; let mut indices = (0..items.len()).filter(|i| { - !cx.ignore_private_item(&items[*i]) + !cx.maybe_ignore_item(&items[*i]) }).collect::>(); // the order of item types in the listing @@ -1670,6 +1649,9 @@ fn item_module(w: &mut fmt::Formatter, cx: &Context, let mut curty = None; for &idx in &indices { let myitem = &items[idx]; + if myitem.is_stripped() { + continue; + } let myty = Some(shortty(myitem)); if curty == Some(ItemType::ExternCrate) && myty == Some(ItemType::Import) { @@ -2146,6 +2128,7 @@ fn render_assoc_item(w: &mut fmt::Formatter, where_clause = WhereClause(g)) } match item.inner { + clean::StrippedItem(..) => Ok(()), clean::TyMethodItem(ref m) => { method(w, item, m.unsafety, hir::Constness::NotConst, m.abi, &m.generics, &m.self_, &m.decl, link) @@ -2540,6 +2523,7 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi assoc_type(w, item, bounds, default.as_ref(), link)?; write!(w, "

\n")?; } + clean::StrippedItem(..) => return Ok(()), _ => panic!("can't make docs for trait item with name {:?}", item.name) } diff --git a/src/librustdoc/passes.rs b/src/librustdoc/passes.rs index 88cb20991d6..06d84fc8822 100644 --- a/src/librustdoc/passes.rs +++ b/src/librustdoc/passes.rs @@ -21,6 +21,7 @@ use clean::Item; use plugins; use fold; use fold::DocFolder; +use fold::FoldItem::Strip; /// Strip items marked `#[doc(hidden)]` pub fn strip_hidden(krate: clean::Crate) -> plugins::PluginResult { @@ -45,12 +46,10 @@ pub fn strip_hidden(krate: clean::Crate) -> plugins::PluginResult { ..i }); } - _ => { - return None; - } + clean::ModuleItem(..) => return Strip(i).fold(), + _ => return None, } } - self.fold_item_recur(i) } } @@ -125,6 +124,7 @@ struct Stripper<'a> { impl<'a> fold::DocFolder for Stripper<'a> { fn fold_item(&mut self, i: Item) -> Option { match i.inner { + clean::StrippedItem(..) => return Some(i), // These items can all get re-exported clean::TypedefItem(..) | clean::StaticItem(..) | clean::StructItem(..) | clean::EnumItem(..) | @@ -153,8 +153,11 @@ impl<'a> fold::DocFolder for Stripper<'a> { } } - // handled below - clean::ModuleItem(..) => {} + clean::ModuleItem(..) => { + if i.def_id.is_local() && i.visibility != Some(hir::Public) { + return Strip(self.fold_item_recur(i).unwrap()).fold() + } + } // trait impls for private items should be stripped clean::ImplItem(clean::Impl{ @@ -165,7 +168,7 @@ impl<'a> fold::DocFolder for Stripper<'a> { } } // handled in the `strip-priv-imports` pass - clean::ExternCrateItem(..) | clean::ImportItem(_) => {} + clean::ExternCrateItem(..) | clean::ImportItem(..) => {} clean::DefaultImplItem(..) | clean::ImplItem(..) => {} @@ -187,7 +190,6 @@ impl<'a> fold::DocFolder for Stripper<'a> { // implementations of traits are always public. clean::ImplItem(ref imp) if imp.trait_.is_some() => true, - // Struct variant fields have inherited visibility clean::VariantItem(clean::Variant { kind: clean::StructVariant(..) @@ -202,19 +204,17 @@ impl<'a> fold::DocFolder for Stripper<'a> { self.fold_item_recur(i) }; - i.and_then(|i| { - match i.inner { - // emptied modules/impls have no need to exist - clean::ModuleItem(ref m) - if m.items.is_empty() && - i.doc_value().is_none() => None, - clean::ImplItem(ref i) if i.items.is_empty() => None, - _ => { - self.retained.insert(i.def_id); - Some(i) - } + i.and_then(|i| { match i.inner { + // emptied modules/impls have no need to exist + clean::ModuleItem(ref m) + if m.items.is_empty() && + i.doc_value().is_none() => None, + clean::ImplItem(ref i) if i.items.is_empty() => None, + _ => { + self.retained.insert(i.def_id); + Some(i) } - }) + }}) } } diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 331e3431cee..5bd3b9c4f59 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -431,7 +431,7 @@ impl Collector { // compiler failures are test failures should_panic: testing::ShouldPanic::No, }, - testfn: testing::DynTestFn(Box::new(move|| { + testfn: testing::DynTestFn(box move|| { runtest(&test, &cratename, cfgs, @@ -442,7 +442,7 @@ impl Collector { as_test_harness, compile_fail, &opts); - })) + }) }); } diff --git a/src/test/auxiliary/reexp_stripped.rs b/src/test/auxiliary/reexp_stripped.rs new file mode 100644 index 00000000000..2b061e3997d --- /dev/null +++ b/src/test/auxiliary/reexp_stripped.rs @@ -0,0 +1,21 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +pub use private::Quz; +pub use hidden::Bar; + +mod private { + pub struct Quz; +} + +#[doc(hidden)] +pub mod hidden { + pub struct Bar; +} diff --git a/src/test/rustdoc/redirect.rs b/src/test/rustdoc/redirect.rs new file mode 100644 index 00000000000..98e66e8c024 --- /dev/null +++ b/src/test/rustdoc/redirect.rs @@ -0,0 +1,48 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// aux-build:reexp_stripped.rs +// build-aux-docs +// ignore-cross-compile + +extern crate reexp_stripped; + +pub trait Foo {} + +// @has redirect/index.html +// @has - '//code' 'pub use reexp_stripped::Bar' +// @has - '//code/a' 'Bar' +// @has reexp_stripped/hidden/struct.Bar.html +// @has - '//p/a' '../../reexp_stripped/struct.Bar.html' +// @has 'reexp_stripped/struct.Bar.html' +#[doc(no_inline)] +pub use reexp_stripped::Bar; +impl Foo for Bar {} + +// @has redirect/index.html +// @has - '//code' 'pub use reexp_stripped::Quz' +// @has - '//code/a' 'Quz' +// @has reexp_stripped/private/struct.Quz.html +// @has - '//p/a' '../../reexp_stripped/struct.Quz.html' +// @has 'reexp_stripped/struct.Quz.html' +#[doc(no_inline)] +pub use reexp_stripped::Quz; +impl Foo for Quz {} + +mod private_no_inline { + pub struct Qux; + impl ::Foo for Qux {} +} + +// @has redirect/index.html +// @has - '//code' 'pub use private_no_inline::Qux' +// @!has - '//code/a' 'Qux' +#[doc(no_inline)] +pub use private_no_inline::Qux; From 0ef85c1e6a573d736592f00402456616a25eee0f Mon Sep 17 00:00:00 2001 From: mitaa Date: Sat, 2 Apr 2016 08:17:59 +0200 Subject: [PATCH 2/3] Refactor `HiddenStructField` into `StrippedItem` --- src/librustdoc/clean/mod.rs | 26 +++++++++---------- src/librustdoc/html/render.rs | 40 ++++++++++------------------- src/librustdoc/passes.rs | 22 ++++------------ src/test/rustdoc/structfields.rs | 44 ++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 56 deletions(-) create mode 100644 src/test/rustdoc/structfields.rs diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 9c60b90a1fc..7437d608771 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -14,7 +14,6 @@ pub use self::Type::*; pub use self::PrimitiveType::*; pub use self::TypeKind::*; -pub use self::StructField::*; pub use self::VariantKind::*; pub use self::Mutability::*; pub use self::Import::*; @@ -309,6 +308,15 @@ impl Item { pub fn is_stripped(&self) -> bool { match self.inner { StrippedItem(..) => true, _ => false } } + pub fn has_stripped_fields(&self) -> Option { + match self.inner { + StructItem(ref _struct) => Some(_struct.fields_stripped), + VariantItem(Variant { kind: StructVariant(ref vstruct)} ) => { + Some(vstruct.fields_stripped) + }, + _ => None, + } + } pub fn stability_class(&self) -> String { self.stability.as_ref().map(|ref s| { @@ -346,7 +354,7 @@ pub enum ItemEnum { TyMethodItem(TyMethod), /// A method with a body. MethodItem(Method), - StructFieldItem(StructField), + StructFieldItem(Type), VariantItem(Variant), /// `fn`s from an extern block ForeignFunctionItem(Function), @@ -1740,12 +1748,6 @@ impl<'tcx> Clean for ty::Ty<'tcx> { } } -#[derive(Clone, RustcEncodable, RustcDecodable, Debug)] -pub enum StructField { - HiddenStructField, // inserted later by strip passes - TypedStructField(Type), -} - impl Clean for hir::StructField { fn clean(&self, cx: &DocContext) -> Item { Item { @@ -1756,7 +1758,7 @@ impl Clean for hir::StructField { stability: get_stability(cx, cx.map.local_def_id(self.id)), deprecation: get_deprecation(cx, cx.map.local_def_id(self.id)), def_id: cx.map.local_def_id(self.id), - inner: StructFieldItem(TypedStructField(self.ty.clean(cx))), + inner: StructFieldItem(self.ty.clean(cx)), } } } @@ -1773,7 +1775,7 @@ impl<'tcx> Clean for ty::FieldDefData<'tcx, 'static> { stability: get_stability(cx, self.did), deprecation: get_deprecation(cx, self.did), def_id: self.did, - inner: StructFieldItem(TypedStructField(self.unsubst_ty().clean(cx))), + inner: StructFieldItem(self.unsubst_ty().clean(cx)), } } } @@ -1904,9 +1906,7 @@ impl<'tcx> Clean for ty::VariantDefData<'tcx, 'static> { def_id: field.did, stability: get_stability(cx, field.did), deprecation: get_deprecation(cx, field.did), - inner: StructFieldItem( - TypedStructField(field.unsubst_ty().clean(cx)) - ) + inner: StructFieldItem(field.unsubst_ty().clean(cx)) } }).collect() }) diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 782b3cc52e8..678a9d75f96 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -1020,13 +1020,9 @@ impl DocFolder for Cache { } _ => ((None, Some(&*self.stack)), false) }; - let hidden_field = match item.inner { - clean::StructFieldItem(clean::HiddenStructField) => true, - _ => false - }; match parent { - (parent, Some(path)) if is_method || (!self.stripped_mod && !hidden_field) => { + (parent, Some(path)) if is_method || (!self.stripped_mod) => { // Needed to determine `self` type. let parent_basename = self.parent_stack.first().and_then(|parent| { match self.paths.get(parent) { @@ -1051,7 +1047,7 @@ impl DocFolder for Cache { }); } } - (Some(parent), None) if is_method || (!self.stripped_mod && !hidden_field)=> { + (Some(parent), None) if is_method || (!self.stripped_mod)=> { if parent.is_local() { // We have a parent, but we don't know where they're // defined yet. Wait for later to index this item. @@ -2165,8 +2161,7 @@ fn item_struct(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item, document(w, cx, it)?; let mut fields = s.fields.iter().filter(|f| { match f.inner { - clean::StructFieldItem(clean::HiddenStructField) => false, - clean::StructFieldItem(clean::TypedStructField(..)) => true, + clean::StructFieldItem(..) => true, _ => false, } }).peekable(); @@ -2256,7 +2251,7 @@ fn item_enum(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item, if let clean::VariantItem( Variant { kind: StructVariant(ref s) } ) = variant.inner { let fields = s.fields.iter().filter(|f| { match f.inner { - clean::StructFieldItem(clean::TypedStructField(..)) => true, + clean::StructFieldItem(..) => true, _ => false, } }); @@ -2315,24 +2310,17 @@ fn render_struct(w: &mut fmt::Formatter, it: &clean::Item, match ty { doctree::Plain => { write!(w, " {{\n{}", tab)?; - let mut fields_stripped = false; for field in fields { - match field.inner { - clean::StructFieldItem(clean::HiddenStructField) => { - fields_stripped = true; - } - clean::StructFieldItem(clean::TypedStructField(ref ty)) => { - write!(w, " {}{}: {},\n{}", - VisSpace(field.visibility), - field.name.as_ref().unwrap(), - *ty, - tab)?; - } - _ => unreachable!(), - }; + if let clean::StructFieldItem(ref ty) = field.inner { + write!(w, " {}{}: {},\n{}", + VisSpace(field.visibility), + field.name.as_ref().unwrap(), + *ty, + tab)?; + } } - if fields_stripped { + if it.has_stripped_fields().unwrap() { write!(w, " // some fields omitted\n{}", tab)?; } write!(w, "}}")?; @@ -2344,10 +2332,10 @@ fn render_struct(w: &mut fmt::Formatter, it: &clean::Item, write!(w, ", ")?; } match field.inner { - clean::StructFieldItem(clean::HiddenStructField) => { + clean::StrippedItem(box clean::StructFieldItem(..)) => { write!(w, "_")? } - clean::StructFieldItem(clean::TypedStructField(ref ty)) => { + clean::StructFieldItem(ref ty) => { write!(w, "{}{}", VisSpace(field.visibility), *ty)? } _ => unreachable!() diff --git a/src/librustdoc/passes.rs b/src/librustdoc/passes.rs index 06d84fc8822..f93ecb46228 100644 --- a/src/librustdoc/passes.rs +++ b/src/librustdoc/passes.rs @@ -40,13 +40,9 @@ pub fn strip_hidden(krate: clean::Crate) -> plugins::PluginResult { // use a dedicated hidden item for given item type if any match i.inner { - clean::StructFieldItem(..) => { - return Some(clean::Item { - inner: clean::StructFieldItem(clean::HiddenStructField), - ..i - }); + clean::StructFieldItem(..) | clean::ModuleItem(..) => { + return Strip(i).fold() } - clean::ModuleItem(..) => return Strip(i).fold(), _ => return None, } } @@ -130,7 +126,8 @@ impl<'a> fold::DocFolder for Stripper<'a> { clean::StructItem(..) | clean::EnumItem(..) | clean::TraitItem(..) | clean::FunctionItem(..) | clean::VariantItem(..) | clean::MethodItem(..) | - clean::ForeignFunctionItem(..) | clean::ForeignStaticItem(..) => { + clean::ForeignFunctionItem(..) | clean::ForeignStaticItem(..) | + clean::ConstantItem(..) => { if i.def_id.is_local() { if !self.access_levels.is_exported(i.def_id) { return None; @@ -138,18 +135,9 @@ impl<'a> fold::DocFolder for Stripper<'a> { } } - clean::ConstantItem(..) => { - if i.def_id.is_local() && !self.access_levels.is_exported(i.def_id) { - return None; - } - } - clean::StructFieldItem(..) => { if i.visibility != Some(hir::Public) { - return Some(clean::Item { - inner: clean::StructFieldItem(clean::HiddenStructField), - ..i - }) + return Strip(i).fold(); } } diff --git a/src/test/rustdoc/structfields.rs b/src/test/rustdoc/structfields.rs new file mode 100644 index 00000000000..c4327f70728 --- /dev/null +++ b/src/test/rustdoc/structfields.rs @@ -0,0 +1,44 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// @has structfields/struct.Foo.html +pub struct Foo { + // @has - //pre "pub a: ()" + pub a: (), + // @has - //pre "// some fields omitted" + // @!has - //pre "b: ()" + b: (), + // @!has - //pre "c: usize" + #[doc(hidden)] + c: usize, + // @has - //pre "pub d: usize" + pub d: usize, +} + +// @has structfields/struct.Bar.html +pub struct Bar { + // @has - //pre "pub a: ()" + pub a: (), + // @!has - //pre "// some fields omitted" +} + +// @has structfields/enum.Qux.html +pub enum Qux { + Quz { + // @has - //pre "a: ()" + a: (), + // @!has - //pre "b: ()" + #[doc(hidden)] + b: (), + // @has - //pre "c: usize" + c: usize, + // @has - //pre "// some fields omitted" + }, +} From 95eb8a68aa38ebeaadcca337d6005efabcf4a05e Mon Sep 17 00:00:00 2001 From: mitaa Date: Sat, 2 Apr 2016 09:03:55 +0200 Subject: [PATCH 3/3] Slim down `rustdoc::html::render::Context` Like the comment on `Context` explains, `Context` is supposed to be lightweight, so we're putting everything that's immutable after creation of the Context behind an `Arc`. --- src/librustdoc/html/render.rs | 87 +++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 40 deletions(-) diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 678a9d75f96..78dd14766e7 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -91,12 +91,20 @@ pub struct Context { /// String representation of how to get back to the root path of the 'doc/' /// folder in terms of a relative URL. pub root_path: String, - /// The path to the crate root source minus the file name. - /// Used for simplifying paths to the highlighted source code files. - pub src_root: PathBuf, /// The current destination folder of where HTML artifacts should be placed. /// This changes as the context descends into the module hierarchy. pub dst: PathBuf, + /// A flag, which when `true`, will render pages which redirect to the + /// real location of an item. This is used to allow external links to + /// publicly reused items to redirect to the right location. + pub render_redirect_pages: bool, + pub shared: Arc, +} + +pub struct SharedContext { + /// The path to the crate root source minus the file name. + /// Used for simplifying paths to the highlighted source code files. + pub src_root: PathBuf, /// This describes the layout of each page, and is not modified after /// creation of the context (contains info like the favicon and added html). pub layout: layout::Layout, @@ -106,10 +114,6 @@ pub struct Context { pub include_sources: bool, /// The local file sources we've emitted and their respective url-paths. pub local_sources: HashMap, - /// A flag, which when turned off, will render pages which redirect to the - /// real location of an item. This is used to allow external links to - /// publicly reused items to redirect to the right location. - pub render_redirect_pages: bool, /// All the passes that were run on this crate. pub passes: HashSet, /// The base-URL of the issue tracker for when an item has been tagged with @@ -259,7 +263,7 @@ pub struct Cache { /// Helper struct to render all source code to HTML pages struct SourceCollector<'a> { - cx: &'a mut Context, + scx: &'a mut SharedContext, /// Root destination to place all HTML output into dst: PathBuf, @@ -412,12 +416,12 @@ pub fn run(mut krate: clean::Crate, Some(p) => p.to_path_buf(), None => PathBuf::new(), }; - let mut cx = Context { - dst: dst, + let mut scx = SharedContext { src_root: src_root, passes: passes, - current: Vec::new(), - root_path: String::new(), + include_sources: true, + local_sources: HashMap::new(), + issue_tracker_base_url: None, layout: layout::Layout { logo: "".to_string(), favicon: "".to_string(), @@ -425,14 +429,8 @@ pub fn run(mut krate: clean::Crate, krate: krate.name.clone(), playground_url: "".to_string(), }, - include_sources: true, - local_sources: HashMap::new(), - render_redirect_pages: false, - issue_tracker_base_url: None, }; - try_err!(mkdir(&cx.dst), &cx.dst); - // Crawl the crate attributes looking for attributes which control how we're // going to emit HTML if let Some(attrs) = krate.module.as_ref().map(|m| m.attrs.list("doc")) { @@ -440,15 +438,15 @@ pub fn run(mut krate: clean::Crate, match *attr { clean::NameValue(ref x, ref s) if "html_favicon_url" == *x => { - cx.layout.favicon = s.to_string(); + scx.layout.favicon = s.to_string(); } clean::NameValue(ref x, ref s) if "html_logo_url" == *x => { - cx.layout.logo = s.to_string(); + scx.layout.logo = s.to_string(); } clean::NameValue(ref x, ref s) if "html_playground_url" == *x => { - cx.layout.playground_url = s.to_string(); + scx.layout.playground_url = s.to_string(); markdown::PLAYGROUND_KRATE.with(|slot| { if slot.borrow().is_none() { let name = krate.name.clone(); @@ -458,16 +456,25 @@ pub fn run(mut krate: clean::Crate, } clean::NameValue(ref x, ref s) if "issue_tracker_base_url" == *x => { - cx.issue_tracker_base_url = Some(s.to_string()); + scx.issue_tracker_base_url = Some(s.to_string()); } clean::Word(ref x) if "html_no_source" == *x => { - cx.include_sources = false; + scx.include_sources = false; } _ => {} } } } + try_err!(mkdir(&dst), &dst); + krate = render_sources(&dst, &mut scx, krate)?; + let cx = Context { + current: Vec::new(), + root_path: String::new(), + dst: dst, + render_redirect_pages: false, + shared: Arc::new(scx), + }; // Crawl the crate to build various caches used for the output let analysis = ::ANALYSISKEY.with(|a| a.clone()); @@ -538,7 +545,6 @@ pub fn run(mut krate: clean::Crate, CURRENT_LOCATION_KEY.with(|s| s.borrow_mut().clear()); write_shared(&cx, &krate, &*cache, index)?; - let krate = render_sources(&mut cx, krate)?; // And finally render the whole crate's documentation cx.krate(krate) @@ -760,16 +766,16 @@ fn write_shared(cx: &Context, Ok(()) } -fn render_sources(cx: &mut Context, +fn render_sources(dst: &Path, scx: &mut SharedContext, krate: clean::Crate) -> Result { info!("emitting source files"); - let dst = cx.dst.join("src"); + let dst = dst.join("src"); try_err!(mkdir(&dst), &dst); let dst = dst.join(&krate.name); try_err!(mkdir(&dst), &dst); let mut folder = SourceCollector { dst: dst, - cx: cx, + scx: scx, }; Ok(folder.fold_crate(krate)) } @@ -847,7 +853,7 @@ impl<'a> DocFolder for SourceCollector<'a> { fn fold_item(&mut self, item: clean::Item) -> Option { // If we're including source files, and we haven't seen this file yet, // then we need to render it out to the filesystem - if self.cx.include_sources + if self.scx.include_sources // skip all invalid spans && item.source.filename != "" // macros from other libraries get special filenames which we can @@ -860,7 +866,7 @@ impl<'a> DocFolder for SourceCollector<'a> { // something like that), so just don't include sources for the // entire crate. The other option is maintaining this mapping on a // per-file basis, but that's probably not worth it... - self.cx + self.scx .include_sources = match self.emit_source(&item.source.filename) { Ok(()) => true, Err(e) => { @@ -880,7 +886,7 @@ impl<'a> SourceCollector<'a> { /// Renders the given filename into its corresponding HTML source file. fn emit_source(&mut self, filename: &str) -> io::Result<()> { let p = PathBuf::from(filename); - if self.cx.local_sources.contains_key(&p) { + if self.scx.local_sources.contains_key(&p) { // We've already emitted this source return Ok(()); } @@ -901,7 +907,7 @@ impl<'a> SourceCollector<'a> { let mut cur = self.dst.clone(); let mut root_path = String::from("../../"); let mut href = String::new(); - clean_srcpath(&self.cx.src_root, &p, false, |component| { + clean_srcpath(&self.scx.src_root, &p, false, |component| { cur.push(component); mkdir(&cur).unwrap(); root_path.push_str("../"); @@ -925,10 +931,10 @@ impl<'a> SourceCollector<'a> { description: &desc, keywords: BASIC_KEYWORDS, }; - layout::render(&mut w, &self.cx.layout, + layout::render(&mut w, &self.scx.layout, &page, &(""), &Source(contents))?; w.flush()?; - self.cx.local_sources.insert(p, href); + self.scx.local_sources.insert(p, href); Ok(()) } } @@ -1265,10 +1271,10 @@ impl Context { let tyname = shortty(it).to_static_str(); let desc = if it.is_crate() { format!("API documentation for the Rust `{}` crate.", - cx.layout.krate) + cx.shared.layout.krate) } else { format!("API documentation for the Rust `{}` {} in crate `{}`.", - it.name.as_ref().unwrap(), tyname, cx.layout.krate) + it.name.as_ref().unwrap(), tyname, cx.shared.layout.krate) }; let keywords = make_item_keywords(it); let page = layout::Page { @@ -1286,7 +1292,7 @@ impl Context { // write syscall all the time. let mut writer = BufWriter::new(w); if !cx.render_redirect_pages { - layout::render(&mut writer, &cx.layout, &page, + layout::render(&mut writer, &cx.shared.layout, &page, &Sidebar{ cx: cx, item: it }, &Item{ cx: cx, item: it })?; @@ -1434,10 +1440,11 @@ impl<'a> Item<'a> { // know the span, so we plow forward and generate a proper url. The url // has anchors for the line numbers that we're linking to. } else if self.item.def_id.is_local() { - self.cx.local_sources.get(&PathBuf::from(&self.item.source.filename)).map(|path| { + let path = PathBuf::from(&self.item.source.filename); + self.cx.shared.local_sources.get(&path).map(|path| { format!("{root}src/{krate}/{path}#{href}", root = self.cx.root_path, - krate = self.cx.layout.krate, + krate = self.cx.shared.layout.krate, path = path, href = href) }) @@ -1520,7 +1527,7 @@ impl<'a> fmt::Display for Item<'a> { // [src] link in the downstream documentation will actually come back to // this page, and this link will be auto-clicked. The `id` attribute is // used to find the link to auto-click. - if self.cx.include_sources && !is_primitive { + if self.cx.shared.include_sources && !is_primitive { if let Some(l) = self.href() { write!(fmt, "[src]", @@ -1752,7 +1759,7 @@ fn short_stability(item: &clean::Item, cx: &Context, show_reason: bool) -> Optio format!("Deprecated{}{}", since, Markdown(&reason)) } else if stab.level == stability::Unstable { let unstable_extra = if show_reason { - match (!stab.feature.is_empty(), &cx.issue_tracker_base_url, stab.issue) { + 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!(" ({} #{})", Escape(&stab.feature), tracker_url, issue_no, issue_no),