Improve code

Author: GuillaumeGomez

src/librustdoc/clean/inline.rs

@@ -227,7 +227,7 @@ pub(crate) fn item_relative_path(tcx: TyCtxt<'_>, def_id: DefId) -> Vec<Symbol>
     tcx.def_path(def_id).data.into_iter().filter_map(|elem| elem.data.get_opt_name()).collect()
 }
 
-/// Get the path to an item in a URL sense: we use it to generate the URL to the actual item.
+/// Get the public Rust path to an item. This is used to generate the URL to the item's page.
 ///
 /// In particular: we handle macro differently: if it's not a macro 2.0 oe a built-in macro, then
 /// it is generated at the top-level of the crate and its path will be `[crate_name, macro_name]`.

src/librustdoc/clean/types.rs

@@ -40,6 +40,7 @@ use crate::clean::utils::{is_literal_expr, print_evaluated_const};
 use crate::core::DocContext;
 use crate::formats::cache::Cache;
 use crate::formats::item_type::ItemType;
+use crate::html::format::HrefInfo;
 use crate::html::render::Context;
 use crate::passes::collect_intra_doc_links::UrlFragment;
 
@@ -519,16 +520,16 @@ impl Item {
             .iter()
             .filter_map(|ItemLink { link: s, link_text, page_id: id, fragment }| {
                 debug!(?id);
-                if let Ok((mut href, ..)) = href(*id, cx) {
-                    debug!(?href);
+                if let Ok(HrefInfo { mut url, .. }) = href(*id, cx) {
+                    debug!(?url);
                     if let Some(ref fragment) = *fragment {
-                        fragment.render(&mut href, cx.tcx())
+                        fragment.render(&mut url, cx.tcx())
                     }
                     Some(RenderedLink {
                         original_text: s.clone(),
                         new_text: link_text.clone(),
                         tooltip: link_tooltip(*id, fragment, cx).to_string(),
-                        href,
+                        href: url,
                     })
                 } else {
                     None

src/librustdoc/display.rs

@@ -10,19 +10,18 @@ pub(crate) trait Joined: IntoIterator {
     ///
     /// The performance of `joined` is slightly better than `format`, since it doesn't need to use a `Cell` to keep track of whether [`fmt`](Display::fmt)
     /// was already called (`joined`'s API doesn't allow it be called more than once).
-    fn joined(self, sep: impl Display, f: &mut Formatter<'_>) -> fmt::Result;
+    fn joined(&mut self, sep: impl Display, f: &mut Formatter<'_>) -> fmt::Result;
 }
 
 impl<I, T> Joined for I
 where
-    I: IntoIterator<Item = T>,
+    I: Iterator<Item = T>,
     T: Display,
 {
-    fn joined(self, sep: impl Display, f: &mut Formatter<'_>) -> fmt::Result {
-        let mut iter = self.into_iter();
-        let Some(first) = iter.next() else { return Ok(()) };
+    fn joined(&mut self, sep: impl Display, f: &mut Formatter<'_>) -> fmt::Result {
+        let Some(first) = self.next() else { return Ok(()) };
         first.fmt(f)?;
-        for item in iter {
+        for item in self {
             sep.fmt(f)?;
             item.fmt(f)?;
         }

src/librustdoc/html/format.rs

@@ -349,13 +349,23 @@ pub(crate) enum HrefError {
     UnnamableItem,
 }
 
+/// Type representing information of an `href` attribute.
+pub(crate) struct HrefInfo {
+    /// URL to the item page.
+    pub(crate) url: String,
+    /// Kind of the item (used to generate the `title` attribute).
+    pub(crate) kind: ItemType,
+    /// Rust path to the item (used to generate the `title` attribute).
+    pub(crate) rust_path: Vec<Symbol>,
+}
+
 /// This function is to get the external macro path because they are not in the cache used in
 /// `href_with_root_path`.
 fn generate_macro_def_id_path(
     def_id: DefId,
     cx: &Context<'_>,
     root_path: Option<&str>,
-) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
+) -> Result<HrefInfo, HrefError> {
     let tcx = cx.tcx();
     let crate_name = tcx.crate_name(def_id.krate);
     let cache = cx.cache();
@@ -384,9 +394,11 @@ fn generate_macro_def_id_path(
         return Err(HrefError::NotInExternalCache);
     }
 
-    let fqp = path.clone();
-    if let Some(last) = path.last_mut() {
-        *last = Symbol::intern(&format!("{}.{last}.html", item_type.as_str()));
+    // FIXME: Try to use `iter().chain().once()` instead.
+    let mut prev = None;
+    if let Some(last) = path.pop() {
+        path.push(Symbol::intern(&format!("{}.{last}.html", item_type.as_str())));
+        prev = Some(last);
     }
 
     let url = match cache.extern_locations[&def_id.krate] {
@@ -407,7 +419,11 @@ fn generate_macro_def_id_path(
             return Err(HrefError::NotInExternalCache);
         }
     };
-    Ok((url, item_type, fqp))
+    if let Some(prev) = prev {
+        path.pop();
+        path.push(prev);
+    }
+    Ok(HrefInfo { url, kind: item_type, rust_path: path })
 }
 
 /// If the function succeeded, it will return the full URL to the item, its type and a `Vec`
@@ -418,7 +434,7 @@ fn generate_item_def_id_path(
     cx: &Context<'_>,
     root_path: Option<&str>,
     original_def_kind: DefKind,
-) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
+) -> Result<HrefInfo, HrefError> {
     use rustc_middle::traits::ObligationCause;
     use rustc_trait_selection::infer::TyCtxtInferExt;
     use rustc_trait_selection::traits::query::normalize::QueryNormalizeExt;
@@ -454,7 +470,7 @@ fn generate_item_def_id_path(
         let kind = ItemType::from_def_kind(original_def_kind, Some(def_kind));
         url_parts = format!("{url_parts}#{kind}.{}", tcx.item_name(original_def_id))
     };
-    Ok((url_parts, shortty, fqp))
+    Ok(HrefInfo { url: url_parts, kind: shortty, rust_path: fqp })
 }
 
 /// Checks if the given defid refers to an item that is unnamable, such as one defined in a const block.
@@ -529,7 +545,7 @@ pub(crate) fn href_with_root_path(
     original_did: DefId,
     cx: &Context<'_>,
     root_path: Option<&str>,
-) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
+) -> Result<HrefInfo, HrefError> {
     let tcx = cx.tcx();
     let def_kind = tcx.def_kind(original_did);
     let did = match def_kind {
@@ -595,14 +611,14 @@ pub(crate) fn href_with_root_path(
             }
         }
     };
-    let url_parts = make_href(root_path, shortty, url_parts, fqp, is_remote);
-    Ok((url_parts, shortty, fqp.clone()))
+    Ok(HrefInfo {
+        url: make_href(root_path, shortty, url_parts, fqp, is_remote),
+        kind: shortty,
+        rust_path: fqp.clone(),
+    })
 }
 
-pub(crate) fn href(
-    did: DefId,
-    cx: &Context<'_>,
-) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
+pub(crate) fn href(did: DefId, cx: &Context<'_>) -> Result<HrefInfo, HrefError> {
     href_with_root_path(did, cx, None)
 }
 
@@ -689,12 +705,12 @@ fn resolved_path(
     } else {
         let path = fmt::from_fn(|f| {
             if use_absolute {
-                if let Ok((_, _, fqp)) = href(did, cx) {
+                if let Ok(HrefInfo { rust_path, .. }) = href(did, cx) {
                     write!(
                         f,
                         "{path}::{anchor}",
-                        path = join_path_syms(&fqp[..fqp.len() - 1]),
-                        anchor = print_anchor(did, *fqp.last().unwrap(), cx)
+                        path = join_path_syms(&rust_path[..rust_path.len() - 1]),
+                        anchor = print_anchor(did, *rust_path.last().unwrap(), cx)
                     )
                 } else {
                     write!(f, "{}", last.name)
@@ -823,12 +839,11 @@ fn print_higher_ranked_params_with_space(
 
 pub(crate) fn print_anchor(did: DefId, text: Symbol, cx: &Context<'_>) -> impl Display {
     fmt::from_fn(move |f| {
-        let parts = href(did, cx);
-        if let Ok((url, short_ty, fqp)) = parts {
+        if let Ok(HrefInfo { url, kind, rust_path }) = href(did, cx) {
             write!(
                 f,
-                r#"<a class="{short_ty}" href="{url}" title="{short_ty} {path}">{text}</a>"#,
-                path = join_path_syms(fqp),
+                r#"<a class="{kind}" href="{url}" title="{kind} {path}">{text}</a>"#,
+                path = join_path_syms(rust_path),
                 text = EscapeBodyText(text.as_str()),
             )
         } else {
@@ -1055,14 +1070,14 @@ fn print_qpath_data(qpath_data: &clean::QPathData, cx: &Context<'_>) -> impl Dis
                 None => self_type.def_id(cx.cache()).and_then(|did| href(did, cx).ok()),
             };
 
-            if let Some((url, _, path)) = parent_href {
+            if let Some(HrefInfo { url, rust_path, .. }) = parent_href {
                 write!(
                     f,
                     "<a class=\"associatedtype\" href=\"{url}#{shortty}.{name}\" \
                                 title=\"type {path}::{name}\">{name}</a>",
                     shortty = ItemType::AssocType,
                     name = assoc.name,
-                    path = join_path_syms(path),
+                    path = join_path_syms(rust_path),
                 )
             } else {
                 write!(f, "{}", assoc.name)

src/librustdoc/html/highlight.rs

@@ -21,6 +21,7 @@ use super::format;
 use crate::clean::PrimitiveType;
 use crate::display::Joined as _;
 use crate::html::escape::EscapeBodyText;
+use crate::html::format::HrefInfo;
 use crate::html::macro_expansion::ExpandedCode;
 use crate::html::render::span_map::{DUMMY_SP, Span};
 use crate::html::render::{Context, LinkFromSrc};
@@ -1357,19 +1358,19 @@ fn generate_link_to_def(
                     LinkFromSrc::External(def_id) => {
                         format::href_with_root_path(*def_id, context, Some(href_context.root_path))
                             .ok()
-                            .map(|(url, _, _)| url)
+                            .map(|HrefInfo { url, .. }| url)
                     }
                     LinkFromSrc::Primitive(prim) => format::href_with_root_path(
                         PrimitiveType::primitive_locations(context.tcx())[prim],
                         context,
                         Some(href_context.root_path),
                     )
                     .ok()
-                    .map(|(url, _, _)| url),
+                    .map(|HrefInfo { url, .. }| url),
                     LinkFromSrc::Doc(def_id) => {
                         format::href_with_root_path(*def_id, context, Some(href_context.root_path))
                             .ok()
-                            .map(|(doc_link, _, _)| doc_link)
+                            .map(|HrefInfo { url, .. }| url)
                     }
                 }
             })

src/librustdoc/html/render/mod.rs

@@ -74,7 +74,7 @@ use crate::formats::cache::Cache;
 use crate::formats::item_type::ItemType;
 use crate::html::escape::Escape;
 use crate::html::format::{
-    Ending, HrefError, PrintWithSpace, full_print_fn_decl, href, print_abi_with_space,
+    Ending, HrefError, HrefInfo, PrintWithSpace, full_print_fn_decl, href, print_abi_with_space,
     print_constness_with_space, print_default_space, print_generic_bounds, print_generics,
     print_impl, print_path, print_type, print_where_clause, visibility_print_with_space,
 };
@@ -982,7 +982,7 @@ fn assoc_href_attr(
             };
 
             match href(did.expect_def_id(), cx) {
-                Ok((url, ..)) => Href::Url(url, item_type),
+                Ok(HrefInfo { url, .. }) => Href::Url(url, item_type),
                 // The link is broken since it points to an external crate that wasn't documented.
                 // Do not create any link in such case. This is better than falling back to a
                 // dummy anchor like `#{item_type}.{name}` representing the `id` of *this* impl item