about summary refs log tree commit diff
path: root/src
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2024-11-13 21:04:22 +0100
committerGitHub <noreply@github.com>2024-11-13 21:04:22 +0100
commit3376edac6d2c3648b398f59ac599dc6562faacdc (patch)
tree4e754b1080a7969f8a47ceb49b5baaefda2b48f6 /src
parentf3df2a212607be650cb802b633dcb166307e12fd (diff)
parent9016711bf1f43db68f0ba51a9852b65a2270ae61 (diff)
downloadrust-3376edac6d2c3648b398f59ac599dc6562faacdc.tar.gz
rust-3376edac6d2c3648b398f59ac599dc6562faacdc.zip
Rollup merge of #132302 - fmease:rustdoc-better-vis-for-macro-decl, r=notriddle
rustdoc: Treat declarative macros more like other item kinds

Apparently at some time in the past we were unable to generate an href for the module path inside the visibility of decl macros 2.0 (`pub(in ...)`). As a result of this, a whole separate function was introduced specifically for printing the visibility of decl macros that didn't attempt to generate any links. The description of PR https://github.com/rust-lang/rust/pull/84074 states:

> This fixes the overly-complex invariant mentioned in https://github.com/rust-lang/rust/pull/83237#issuecomment-815346570, where the macro source can't have any links in it only because the cache hasn't been populated yet.

I can no longer reproduce the original issue. Reusing the existing visibility rendering logic *seems* to work just fine (I couldn't come up with any counterexamples, though I invite you to prove me wrong).

* Fixes #83000
* Fixes the visibility showing up "twice" in rustdoc-JSON output: Once as the `visibility` field, once baked into the source[^1]
* Fixes `#[doc(hidden)]` not getting rendered on doc(hidden) decl macros 2.0 under `--document-hiden-items` (for decl macros 1.2 the issue remains; I will address this separately when fixing #132304).

---

<details><summary>Outdated Section</summary>

NOTE: The current version of this PR is committing a UI crime, I'd like to receive feedback on that. Maybe you have a satisfactory solution for how to remedy it. Namely, as you know we have two different ways of / modes for highlighting code with color:

1. Only highlighting links / item paths and avoiding to highlight tokens by kind like keywords (to reduce visual noise and maybe also artifact size). Used for item declarations(\*).
2. Highlighting tokens by kind. Used for code blocks written by the user.

(\*): With the notable exception being macro declarations! Well, since this PR reuses the same function for rendering the item visibility (which only makes sense), we have a clash of modes: We now use both ways of highlighting code for decl macros: №1 for the visibility, №2 for the rest. This awkward. See for yourself:

* On master: ![Screenshot 2024-10-29 at 03-37-48 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/22f0ab6e-9ba9-4c4e-8fb0-0741c91d360b)
* On this branch: ![Screenshot 2024-10-29 at 03-36-41 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/b11d81a3-3e2e-43cb-a5b8-6773a3048732)

</details>

Furthermore, we now no longer syntax-highlight declarative macros (be it `macro_rules!` or `macro`) since that was inconsistent with the way we render all other item kinds. See (collapsed) *Outdated Section* above. See also https://github.com/rust-lang/rust/pull/132302#discussion_r1821310783.

| On master | On this branch |
|---|---|
| ![Screenshot 2024-11-13 at 16-12-46 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/cb3aeb42-a56d-4ced-80d9-f2694f369af1) | ![Screenshot 2024-11-13 at 16-13-22 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/b73bee50-1b85-4862-afba-5ad471443ccc) |

[^1]: E.g., `"visibility":{"restricted":{"parent":1,"path":"::a"}},/*OMITTED*/,"inner":{"macro":"pub(in a) macro by_example_vis_named($foo:expr) {\n    ...\n}"}`
Diffstat (limited to 'src')
-rw-r--r--src/librustdoc/clean/inline.rs18
-rw-r--r--src/librustdoc/clean/mod.rs11
-rw-r--r--src/librustdoc/clean/types.rs2
-rw-r--r--src/librustdoc/clean/utils.rs15
-rw-r--r--src/librustdoc/html/format.rs41
-rw-r--r--src/librustdoc/html/highlight.rs7
-rw-r--r--src/librustdoc/html/render/print_item.rs9
7 files changed, 21 insertions, 82 deletions
diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs
index 97529e420e3..5ddd6188c2d 100644
--- a/src/librustdoc/clean/inline.rs
+++ b/src/librustdoc/clean/inline.rs
@@ -134,11 +134,7 @@ pub(crate) fn try_inline(
             })
         }
         Res::Def(DefKind::Macro(kind), did) => {
-            let is_doc_hidden = cx.tcx.is_doc_hidden(did)
-                || attrs_without_docs
-                    .map(|(attrs, _)| attrs)
-                    .is_some_and(|attrs| utils::attrs_have_doc_flag(attrs.iter(), sym::hidden));
-            let mac = build_macro(cx, did, name, import_def_id, kind, is_doc_hidden);
+            let mac = build_macro(cx, did, name, kind);
 
             let type_kind = match kind {
                 MacroKind::Bang => ItemType::Macro,
@@ -740,18 +736,14 @@ fn build_macro(
     cx: &mut DocContext<'_>,
     def_id: DefId,
     name: Symbol,
-    import_def_id: Option<LocalDefId>,
     macro_kind: MacroKind,
-    is_doc_hidden: bool,
 ) -> clean::ItemKind {
     match CStore::from_tcx(cx.tcx).load_macro_untracked(def_id, cx.tcx) {
         LoadedMacro::MacroDef { def, .. } => match macro_kind {
-            MacroKind::Bang => {
-                let vis = cx.tcx.visibility(import_def_id.map(|d| d.to_def_id()).unwrap_or(def_id));
-                clean::MacroItem(clean::Macro {
-                    source: utils::display_macro_source(cx, name, &def, def_id, vis, is_doc_hidden),
-                })
-            }
+            MacroKind::Bang => clean::MacroItem(clean::Macro {
+                source: utils::display_macro_source(cx, name, &def),
+                macro_rules: def.macro_rules,
+            }),
             MacroKind::Derive | MacroKind::Attr => {
                 clean::ProcMacroItem(clean::ProcMacro { kind: macro_kind, helpers: Vec::new() })
             }
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index 15f75384b89..9e11360cab4 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -2787,13 +2787,10 @@ fn clean_maybe_renamed_item<'tcx>(
                 fields: variant_data.fields().iter().map(|x| clean_field(x, cx)).collect(),
             }),
             ItemKind::Impl(impl_) => return clean_impl(impl_, item.owner_id.def_id, cx),
-            ItemKind::Macro(macro_def, MacroKind::Bang) => {
-                let ty_vis = cx.tcx.visibility(def_id);
-                MacroItem(Macro {
-                    // FIXME this shouldn't be false
-                    source: display_macro_source(cx, name, macro_def, def_id, ty_vis, false),
-                })
-            }
+            ItemKind::Macro(macro_def, MacroKind::Bang) => MacroItem(Macro {
+                source: display_macro_source(cx, name, macro_def),
+                macro_rules: macro_def.macro_rules,
+            }),
             ItemKind::Macro(_, macro_kind) => clean_proc_macro(item, &mut name, macro_kind, cx),
             // proc macros can have a name set by attributes
             ItemKind::Fn(ref sig, generics, body_id) => {
diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
index e5c9539b5e7..e3a0dbe1a7f 100644
--- a/src/librustdoc/clean/types.rs
+++ b/src/librustdoc/clean/types.rs
@@ -2543,6 +2543,8 @@ pub(crate) struct ImportSource {
 #[derive(Clone, Debug)]
 pub(crate) struct Macro {
     pub(crate) source: String,
+    /// Whether the macro was defined via `macro_rules!` as opposed to `macro`.
+    pub(crate) macro_rules: bool,
 }
 
 #[derive(Clone, Debug)]
diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs
index ec41ad0e8e3..e551e0170c6 100644
--- a/src/librustdoc/clean/utils.rs
+++ b/src/librustdoc/clean/utils.rs
@@ -24,7 +24,6 @@ use crate::clean::{
     clean_middle_ty, inline,
 };
 use crate::core::DocContext;
-use crate::html::format::visibility_to_src_with_space;
 
 #[cfg(test)]
 mod tests;
@@ -593,7 +592,7 @@ pub(crate) static DOC_CHANNEL: Lazy<&'static str> =
 
 /// Render a sequence of macro arms in a format suitable for displaying to the user
 /// as part of an item declaration.
-pub(super) fn render_macro_arms<'a>(
+fn render_macro_arms<'a>(
     tcx: TyCtxt<'_>,
     matchers: impl Iterator<Item = &'a TokenTree>,
     arm_delim: &str,
@@ -614,9 +613,6 @@ pub(super) fn display_macro_source(
     cx: &mut DocContext<'_>,
     name: Symbol,
     def: &ast::MacroDef,
-    def_id: DefId,
-    vis: ty::Visibility<DefId>,
-    is_doc_hidden: bool,
 ) -> String {
     // Extract the spans of all matchers. They represent the "interface" of the macro.
     let matchers = def.body.tokens.chunks(4).map(|arm| &arm[0]);
@@ -626,18 +622,13 @@ pub(super) fn display_macro_source(
     } else {
         if matchers.len() <= 1 {
             format!(
-                "{vis}macro {name}{matchers} {{\n    ...\n}}",
-                vis = visibility_to_src_with_space(Some(vis), cx.tcx, def_id, is_doc_hidden),
+                "macro {name}{matchers} {{\n    ...\n}}",
                 matchers = matchers
                     .map(|matcher| render_macro_matcher(cx.tcx, matcher))
                     .collect::<String>(),
             )
         } else {
-            format!(
-                "{vis}macro {name} {{\n{arms}}}",
-                vis = visibility_to_src_with_space(Some(vis), cx.tcx, def_id, is_doc_hidden),
-                arms = render_macro_arms(cx.tcx, matchers, ","),
-            )
+            format!("macro {name} {{\n{arms}}}", arms = render_macro_arms(cx.tcx, matchers, ","))
         }
     }
 }
diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
index e9d5ba2ea57..2f9e7976ca1 100644
--- a/src/librustdoc/html/format.rs
+++ b/src/librustdoc/html/format.rs
@@ -1615,47 +1615,6 @@ pub(crate) fn visibility_print_with_space<'a, 'tcx: 'a>(
     })
 }
 
-/// This function is the same as print_with_space, except that it renders no links.
-/// It's used for macros' rendered source view, which is syntax highlighted and cannot have
-/// any HTML in it.
-pub(crate) fn visibility_to_src_with_space<'a, 'tcx: 'a>(
-    visibility: Option<ty::Visibility<DefId>>,
-    tcx: TyCtxt<'tcx>,
-    item_did: DefId,
-    is_doc_hidden: bool,
-) -> impl Display + 'a + Captures<'tcx> {
-    let vis: Cow<'static, str> = match visibility {
-        None => "".into(),
-        Some(ty::Visibility::Public) => "pub ".into(),
-        Some(ty::Visibility::Restricted(vis_did)) => {
-            // FIXME(camelid): This may not work correctly if `item_did` is a module.
-            //                 However, rustdoc currently never displays a module's
-            //                 visibility, so it shouldn't matter.
-            let parent_module = find_nearest_parent_module(tcx, item_did);
-
-            if vis_did.is_crate_root() {
-                "pub(crate) ".into()
-            } else if parent_module == Some(vis_did) {
-                // `pub(in foo)` where `foo` is the parent module
-                // is the same as no visibility modifier
-                "".into()
-            } else if parent_module.and_then(|parent| find_nearest_parent_module(tcx, parent))
-                == Some(vis_did)
-            {
-                "pub(super) ".into()
-            } else {
-                format!("pub(in {}) ", tcx.def_path_str(vis_did)).into()
-            }
-        }
-    };
-    display_fn(move |f| {
-        if is_doc_hidden {
-            f.write_str("#[doc(hidden)] ")?;
-        }
-        f.write_str(&vis)
-    })
-}
-
 pub(crate) trait PrintWithSpace {
     fn print_with_space(&self) -> &str;
 }
diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs
index b68b7295096..4def80764ea 100644
--- a/src/librustdoc/html/highlight.rs
+++ b/src/librustdoc/html/highlight.rs
@@ -58,13 +58,6 @@ pub(crate) fn render_example_with_highlighting(
     write_footer(out, playground_button);
 }
 
-/// Highlights `src` as an item-decl, returning the HTML output.
-pub(crate) fn render_item_decl_with_highlighting(src: &str, out: &mut Buffer) {
-    write!(out, "<pre class=\"rust item-decl\">");
-    write_code(out, src, None, None);
-    write!(out, "</pre>");
-}
-
 fn write_header(
     out: &mut Buffer,
     class: &str,
diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs
index c6a2d87cbd0..d247e90d298 100644
--- a/src/librustdoc/html/render/print_item.rs
+++ b/src/librustdoc/html/render/print_item.rs
@@ -35,7 +35,6 @@ use crate::html::format::{
     Buffer, Ending, PrintWithSpace, display_fn, join_with_double_colon, print_abi_with_space,
     print_constness_with_space, print_where_clause, visibility_print_with_space,
 };
-use crate::html::highlight;
 use crate::html::markdown::{HeadingOffset, MarkdownSummaryLine};
 use crate::html::render::{document_full, document_item_info};
 use crate::html::url_parts_builder::UrlPartsBuilder;
@@ -1745,7 +1744,13 @@ fn item_variants(
 }
 
 fn item_macro(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean::Macro) {
-    highlight::render_item_decl_with_highlighting(&t.source, w);
+    wrap_item(w, |w| {
+        // FIXME: Also print `#[doc(hidden)]` for `macro_rules!` if it `is_doc_hidden`.
+        if !t.macro_rules {
+            write!(w, "{}", visibility_print_with_space(it, cx));
+        }
+        write!(w, "{}", Escape(&t.source));
+    });
     write!(w, "{}", document(cx, it, None, HeadingOffset::H2))
 }