Auto merge of #92601 - camelid:more-intra-doc-cleanup, r=Manishearth

rustdoc: Remove the intra-doc links side channel

The side channel made the code much more complex and harder to
understand. It was added as a temporary workaround in
0c99d806eabd32a2ee2e6c71b400222b99c659e1, but it's no longer necessary.

The addition of `UrlFragment` in #92088 was the key to getting rid of
the side channel. The semantic information (rather than the strings that
used to be used for fragments) that is now captured by `UrlFragment` is
enough to obviate the side channel. An additional change had to be made
to `UrlFragment` in this PR to make this possible: it now records
`DefId`s rather than item names.

This PR also consolidates the checks for anchor conflicts into one place.

r? `@Manishearth`

2 files changed, 148 insertions, 142 deletions

diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
index 491f7b2fa69..00c6e38839f 100644
--- a/src/librustdoc/clean/types.rs
+++ b/src/librustdoc/clean/types.rs
@@ -1,6 +1,5 @@
 use std::cell::RefCell;
 use std::default::Default;
-use std::fmt::Write;
 use std::hash::Hash;
 use std::lazy::SyncOnceCell as OnceCell;
 use std::path::PathBuf;
@@ -496,7 +495,7 @@ impl Item {
                 if let Ok((mut href, ..)) = href(*did, cx) {
                     debug!(?href);
                     if let Some(ref fragment) = *fragment {
-                        write!(href, "{}", fragment).unwrap()
+                        fragment.render(&mut href, cx.tcx()).unwrap()
                     }
                     Some(RenderedLink {
                         original_text: s.clone(),
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index 49ff4517a4e..9d1a8b3f80f 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -13,7 +13,7 @@ use rustc_hir::def::{
     PerNS,
 };
 use rustc_hir::def_id::{CrateNum, DefId};
-use rustc_middle::ty::{Ty, TyCtxt};
+use rustc_middle::ty::{DefIdTree, Ty, TyCtxt};
 use rustc_middle::{bug, span_bug, ty};
 use rustc_resolve::ParentScope;
 use rustc_session::lint::Lint;
@@ -25,8 +25,8 @@ use smallvec::{smallvec, SmallVec};
 use pulldown_cmark::LinkType;
 
 use std::borrow::Cow;
-use std::cell::Cell;
 use std::convert::{TryFrom, TryInto};
+use std::fmt::Write;
 use std::mem;
 use std::ops::Range;
 
@@ -47,12 +47,8 @@ crate const COLLECT_INTRA_DOC_LINKS: Pass = Pass {
 };
 
 fn collect_intra_doc_links(krate: Crate, cx: &mut DocContext<'_>) -> Crate {
-    let mut collector = LinkCollector {
-        cx,
-        mod_ids: Vec::new(),
-        kind_side_channel: Cell::new(None),
-        visited_links: FxHashMap::default(),
-    };
+    let mut collector =
+        LinkCollector { cx, mod_ids: Vec::new(), visited_links: FxHashMap::default() };
     collector.visit_crate(&krate);
     krate
 }
@@ -240,53 +236,73 @@ enum AnchorFailure {
 
 #[derive(Clone, Debug, Hash, PartialEq, Eq)]
 crate enum UrlFragment {
-    Method(Symbol),
-    TyMethod(Symbol),
-    AssociatedConstant(Symbol),
-    AssociatedType(Symbol),
-
-    StructField(Symbol),
-    Variant(Symbol),
-    VariantField { variant: Symbol, field: Symbol },
-
+    Item(ItemFragment),
     UserWritten(String),
 }
 
 impl UrlFragment {
+    /// Render the fragment, including the leading `#`.
+    crate fn render(&self, s: &mut String, tcx: TyCtxt<'_>) -> std::fmt::Result {
+        match self {
+            UrlFragment::Item(frag) => frag.render(s, tcx),
+            UrlFragment::UserWritten(raw) => write!(s, "#{}", raw),
+        }
+    }
+}
+
+#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
+crate struct ItemFragment(FragmentKind, DefId);
+
+#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
+crate enum FragmentKind {
+    Method,
+    TyMethod,
+    AssociatedConstant,
+    AssociatedType,
+
+    StructField,
+    Variant,
+    VariantField,
+}
+
+impl ItemFragment {
     /// Create a fragment for an associated item.
     ///
     /// `is_prototype` is whether this associated item is a trait method
     /// without a default definition.
-    fn from_assoc_item(name: Symbol, kind: ty::AssocKind, is_prototype: bool) -> Self {
+    fn from_assoc_item(def_id: DefId, kind: ty::AssocKind, is_prototype: bool) -> Self {
         match kind {
             ty::AssocKind::Fn => {
                 if is_prototype {
-                    UrlFragment::TyMethod(name)
+                    ItemFragment(FragmentKind::TyMethod, def_id)
                 } else {
-                    UrlFragment::Method(name)
+                    ItemFragment(FragmentKind::Method, def_id)
                 }
             }
-            ty::AssocKind::Const => UrlFragment::AssociatedConstant(name),
-            ty::AssocKind::Type => UrlFragment::AssociatedType(name),
+            ty::AssocKind::Const => ItemFragment(FragmentKind::AssociatedConstant, def_id),
+            ty::AssocKind::Type => ItemFragment(FragmentKind::AssociatedType, def_id),
         }
     }
-}
 
-/// Render the fragment, including the leading `#`.
-impl std::fmt::Display for UrlFragment {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "#")?;
-        match self {
-            UrlFragment::Method(name) => write!(f, "method.{}", name),
-            UrlFragment::TyMethod(name) => write!(f, "tymethod.{}", name),
-            UrlFragment::AssociatedConstant(name) => write!(f, "associatedconstant.{}", name),
-            UrlFragment::AssociatedType(name) => write!(f, "associatedtype.{}", name),
-            UrlFragment::StructField(name) => write!(f, "structfield.{}", name),
-            UrlFragment::Variant(name) => write!(f, "variant.{}", name),
-            UrlFragment::VariantField { variant, field } => {
-                write!(f, "variant.{}.field.{}", variant, field)
+    /// Render the fragment, including the leading `#`.
+    crate fn render(&self, s: &mut String, tcx: TyCtxt<'_>) -> std::fmt::Result {
+        write!(s, "#")?;
+        match *self {
+            ItemFragment(kind, def_id) => {
+                let name = tcx.item_name(def_id);
+                match kind {
+                    FragmentKind::Method => write!(s, "method.{}", name),
+                    FragmentKind::TyMethod => write!(s, "tymethod.{}", name),
+                    FragmentKind::AssociatedConstant => write!(s, "associatedconstant.{}", name),
+                    FragmentKind::AssociatedType => write!(s, "associatedtype.{}", name),
+                    FragmentKind::StructField => write!(s, "structfield.{}", name),
+                    FragmentKind::Variant => write!(s, "variant.{}", name),
+                    FragmentKind::VariantField => {
+                        let variant = tcx.item_name(tcx.parent(def_id).unwrap());
+                        write!(s, "variant.{}.field.{}", variant, name)
+                    }
+                }
             }
-            UrlFragment::UserWritten(raw) => write!(f, "{}", raw),
         }
     }
 }
@@ -296,7 +312,7 @@ struct ResolutionInfo {
     module_id: DefId,
     dis: Option<Disambiguator>,
     path_str: String,
-    extra_fragment: Option<UrlFragment>,
+    extra_fragment: Option<String>,
 }
 
 #[derive(Clone)]
@@ -310,7 +326,6 @@ struct DiagnosticInfo<'a> {
 #[derive(Clone, Debug, Hash)]
 struct CachedLink {
     pub res: (Res, Option<UrlFragment>),
-    pub side_channel: Option<(DefKind, DefId)>,
 }
 
 struct LinkCollector<'a, 'tcx> {
@@ -320,10 +335,6 @@ struct LinkCollector<'a, 'tcx> {
     /// The last module will be used if the parent scope of the current item is
     /// unknown.
     mod_ids: Vec<DefId>,
-    /// This is used to store the kind of associated items,
-    /// because `clean` and the disambiguator code expect them to be different.
-    /// See the code for associated items on inherent impls for details.
-    kind_side_channel: Cell<Option<(DefKind, DefId)>>,
     /// Cache the resolved links so we can avoid resolving (and emitting errors for) the same link.
     /// The link will be `None` if it could not be resolved (i.e. the error was cached).
     visited_links: FxHashMap<ResolutionInfo, Option<CachedLink>>,
@@ -340,7 +351,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
         &self,
         path_str: &'path str,
         module_id: DefId,
-    ) -> Result<(Res, Option<UrlFragment>), ErrorKind<'path>> {
+    ) -> Result<(Res, Option<ItemFragment>), ErrorKind<'path>> {
         let tcx = self.cx.tcx;
         let no_res = || ResolutionFailure::NotResolved {
             module_id,
@@ -387,14 +398,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
                 }
                 match tcx.type_of(did).kind() {
                     ty::Adt(def, _) if def.is_enum() => {
-                        if def.all_fields().any(|item| item.ident.name == variant_field_name) {
-                            Ok((
-                                ty_res,
-                                Some(UrlFragment::VariantField {
-                                    variant: variant_name,
-                                    field: variant_field_name,
-                                }),
-                            ))
+                        if let Some(field) =
+                            def.all_fields().find(|f| f.ident.name == variant_field_name)
+                        {
+                            Ok((ty_res, Some(ItemFragment(FragmentKind::VariantField, field.did))))
                         } else {
                             Err(ResolutionFailure::NotResolved {
                                 module_id,
@@ -422,7 +429,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
         prim_ty: PrimitiveType,
         ns: Namespace,
         item_name: Symbol,
-    ) -> Option<(Res, UrlFragment, Option<(DefKind, DefId)>)> {
+    ) -> Option<(Res, ItemFragment)> {
         let tcx = self.cx.tcx;
 
         prim_ty.impls(tcx).into_iter().find_map(|&impl_| {
@@ -430,8 +437,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
                 .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, impl_)
                 .map(|item| {
                     let kind = item.kind;
-                    let fragment = UrlFragment::from_assoc_item(item_name, kind, false);
-                    (Res::Primitive(prim_ty), fragment, Some((kind.as_def_kind(), item.def_id)))
+                    let fragment = ItemFragment::from_assoc_item(item.def_id, kind, false);
+                    (Res::Primitive(prim_ty), fragment)
                 })
         })
     }
@@ -505,8 +512,30 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
         path_str: &'path str,
         ns: Namespace,
         module_id: DefId,
-        extra_fragment: &Option<UrlFragment>,
+        user_fragment: &Option<String>,
     ) -> Result<(Res, Option<UrlFragment>), ErrorKind<'path>> {
+        let (res, rustdoc_fragment) = self.resolve_inner(path_str, ns, module_id)?;
+        let chosen_fragment = match (user_fragment, rustdoc_fragment) {
+            (Some(_), Some(r_frag)) => {
+                let diag_res = match r_frag {
+                    ItemFragment(_, did) => Res::Def(self.cx.tcx.def_kind(did), did),
+                };
+                let failure = AnchorFailure::RustdocAnchorConflict(diag_res);
+                return Err(ErrorKind::AnchorFailure(failure));
+            }
+            (Some(u_frag), None) => Some(UrlFragment::UserWritten(u_frag.clone())),
+            (None, Some(r_frag)) => Some(UrlFragment::Item(r_frag)),
+            (None, None) => None,
+        };
+        Ok((res, chosen_fragment))
+    }
+
+    fn resolve_inner<'path>(
+        &mut self,
+        path_str: &'path str,
+        ns: Namespace,
+        module_id: DefId,
+    ) -> Result<(Res, Option<ItemFragment>), ErrorKind<'path>> {
         if let Some(res) = self.resolve_path(path_str, ns, module_id) {
             match res {
                 // FIXME(#76467): make this fallthrough to lookup the associated
@@ -514,10 +543,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
                 Res::Def(DefKind::AssocFn | DefKind::AssocConst, _) => assert_eq!(ns, ValueNS),
                 Res::Def(DefKind::AssocTy, _) => assert_eq!(ns, TypeNS),
                 Res::Def(DefKind::Variant, _) => {
-                    return handle_variant(self.cx, res, extra_fragment);
+                    return handle_variant(self.cx, res);
                 }
                 // Not a trait item; just return what we found.
-                _ => return Ok((res, extra_fragment.clone())),
+                _ => return Ok((res, None)),
             }
         }
 
@@ -548,23 +577,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
         resolve_primitive(&path_root, TypeNS)
             .or_else(|| self.resolve_path(&path_root, TypeNS, module_id))
             .and_then(|ty_res| {
-                let (res, fragment, side_channel) =
+                let (res, fragment) =
                     self.resolve_associated_item(ty_res, item_name, ns, module_id)?;
-                let result = if extra_fragment.is_some() {
-                    // NOTE: can never be a primitive since `side_channel.is_none()` only when `res`
-                    // is a trait (and the side channel DefId is always an associated item).
-                    let diag_res = side_channel.map_or(res, |(k, r)| Res::Def(k, r));
-                    Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(diag_res)))
-                } else {
-                    // HACK(jynelson): `clean` expects the type, not the associated item
-                    // but the disambiguator logic expects the associated item.
-                    // Store the kind in a side channel so that only the disambiguator logic looks at it.
-                    if let Some((kind, id)) = side_channel {
-                        self.kind_side_channel.set(Some((kind, id)));
-                    }
-                    Ok((res, Some(fragment)))
-                };
-                Some(result)
+
+                Some(Ok((res, Some(fragment))))
             })
             .unwrap_or_else(|| {
                 if ns == Namespace::ValueNS {
@@ -661,7 +677,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
         item_name: Symbol,
         ns: Namespace,
         module_id: DefId,
-    ) -> Option<(Res, UrlFragment, Option<(DefKind, DefId)>)> {
+    ) -> Option<(Res, ItemFragment)> {
         let tcx = self.cx.tcx;
 
         match root_res {
@@ -676,11 +692,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
 
                     assoc_item.map(|item| {
                         let kind = item.kind;
-                        let fragment = UrlFragment::from_assoc_item(item_name, kind, false);
-                        // HACK(jynelson): `clean` expects the type, not the associated item
-                        // but the disambiguator logic expects the associated item.
-                        // Store the kind in a side channel so that only the disambiguator logic looks at it.
-                        (root_res, fragment, Some((kind.as_def_kind(), item.def_id)))
+                        let fragment = ItemFragment::from_assoc_item(item.def_id, kind, false);
+                        (root_res, fragment)
                     })
                 })
             }
@@ -730,11 +743,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
 
                 if let Some(item) = assoc_item {
                     let kind = item.kind;
-                    let fragment = UrlFragment::from_assoc_item(item_name, kind, false);
-                    // HACK(jynelson): `clean` expects the type, not the associated item
-                    // but the disambiguator logic expects the associated item.
-                    // Store the kind in a side channel so that only the disambiguator logic looks at it.
-                    return Some((root_res, fragment, Some((kind.as_def_kind(), item.def_id))));
+                    let fragment = ItemFragment::from_assoc_item(item.def_id, kind, false);
+                    return Some((root_res, fragment));
                 }
 
                 if ns != Namespace::ValueNS {
@@ -765,23 +775,19 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
                     .fields
                     .iter()
                     .find(|item| item.ident.name == item_name)?;
-                Some((
-                    root_res,
-                    UrlFragment::StructField(field.ident.name),
-                    Some((DefKind::Field, field.did)),
-                ))
+                Some((root_res, ItemFragment(FragmentKind::StructField, field.did)))
             }
             Res::Def(DefKind::Trait, did) => tcx
                 .associated_items(did)
                 .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, did)
                 .map(|item| {
-                    let fragment = UrlFragment::from_assoc_item(
-                        item_name,
+                    let fragment = ItemFragment::from_assoc_item(
+                        item.def_id,
                         item.kind,
                         !item.defaultness.has_value(),
                     );
                     let res = Res::Def(item.kind.as_def_kind(), item.def_id);
-                    (res, fragment, None)
+                    (res, fragment)
                 }),
             _ => None,
         }
@@ -798,23 +804,32 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
         ns: Namespace,
         path_str: &str,
         module_id: DefId,
-        extra_fragment: &Option<UrlFragment>,
+        extra_fragment: &Option<String>,
     ) -> Option<Res> {
         // resolve() can't be used for macro namespace
         let result = match ns {
-            Namespace::MacroNS => self.resolve_macro(path_str, module_id).map_err(ErrorKind::from),
+            Namespace::MacroNS => self
+                .resolve_macro(path_str, module_id)
+                .map(|res| (res, None))
+                .map_err(ErrorKind::from),
             Namespace::TypeNS | Namespace::ValueNS => {
-                self.resolve(path_str, ns, module_id, extra_fragment).map(|(res, _)| res)
+                self.resolve(path_str, ns, module_id, extra_fragment)
             }
         };
 
         let res = match result {
-            Ok(res) => Some(res),
+            Ok((res, frag)) => {
+                if let Some(UrlFragment::Item(ItemFragment(_, id))) = frag {
+                    Some(Res::Def(self.cx.tcx.def_kind(id), id))
+                } else {
+                    Some(res)
+                }
+            }
             Err(ErrorKind::Resolve(box kind)) => kind.full_res(),
             Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))) => Some(res),
             Err(ErrorKind::AnchorFailure(AnchorFailure::MultipleAnchors)) => None,
         };
-        self.kind_side_channel.take().map(|(kind, id)| Res::Def(kind, id)).or(res)
+        res
     }
 }
 
@@ -912,8 +927,6 @@ fn is_derive_trait_collision<T>(ns: &PerNS<Result<(Res, T), ResolutionFailure<'_
 
 impl<'a, 'tcx> DocVisitor for LinkCollector<'a, 'tcx> {
     fn visit_item(&mut self, item: &Item) {
-        use rustc_middle::ty::DefIdTree;
-
         let parent_node =
             item.def_id.as_def_id().and_then(|did| find_nearest_parent_module(self.cx.tcx, did));
         if parent_node.is_some() {
@@ -1033,7 +1046,7 @@ impl From<AnchorFailure> for PreprocessingError<'_> {
 struct PreprocessingInfo {
     path_str: String,
     disambiguator: Option<Disambiguator>,
-    extra_fragment: Option<UrlFragment>,
+    extra_fragment: Option<String>,
     link_text: String,
 }
 
@@ -1119,7 +1132,7 @@ fn preprocess_link<'a>(
     Some(Ok(PreprocessingInfo {
         path_str,
         disambiguator,
-        extra_fragment: extra_fragment.map(|frag| UrlFragment::UserWritten(frag.to_owned())),
+        extra_fragment: extra_fragment.map(|frag| frag.to_owned()),
         link_text: link_text.to_owned(),
     }))
 }
@@ -1286,7 +1299,11 @@ impl LinkCollector<'_, '_> {
         };
 
         let verify = |kind: DefKind, id: DefId| {
-            let (kind, id) = self.kind_side_channel.take().unwrap_or((kind, id));
+            let (kind, id) = if let Some(UrlFragment::Item(ItemFragment(_, id))) = fragment {
+                (self.cx.tcx.def_kind(id), id)
+            } else {
+                (kind, id)
+            };
             debug!("intra-doc link to {} resolved to {:?} (id: {:?})", path_str, res, id);
 
             // Disallow e.g. linking to enums with `struct@`
@@ -1330,7 +1347,9 @@ impl LinkCollector<'_, '_> {
 
         match res {
             Res::Primitive(prim) => {
-                if let Some((kind, id)) = self.kind_side_channel.take() {
+                if let Some(UrlFragment::Item(ItemFragment(_, id))) = fragment {
+                    let kind = self.cx.tcx.def_kind(id);
+
                     // 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,
@@ -1347,22 +1366,7 @@ impl LinkCollector<'_, '_> {
                         && item.def_id.is_local()
                         && !self.cx.tcx.features().intra_doc_pointers
                     {
-                        let span = super::source_span_for_markdown_range(
-                            self.cx.tcx,
-                            dox,
-                            &ori_link.range,
-                            &item.attrs,
-                        )
-                        .unwrap_or_else(|| item.attr_span(self.cx.tcx));
-
-                        rustc_session::parse::feature_err(
-                            &self.cx.tcx.sess.parse_sess,
-                            sym::intra_doc_pointers,
-                            span,
-                            "linking to associated items of raw pointers is experimental",
-                        )
-                        .note("rustdoc does not allow disambiguating between `*const` and `*mut`, and pointers are unstable until it does")
-                        .emit();
+                        self.report_rawptr_assoc_feature_gate(dox, &ori_link, item);
                     }
                 } else {
                     match disambiguator {
@@ -1389,6 +1393,20 @@ impl LinkCollector<'_, '_> {
         }
     }
 
+    fn report_rawptr_assoc_feature_gate(&self, dox: &str, ori_link: &MarkdownLink, item: &Item) {
+        let span =
+            super::source_span_for_markdown_range(self.cx.tcx, dox, &ori_link.range, &item.attrs)
+                .unwrap_or_else(|| item.attr_span(self.cx.tcx));
+        rustc_session::parse::feature_err(
+            &self.cx.tcx.sess.parse_sess,
+            sym::intra_doc_pointers,
+            span,
+            "linking to associated items of raw pointers is experimental",
+        )
+        .note("rustdoc does not allow disambiguating between `*const` and `*mut`, and pointers are unstable until it does")
+        .emit();
+    }
+
     fn resolve_with_disambiguator_cached(
         &mut self,
         key: ResolutionInfo,
@@ -1399,7 +1417,6 @@ impl LinkCollector<'_, '_> {
         if let Some(ref cached) = self.visited_links.get(&key) {
             match cached {
                 Some(cached) => {
-                    self.kind_side_channel.set(cached.side_channel);
                     return Some(cached.res.clone());
                 }
                 None if cache_resolution_failure => return None,
@@ -1416,13 +1433,7 @@ impl LinkCollector<'_, '_> {
         // Cache only if resolved successfully - don't silence duplicate errors
         if let Some(res) = res {
             // Store result for the actual namespace
-            self.visited_links.insert(
-                key,
-                Some(CachedLink {
-                    res: res.clone(),
-                    side_channel: self.kind_side_channel.clone().into_inner(),
-                }),
-            );
+            self.visited_links.insert(key, Some(CachedLink { res: res.clone() }));
 
             Some(res)
         } else {
@@ -1484,7 +1495,7 @@ impl LinkCollector<'_, '_> {
                 let mut candidates = PerNS {
                     macro_ns: self
                         .resolve_macro(path_str, base_node)
-                        .map(|res| (res, extra_fragment.clone())),
+                        .map(|res| (res, extra_fragment.clone().map(UrlFragment::UserWritten))),
                     type_ns: match self.resolve(path_str, TypeNS, base_node, extra_fragment) {
                         Ok(res) => {
                             debug!("got res in TypeNS: {:?}", res);
@@ -1516,7 +1527,10 @@ impl LinkCollector<'_, '_> {
                                         // Shouldn't happen but who knows?
                                         Ok((res, Some(fragment)))
                                     }
-                                    (fragment, None) | (None, fragment) => Ok((res, fragment)),
+                                    (fragment, None) => Ok((res, fragment)),
+                                    (None, fragment) => {
+                                        Ok((res, fragment.map(UrlFragment::UserWritten)))
+                                    }
                                 }
                             }
                         }
@@ -1553,7 +1567,7 @@ impl LinkCollector<'_, '_> {
             }
             Some(MacroNS) => {
                 match self.resolve_macro(path_str, base_node) {
-                    Ok(res) => Some((res, extra_fragment.clone())),
+                    Ok(res) => Some((res, extra_fragment.clone().map(UrlFragment::UserWritten))),
                     Err(mut kind) => {
                         // `resolve_macro` only looks in the macro namespace. Try to give a better error if possible.
                         for ns in [TypeNS, ValueNS] {
@@ -2272,20 +2286,13 @@ fn privacy_error(cx: &DocContext<'_>, diag_info: &DiagnosticInfo<'_>, path_str:
 fn handle_variant(
     cx: &DocContext<'_>,
     res: Res,
-    extra_fragment: &Option<UrlFragment>,
-) -> Result<(Res, Option<UrlFragment>), ErrorKind<'static>> {
-    use rustc_middle::ty::DefIdTree;
-
-    if extra_fragment.is_some() {
-        // NOTE: `res` can never be a primitive since this function is only called when `tcx.def_kind(res) == DefKind::Variant`.
-        return Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res)));
-    }
+) -> Result<(Res, Option<ItemFragment>), ErrorKind<'static>> {
     cx.tcx
         .parent(res.def_id(cx.tcx))
         .map(|parent| {
             let parent_def = Res::Def(DefKind::Enum, parent);
             let variant = cx.tcx.expect_variant_res(res.as_hir_res().unwrap());
-            (parent_def, Some(UrlFragment::Variant(variant.ident.name)))
+            (parent_def, Some(ItemFragment(FragmentKind::Variant, variant.def_id)))
         })
         .ok_or_else(|| ResolutionFailure::NoParentItem.into())
 }