From d3846aa8239b12e720fbe4c1578c5db9605d899a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 3 Mar 2023 14:54:41 +0100 Subject: [PATCH 1/2] Fix invalid suggestions on ambiguous intra doc links --- .../passes/collect_intra_doc_links.rs | 72 +++++++++++++++++-- 1 file changed, 68 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index bcb69d1a4ca8b..b07104e49bb1d 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1018,7 +1018,7 @@ impl LinkCollector<'_, '_> { } else { // `[char]` when a `char` module is in scope let candidates = vec![res, prim]; - ambiguity_error(self.cx, diag_info, path_str, candidates); + ambiguity_error(self.cx, diag_info, path_str, candidates, module_id); return None; } } @@ -1304,7 +1304,13 @@ impl LinkCollector<'_, '_> { if ignore_macro { candidates.macro_ns = None; } - ambiguity_error(self.cx, diag, path_str, candidates.present_items().collect()); + ambiguity_error( + self.cx, + diag, + path_str, + candidates.present_items().collect(), + base_node, + ); None } } @@ -1888,15 +1894,73 @@ fn report_malformed_generics( ); } +fn get_matching_items( + cx: &DocContext<'_>, + item_name: &str, + candidates: &mut Vec, + def_id: DefId, +) { + let assoc_items = cx.tcx.associated_items(def_id); + for assoc_item in assoc_items.in_definition_order() { + if assoc_item.name.as_str() == item_name { + candidates.push(Res::Def(assoc_item.kind.as_def_kind(), assoc_item.def_id)); + } + } +} + /// Report an ambiguity error, where there were multiple possible resolutions. fn ambiguity_error( - cx: &DocContext<'_>, + cx: &mut DocContext<'_>, diag_info: DiagnosticInfo<'_>, path_str: &str, - candidates: Vec, + mut candidates: Vec, + module_id: DefId, ) { let mut msg = format!("`{}` is ", path_str); + let mut def_id_counts = FxHashMap::default(); + let mut need_dedup = false; + + // If the item is an impl block or a trait, we need to disambiguate even further, otherwise + // `Res` will always tell it's an impl/trait. + for candidate in &candidates { + if let Some(def_id) = candidate.def_id(cx.tcx) { + def_id_counts + .entry(def_id) + .and_modify(|(count, _)| { + *count += 1; + need_dedup = true; + }) + .or_insert((1, *candidate)); + } + } + if need_dedup && let Some(item_name) = path_str.split("::").last() { + candidates.clear(); + for (def_id, (count, candidate)) in def_id_counts.into_iter() { + if count > 1 { + if matches!(cx.tcx.def_kind(def_id), DefKind::Trait | DefKind::Impl { .. }) { + // If it's a trait or an impl block, we can directly get the items from it. + get_matching_items(cx, item_name, &mut candidates, def_id); + } else { + // We go through the type's impl blocks. + for impl_ in cx.tcx.inherent_impls(def_id) { + get_matching_items(cx, item_name, &mut candidates, *impl_); + } + // We now go through trait impls. + let trait_impls = trait_impls_for( + cx, + cx.tcx.type_of(def_id).subst_identity(), + module_id, + ); + for (impl_, _) in trait_impls { + get_matching_items(cx, item_name, &mut candidates, impl_); + } + } + } else { + candidates.push(candidate); + } + } + } match candidates.as_slice() { [first_def, second_def] => { msg += &format!( From ff4352e85031c469fcc36e4f68f896aef2a0623b Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 3 Mar 2023 14:54:56 +0100 Subject: [PATCH 2/2] Add regression tests for #108653 --- .../issue-108653-associated-items-2.rs | 17 +++++ .../issue-108653-associated-items-2.stderr | 37 ++++++++++ .../issue-108653-associated-items-3.rs | 16 +++++ .../issue-108653-associated-items-3.stderr | 37 ++++++++++ .../issue-108653-associated-items-4.rs | 21 ++++++ .../issue-108653-associated-items-4.stderr | 22 ++++++ .../issue-108653-associated-items.rs | 35 ++++++++++ .../issue-108653-associated-items.stderr | 67 +++++++++++++++++++ 8 files changed, 252 insertions(+) create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.rs create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.stderr create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.rs create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.stderr create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.rs create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.stderr create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items.rs create mode 100644 tests/rustdoc-ui/intra-doc/issue-108653-associated-items.stderr diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.rs new file mode 100644 index 0000000000000..06583ce80eb35 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.rs @@ -0,0 +1,17 @@ +// This is ensuring that the UI output for associated items is as expected. + +#![deny(rustdoc::broken_intra_doc_links)] + +/// [`Trait::IDENT`] +//~^ ERROR +pub trait Trait { + type IDENT; + const IDENT: usize; +} + +/// [`Trait2::IDENT`] +//~^ ERROR +pub trait Trait2 { + type IDENT; + fn IDENT() {} +} diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.stderr new file mode 100644 index 0000000000000..c516cbc5d1803 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-2.stderr @@ -0,0 +1,37 @@ +error: `Trait::IDENT` is both an associated type and an associated constant + --> $DIR/issue-108653-associated-items-2.rs:5:7 + | +LL | /// [`Trait::IDENT`] + | ^^^^^^^^^^^^ ambiguous link + | +note: the lint level is defined here + --> $DIR/issue-108653-associated-items-2.rs:3:9 + | +LL | #![deny(rustdoc::broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Trait::IDENT`] + | +++++ +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@Trait::IDENT`] + | ++++++ + +error: `Trait2::IDENT` is both an associated type and an associated function + --> $DIR/issue-108653-associated-items-2.rs:12:7 + | +LL | /// [`Trait2::IDENT`] + | ^^^^^^^^^^^^^ ambiguous link + | +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Trait2::IDENT`] + | +++++ +help: to link to the associated function, add parentheses + | +LL | /// [`Trait2::IDENT()`] + | ++ + +error: aborting due to 2 previous errors + diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.rs new file mode 100644 index 0000000000000..a8fd6c834c0eb --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.rs @@ -0,0 +1,16 @@ +// This is ensuring that the UI output for associated items works when it's being documented +// from another item. + +#![deny(rustdoc::broken_intra_doc_links)] +#![allow(nonstandard_style)] + +pub trait Trait { + type Trait; + const Trait: usize; +} + +/// [`Trait`] +//~^ ERROR +/// [`Trait::Trait`] +//~^ ERROR +pub const Trait: usize = 0; diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.stderr new file mode 100644 index 0000000000000..f6ea969345b1b --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-3.stderr @@ -0,0 +1,37 @@ +error: `Trait` is both a trait and a constant + --> $DIR/issue-108653-associated-items-3.rs:12:7 + | +LL | /// [`Trait`] + | ^^^^^ ambiguous link + | +note: the lint level is defined here + --> $DIR/issue-108653-associated-items-3.rs:4:9 + | +LL | #![deny(rustdoc::broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the trait, prefix with `trait@` + | +LL | /// [`trait@Trait`] + | ++++++ +help: to link to the constant, prefix with `const@` + | +LL | /// [`const@Trait`] + | ++++++ + +error: `Trait::Trait` is both an associated type and an associated constant + --> $DIR/issue-108653-associated-items-3.rs:14:7 + | +LL | /// [`Trait::Trait`] + | ^^^^^^^^^^^^ ambiguous link + | +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Trait::Trait`] + | +++++ +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@Trait::Trait`] + | ++++++ + +error: aborting due to 2 previous errors + diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.rs new file mode 100644 index 0000000000000..85eb23101fe5c --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.rs @@ -0,0 +1,21 @@ +// This is ensuring that the UI output for associated items works when it's being documented +// from another item. + +#![deny(rustdoc::broken_intra_doc_links)] +#![allow(nonstandard_style)] + +pub trait Trait { + type Trait; +} + +/// [`Struct::Trait`] +//~^ ERROR +pub struct Struct; + +impl Trait for Struct { + type Trait = Struct; +} + +impl Struct { + pub const Trait: usize = 0; +} diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.stderr new file mode 100644 index 0000000000000..a8dc91204c083 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items-4.stderr @@ -0,0 +1,22 @@ +error: `Struct::Trait` is both an associated constant and an associated type + --> $DIR/issue-108653-associated-items-4.rs:11:7 + | +LL | /// [`Struct::Trait`] + | ^^^^^^^^^^^^^ ambiguous link + | +note: the lint level is defined here + --> $DIR/issue-108653-associated-items-4.rs:4:9 + | +LL | #![deny(rustdoc::broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@Struct::Trait`] + | ++++++ +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Struct::Trait`] + | +++++ + +error: aborting due to previous error + diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.rs b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.rs new file mode 100644 index 0000000000000..9f311e34f3cd1 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.rs @@ -0,0 +1,35 @@ +// This is ensuring that the UI output for associated items is as expected. + +#![deny(rustdoc::broken_intra_doc_links)] + +pub enum Enum { + IDENT, +} + +/// [`Self::IDENT`] +//~^ ERROR +pub trait Trait { + type IDENT; + fn IDENT(); +} + +/// [`Self::IDENT`] +//~^ ERROR +impl Trait for Enum { + type IDENT = usize; + fn IDENT() {} +} + +/// [`Self::IDENT2`] +//~^ ERROR +pub trait Trait2 { + type IDENT2; + const IDENT2: usize; +} + +/// [`Self::IDENT2`] +//~^ ERROR +impl Trait2 for Enum { + type IDENT2 = usize; + const IDENT2: usize = 0; +} diff --git a/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.stderr b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.stderr new file mode 100644 index 0000000000000..f189f7efe6cb7 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/issue-108653-associated-items.stderr @@ -0,0 +1,67 @@ +error: `Self::IDENT` is both an associated type and an associated function + --> $DIR/issue-108653-associated-items.rs:9:7 + | +LL | /// [`Self::IDENT`] + | ^^^^^^^^^^^ ambiguous link + | +note: the lint level is defined here + --> $DIR/issue-108653-associated-items.rs:3:9 + | +LL | #![deny(rustdoc::broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Self::IDENT`] + | +++++ +help: to link to the associated function, add parentheses + | +LL | /// [`Self::IDENT()`] + | ++ + +error: `Self::IDENT2` is both an associated type and an associated constant + --> $DIR/issue-108653-associated-items.rs:23:7 + | +LL | /// [`Self::IDENT2`] + | ^^^^^^^^^^^^ ambiguous link + | +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Self::IDENT2`] + | +++++ +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@Self::IDENT2`] + | ++++++ + +error: `Self::IDENT2` is both an associated type and an associated constant + --> $DIR/issue-108653-associated-items.rs:30:7 + | +LL | /// [`Self::IDENT2`] + | ^^^^^^^^^^^^ ambiguous link + | +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Self::IDENT2`] + | +++++ +help: to link to the associated constant, prefix with `const@` + | +LL | /// [`const@Self::IDENT2`] + | ++++++ + +error: `Self::IDENT` is both an associated type and an associated function + --> $DIR/issue-108653-associated-items.rs:16:7 + | +LL | /// [`Self::IDENT`] + | ^^^^^^^^^^^ ambiguous link + | +help: to link to the associated type, prefix with `type@` + | +LL | /// [`type@Self::IDENT`] + | +++++ +help: to link to the associated function, add parentheses + | +LL | /// [`Self::IDENT()`] + | ++ + +error: aborting due to 4 previous errors +