rustdoc: Inherit inline attributes for declarative macros

[shivendra02467]

When explicitly re-exporting a declarative macro by name, rustdoc previously bypassed intermediate re-exports and dropped #[doc(inline)] attributes, causing the macro to be incorrectly stripped if the original definition was #[doc(hidden)].

This updates generate_item_with_correct_attrs to walk the reexport_chain specifically for declarative macros, allowing them to inherit inline attributes exactly as glob imports do, while preserving strict visibility rules for standard items.

Fixes #154694

[rustbot]

r? @lolbinarycat

rustbot has assigned @lolbinarycat.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: rustdoc
• rustdoc expanded to 9 candidates
• Random selection from GuillaumeGomez, camelid, fmease, lolbinarycat, notriddle

[lolbinarycat]

thanks for the contribution!

I haven't looked at the code in-depth yet, but this will need a regression test, I would recommend the rustdoc-html test suite. If you have any issues creating a test, please let me know, both so I can help and so the documentation can be improved for future contributors.

EDIT: nvm there is a regression test, not sure how i missed that.

[shivendra02467]

thanks for the contribution!

I haven't looked at the code in-depth yet, but this will need a regression test, I would recommend the rustdoc-html test suite. If you have any issues creating a test, please let me know, both so I can help and so the documentation can be improved for future contributors.

EDIT: nvm there is a regression test, not sure how i missed that.

No worries at all! Thanks for taking a look. Take your time with the review, and just let me know if you need any adjustments to the test or the logic once you get a chance to dig into it.

[lolbinarycat]

The logic is a bit naive, reexport_chain has no knowledge of doc(hidden), so we need to handle that properly.

Specifically, as we walk backwards through the import chain¹, if we hit a doc(hidden) attribute before or at the same time as we find a doc(inline) attribute, we need to stop searching and say the macro is hidden.

The code already had a decent bit of duplication going on, and the extra logic would only add to that, so please factor out the check (or at least a decent portion of it) into a utility function or method, perhaps named something like should_macro_reexport_be_hidden.

Modified test with extra details in comments

// Regression test for <https://github.com/rust-lang/rust/issues/154694>.
// The goal is to ensure that declarative macros re-exported by name
// inherit the `#[doc(inline)]` attribute from intermediate re-exports,
// matching the behavior of glob re-exports.

#![crate_name = "foo"]

#[macro_use]
mod macros {
    #[macro_export]
    #[doc(hidden)]
    macro_rules! explicit_macro {
        () => {};
    }

    #[macro_export]
    #[doc(hidden)]
    macro_rules! wild_macro {
        () => {};
    }

    #[macro_export]
    #[doc(hidden)]
    macro_rules! actually_hidden_macro {
        () => {};
    }

    #[macro_export]
    #[doc(hidden)]
    macro_rules! actually_hidden_wild_macro {
        () => {};
    }


    #[macro_export]
    #[doc(hidden)]
    macro_rules! actually_hidden_indirect_macro {
        () => {};
    }

}

// We would like to have parity between macro inlining and inlining other items,
// so I'm providing a struct for reference.
#[doc(hidden)]
pub struct HiddenStruct;

#[doc(hidden)]
pub struct IndirectlyHiddenStruct;


pub mod bar {
    mod hidden_explicit {
        #[doc(inline)]
        pub use crate::explicit_macro;
    }

    mod hidden_wild {
        #[doc(inline)]
        pub use crate::wild_macro;
    }

    mod actually_hidden {
        // BUG: as demonstrated by the `actually_hidden_struct` module, when both
        // `doc(hidden)` and `doc(inline)` are specified, `doc(hidden)`
        // should take priority.
        #[doc(hidden)]
        #[doc(inline)]
        pub use crate::actually_hidden_macro;
    }

    mod actually_hidden_indirect_inner {
        #[doc(inline)]
        pub use crate::actually_hidden_indirect_macro;
    }

    mod actually_hidden_indirect {
        // BUG: when there is a chain of imports, we should stop looking as soon as soon as we hit
        // something with `doc(hidden)`.
        // IMO this is the more important of the two bugs.
        #[doc(hidden)]
        pub use super::actually_hidden_indirect_inner::actually_hidden_indirect_macro;
    }

    mod actually_hidden_indirect_struct_inner {
        #[doc(inline)]
        pub use crate::IndirectlyHiddenStruct;
    }

    mod actually_hidden_indirect_struct {
        #[doc(hidden)]
        pub use super::actually_hidden_indirect_struct_inner::IndirectlyHiddenStruct;
    }

    mod actually_hidden_wild {
        #[doc(hidden)]
        #[doc(inline)]
        pub use crate::actually_hidden_wild_macro;
    }

    mod actually_hidden_struct {
        #[doc(inline)]
        #[doc(hidden)]
        pub use crate::HiddenStruct;
    }


    // First, we check that the explicitly named macro inherits the inline attribute
    // from `hidden_explicit` and is successfully rendered.
    //@ has 'foo/bar/macro.explicit_macro.html'
    //@ has 'foo/bar/index.html' '//a[@href="macro.explicit_macro.html"]' 'explicit_macro'
    pub use self::hidden_explicit::explicit_macro;

    // Next, we ensure that the glob-imported macro continues to render correctly
    // as a control case.
    //@ has 'foo/bar/macro.wild_macro.html'
    //@ has 'foo/bar/index.html' '//a[@href="macro.wild_macro.html"]' 'wild_macro'
    pub use self::hidden_wild::*;

    //@ !has 'foo/bar/macro.actually_hidden_macro.html'
    pub use self::actually_hidden::actually_hidden_macro;
    //@ !has 'foo/bar/macro.actually_hidden_wild_macro.html'
    pub use self::actually_hidden_wild::*;
    //@ !has 'foo/bar/struct.HiddenStruct.html'
    pub use self::actually_hidden_struct::HiddenStruct;
    //@ !has 'foo/bar/macro.actually_hidden_indirect_macro.html'
    pub use self::actually_hidden_indirect::actually_hidden_indirect_macro;
    //@ !has 'foo/bar/struct.IndirectlyHiddenStruct.html'
    pub use self::actually_hidden_indirect_struct::IndirectlyHiddenStruct;
}

Footnotes

1. I don't know if reexport_chain walks forwards or backwards by default, will require some experimentation. ↩

[lolbinarycat]

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[shivendra02467]

Thanks for the detailed test and catching the doc(hidden) edge case!

I've factored the check into a helper named macro_reexport_is_inline (kept the positive naming to avoid inverting the existing booleans). It correctly returns the moment it hits doc(hidden).

Regarding your footnote: I verified that reexport_chain walks backwards by default. A standard .iter() hits the doc(hidden) node first, so the precedence works perfectly! I've also included your comments in the test with the conversational comments trimmed.

@rustbot ready

[lolbinarycat]

Two small suggestions, but I think it looks good otherwise!

View changes since this review

[lolbinarycat]

@bors r+ rollup

[rust-bors]

📌 Commit f8d3c27 has been approved by lolbinarycat

It is now in the queue for this repository.