Improve code and docs

Author: GuillaumeGomez

src/librustdoc/clean/mod.rs

@@ -2899,13 +2899,6 @@ fn clean_maybe_renamed_item<'tcx>(
                 fields: variant_data.fields().iter().map(|x| clean_field(x, cx)).collect(),
             }),
             ItemKind::Macro(_, macro_def, kinds) => match kinds {
-                MacroKinds::BANG => MacroItem(
-                    Macro {
-                        source: display_macro_source(cx.tcx, name, macro_def),
-                        macro_rules: macro_def.macro_rules,
-                    },
-                    MacroKinds::BANG,
-                ),
                 MacroKinds::ATTR => clean_proc_macro(item, &mut name, MacroKind::Attr, cx.tcx),
                 MacroKinds::DERIVE => clean_proc_macro(item, &mut name, MacroKind::Derive, cx.tcx),
                 _ => MacroItem(

src/librustdoc/clean/types.rs

@@ -745,31 +745,30 @@ impl Item {
         find_attr!(&self.attrs.other_attrs, NonExhaustive(..))
     }
 
-    /// Returns a documentation-level item type from the item.
+    /// Returns a documentation-level item type from the item. In case of a `macro_rules!` which
+    /// contains an attr/derive kind, it will always return `ItemType::Macro`. If you want all
+    /// kinds, you need to use [`Item::types`].
     pub(crate) fn type_(&self) -> ItemType {
         ItemType::from(self)
     }
 
-    // FIXME: Return an iterator instead of a `ThinVec`.
-    pub(crate) fn types(&self) -> ThinVec<ItemType> {
+    /// Returns an item types. There is only one case where it can return more than one kind:
+    /// for `macro_rules!` items which contain an attr/derive kind.
+    pub(crate) fn types(&self) -> impl Iterator<Item = ItemType> {
         if let ItemKind::MacroItem(_, macro_kinds) = self.kind {
-            let mut types = ThinVec::with_capacity(3);
-            for kind in macro_kinds.iter() {
-                match kind {
-                    MacroKinds::ATTR => types.push(ItemType::BangMacroAttribute),
-                    MacroKinds::DERIVE => types.push(ItemType::BangMacroDerive),
-                    MacroKinds::BANG => types.push(ItemType::Macro),
-                    _ => panic!("unsupported macro kind {kind:?}"),
-                }
-            }
-            return types;
+            Either::Right(macro_kinds.iter().map(|kind| match kind {
+                MacroKinds::ATTR => ItemType::BangMacroAttribute,
+                MacroKinds::DERIVE => ItemType::BangMacroDerive,
+                MacroKinds::BANG => ItemType::Macro,
+                _ => panic!("unsupported macro kind {kind:?}"),
+            }))
+        } else {
+            Either::Left(std::iter::once(self.type_()))
         }
-        let mut types = ThinVec::with_capacity(1);
-        types.push(self.type_());
-        types
     }
 
-    pub(crate) fn is_macro_rules(&self) -> bool {
+    /// Returns true if this a macro declared with the `macro` keyword or with `macro_rules!.
+    pub(crate) fn is_bang_macro_or_macro_rules(&self) -> bool {
         matches!(self.kind, ItemKind::MacroItem(..))
     }
 
@@ -946,8 +945,12 @@ pub(crate) enum ItemKind {
     ForeignStaticItem(Static, hir::Safety),
     /// `type`s from an extern block
     ForeignTypeItem,
-    /// A bang macro. it can be multiple things (macro, derive and attribute, potentially multiple
-    /// at once). Don't forget to look into the `MacroKinds` values.
+    /// A macro defined with `macro_rules` or the `macro` keyword. It can be multiple things (macro,
+    /// derive and attribute, potentially multiple at once). Don't forget to look into the
+    ///`MacroKinds` values.
+    ///
+    /// If a `macro_rules!` only contains a `attr`/`derive` branch, then it's not stored in this
+    /// variant but in the `ProcMacroItem` variant.
     MacroItem(Macro, MacroKinds),
     ProcMacroItem(ProcMacro),
     PrimitiveItem(PrimitiveType),

src/librustdoc/clean/utils.rs

@@ -5,6 +5,7 @@ use std::{ascii, mem};
 
 use rustc_ast as ast;
 use rustc_ast::join_path_idents;
+use rustc_ast::token::{Token, TokenKind};
 use rustc_ast::tokenstream::TokenTree;
 use rustc_data_structures::thin_vec::{ThinVec, thin_vec};
 use rustc_hir as hir;
@@ -617,9 +618,18 @@ fn render_macro_arms(
         .unwrap();
         // We skip the `=>`, macro "body" and the delimiter closing that "body" since we don't
         // render them.
-        tokens.next();
-        tokens.next();
-        tokens.next();
+        let _token = tokens.next();
+        // The `=>`.
+        debug_assert_matches!(
+            _token,
+            Some(TokenTree::Token(Token { kind: TokenKind::FatArrow, .. }, _))
+        );
+        let _token = tokens.next();
+        // The arm body.
+        debug_assert_matches!(_token, Some(TokenTree::Delimited(..)));
+        // The delimiter (which may be omitted on the last arm's body).
+        let _token = tokens.next();
+        debug_assert_matches!(_token, None | Some(TokenTree::Token(Token { .. }, _)));
     }
     out
 }

src/librustdoc/formats/cache.rs

@@ -382,8 +382,8 @@ impl DocFolder for CacheBuilder<'_, '_> {
             | clean::RequiredAssocTypeItem(..)
             | clean::AssocTypeItem(..)
             | clean::StrippedItem(..)
-            | clean::AttributeItem
-            | clean::KeywordItem => {
+            | clean::KeywordItem
+            | clean::AttributeItem => {
                 // FIXME: Do these need handling?
                 // The person writing this comment doesn't know.
                 // So would rather leave them to an expert,
@@ -599,7 +599,7 @@ fn add_item_to_search_index(tcx: TyCtxt<'_>, cache: &mut Cache, item: &clean::It
     let is_unstable = item.is_unstable();
     let mut types = item.types();
     let index_item = IndexItem {
-        ty: types.pop().unwrap(),
+        ty: types.next().unwrap(),
         defid: Some(defid),
         name,
         module_path: parent_path.to_vec(),

src/librustdoc/formats/item_type.rs

@@ -101,6 +101,10 @@ item_type! {
     // This number is reserved for use in JavaScript
     // Generic = 26,
     Attribute = 27,
+    // The two next ones represent an attr/derive macro declared as a `macro_rules!`. We need this
+    // distinction because they will point to a `macro.[name].html` file and not
+    // `[attr|derive].[name].html` file, so the link generation needs to take it into account while
+    // still having the filtering working as expected.
     BangMacroAttribute = 28,
     BangMacroDerive = 29,
 }

src/librustdoc/html/render/context.rs

@@ -351,7 +351,7 @@ impl<'tcx> Context<'tcx> {
                 Some(s) => s,
             };
 
-            let is_macro_rules = item.is_macro_rules();
+            let is_macro_rules = item.is_bang_macro_or_macro_rules();
             for type_ in item.types() {
                 if inserted.entry(type_).or_default().insert(name) {
                     let type_ = type_.to_string();

src/librustdoc/html/render/mod.rs

@@ -536,13 +536,8 @@ impl AllTypes {
             let new_url = format!("{}/{}", url.join("/"), item.html_filename());
             url.push(name);
             let name = url.join("::");
-            let mut types = item.types();
-            if types.len() == 1 {
-                self.add_item_entry(types.pop().unwrap(), new_url, name);
-            } else {
-                for type_ in types {
-                    self.add_item_entry(type_, new_url.clone(), name.clone());
-                }
+            for type_ in item.types() {
+                self.add_item_entry(type_, new_url.clone(), name.clone());
             }
         }
     }

src/librustdoc/html/render/print_item.rs

@@ -129,9 +129,9 @@ pub(super) fn print_item(cx: &Context<'_>, item: &clean::Item) -> impl fmt::Disp
         let item_vars = ItemVars {
             typ,
             name: item.name.as_ref().unwrap().as_str(),
-            // If `type_` returns `None`, it means it's a bang macro with multiple kinds, but
+            // It's fine to use `type_` here because, even if it's a bang macro with multiple kinds,
             // since we're generating its documentation page, we can default to the "parent" type,
-            // ie "bang macro".
+            // ie "macro".
             item_type: &item.type_().to_string(),
             path_components,
             stability_since_raw: &stability_since_raw,
@@ -464,7 +464,7 @@ fn item_module(cx: &Context<'_>, item: &clean::Item, items: &[clean::Item]) -> i
                             stab_tags = print_extra_info_tags(tcx, myitem, item, None),
                             class = type_,
                             unsafety_flag = unsafety_flag,
-                            href = print_item_path(myitem),
+                            href = myitem.html_filename(),
                             title1 = myitem.type_(),
                             title2 = full_path(cx, myitem),
                         )?;

tests/rustdoc-html/macro/decl_macro-sidebar.rs

@@ -0,0 +1,15 @@
+// This test ensures that the `foo` decl macro is present in the module sidebar.
+
+#![feature(decl_macro)]
+#![crate_name = "foo"]
+
+//@has 'foo/bar/index.html'
+//@has - '//*[@id="rustdoc-modnav"]/ul[@class="block macro"]//a[@href="../macro.foo.html"]' 'foo'
+
+pub macro foo {
+    () => { "bar" }
+}
+
+/// docs
+pub mod bar {
+}