From: Tyler Mandry Date: Fri, 1 Nov 2019 18:20:12 +0000 (-0700) Subject: Rollup merge of #65857 - kinnison:kinnison/issue-55364, r=Manisheart,GuillaumeGomez X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=48a9f59ae7e1444ee38fb5afa849daacfb7d02e7;hp=19182058914b0b7fb246beaeb9469d6df8f509b8;p=rust.git Rollup merge of #65857 - kinnison:kinnison/issue-55364, r=Manisheart,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 #55364 though right now I'm not sure it's the right fix. r? @GuillaumeGomez --- diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index caa7f08f68c..ab34f8daad7 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -322,9 +322,26 @@ fn fold_item(&mut self, mut item: Item) -> Option { 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, ¤t_item, parent_node) { + if let Ok(res) = self.resolve(path_str, ns, ¤t_item, base_node) { res } else { resolution_failure(cx, &item, path_str, &dox, link_range); @@ -335,7 +352,7 @@ fn fold_item(&mut self, mut item: Item) -> Option { } } Some(ns @ TypeNS) => { - if let Ok(res) = self.resolve(path_str, ns, ¤t_item, parent_node) { + if let Ok(res) = self.resolve(path_str, ns, ¤t_item, base_node) { res } else { resolution_failure(cx, &item, path_str, &dox, link_range); @@ -348,10 +365,10 @@ fn fold_item(&mut self, mut item: Item) -> Option { let candidates = PerNS { macro_ns: macro_resolve(cx, path_str).map(|res| (res, None)), type_ns: self - .resolve(path_str, TypeNS, ¤t_item, parent_node) + .resolve(path_str, TypeNS, ¤t_item, base_node) .ok(), value_ns: self - .resolve(path_str, ValueNS, ¤t_item, parent_node) + .resolve(path_str, ValueNS, ¤t_item, base_node) .ok() .and_then(|(res, fragment)| { // Constructors are picked up in the type namespace. diff --git a/src/test/rustdoc/issue-55364.rs b/src/test/rustdoc/issue-55364.rs new file mode 100644 index 00000000000..200a29fc7ee --- /dev/null +++ b/src/test/rustdoc/issue-55364.rs @@ -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() {} + } + } + } + } +}