Skip to content

Commit

Permalink
Rollup merge of rust-lang#65857 - kinnison:kinnison/issue-55364, r=Ma…
Browse files Browse the repository at this point in the history
…nisheart,GuillaumeGomez

rustdoc: Resolve module-level doc references more locally

Module level docs should resolve intra-doc links as locally as
possible.  As such, this commit alters the heuristic for finding
intra-doc links such that we attempt to resolve names mentioned
in *inner* documentation comments within the (sub-)module rather
that from the context of its parent.

I'm hoping that this fixes rust-lang#55364 though right now I'm not sure it's the right fix.

r? @GuillaumeGomez
  • Loading branch information
tmandry authored Oct 31, 2019
2 parents 19550e4 + c24a099 commit ee58559
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 4 deletions.
25 changes: 21 additions & 4 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,26 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
continue;
}

// 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 {
None
} else {
parent_node
};

match kind {
Some(ns @ ValueNS) => {
if let Ok(res) = self.resolve(path_str, ns, &current_item, parent_node) {
if let Ok(res) = self.resolve(path_str, ns, &current_item, base_node) {
res
} else {
resolution_failure(cx, &item, path_str, &dox, link_range);
Expand All @@ -335,7 +352,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
}
}
Some(ns @ TypeNS) => {
if let Ok(res) = self.resolve(path_str, ns, &current_item, parent_node) {
if let Ok(res) = self.resolve(path_str, ns, &current_item, base_node) {
res
} else {
resolution_failure(cx, &item, path_str, &dox, link_range);
Expand All @@ -348,10 +365,10 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
let candidates = PerNS {
macro_ns: macro_resolve(cx, path_str).map(|res| (res, None)),
type_ns: self
.resolve(path_str, TypeNS, &current_item, parent_node)
.resolve(path_str, TypeNS, &current_item, base_node)
.ok(),
value_ns: self
.resolve(path_str, ValueNS, &current_item, parent_node)
.resolve(path_str, ValueNS, &current_item, base_node)
.ok()
.and_then(|(res, fragment)| {
// Constructors are picked up in the type namespace.
Expand Down
88 changes: 88 additions & 0 deletions src/test/rustdoc/issue-55364.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// ignore-tidy-linelength

// First a module with inner documentation

// @has issue_55364/subone/index.html
// These foo/bar links in the module's documentation should refer inside `subone`
// @has - '//section[@id="main"]/div[@class="docblock"]//a[@href="../../issue_55364/subone/fn.foo.html"]' 'foo'
// @has - '//section[@id="main"]/div[@class="docblock"]//a[@href="../../issue_55364/subone/fn.bar.html"]' 'bar'
pub mod subone {
//! See either [foo] or [bar].

// This should refer to subone's `bar`
// @has issue_55364/subone/fn.foo.html
// @has - '//section[@id="main"]/div[@class="docblock"]//a[@href="../../issue_55364/subone/fn.bar.html"]' 'bar'
/// See [bar]
pub fn foo() {}
// This should refer to subone's `foo`
// @has issue_55364/subone/fn.bar.html
// @has - '//section[@id="main"]/div[@class="docblock"]//a[@href="../../issue_55364/subone/fn.foo.html"]' 'foo'
/// See [foo]
pub fn bar() {}
}

// A module with outer documentation

// @has issue_55364/subtwo/index.html
// These foo/bar links in the module's documentation should not reference inside `subtwo`
// @!has - '//section[@id="main"]/div[@class="docblock"]//a[@href="../../issue_55364/subtwo/fn.foo.html"]' 'foo'
// @!has - '//section[@id="main"]/div[@class="docblock"]//a[@href="../../issue_55364/subtwo/fn.bar.html"]' 'bar'
// Instead it should be referencing the top level functions
// @has - '//section[@id="main"]/div[@class="docblock"]//a[@href="../../issue_55364/fn.foo.html"]' 'foo'
// @has - '//section[@id="main"]/div[@class="docblock"]//a[@href="../../issue_55364/fn.bar.html"]' 'bar'
// Though there should be such links later
// @has - '//section[@id="main"]/table//tr[@class="module-item"]/td/a[@class="fn"][@href="fn.foo.html"]' 'foo'
// @has - '//section[@id="main"]/table//tr[@class="module-item"]/td/a[@class="fn"][@href="fn.bar.html"]' 'bar'
/// See either [foo] or [bar].
pub mod subtwo {

// Despite the module's docs referring to the top level foo/bar,
// this should refer to subtwo's `bar`
// @has issue_55364/subtwo/fn.foo.html
// @has - '//section[@id="main"]/div[@class="docblock"]//a[@href="../../issue_55364/subtwo/fn.bar.html"]' 'bar'
/// See [bar]
pub fn foo() {}
// Despite the module's docs referring to the top level foo/bar,
// this should refer to subtwo's `foo`
// @has issue_55364/subtwo/fn.bar.html
// @has - '//section[@id="main"]/div[@class="docblock"]//a[@href="../../issue_55364/subtwo/fn.foo.html"]' 'foo'
/// See [foo]
pub fn bar() {}
}

// These are the function referred to by the module above with outer docs

/// See [bar]
pub fn foo() {}
/// See [foo]
pub fn bar() {}

// This module refers to the outer foo/bar by means of `super::`

// @has issue_55364/subthree/index.html
// This module should also refer to the top level foo/bar
// @has - '//section[@id="main"]/div[@class="docblock"]//a[@href="../../issue_55364/fn.foo.html"]' 'foo'
// @has - '//section[@id="main"]/div[@class="docblock"]//a[@href="../../issue_55364/fn.bar.html"]' 'bar'
pub mod subthree {
//! See either [foo][super::foo] or [bar][super::bar]
}

// Next we go *deeper* - In order to ensure it's not just "this or parent"
// we test `crate::` and a `super::super::...` chain
// @has issue_55364/subfour/subfive/subsix/subseven/subeight/index.html
// @has - '//section[@id="main"]/table//tr[@class="module-item"]/td[@class="docblock-short"]//a[@href="../../../../../../issue_55364/subone/fn.foo.html"]' 'other foo'
// @has - '//section[@id="main"]/table//tr[@class="module-item"]/td[@class="docblock-short"]//a[@href="../../../../../../issue_55364/subtwo/fn.bar.html"]' 'other bar'
pub mod subfour {
pub mod subfive {
pub mod subsix {
pub mod subseven {
pub mod subeight {
/// See [other foo][crate::subone::foo]
pub fn foo() {}
/// See [other bar][super::super::super::super::super::subtwo::bar]
pub fn bar() {}
}
}
}
}
}

0 comments on commit ee58559

Please sign in to comment.