From 42232ba70add056cf422960ac96986264870b313 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 18 Jul 2020 16:48:03 -0400 Subject: rustdoc: Add support for associated items even outside the impl itself Previously, associated items would only be available for linking directly on the `impl Trait for Type`. Now they can be used anywhere. - Make `item` for resolve mandatory - Refactor resolving associated items into a separate function - Remove broken trait_item logic - Don't skip the type namespace for associated items - Only call `variant_field` for `TypeNS` - Add test for associated items - Use exhaustive matches instead of wildcards Wildcards caused several bugs while implementing this. --- src/test/rustdoc/intra-link-associated-defaults.rs | 27 ++++++++++ src/test/rustdoc/intra-link-associated-items.rs | 59 ++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 src/test/rustdoc/intra-link-associated-defaults.rs create mode 100644 src/test/rustdoc/intra-link-associated-items.rs (limited to 'src/test') diff --git a/src/test/rustdoc/intra-link-associated-defaults.rs b/src/test/rustdoc/intra-link-associated-defaults.rs new file mode 100644 index 00000000000..2051129b948 --- /dev/null +++ b/src/test/rustdoc/intra-link-associated-defaults.rs @@ -0,0 +1,27 @@ +// ignore-tidy-linelength +#![deny(intra_doc_link_resolution_failure)] +#![feature(associated_type_defaults)] + +pub trait TraitWithDefault { + type T = usize; + fn f() -> Self::T { + 0 + } +} + +/// Link to [UsesDefaults::T] and [UsesDefaults::f] +// @has 'intra_link_associated_defaults/struct.UsesDefaults.html' '//a[@href="../intra_link_associated_defaults/struct.UsesDefaults.html#associatedtype.T"]' 'UsesDefaults::T' +// @has 'intra_link_associated_defaults/struct.UsesDefaults.html' '//a[@href="../intra_link_associated_defaults/struct.UsesDefaults.html#method.f"]' 'UsesDefaults::f' +pub struct UsesDefaults; +impl TraitWithDefault for UsesDefaults {} + +/// Link to [OverridesDefaults::T] and [OverridesDefaults::f] +// @has 'intra_link_associated_defaults/struct.OverridesDefaults.html' '//a[@href="../intra_link_associated_defaults/struct.OverridesDefaults.html#associatedtype.T"]' 'OverridesDefaults::T' +// @has 'intra_link_associated_defaults/struct.OverridesDefaults.html' '//a[@href="../intra_link_associated_defaults/struct.OverridesDefaults.html#method.f"]' 'OverridesDefaults::f' +pub struct OverridesDefaults; +impl TraitWithDefault for OverridesDefaults { + type T = bool; + fn f() -> bool { + true + } +} diff --git a/src/test/rustdoc/intra-link-associated-items.rs b/src/test/rustdoc/intra-link-associated-items.rs new file mode 100644 index 00000000000..16a21e33748 --- /dev/null +++ b/src/test/rustdoc/intra-link-associated-items.rs @@ -0,0 +1,59 @@ +// ignore-tidy-linelength +#![deny(intra_doc_link_resolution_failure)] + +/// [`std::collections::BTreeMap::into_iter`] +/// [`String::from`] is ambiguous as to which `From` impl +// @has 'intra_link_associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/collections/btree/map/struct.BTreeMap.html#method.into_iter"]' 'std::collections::BTreeMap::into_iter' +// @has 'intra_link_associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/string/struct.String.html#method.from"]' 'String::from' +pub fn foo() {} + +/// Link to [MyStruct], [link from struct][MyStruct::method], [MyStruct::clone], [MyStruct::Input] +// @has 'intra_link_associated_items/struct.MyStruct.html' '//a[@href="../intra_link_associated_items/struct.MyStruct.html"]' 'MyStruct' +// @has 'intra_link_associated_items/struct.MyStruct.html' '//a[@href="../intra_link_associated_items/struct.MyStruct.html#method.method"]' 'link from struct' +// @has 'intra_link_associated_items/struct.MyStruct.html' '//a[@href="../intra_link_associated_items/struct.MyStruct.html#method.clone"]' 'MyStruct::clone' +// @has 'intra_link_associated_items/struct.MyStruct.html' '//a[@href="../intra_link_associated_items/struct.MyStruct.html#associatedtype.Input"]' 'MyStruct::Input' +pub struct MyStruct { foo: () } + +impl Clone for MyStruct { + fn clone(&self) -> Self { + MyStruct + } +} + +pub trait T { + type Input; + fn method(i: Self::Input); +} + +impl T for MyStruct { + type Input = usize; + + /// [link from method][MyStruct::method] on method + // @has 'intra_link_associated_items/struct.MyStruct.html' '//a[@href="../intra_link_associated_items/struct.MyStruct.html#method.method"]' 'link from method' + fn method(i: usize) { + } +} + +/// Ambiguity between which trait to use +pub trait T1 { + fn ambiguous_method(); +} + +pub trait T2 { + fn ambiguous_method(); +} + +/// Link to [S::ambiguous_method] +// FIXME: there is no way to disambiguate these. +// Since we have `#[deny(intra_doc_failure)]`, we still know it was one or the other. +pub struct S; + +impl T1 for S { + fn ambiguous_method() {} +} + +impl T2 for S { + fn ambiguous_method() {} +} + +fn main() {} -- cgit 1.4.1-3-g733a5 From da921e97f4d3702ab60fb696699712444e8ec48f Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 8 Aug 2020 14:57:35 -0400 Subject: rustdoc: Only resolve traits in scope --- src/librustdoc/core.rs | 8 +++--- src/librustdoc/passes/collect_intra_doc_links.rs | 31 +++++++++++++--------- src/test/rustdoc-ui/assoc-item-not-in-scope.rs | 22 +++++++++++++++ src/test/rustdoc-ui/assoc-item-not-in-scope.stderr | 15 +++++++++++ 4 files changed, 59 insertions(+), 17 deletions(-) create mode 100644 src/test/rustdoc-ui/assoc-item-not-in-scope.rs create mode 100644 src/test/rustdoc-ui/assoc-item-not-in-scope.stderr (limited to 'src/test') diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 51d3b92b3e1..27debfc1743 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -69,11 +69,11 @@ pub struct DocContext<'tcx> { pub auto_traits: Vec, /// The options given to rustdoc that could be relevant to a pass. pub render_options: RenderOptions, - /// The traits implemented by a given type. + /// The traits in scope for a given module. /// /// See `collect_intra_doc_links::traits_implemented_by` for more details. - /// `map>` - pub type_trait_cache: RefCell>>, + /// `map>` + pub module_trait_cache: RefCell>>, } impl<'tcx> DocContext<'tcx> { @@ -515,7 +515,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt .filter(|trait_def_id| tcx.trait_is_auto(*trait_def_id)) .collect(), render_options, - type_trait_cache: RefCell::new(FxHashMap::default()), + module_trait_cache: RefCell::new(FxHashMap::default()), }; debug!("crate: {:?}", tcx.hir().krate()); diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index d6417ec2e15..2ed3d07974f 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -10,7 +10,7 @@ use rustc_hir::def::{ PerNS, Res, }; use rustc_hir::def_id::DefId; -use rustc_middle::ty::{self, TyCtxt}; +use rustc_middle::ty; use rustc_resolve::ParentScope; use rustc_session::lint; use rustc_span::hygiene::MacroKind; @@ -327,7 +327,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // To handle that properly resolve() would have to support // something like [`ambi_fn`](::ambi_fn) .or_else(|| { - let kind = resolve_associated_trait_item(did, item_name, ns, &self.cx); + let kind = resolve_associated_trait_item( + did, module_id, item_name, ns, &self.cx, + ); debug!("got associated item kind {:?}", kind); kind }); @@ -440,6 +442,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { fn resolve_associated_trait_item( did: DefId, + module: DefId, item_name: Symbol, ns: Namespace, cx: &DocContext<'_>, @@ -504,8 +507,7 @@ fn resolve_associated_trait_item( // Next consider explicit impls: `impl MyTrait for MyType` // Give precedence to inherent impls. if candidates.is_empty() { - let mut cache = cx.type_trait_cache.borrow_mut(); - let traits = cache.entry(did).or_insert_with(|| traits_implemented_by(cx.tcx, did)); + let traits = traits_implemented_by(cx, did, module); debug!("considering traits {:?}", traits); candidates.extend(traits.iter().filter_map(|&trait_| { cx.tcx @@ -519,27 +521,30 @@ fn resolve_associated_trait_item( candidates.pop().map(|(_, kind)| kind) } -/// Given a type, return all traits implemented by that type. +/// Given a type, return all traits in scope in `module` implemented by that type. /// /// NOTE: this cannot be a query because more traits could be available when more crates are compiled! /// So it is not stable to serialize cross-crate. -/// FIXME: this should only search traits in scope -fn traits_implemented_by<'a>(tcx: TyCtxt<'a>, type_: DefId) -> FxHashSet { - use rustc_hir::def_id::LOCAL_CRATE; +fn traits_implemented_by(cx: &DocContext<'_>, type_: DefId, module: DefId) -> FxHashSet { + let mut cache = cx.module_trait_cache.borrow_mut(); + let in_scope_traits = cache.entry(module).or_insert_with(|| { + cx.enter_resolver(|resolver| { + resolver.traits_in_scope(module).into_iter().map(|candidate| candidate.def_id).collect() + }) + }); - let all_traits = tcx.all_traits(LOCAL_CRATE).iter().copied(); - let ty = tcx.type_of(type_); - let iter = all_traits.flat_map(|trait_| { + let ty = cx.tcx.type_of(type_); + let iter = in_scope_traits.iter().flat_map(|&trait_| { trace!("considering explicit impl for trait {:?}", trait_); let mut saw_impl = false; // Look at each trait implementation to see if it's an impl for `did` - tcx.for_each_relevant_impl(trait_, ty, |impl_| { + cx.tcx.for_each_relevant_impl(trait_, ty, |impl_| { // FIXME: this is inefficient, find a way to short-circuit for_each_* so this doesn't take as long if saw_impl { return; } - let trait_ref = tcx.impl_trait_ref(impl_).expect("this is not an inherent impl"); + let trait_ref = cx.tcx.impl_trait_ref(impl_).expect("this is not an inherent impl"); // Check if these are the same type. let impl_type = trait_ref.self_ty(); debug!( diff --git a/src/test/rustdoc-ui/assoc-item-not-in-scope.rs b/src/test/rustdoc-ui/assoc-item-not-in-scope.rs new file mode 100644 index 00000000000..c5bb4305db7 --- /dev/null +++ b/src/test/rustdoc-ui/assoc-item-not-in-scope.rs @@ -0,0 +1,22 @@ +#![deny(broken_intra_doc_links)] + +#[derive(Debug)] +/// Link to [`S::fmt`] +//~^ ERROR unresolved link +pub struct S; + +pub mod inner { + use std::fmt::Debug; + use super::S; + + /// Link to [`S::fmt`] + pub fn f() {} +} + +pub mod ambiguous { + use std::fmt::{Display, Debug}; + use super::S; + + /// Link to [`S::fmt`] + pub fn f() {} +} diff --git a/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr b/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr new file mode 100644 index 00000000000..8827c9351a6 --- /dev/null +++ b/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr @@ -0,0 +1,15 @@ +error: unresolved link to `S::fmt` + --> $DIR/assoc-item-not-in-scope.rs:4:14 + | +LL | /// Link to [`S::fmt`] + | ^^^^^^^^ unresolved link + | +note: the lint level is defined here + --> $DIR/assoc-item-not-in-scope.rs:1:9 + | +LL | #![deny(broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^ + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + +error: aborting due to previous error + -- cgit 1.4.1-3-g733a5