diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 7d5e235c88526..1bf6117c9a955 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -2388,7 +2388,7 @@ impl UseTree { /// Distinguishes between `Attribute`s that decorate items and Attributes that /// are contained as statements within items. These two cases need to be /// distinguished for pretty-printing. -#[derive(Clone, PartialEq, Encodable, Decodable, Debug, Copy, HashStable_Generic)] +#[derive(Clone, PartialEq, Eq, Hash, Encodable, Decodable, Debug, Copy, HashStable_Generic)] pub enum AttrStyle { Outer, Inner, diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 32b3f69ecd4f0..916d3b4567d45 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -385,6 +385,7 @@ pub struct DocFragment { pub parent_module: Option, pub doc: String, pub kind: DocFragmentKind, + pub style: AttrStyle, } #[derive(Clone, PartialEq, Eq, Debug, Hash)] @@ -421,7 +422,6 @@ pub struct Attributes { pub span: Option, /// map from Rust paths to resolved defs and potential URL fragments pub links: Vec, - pub inner_docs: bool, } #[derive(Clone, Debug, Default, PartialEq, Eq, Hash)] @@ -559,6 +559,7 @@ impl Attributes { doc: value, kind, parent_module, + style: attr.style, }); if sp.is_none() { @@ -584,6 +585,7 @@ impl Attributes { doc: contents, kind: DocFragmentKind::Include { filename }, parent_module: parent_module, + style: attr.style, }); } } @@ -618,18 +620,12 @@ impl Attributes { } } - let inner_docs = attrs - .iter() - .find(|a| a.doc_str().is_some()) - .map_or(true, |a| a.style == AttrStyle::Inner); - Attributes { doc_strings, other_attrs, cfg: if cfg == Cfg::True { None } else { Some(Arc::new(cfg)) }, span: sp, links: vec![], - inner_docs, } } diff --git a/src/librustdoc/passes/collapse_docs.rs b/src/librustdoc/passes/collapse_docs.rs index c2f7f97a673ce..6f928015b8a92 100644 --- a/src/librustdoc/passes/collapse_docs.rs +++ b/src/librustdoc/passes/collapse_docs.rs @@ -39,7 +39,9 @@ fn collapse(doc_strings: &mut Vec) { if matches!(*curr_kind, DocFragmentKind::Include { .. }) || curr_kind != new_kind || curr_frag.parent_module != frag.parent_module + || curr_frag.style != frag.style { + // Keep these as two separate attributes if *curr_kind == DocFragmentKind::SugaredDoc || *curr_kind == DocFragmentKind::RawDoc { @@ -49,6 +51,7 @@ fn collapse(doc_strings: &mut Vec) { docs.push(curr_frag); last_frag = Some(frag); } else { + // Combine both attributes into one curr_frag.doc.push('\n'); curr_frag.doc.push_str(&frag.doc); curr_frag.span = curr_frag.span.to(frag.span); diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 8be9482acffde..ac3b3fec25ec5 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -737,6 +737,7 @@ fn is_derive_trait_collision(ns: &PerNS DocFolder for LinkCollector<'a, 'tcx> { fn fold_item(&mut self, mut item: Item) -> Option { + use rustc_ast::AttrStyle; use rustc_middle::ty::DefIdTree; let parent_node = if item.is_fake() { @@ -773,7 +774,8 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { let current_item = match item.inner { ModuleItem(..) => { - if item.attrs.inner_docs { + // FIXME: this will be wrong if there are both inner and outer docs. + if item.attrs.doc_strings.iter().any(|attr| attr.style == AttrStyle::Inner) { if item.def_id.is_top_level_module() { item.name.clone() } else { None } } else { match parent_node.or(self.mod_ids.last().copied()) { @@ -798,10 +800,6 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { _ => item.name.clone(), }; - if item.is_mod() && item.attrs.inner_docs { - self.mod_ids.push(item.def_id); - } - // find item's parent to resolve `Self` in item's docs below let parent_name = self.cx.as_local_hir_id(item.def_id).and_then(|item_hir| { let parent_hir = self.cx.tcx.hir().get_parent_item(item_hir); @@ -839,6 +837,10 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } }); + // If there are both inner and outer docs, we want to only resolve the inner docs + // within the module. + let mut seen_inner_docs = false; + // We want to resolve in the lexical scope of the documentation. // In the presence of re-exports, this is not the same as the module of the item. // Rather than merging all documentation into one, resolve it one attribute at a time @@ -849,10 +851,14 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { // we want `///` and `#[doc]` to count as the same attribute, // but currently it will treat them as separate. // As a workaround, combine all attributes with the same parent module into the same attribute. + // NOTE: this can combine attributes across different spans, + // for example both inside and outside a crate. let mut combined_docs = attr.doc.clone(); loop { match attrs.peek() { - Some(next) if next.parent_module == attr.parent_module => { + Some(next) + if next.parent_module == attr.parent_module && next.style == attr.style => + { combined_docs.push('\n'); combined_docs.push_str(&attrs.next().unwrap().doc); } @@ -868,15 +874,39 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { trace!("no parent found for {:?}", attr.doc); (item.def_id.krate, parent_node) }; + + // In order to correctly resolve intra-doc-links we need to + // pick a base AST node to work from. If the documentation for + // this module came from an inner comment (//!) then we anchor + // our name resolution *inside* the module. If, on the other + // hand it was an outer comment (///) then we anchor the name + // resolution in the parent module on the basis that the names + // used are more likely to be intended to be parent names. For + // this, we set base_node to None for inner comments since + // we've already pushed this node onto the resolution stack but + // for outer comments we explicitly try and resolve against the + // parent_node first. + + // NOTE: there is an implicit assumption here that outer docs will always come + // before inner docs. + let base_node = if !seen_inner_docs && item.is_mod() && attr.style == AttrStyle::Inner { + // FIXME(jynelson): once `Self` handling is cleaned up I think we can get rid + // of `mod_ids` altogether + self.mod_ids.push(item.def_id); + seen_inner_docs = true; + Some(item.def_id) + } else { + parent_node + }; + // NOTE: if there are links that start in one crate and end in another, this will not resolve them. // This is a degenerate case and it's not supported by rustdoc. - // FIXME: this will break links that start in `#[doc = ...]` and end as a sugared doc. Should this be supported? for (ori_link, link_range) in markdown_links(&combined_docs) { let link = self.resolve_link( &item, &combined_docs, ¤t_item, - parent_node, + base_node, &parent_name, krate, ori_link, @@ -888,11 +918,10 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } - if item.is_mod() && !item.attrs.inner_docs { - self.mod_ids.push(item.def_id); - } - if item.is_mod() { + if !seen_inner_docs { + self.mod_ids.push(item.def_id); + } let ret = self.fold_item_recur(item); self.mod_ids.pop(); @@ -910,7 +939,7 @@ impl LinkCollector<'_, '_> { item: &Item, dox: &str, current_item: &Option, - parent_node: Option, + base_node: Option, parent_name: &Option, krate: CrateNum, ori_link: String, @@ -968,23 +997,6 @@ impl LinkCollector<'_, '_> { .map(|d| d.display_for(path_str)) .unwrap_or_else(|| path_str.to_owned()); - // In order to correctly resolve intra-doc-links we need to - // pick a base AST node to work from. If the documentation for - // this module came from an inner comment (//!) then we anchor - // our name resolution *inside* the module. If, on the other - // hand it was an outer comment (///) then we anchor the name - // resolution in the parent module on the basis that the names - // used are more likely to be intended to be parent names. For - // this, we set base_node to None for inner comments since - // we've already pushed this node onto the resolution stack but - // for outer comments we explicitly try and resolve against the - // parent_node first. - let base_node = if item.is_mod() && item.attrs.inner_docs { - self.mod_ids.last().copied() - } else { - parent_node - }; - let mut module_id = if let Some(id) = base_node { id } else { @@ -1185,6 +1197,8 @@ impl LinkCollector<'_, '_> { ori_link: &str, link_range: Option>, ) -> Option<(Res, Option)> { + debug!("resolving {} relative to {:?}", path_str, base_node); + match disambiguator.map(Disambiguator::ns) { Some(ns @ (ValueNS | TypeNS)) => { match self.resolve(path_str, ns, ¤t_item, base_node, &extra_fragment) { diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 2591650e3f97f..098ef2da419df 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -179,7 +179,9 @@ crate fn source_span_for_markdown_range( return None; } + trace!("markdown is {}", markdown); let snippet = cx.sess().source_map().span_to_snippet(span_of_attrs(attrs)?).ok()?; + trace!("snippet is {}", snippet); let starting_line = markdown[..md_range.start].matches('\n').count(); let ending_line = starting_line + markdown[md_range.start..md_range.end].matches('\n').count(); @@ -196,7 +198,9 @@ crate fn source_span_for_markdown_range( 'outer: for (line_no, md_line) in md_lines.enumerate() { loop { - let source_line = src_lines.next().expect("could not find markdown in source"); + let source_line = src_lines.next().unwrap_or_else(|| { + panic!("could not find markdown range {:?} in source", md_range) + }); match source_line.find(md_line) { Some(offset) => { if line_no == starting_line { diff --git a/src/test/rustdoc-ui/intra-link-inner-outer.rs b/src/test/rustdoc-ui/intra-link-inner-outer.rs new file mode 100644 index 0000000000000..4d1acb1f42952 --- /dev/null +++ b/src/test/rustdoc-ui/intra-link-inner-outer.rs @@ -0,0 +1,12 @@ +// check-pass + +pub enum A {} + +/// Links to [outer][A] and [outer][B] +//~^ WARNING: unresolved link to `B` +pub mod M { + //! Links to [inner][A] and [inner][B] + //~^ WARNING: unresolved link to `A` + + pub struct B; +} diff --git a/src/test/rustdoc/intra-link-inner-outer.rs b/src/test/rustdoc/intra-link-inner-outer.rs new file mode 100644 index 0000000000000..5b6ac1f7527e6 --- /dev/null +++ b/src/test/rustdoc/intra-link-inner-outer.rs @@ -0,0 +1,24 @@ +#![crate_name = "foo"] + +pub enum A {} + +/// Links to [outer A][A] and [outer B][B] +// @has foo/M/index.html '//*[@href="../foo/enum.A.html"]' 'outer A' +// @!has foo/M/index.html '//*[@href="../foo/struct.B.html"]' 'outer B' +// doesn't resolve unknown links +pub mod M { + //! Links to [inner A][A] and [inner B][B] + // @!has foo/M/index.html '//*[@href="../foo/enum.A.html"]' 'inner A' + // @has foo/M/index.html '//*[@href="../foo/struct.B.html"]' 'inner B' + pub struct B; +} + +// distinguishes between links to inner and outer attributes +/// Links to [outer A][A] +// @has foo/N/index.html '//*[@href="../foo/enum.A.html"]' 'outer A' +pub mod N { + //! Links to [inner A][A] + // @has foo/N/index.html '//*[@href="../foo/struct.A.html"]' 'inner A' + + pub struct A; +}