diff --git a/src/doc/style/features/traits/generics.md b/src/doc/style/features/traits/generics.md index 26ffda50ac5..a09640c3055 100644 --- a/src/doc/style/features/traits/generics.md +++ b/src/doc/style/features/traits/generics.md @@ -27,8 +27,7 @@ explicitly implement to be used by this generic function. * _Inference_. Since the type parameters to generic functions can usually be inferred, generic functions can help cut down on verbosity in code where explicit conversions or other method calls would usually be necessary. See the - [overloading/implicits use case](#use-case-limited-overloading-andor-implicit-conversions) - below. + overloading/implicits use case below. * _Precise types_. Because generics give a _name_ to the specific type implementing a trait, it is possible to be precise about places where that exact type is required or produced. For example, a function @@ -51,7 +50,7 @@ explicitly implement to be used by this generic function. a `Vec` contains elements of a single concrete type (and, indeed, the vector representation is specialized to lay these out in line). Sometimes heterogeneous collections are useful; see - [trait objects](#use-case-trait-objects) below. + trait objects below. * _Signature verbosity_. Heavy use of generics can bloat function signatures. **[Ed. note]** This problem may be mitigated by some language improvements; stay tuned. diff --git a/src/libcore/iter.rs b/src/libcore/iter.rs index 72421e94a43..4dbcf7ab4e3 100644 --- a/src/libcore/iter.rs +++ b/src/libcore/iter.rs @@ -434,7 +434,7 @@ pub trait Iterator { /// `None`. Once `None` is encountered, `count()` returns the number of /// times it called [`next()`]. /// - /// [`next()`]: #method.next + /// [`next()`]: #tymethod.next /// /// # Overflow Behavior /// @@ -497,7 +497,7 @@ pub trait Iterator { /// This method will evaluate the iterator `n` times, discarding those elements. /// After it does so, it will call [`next()`] and return its value. /// - /// [`next()`]: #method.next + /// [`next()`]: #tymethod.next /// /// Like most indexing operations, the count starts from zero, so `nth(0)` /// returns the first value, `nth(1)` the second, and so on. diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 94dead3c005..702c6dd8211 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -26,7 +26,7 @@ use rustc::middle::const_eval; use core::DocContext; use doctree; -use clean::{self, Attributes}; +use clean::{self, Attributes, GetDefId}; use super::{Clean, ToSource}; @@ -414,15 +414,22 @@ pub fn build_impl(cx: &DocContext, clean::RegionBound(..) => unreachable!(), } }); - if let Some(clean::ResolvedPath { did, .. }) = trait_ { - if Some(did) == cx.deref_trait_did.get() { - super::build_deref_target_impls(cx, &trait_items, ret); - } + if trait_.def_id() == cx.deref_trait_did.get() { + super::build_deref_target_impls(cx, &trait_items, ret); } + + let provided = trait_.def_id().map(|did| { + cx.tcx().provided_trait_methods(did) + .into_iter() + .map(|meth| meth.name.to_string()) + .collect() + }).unwrap_or(HashSet::new()); + ret.push(clean::Item { inner: clean::ImplItem(clean::Impl { unsafety: hir::Unsafety::Normal, // FIXME: this should be decoded derived: clean::detect_derived(&attrs), + provided_trait_methods: provided, trait_: trait_, for_: ty.ty.clean(cx), generics: (&ty.generics, &predicates, subst::TypeSpace).clean(cx), diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index e56286c8bed..b26e56008ac 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -44,7 +44,7 @@ use rustc::middle::stability; use rustc_front::hir; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::PathBuf; use std::rc::Rc; use std::u32; @@ -559,15 +559,9 @@ impl TyParamBound { fn is_sized_bound(&self, cx: &DocContext) -> bool { use rustc_front::hir::TraitBoundModifier as TBM; if let Some(tcx) = cx.tcx_opt() { - let sized_did = match tcx.lang_items.sized_trait() { - Some(did) => did, - None => return false - }; - if let TyParamBound::TraitBound(PolyTrait { - trait_: Type::ResolvedPath { did, .. }, .. - }, TBM::None) = *self { - if did == sized_did { - return true + if let TyParamBound::TraitBound(PolyTrait { ref trait_, .. }, TBM::None) = *self { + if trait_.def_id() == tcx.lang_items.sized_trait() { + return true; } } } @@ -724,15 +718,18 @@ impl<'tcx> Clean for ty::TraitRef<'tcx> { } } - TraitBound(PolyTrait { - trait_: ResolvedPath { - path: path, - typarams: None, - did: self.def_id, - is_generic: false, + TraitBound( + PolyTrait { + trait_: ResolvedPath { + path: path, + typarams: None, + did: self.def_id, + is_generic: false, + }, + lifetimes: late_bounds, }, - lifetimes: late_bounds - }, hir::TraitBoundModifier::None) + hir::TraitBoundModifier::None + ) } } @@ -932,7 +929,6 @@ impl<'a, 'tcx> Clean for (&'a ty::Generics<'tcx>, &'a ty::GenericPredicates<'tcx>, subst::ParamSpace) { fn clean(&self, cx: &DocContext) -> Generics { - use std::collections::HashSet; use self::WherePredicate as WP; let (gens, preds, space) = *self; @@ -1486,6 +1482,16 @@ pub enum TypeKind { TypeTypedef, } +pub trait GetDefId { + fn def_id(&self) -> Option; +} + +impl GetDefId for Option { + fn def_id(&self) -> Option { + self.as_ref().and_then(|d| d.def_id()) + } +} + impl Type { pub fn primitive_type(&self) -> Option { match *self { @@ -1499,7 +1505,9 @@ impl Type { _ => None, } } +} +impl GetDefId for Type { fn def_id(&self) -> Option { match *self { ResolvedPath { did, .. } => Some(did), @@ -1884,18 +1892,11 @@ impl<'tcx> Clean for ty::VariantDefData<'tcx, 'static> { Item { source: Span::empty(), name: Some(field.name.clean(cx)), - attrs: Vec::new(), + attrs: cx.tcx().get_attrs(field.did).clean(cx), visibility: Some(field.vis), - // FIXME: this is not accurate, we need an id for - // the specific field but we're using the id - // for the whole variant. Thus we read the - // stability from the whole variant as well. - // Struct variants are experimental and need - // more infrastructure work before we can get - // at the needed information here. - def_id: self.did, - stability: get_stability(cx, self.did), - deprecation: get_deprecation(cx, self.did), + def_id: field.did, + stability: get_stability(cx, field.did), + deprecation: get_deprecation(cx, field.did), inner: StructFieldItem( TypedStructField(field.unsubst_ty().clean(cx)) ) @@ -1908,7 +1909,7 @@ impl<'tcx> Clean for ty::VariantDefData<'tcx, 'static> { name: Some(self.name.clean(cx)), attrs: inline::load_attrs(cx, cx.tcx(), self.did), source: Span::empty(), - visibility: Some(hir::Public), + visibility: Some(hir::Inherited), def_id: self.did, inner: VariantItem(Variant { kind: kind }), stability: get_stability(cx, self.did), @@ -2208,6 +2209,7 @@ impl Clean for hir::ImplPolarity { pub struct Impl { pub unsafety: hir::Unsafety, pub generics: Generics, + pub provided_trait_methods: HashSet, pub trait_: Option, pub for_: Type, pub items: Vec, @@ -2227,12 +2229,19 @@ impl Clean> for doctree::Impl { // If this impl block is an implementation of the Deref trait, then we // need to try inlining the target's inherent impl blocks as well. - if let Some(ResolvedPath { did, .. }) = trait_ { - if Some(did) == cx.deref_trait_did.get() { - build_deref_target_impls(cx, &items, &mut ret); - } + if trait_.def_id() == cx.deref_trait_did.get() { + build_deref_target_impls(cx, &items, &mut ret); } + let provided = trait_.def_id().and_then(|did| { + cx.tcx_opt().map(|tcx| { + tcx.provided_trait_methods(did) + .into_iter() + .map(|meth| meth.name.to_string()) + .collect() + }) + }).unwrap_or(HashSet::new()); + ret.push(Item { name: None, attrs: self.attrs.clean(cx), @@ -2244,6 +2253,7 @@ impl Clean> for doctree::Impl { inner: ImplItem(Impl { unsafety: self.unsafety, generics: self.generics.clean(cx), + provided_trait_methods: provided, trait_: trait_, for_: self.for_.clean(cx), items: items, diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 9fd476bfbab..e7304b69510 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -607,7 +607,7 @@ mod tests { fn issue_17736() { let markdown = "# title"; format!("{}", Markdown(markdown)); - reset_ids(); + reset_ids(true); } #[test] @@ -615,7 +615,7 @@ mod tests { fn t(input: &str, expect: &str) { let output = format!("{}", Markdown(input)); assert_eq!(output, expect); - reset_ids(); + reset_ids(true); } t("# Foo bar", "\n

\ @@ -654,7 +654,7 @@ mod tests { Panics

"); }; test(); - reset_ids(); + reset_ids(true); test(); } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 071e3dd6bd0..0f436efd70b 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -62,7 +62,7 @@ use rustc::middle::stability; use rustc::session::config::get_unstable_features_setting; use rustc_front::hir; -use clean::{self, SelfTy, Attributes}; +use clean::{self, SelfTy, Attributes, GetDefId}; use doctree; use fold::DocFolder; use html::escape::Escape; @@ -144,9 +144,7 @@ pub struct Impl { impl Impl { fn trait_did(&self) -> Option { - self.impl_.trait_.as_ref().and_then(|tr| { - if let clean::ResolvedPath { did, .. } = *tr {Some(did)} else {None} - }) + self.impl_.trait_.def_id() } } @@ -380,8 +378,14 @@ fn init_ids() -> HashMap { /// This method resets the local table of used ID attributes. This is typically /// used at the beginning of rendering an entire HTML page to reset from the /// previous state (if any). -pub fn reset_ids() { - USED_ID_MAP.with(|s| *s.borrow_mut() = init_ids()); +pub fn reset_ids(embedded: bool) { + USED_ID_MAP.with(|s| { + *s.borrow_mut() = if embedded { + init_ids() + } else { + HashMap::new() + }; + }); } pub fn derive_id(candidate: String) -> String { @@ -967,7 +971,7 @@ impl DocFolder for Cache { // Collect all the implementors of traits. if let clean::ImplItem(ref i) = item.inner { - if let Some(clean::ResolvedPath{ did, .. }) = i.trait_ { + if let Some(did) = i.trait_.def_id() { self.implementors.entry(did).or_insert(vec![]).push(Implementor { def_id: item.def_id, stability: item.stability.clone(), @@ -1282,7 +1286,7 @@ impl Context { keywords: &keywords, }; - reset_ids(); + reset_ids(true); // We have a huge number of calls to write, so try to alleviate some // of the pain by using a buffered writer instead of invoking the @@ -2023,10 +2027,33 @@ fn item_trait(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item, Ok(()) } -fn assoc_const(w: &mut fmt::Formatter, it: &clean::Item, - ty: &clean::Type, default: Option<&String>) - -> fmt::Result { - write!(w, "const {}", it.name.as_ref().unwrap())?; +fn naive_assoc_href(it: &clean::Item, link: AssocItemLink) -> String { + use html::item_type::ItemType::*; + + let name = it.name.as_ref().unwrap(); + let ty = match shortty(it) { + Typedef | AssociatedType => AssociatedType, + s@_ => s, + }; + + let anchor = format!("#{}.{}", ty, name); + match link { + AssocItemLink::Anchor => anchor, + AssocItemLink::GotoSource(did, _) => { + href(did).map(|p| format!("{}{}", p.0, anchor)).unwrap_or(anchor) + } + } +} + +fn assoc_const(w: &mut fmt::Formatter, + it: &clean::Item, + ty: &clean::Type, + default: Option<&String>, + link: AssocItemLink) -> fmt::Result { + write!(w, "const {}", + naive_assoc_href(it, link), + it.name.as_ref().unwrap())?; + write!(w, ": {}", ty)?; if let Some(default) = default { write!(w, " = {}", default)?; @@ -2036,13 +2063,15 @@ fn assoc_const(w: &mut fmt::Formatter, it: &clean::Item, fn assoc_type(w: &mut fmt::Formatter, it: &clean::Item, bounds: &Vec, - default: &Option) - -> fmt::Result { - write!(w, "type {}", it.name.as_ref().unwrap())?; + default: Option<&clean::Type>, + link: AssocItemLink) -> fmt::Result { + write!(w, "type {}", + naive_assoc_href(it, link), + it.name.as_ref().unwrap())?; if !bounds.is_empty() { write!(w, ": {}", TyParamBounds(bounds))? } - if let Some(ref default) = *default { + if let Some(default) = default { write!(w, " = {}", default)?; } Ok(()) @@ -2066,10 +2095,11 @@ fn render_stability_since(w: &mut fmt::Formatter, render_stability_since_raw(w, item.stable_since(), containing_item.stable_since()) } -fn render_assoc_item(w: &mut fmt::Formatter, meth: &clean::Item, +fn render_assoc_item(w: &mut fmt::Formatter, + item: &clean::Item, link: AssocItemLink) -> fmt::Result { fn method(w: &mut fmt::Formatter, - it: &clean::Item, + meth: &clean::Item, unsafety: hir::Unsafety, constness: hir::Constness, abi: abi::Abi, @@ -2080,14 +2110,23 @@ fn render_assoc_item(w: &mut fmt::Formatter, meth: &clean::Item, -> fmt::Result { use syntax::abi::Abi; - let name = it.name.as_ref().unwrap(); - let anchor = format!("#{}.{}", shortty(it), name); + let name = meth.name.as_ref().unwrap(); + let anchor = format!("#{}.{}", shortty(meth), name); let href = match link { AssocItemLink::Anchor => anchor, - AssocItemLink::GotoSource(did) => { - href(did).map(|p| format!("{}{}", p.0, anchor)).unwrap_or(anchor) + AssocItemLink::GotoSource(did, provided_methods) => { + // We're creating a link from an impl-item to the corresponding + // trait-item and need to map the anchored type accordingly. + let ty = if provided_methods.contains(name) { + ItemType::Method + } else { + ItemType::TyMethod + }; + + href(did).map(|p| format!("{}#{}.{}", p.0, ty, name)).unwrap_or(anchor) } }; + // FIXME(#24111): remove when `const_fn` is stabilized let vis_constness = match get_unstable_features_setting() { UnstableFeatures::Allow => constness, _ => hir::Constness::NotConst @@ -2106,21 +2145,21 @@ fn render_assoc_item(w: &mut fmt::Formatter, meth: &clean::Item, decl = Method(selfty, d), where_clause = WhereClause(g)) } - match meth.inner { + match item.inner { clean::TyMethodItem(ref m) => { - method(w, meth, m.unsafety, hir::Constness::NotConst, + method(w, item, m.unsafety, hir::Constness::NotConst, m.abi, &m.generics, &m.self_, &m.decl, link) } clean::MethodItem(ref m) => { - method(w, meth, m.unsafety, m.constness, + method(w, item, m.unsafety, m.constness, m.abi, &m.generics, &m.self_, &m.decl, link) } clean::AssociatedConstItem(ref ty, ref default) => { - assoc_const(w, meth, ty, default.as_ref()) + assoc_const(w, item, ty, default.as_ref(), link) } clean::AssociatedTypeItem(ref bounds, ref default) => { - assoc_type(w, meth, bounds, default) + assoc_type(w, item, bounds, default.as_ref(), link) } _ => panic!("render_assoc_item called on non-associated-item") } @@ -2153,8 +2192,9 @@ fn item_struct(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item, write!(w, "

Fields

\n")?; for field in fields { write!(w, " -
\ + \ {name}", + shortty = ItemType::StructField, stab = field.stability_class(), name = field.name.as_ref().unwrap())?; document(w, cx, field)?; @@ -2224,7 +2264,8 @@ fn item_enum(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item, if !e.variants.is_empty() { write!(w, "

Variants

\n")?; for variant in &e.variants { - write!(w, "
{name}", + write!(w, "
{name}", + shortty = ItemType::Variant, name = variant.name.as_ref().unwrap())?; document(w, cx, variant)?; @@ -2240,8 +2281,9 @@ fn item_enum(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item, ")?; for field in fields { write!(w, "
\ + id='{shortty}.{v}.field.{f}'>\ {f}", + shortty = ItemType::Variant, v = variant.name.as_ref().unwrap(), f = field.name.as_ref().unwrap())?; document(w, cx, field)?; @@ -2338,9 +2380,9 @@ fn render_struct(w: &mut fmt::Formatter, it: &clean::Item, } #[derive(Copy, Clone)] -enum AssocItemLink { +enum AssocItemLink<'a> { Anchor, - GotoSource(DefId), + GotoSource(DefId, &'a HashSet), } enum AssocItemRender<'a> { @@ -2383,12 +2425,7 @@ fn render_assoc_items(w: &mut fmt::Formatter, } if !traits.is_empty() { let deref_impl = traits.iter().find(|t| { - match *t.impl_.trait_.as_ref().unwrap() { - clean::ResolvedPath { did, .. } => { - Some(did) == c.deref_trait_did - } - _ => false - } + t.impl_.trait_.def_id() == c.deref_trait_did }); if let Some(impl_) = deref_impl { render_deref_methods(w, cx, impl_, containing_item)?; @@ -2400,8 +2437,8 @@ fn render_assoc_items(w: &mut fmt::Formatter, }); for i in &manual { let did = i.trait_did().unwrap(); - render_impl(w, cx, i, AssocItemLink::GotoSource(did), true, - containing_item.stable_since())?; + let assoc_link = AssocItemLink::GotoSource(did, &i.impl_.provided_trait_methods); + render_impl(w, cx, i, assoc_link, true, containing_item.stable_since())?; } if !derived.is_empty() { write!(w, "

\ @@ -2409,8 +2446,8 @@ fn render_assoc_items(w: &mut fmt::Formatter,

")?; for i in &derived { let did = i.trait_did().unwrap(); - render_impl(w, cx, i, AssocItemLink::GotoSource(did), true, - containing_item.stable_since())?; + let assoc_link = AssocItemLink::GotoSource(did, &i.impl_.provided_trait_methods); + render_impl(w, cx, i, assoc_link, true, containing_item.stable_since())?; } } } @@ -2427,17 +2464,16 @@ fn render_deref_methods(w: &mut fmt::Formatter, cx: &Context, impl_: &Impl, } }).next().expect("Expected associated type binding"); let what = AssocItemRender::DerefFor { trait_: deref_type, type_: target }; - match *target { - clean::ResolvedPath { did, .. } => render_assoc_items(w, cx, container_item, did, what), - _ => { - if let Some(prim) = target.primitive_type() { - if let Some(c) = cache().primitive_locations.get(&prim) { - let did = DefId { krate: *c, index: prim.to_def_index() }; - render_assoc_items(w, cx, container_item, did, what)?; - } + if let Some(did) = target.def_id() { + render_assoc_items(w, cx, container_item, did, what) + } else { + if let Some(prim) = target.primitive_type() { + if let Some(c) = cache().primitive_locations.get(&prim) { + let did = DefId { krate: *c, index: prim.to_def_index() }; + render_assoc_items(w, cx, container_item, did, what)?; } - Ok(()) } + Ok(()) } } @@ -2459,6 +2495,7 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi fn doctraititem(w: &mut fmt::Formatter, cx: &Context, item: &clean::Item, link: AssocItemLink, render_static: bool, outer_version: Option<&str>) -> fmt::Result { + let shortty = shortty(item); let name = item.name.as_ref().unwrap(); let is_static = match item.inner { @@ -2471,8 +2508,8 @@ 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 || render_static { - let id = derive_id(format!("method.{}", name)); - write!(w, "

", id, shortty(item))?; + let id = derive_id(format!("{}.{}", shortty, name)); + write!(w, "

", id, shortty)?; render_stability_since_raw(w, item.stable_since(), outer_version)?; write!(w, "")?; render_assoc_item(w, item, link)?; @@ -2480,27 +2517,27 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi } } clean::TypedefItem(ref tydef, _) => { - let id = derive_id(format!("associatedtype.{}", name)); - write!(w, "

", id, shortty(item))?; - write!(w, "type {} = {}", name, tydef.type_)?; + let id = derive_id(format!("{}.{}", ItemType::AssociatedType, name)); + write!(w, "

", id, shortty)?; + assoc_type(w, item, &Vec::new(), Some(&tydef.type_), link)?; write!(w, "

\n")?; } clean::AssociatedConstItem(ref ty, ref default) => { - let id = derive_id(format!("associatedconstant.{}", name)); - write!(w, "

", id, shortty(item))?; - assoc_const(w, item, ty, default.as_ref())?; + let id = derive_id(format!("{}.{}", shortty, name)); + write!(w, "

", id, shortty)?; + assoc_const(w, item, ty, default.as_ref(), link)?; write!(w, "

\n")?; } clean::ConstantItem(ref c) => { - let id = derive_id(format!("associatedconstant.{}", name)); - write!(w, "

", id, shortty(item))?; - assoc_const(w, item, &c.type_, Some(&c.expr))?; + let id = derive_id(format!("{}.{}", shortty, name)); + write!(w, "

", id, shortty)?; + assoc_const(w, item, &c.type_, Some(&c.expr), link)?; write!(w, "

\n")?; } clean::AssociatedTypeItem(ref bounds, ref default) => { - let id = derive_id(format!("associatedtype.{}", name)); - write!(w, "

", id, shortty(item))?; - assoc_type(w, item, bounds, default)?; + let id = derive_id(format!("{}.{}", shortty, name)); + write!(w, "

", id, shortty)?; + assoc_type(w, item, bounds, default.as_ref(), link)?; write!(w, "

\n")?; } _ => panic!("can't make docs for trait item with name {:?}", item.name) @@ -2521,30 +2558,29 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi fn render_default_items(w: &mut fmt::Formatter, cx: &Context, - did: DefId, t: &clean::Trait, - i: &clean::Impl, - render_static: bool, - outer_version: Option<&str>) -> fmt::Result { + i: &clean::Impl, + render_static: bool, + outer_version: Option<&str>) -> fmt::Result { for trait_item in &t.items { let n = trait_item.name.clone(); - if i.items.iter().find(|m| { m.name == n }).is_some() { + if i.items.iter().find(|m| m.name == n).is_some() { continue; } + let did = i.trait_.as_ref().unwrap().def_id().unwrap(); + let assoc_link = AssocItemLink::GotoSource(did, &i.provided_trait_methods); - doctraititem(w, cx, trait_item, AssocItemLink::GotoSource(did), render_static, + doctraititem(w, cx, trait_item, assoc_link, render_static, outer_version)?; } Ok(()) } // If we've implemented a trait, then also emit documentation for all - // default methods which weren't overridden in the implementation block. - // FIXME: this also needs to be done for associated types, whenever defaults - // for them work. - if let Some(clean::ResolvedPath { did, .. }) = i.impl_.trait_ { + // default items which weren't overridden in the implementation block. + if let Some(did) = i.trait_did() { if let Some(t) = cache().traits.get(&did) { - render_default_items(w, cx, did, t, &i.impl_, render_header, outer_version)?; + render_default_items(w, cx, t, &i.impl_, render_header, outer_version)?; } } write!(w, "")?; @@ -2718,6 +2754,6 @@ fn test_unique_id() { assert_eq!(&actual[..], expected); }; test(); - reset_ids(); + reset_ids(true); test(); } diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs index 03d2c1a1b4d..d21726dd40f 100644 --- a/src/librustdoc/markdown.rs +++ b/src/librustdoc/markdown.rs @@ -83,7 +83,7 @@ pub fn render(input: &str, mut output: PathBuf, matches: &getopts::Matches, } let title = metadata[0]; - reset_ids(); + reset_ids(false); let rendered = if include_toc { format!("{}", MarkdownWithToc(text)) diff --git a/src/librustdoc/passes.rs b/src/librustdoc/passes.rs index 154b812cdff..88cb20991d6 100644 --- a/src/librustdoc/passes.rs +++ b/src/librustdoc/passes.rs @@ -16,7 +16,7 @@ use std::string::String; use std::usize; use rustc_front::hir; -use clean::{self, Attributes}; +use clean::{self, Attributes, GetDefId}; use clean::Item; use plugins; use fold; @@ -74,7 +74,7 @@ pub fn strip_hidden(krate: clean::Crate) -> plugins::PluginResult { return None; } // Impls of stripped traits also don't need to exist - if let Some(clean::ResolvedPath { did, .. }) = *trait_ { + if let Some(did) = trait_.def_id() { if self.stripped.contains(&did) { return None; } @@ -223,13 +223,10 @@ struct ImplStripper<'a>(&'a DefIdSet); impl<'a> fold::DocFolder for ImplStripper<'a> { fn fold_item(&mut self, i: Item) -> Option { if let clean::ImplItem(ref imp) = i.inner { - match imp.trait_ { - Some(clean::ResolvedPath{ did, .. }) => { - if did.is_local() && !self.0.contains(&did) { - return None; - } + if let Some(did) = imp.trait_.def_id() { + if did.is_local() && !self.0.contains(&did) { + return None; } - Some(..) | None => {} } } self.fold_item_recur(i) diff --git a/src/libstd/io/mod.rs b/src/libstd/io/mod.rs index 28492b30e0f..5362f7086bd 100644 --- a/src/libstd/io/mod.rs +++ b/src/libstd/io/mod.rs @@ -1254,7 +1254,7 @@ pub trait BufRead: Read { /// longer be returned. As such, this function may do odd things if /// `fill_buf` isn't called before calling it. /// - /// [fillbuf]: #tymethod.fill_buff + /// [fillbuf]: #tymethod.fill_buf /// /// The `amt` must be `<=` the number of bytes in the buffer returned by /// `fill_buf`. diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 5608bb646a4..f7b1042592d 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -190,7 +190,7 @@ //! [`thread`]: thread/index.html //! [`use std::env`]: env/index.html //! [`use`]: ../book/crates-and-modules.html#importing-modules-with-use -//! [crate root]: ../book/crates-and-modules.html#basic-terminology:-crates-and-modules +//! [crate root]: ../book/crates-and-modules.html#basic-terminology-crates-and-modules //! [crates.io]: https://crates.io //! [deref coercions]: ../book/deref-coercions.html //! [files]: fs/struct.File.html diff --git a/src/libstd/net/tcp.rs b/src/libstd/net/tcp.rs index 414696413f4..38da74b8903 100644 --- a/src/libstd/net/tcp.rs +++ b/src/libstd/net/tcp.rs @@ -196,7 +196,7 @@ impl TcpStream { /// /// For more information about this option, see [`set_nodelay`][link]. /// - /// [link]: #tymethod.set_nodelay + /// [link]: #method.set_nodelay #[stable(feature = "net2_mutators", since = "1.9.0")] pub fn nodelay(&self) -> io::Result { self.0.nodelay() @@ -215,7 +215,7 @@ impl TcpStream { /// /// For more information about this option, see [`set_ttl`][link]. /// - /// [link]: #tymethod.set_ttl + /// [link]: #method.set_ttl #[stable(feature = "net2_mutators", since = "1.9.0")] pub fn ttl(&self) -> io::Result { self.0.ttl() @@ -238,7 +238,7 @@ impl TcpStream { /// /// For more information about this option, see [`set_only_v6`][link]. /// - /// [link]: #tymethod.set_only_v6 + /// [link]: #method.set_only_v6 #[stable(feature = "net2_mutators", since = "1.9.0")] pub fn only_v6(&self) -> io::Result { self.0.only_v6() @@ -374,7 +374,7 @@ impl TcpListener { /// /// For more information about this option, see [`set_ttl`][link]. /// - /// [link]: #tymethod.set_ttl + /// [link]: #method.set_ttl #[stable(feature = "net2_mutators", since = "1.9.0")] pub fn ttl(&self) -> io::Result { self.0.ttl() @@ -397,7 +397,7 @@ impl TcpListener { /// /// For more information about this option, see [`set_only_v6`][link]. /// - /// [link]: #tymethod.set_only_v6 + /// [link]: #method.set_only_v6 #[stable(feature = "net2_mutators", since = "1.9.0")] pub fn only_v6(&self) -> io::Result { self.0.only_v6() diff --git a/src/libstd/net/udp.rs b/src/libstd/net/udp.rs index da1cf115e8d..0be9f13e817 100644 --- a/src/libstd/net/udp.rs +++ b/src/libstd/net/udp.rs @@ -155,7 +155,7 @@ impl UdpSocket { /// For more information about this option, see /// [`set_broadcast`][link]. /// - /// [link]: #tymethod.set_broadcast + /// [link]: #method.set_broadcast #[stable(feature = "net2_mutators", since = "1.9.0")] pub fn broadcast(&self) -> io::Result { self.0.broadcast() @@ -175,7 +175,7 @@ impl UdpSocket { /// For more information about this option, see /// [`set_multicast_loop_v4`][link]. /// - /// [link]: #tymethod.set_multicast_loop_v4 + /// [link]: #method.set_multicast_loop_v4 #[stable(feature = "net2_mutators", since = "1.9.0")] pub fn multicast_loop_v4(&self) -> io::Result { self.0.multicast_loop_v4() @@ -198,7 +198,7 @@ impl UdpSocket { /// For more information about this option, see /// [`set_multicast_ttl_v4`][link]. /// - /// [link]: #tymethod.set_multicast_ttl_v4 + /// [link]: #method.set_multicast_ttl_v4 #[stable(feature = "net2_mutators", since = "1.9.0")] pub fn multicast_ttl_v4(&self) -> io::Result { self.0.multicast_ttl_v4() @@ -218,7 +218,7 @@ impl UdpSocket { /// For more information about this option, see /// [`set_multicast_loop_v6`][link]. /// - /// [link]: #tymethod.set_multicast_loop_v6 + /// [link]: #method.set_multicast_loop_v6 #[stable(feature = "net2_mutators", since = "1.9.0")] pub fn multicast_loop_v6(&self) -> io::Result { self.0.multicast_loop_v6() @@ -237,7 +237,7 @@ impl UdpSocket { /// /// For more information about this option, see [`set_ttl`][link]. /// - /// [link]: #tymethod.set_ttl + /// [link]: #method.set_ttl #[stable(feature = "net2_mutators", since = "1.9.0")] pub fn ttl(&self) -> io::Result { self.0.ttl() @@ -260,7 +260,7 @@ impl UdpSocket { /// /// For more information about this option, see [`set_only_v6`][link]. /// - /// [link]: #tymethod.set_only_v6 + /// [link]: #method.set_only_v6 #[stable(feature = "net2_mutators", since = "1.9.0")] pub fn only_v6(&self) -> io::Result { self.0.only_v6() @@ -293,7 +293,7 @@ impl UdpSocket { /// For more information about this option, see /// [`join_multicast_v4`][link]. /// - /// [link]: #tymethod.join_multicast_v4 + /// [link]: #method.join_multicast_v4 #[stable(feature = "net2_mutators", since = "1.9.0")] pub fn leave_multicast_v4(&self, multiaddr: &Ipv4Addr, interface: &Ipv4Addr) -> io::Result<()> { self.0.leave_multicast_v4(multiaddr, interface) @@ -304,7 +304,7 @@ impl UdpSocket { /// For more information about this option, see /// [`join_multicast_v6`][link]. /// - /// [link]: #tymethod.join_multicast_v6 + /// [link]: #method.join_multicast_v6 #[stable(feature = "net2_mutators", since = "1.9.0")] pub fn leave_multicast_v6(&self, multiaddr: &Ipv6Addr, interface: u32) -> io::Result<()> { self.0.leave_multicast_v6(multiaddr, interface) diff --git a/src/test/rustdoc/issue-28478.rs b/src/test/rustdoc/issue-28478.rs new file mode 100644 index 00000000000..0db92a491ed --- /dev/null +++ b/src/test/rustdoc/issue-28478.rs @@ -0,0 +1,42 @@ +// 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. + +#![feature(associated_type_defaults)] +#![feature(associated_consts)] + +// @has issue_28478/trait.Bar.html +pub trait Bar { + // @has - '//*[@id="associatedtype.Bar"]' 'type Bar = ()' + // @has - '//*[@href="#associatedtype.Bar"]' 'Bar' + type Bar = (); + // @has - '//*[@id="associatedconstant.Baz"]' 'const Baz: usize = 7' + // @has - '//*[@href="#associatedconstant.Baz"]' 'Baz' + const Baz: usize = 7; + // @has - '//*[@id="tymethod.bar"]' 'fn bar' + fn bar(); + // @has - '//*[@id="method.baz"]' 'fn baz' + fn baz() { } +} + +// @has issue_28478/struct.Foo.html +pub struct Foo; + +impl Foo { + // @has - '//*[@href="#method.foo"]' 'foo' + pub fn foo() {} +} + +impl Bar for Foo { + // @has - '//*[@href="../issue_28478/trait.Bar.html#associatedtype.Bar"]' 'Bar' + // @has - '//*[@href="../issue_28478/trait.Bar.html#associatedconstant.Baz"]' 'Baz' + // @has - '//*[@href="../issue_28478/trait.Bar.html#tymethod.bar"]' 'bar' + fn bar() {} + // @has - '//*[@href="../issue_28478/trait.Bar.html#method.baz"]' 'baz' +} diff --git a/src/test/rustdoc/issue-32395.rs b/src/test/rustdoc/issue-32395.rs index fba2a6ad487..672c8757049 100644 --- a/src/test/rustdoc/issue-32395.rs +++ b/src/test/rustdoc/issue-32395.rs @@ -14,8 +14,10 @@ // @has variant_struct/enum.Foo.html // @!has - 'pub qux' +// @!has - 'pub Bar' extern crate variant_struct; // @has issue_32395/enum.Foo.html // @!has - 'pub qux' +// @!has - 'pub Bar' pub use variant_struct::Foo; diff --git a/src/tools/linkchecker/main.rs b/src/tools/linkchecker/main.rs index 19037a2c4d7..12419d4f7e5 100644 --- a/src/tools/linkchecker/main.rs +++ b/src/tools/linkchecker/main.rs @@ -17,9 +17,9 @@ //! actually point to a valid place. //! //! Currently this doesn't actually do any HTML parsing or anything fancy like -//! that, it just has a simple "regex" to search for `href` tags. These values -//! are then translated to file URLs if possible and then the destination is -//! asserted to exist. +//! that, it just has a simple "regex" to search for `href` and `id` tags. +//! These values are then translated to file URLs if possible and then the +//! destination is asserted to exist. //! //! A few whitelisted exceptions are allowed as there's known bugs in rustdoc, //! but this should catch the majority of "broken link" cases. @@ -29,14 +29,18 @@ extern crate url; use std::env; use std::fs::File; use std::io::prelude::*; -use std::path::Path; +use std::path::{Path, PathBuf}; +use std::collections::{HashMap, HashSet}; +use std::collections::hash_map::Entry; use url::{Url, UrlParser}; +use Redirect::*; + macro_rules! t { ($e:expr) => (match $e { Ok(e) => e, - Err(e) => panic!("{} failed with {}", stringify!($e), e), + Err(e) => panic!("{} failed with {:?}", stringify!($e), e), }) } @@ -45,74 +49,270 @@ fn main() { let docs = env::current_dir().unwrap().join(docs); let mut url = Url::from_file_path(&docs).unwrap(); let mut errors = false; - walk(&docs, &docs, &mut url, &mut errors); + walk(&mut HashMap::new(), &docs, &docs, &mut url, &mut errors); if errors { panic!("found some broken links"); } } -fn walk(root: &Path, dir: &Path, url: &mut Url, errors: &mut bool) { +#[derive(Debug)] +pub enum LoadError { + IOError(std::io::Error), + BrokenRedirect(PathBuf, std::io::Error), + IsRedirect, +} + +enum Redirect { + SkipRedirect, + FromRedirect(bool), +} + +struct FileEntry { + source: String, + ids: HashSet, +} + +type Cache = HashMap; + +impl FileEntry { + fn parse_ids(&mut self, + file: &Path, + contents: &str, + errors: &mut bool) +{ + if self.ids.is_empty() { + with_attrs_in_source(contents, " id", |fragment, i| { + let frag = fragment.trim_left_matches("#").to_owned(); + if !self.ids.insert(frag) { + *errors = true; + println!("{}:{}: id is not unique: `{}`", + file.display(), i, fragment); + } + }); + } + } +} + +fn walk(cache: &mut Cache, + root: &Path, + dir: &Path, + url: &mut Url, + errors: &mut bool) +{ for entry in t!(dir.read_dir()).map(|e| t!(e)) { let path = entry.path(); let kind = t!(entry.file_type()); url.path_mut().unwrap().push(entry.file_name().into_string().unwrap()); if kind.is_dir() { - walk(root, &path, url, errors); + walk(cache, root, &path, url, errors); } else { - check(root, &path, url, errors); + let pretty_path = check(cache, root, &path, url, errors); + if let Some(pretty_path) = pretty_path { + let entry = cache.get_mut(&pretty_path).unwrap(); + // we don't need the source anymore, + // so drop to to reduce memory-usage + entry.source = String::new(); + } } url.path_mut().unwrap().pop(); } } -fn check(root: &Path, file: &Path, base: &Url, errors: &mut bool) { +fn check(cache: &mut Cache, + root: &Path, + file: &Path, + base: &Url, + errors: &mut bool) -> Option +{ // ignore js files as they are not prone to errors as the rest of the // documentation is and they otherwise bring up false positives. if file.extension().and_then(|s| s.to_str()) == Some("js") { - return + return None; } - let pretty_file = file.strip_prefix(root).unwrap_or(file); - // Unfortunately we're not 100% full of valid links today to we need a few // whitelists to get this past `make check` today. // FIXME(#32129) if file.ends_with("std/string/struct.String.html") { - return + return None; + } + // FIXME(#32553) + if file.ends_with("collections/string/struct.String.html") { + return None; } // FIXME(#32130) if file.ends_with("btree_set/struct.BTreeSet.html") || - file.ends_with("collections/struct.BTreeSet.html") { - return + file.ends_with("collections/struct.BTreeSet.html") || + file.ends_with("collections/btree_map/struct.BTreeMap.html") || + file.ends_with("collections/hash_map/struct.HashMap.html") { + return None; } if file.ends_with("std/sys/ext/index.html") { - return + return None; } if let Some(file) = file.to_str() { // FIXME(#31948) if file.contains("ParseFloatError") { - return + return None; } // weird reexports, but this module is on its way out, so chalk it up to // "rustdoc weirdness" and move on from there if file.contains("scoped_tls") { - return + return None; } } let mut parser = UrlParser::new(); parser.base_url(base); - let mut contents = String::new(); - if t!(File::open(file)).read_to_string(&mut contents).is_err() { - return + + let res = load_file(cache, root, PathBuf::from(file), SkipRedirect); + let (pretty_file, contents) = match res { + Ok(res) => res, + Err(_) => return None, + }; + { + cache.get_mut(&pretty_file).unwrap() + .parse_ids(&pretty_file, &contents, errors); } + // Search for anything that's the regex 'href[ ]*=[ ]*".*?"' + with_attrs_in_source(&contents, " href", |url, i| { + // Once we've plucked out the URL, parse it using our base url and + // then try to extract a file path. If either of these fail then we + // just keep going. + let (parsed_url, path) = match url_to_file_path(&parser, url) { + Some((url, path)) => (url, PathBuf::from(path)), + None => return, + }; + + // Alright, if we've found a file name then this file had better + // exist! If it doesn't then we register and print an error. + if path.exists() { + if path.is_dir() { + return; + } + let res = load_file(cache, root, path.clone(), FromRedirect(false)); + let (pretty_path, contents) = match res { + Ok(res) => res, + Err(LoadError::IOError(err)) => panic!(format!("{}", err)), + Err(LoadError::BrokenRedirect(target, _)) => { + print!("{}:{}: broken redirect to {}", + pretty_file.display(), i + 1, target.display()); + return; + } + Err(LoadError::IsRedirect) => unreachable!(), + }; + + if let Some(ref fragment) = parsed_url.fragment { + // Fragments like `#1-6` are most likely line numbers to be + // interpreted by javascript, so we're ignoring these + if fragment.splitn(2, '-') + .all(|f| f.chars().all(|c| c.is_numeric())) { + return; + } + + let entry = &mut cache.get_mut(&pretty_path).unwrap(); + entry.parse_ids(&pretty_path, &contents, errors); + + if !entry.ids.contains(fragment) { + *errors = true; + print!("{}:{}: broken link fragment ", + pretty_file.display(), i + 1); + println!("`#{}` pointing to `{}`", + fragment, pretty_path.display()); + }; + } + } else { + *errors = true; + print!("{}:{}: broken link - ", pretty_file.display(), i + 1); + let pretty_path = path.strip_prefix(root).unwrap_or(&path); + println!("{}", pretty_path.display()); + } + }); + Some(pretty_file) +} + +fn load_file(cache: &mut Cache, + root: &Path, + file: PathBuf, + redirect: Redirect) -> Result<(PathBuf, String), LoadError> { + let mut contents = String::new(); + let pretty_file = PathBuf::from(file.strip_prefix(root).unwrap_or(&file)); + + let maybe_redirect = match cache.entry(pretty_file.clone()) { + Entry::Occupied(entry) => { + contents = entry.get().source.clone(); + None + }, + Entry::Vacant(entry) => { + let mut fp = try!(File::open(file.clone()).map_err(|err| { + if let FromRedirect(true) = redirect { + LoadError::BrokenRedirect(file.clone(), err) + } else { + LoadError::IOError(err) + } + })); + try!(fp.read_to_string(&mut contents) + .map_err(|err| LoadError::IOError(err))); + + let maybe = maybe_redirect(&contents); + if maybe.is_some() { + if let SkipRedirect = redirect { + return Err(LoadError::IsRedirect); + } + } else { + entry.insert(FileEntry { + source: contents.clone(), + ids: HashSet::new(), + }); + } + maybe + }, + }; + let base = Url::from_file_path(&file).unwrap(); + let mut parser = UrlParser::new(); + parser.base_url(&base); + + match maybe_redirect.and_then(|url| url_to_file_path(&parser, &url)) { + Some((_, redirect_file)) => { + let path = PathBuf::from(redirect_file); + load_file(cache, root, path, FromRedirect(true)) + } + None => Ok((pretty_file, contents)) + } +} + +fn maybe_redirect(source: &str) -> Option { + const REDIRECT: &'static str = "

Redirecting to Option<(Url, PathBuf)> { + parser.parse(url).ok().and_then(|parsed_url| { + parsed_url.to_file_path().ok().map(|f| (parsed_url, f)) + }) +} + +fn with_attrs_in_source(contents: &str, + attr: &str, + mut f: F) +{ for (i, mut line) in contents.lines().enumerate() { - // Search for anything that's the regex 'href[ ]*=[ ]*".*?"' - while let Some(j) = line.find(" href") { - let rest = &line[j + 5..]; + while let Some(j) = line.find(attr) { + let rest = &line[j + attr.len() ..]; line = rest; let pos_equals = match rest.find("=") { Some(i) => i, @@ -121,40 +321,24 @@ fn check(root: &Path, file: &Path, base: &Url, errors: &mut bool) { if rest[..pos_equals].trim_left_matches(" ") != "" { continue } + let rest = &rest[pos_equals + 1..]; - let pos_quote = match rest.find("\"").or_else(|| rest.find("'")) { + + let pos_quote = match rest.find(&['"', '\''][..]) { Some(i) => i, None => continue, }; + let quote_delim = rest.as_bytes()[pos_quote] as char; + if rest[..pos_quote].trim_left_matches(" ") != "" { continue } let rest = &rest[pos_quote + 1..]; - let url = match rest.find("\"").or_else(|| rest.find("'")) { + let url = match rest.find(quote_delim) { Some(i) => &rest[..i], None => continue, }; - - // Once we've plucked out the URL, parse it using our base url and - // then try to extract a file path. If either if these fail then we - // just keep going. - let parsed_url = match parser.parse(url) { - Ok(url) => url, - Err(..) => continue, - }; - let path = match parsed_url.to_file_path() { - Ok(path) => path, - Err(..) => continue, - }; - - // Alright, if we've found a file name then this file had better - // exist! If it doesn't then we register and print an error. - if !path.exists() { - *errors = true; - print!("{}:{}: broken link - ", pretty_file.display(), i + 1); - let pretty_path = path.strip_prefix(root).unwrap_or(&path); - println!("{}", pretty_path.display()); - } + f(url, i) } } }