Rollup merge of #80660 - max-heller:issue-80559-fix, r=jyn514

Properly handle primitive disambiguators in rustdoc

Fixes #80559

r? ``@jyn514``

Is there a way to test that the generated intra-doc link is what I expect?

4 files changed, 92 insertions, 46 deletions

diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index 0cefbb34791..11ee59b2401 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -394,10 +394,14 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
                         ns,
                         impl_,
                     )
-                    .map(|item| match item.kind {
-                        ty::AssocKind::Fn => "method",
-                        ty::AssocKind::Const => "associatedconstant",
-                        ty::AssocKind::Type => "associatedtype",
+                    .map(|item| {
+                        let kind = item.kind;
+                        self.kind_side_channel.set(Some((kind.as_def_kind(), item.def_id)));
+                        match kind {
+                            ty::AssocKind::Fn => "method",
+                            ty::AssocKind::Const => "associatedconstant",
+                            ty::AssocKind::Type => "associatedtype",
+                        }
                     })
                     .map(|out| {
                         (
@@ -1142,55 +1146,75 @@ impl LinkCollector<'_, '_> {
                 callback,
             );
         };
-        match res {
-            Res::Primitive(_) => match disambiguator {
-                Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => {
-                    Some(ItemLink { link: ori_link.link, link_text, did: None, fragment })
-                }
-                Some(other) => {
-                    report_mismatch(other, Disambiguator::Primitive);
-                    None
-                }
-            },
-            Res::Def(kind, id) => {
-                debug!("intra-doc link to {} resolved to {:?}", path_str, res);
-
-                // Disallow e.g. linking to enums with `struct@`
-                debug!("saw kind {:?} with disambiguator {:?}", kind, disambiguator);
-                match (self.kind_side_channel.take().map(|(kind, _)| kind).unwrap_or(kind), disambiguator) {
-                    | (DefKind::Const | DefKind::ConstParam | DefKind::AssocConst | DefKind::AnonConst, Some(Disambiguator::Kind(DefKind::Const)))
-                    // NOTE: this allows 'method' to mean both normal functions and associated functions
-                    // This can't cause ambiguity because both are in the same namespace.
-                    | (DefKind::Fn | DefKind::AssocFn, Some(Disambiguator::Kind(DefKind::Fn)))
-                    // These are namespaces; allow anything in the namespace to match
-                    | (_, Some(Disambiguator::Namespace(_)))
-                    // If no disambiguator given, allow anything
-                    | (_, None)
-                    // All of these are valid, so do nothing
-                    => {}
-                    (actual, Some(Disambiguator::Kind(expected))) if actual == expected => {}
-                    (_, Some(specified @ Disambiguator::Kind(_) | specified @ Disambiguator::Primitive)) => {
-                        report_mismatch(specified, Disambiguator::Kind(kind));
-                        return None;
-                    }
+
+        let verify = |kind: DefKind, id: DefId| {
+            debug!("intra-doc link to {} resolved to {:?}", path_str, res);
+
+            // Disallow e.g. linking to enums with `struct@`
+            debug!("saw kind {:?} with disambiguator {:?}", kind, disambiguator);
+            match (self.kind_side_channel.take().map(|(kind, _)| kind).unwrap_or(kind), disambiguator) {
+                | (DefKind::Const | DefKind::ConstParam | DefKind::AssocConst | DefKind::AnonConst, Some(Disambiguator::Kind(DefKind::Const)))
+                // NOTE: this allows 'method' to mean both normal functions and associated functions
+                // This can't cause ambiguity because both are in the same namespace.
+                | (DefKind::Fn | DefKind::AssocFn, Some(Disambiguator::Kind(DefKind::Fn)))
+                // These are namespaces; allow anything in the namespace to match
+                | (_, Some(Disambiguator::Namespace(_)))
+                // If no disambiguator given, allow anything
+                | (_, None)
+                // All of these are valid, so do nothing
+                => {}
+                (actual, Some(Disambiguator::Kind(expected))) if actual == expected => {}
+                (_, Some(specified @ Disambiguator::Kind(_) | specified @ Disambiguator::Primitive)) => {
+                    report_mismatch(specified, Disambiguator::Kind(kind));
+                    return None;
                 }
+            }
+
+            // item can be non-local e.g. when using #[doc(primitive = "pointer")]
+            if let Some((src_id, dst_id)) = id
+                .as_local()
+                .and_then(|dst_id| item.def_id.as_local().map(|src_id| (src_id, dst_id)))
+            {
+                use rustc_hir::def_id::LOCAL_CRATE;
 
-                // item can be non-local e.g. when using #[doc(primitive = "pointer")]
-                if let Some((src_id, dst_id)) = id
-                    .as_local()
-                    .and_then(|dst_id| item.def_id.as_local().map(|src_id| (src_id, dst_id)))
+                let hir_src = self.cx.tcx.hir().local_def_id_to_hir_id(src_id);
+                let hir_dst = self.cx.tcx.hir().local_def_id_to_hir_id(dst_id);
+
+                if self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_src)
+                    && !self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_dst)
                 {
-                    use rustc_hir::def_id::LOCAL_CRATE;
+                    privacy_error(cx, &item, &path_str, dox, &ori_link);
+                }
+            }
 
-                    let hir_src = self.cx.tcx.hir().local_def_id_to_hir_id(src_id);
-                    let hir_dst = self.cx.tcx.hir().local_def_id_to_hir_id(dst_id);
+            Some((kind, id))
+        };
 
-                    if self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_src)
-                        && !self.cx.tcx.privacy_access_levels(LOCAL_CRATE).is_exported(hir_dst)
-                    {
-                        privacy_error(cx, &item, &path_str, dox, &ori_link);
+        match res {
+            Res::Primitive(_) => {
+                if let Some((kind, id)) = self.kind_side_channel.take() {
+                    // We're actually resolving an associated item of a primitive, so we need to
+                    // verify the disambiguator (if any) matches the type of the associated item.
+                    // This case should really follow the same flow as the `Res::Def` branch below,
+                    // but attempting to add a call to `clean::register_res` causes an ICE. @jyn514
+                    // thinks `register_res` is only needed for cross-crate re-exports, but Rust
+                    // doesn't allow statements like `use str::trim;`, making this a (hopefully)
+                    // valid omission. See https://github.com/rust-lang/rust/pull/80660#discussion_r551585677
+                    // for discussion on the matter.
+                    verify(kind, id)?;
+                } else {
+                    match disambiguator {
+                        Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => {}
+                        Some(other) => {
+                            report_mismatch(other, Disambiguator::Primitive);
+                            return None;
+                        }
                     }
                 }
+                Some(ItemLink { link: ori_link.link, link_text, did: None, fragment })
+            }
+            Res::Def(kind, id) => {
+                let (kind, id) = verify(kind, id)?;
                 let id = clean::register_res(cx, rustc_hir::def::Res::Def(kind, id));
                 Some(ItemLink { link: ori_link.link, link_text, did: Some(id), fragment })
             }
diff --git a/src/test/rustdoc-ui/intra-doc/incompatible-primitive-disambiguator.rs b/src/test/rustdoc-ui/intra-doc/incompatible-primitive-disambiguator.rs
new file mode 100644
index 00000000000..0d1d5d1134b
--- /dev/null
+++ b/src/test/rustdoc-ui/intra-doc/incompatible-primitive-disambiguator.rs
@@ -0,0 +1,3 @@
+#![deny(broken_intra_doc_links)]
+//! [static@u8::MIN]
+//~^ ERROR incompatible link kind
diff --git a/src/test/rustdoc-ui/intra-doc/incompatible-primitive-disambiguator.stderr b/src/test/rustdoc-ui/intra-doc/incompatible-primitive-disambiguator.stderr
new file mode 100644
index 00000000000..ed1c10f9e0c
--- /dev/null
+++ b/src/test/rustdoc-ui/intra-doc/incompatible-primitive-disambiguator.stderr
@@ -0,0 +1,15 @@
+error: incompatible link kind for `u8::MIN`
+  --> $DIR/incompatible-primitive-disambiguator.rs:2:6
+   |
+LL | //! [static@u8::MIN]
+   |      ^^^^^^^^^^^^^^ help: to link to the associated constant, prefix with `const@`: `const@u8::MIN`
+   |
+note: the lint level is defined here
+  --> $DIR/incompatible-primitive-disambiguator.rs:1:9
+   |
+LL | #![deny(broken_intra_doc_links)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^
+   = note: this link resolved to an associated constant, which is not a static
+
+error: aborting due to previous error
+
diff --git a/src/test/rustdoc/intra-doc/primitive-disambiguators.rs b/src/test/rustdoc/intra-doc/primitive-disambiguators.rs
new file mode 100644
index 00000000000..acdd07566c9
--- /dev/null
+++ b/src/test/rustdoc/intra-doc/primitive-disambiguators.rs
@@ -0,0 +1,4 @@
+#![deny(broken_intra_doc_links)]
+// @has primitive_disambiguators/index.html
+// @has - '//a/@href' 'https://doc.rust-lang.org/nightly/std/primitive.str.html#method.trim'
+//! [str::trim()]