Switch proc macro docs to be on re-exports so we can have doc links to outer crate items

[sam0x17]

Right now the frame proc macros (especially the pallet:: ones) follow the pattern of documenting proc macros inside their corresponding inner proc macro crates (i.e. frame_support_procedural). I standardized this in a number of places in paritytech/substrate#12333 / paritytech/substrate#12334.

A major reason for this is tools like rust-analyzer right now do not conform with the cargo doc behavior of prepending docs that are attached to re-exported items (i.e. macros, functions, traits, etc) and in fact completely ignore docs on re-exports (see my companion issue in rust-analyzer: https://github.com/rust-lang/rust-analyzer/issues/13431).

This is why in paritytech/substrate#12334 I ended up switching all of the pallet:: macros to have their docs inside frame_support_procedural because then rust-analyzer will actually pick them up upon mouse-over.

The problem with this is inner crates can only generate doc links to items that are visible from the inner crate. For non-proc-macro crates the workaround is easy -- simply set up a circular reference between the inner crate and the outer crate so the outer crate items are visible/linkable from the inner crate. This doesn't work for proc macro crates, however, because rust restricts proc macro crates so that they are only allowed to publicly export proc macros (no other items are allowed), so even if we pub use outer_crate::* to make all the outer crate's items accessible within the inner crate, this will fail to compile since we are trying to export something other than a proc macro in a proc macro crate.

Thus we are at a bit of a stand-still. Ideally rust-analyzer would handle docs on re-exports properly the way cargo doc does and we would switch to only documenting proc macros where their re-exports are defined in the outer crate.

So this is blocked until https://github.com/rust-lang/rust-analyzer/issues/13431 is addressed in rust-analyzer or some other solution (like better support for inter-crate doc linking) is added to rust.

One possible alternative solution would be to link directly to the docs.rs links for the outer items from the inner macro declarations. This has some obvious downsides (like rust-analyzer will open a web browser instead of following the links inline), but if this issue sits for a long period of time this might be the way to go.

Note: the one thing I haven't tried is trying to pub use the outer crate items in the inner proc macro crate while gating this with a #[cfg(doc)] directive. I doubt it will compile but it's the only thing I haven't tried so would be cool if someone could confirm (or if someone knows) that this doesn't work.

Related Issues

• paritytech/substrate#12333
• https://github.com/rust-lang/rust-analyzer/issues/13431

Related PRs

• paritytech/substrate#12334

[bkchr]

IMO the primary goal should be there to have proper cargo doc output and ignore rust-analyzer. Yes it is bad that this doesn't work, but this is a bug on their side and we should not scarify good docs for this.

[sam0x17]

The pre-#12334 structure was that all of the pallet:: macros lacked any kind of stub (because the macro expansion of the pallet macro actually removes all these attributes) so all of their docs were on the docs for the pallet attribute macro.

paritytech/substrate#12334 introduced stubs for these macros with effectively duplicated docs from the pallet page which improved the developer experience both within cargo docs and for those using rust-analyzer. For those using cargo docs, now these macros actually come up in search as macros, and for rust-analyzer, hovering over any of these attributes displays the docs for that attribute from its respective stub and right clicking any of these attributes allowed you to "Go to Definition" without it failing like it did before.

Currently, the version of the docs that is on the pallet macro has working doc links to all the associated items that are mentioned in the docs, just like before. The copy of the docs that is on each stub has been largely stripped of links except those that link to other macros since we can only link to things within frame_support_procedural, but these links are all left in place on the copy of the docs that is on the pallet macro.

IMO this is the best compromise dev UX-wise until rust-analyzer fixes their issue, especially since one of the stated purposes of solving paritytech/substrate#12333 was to get rust-analyzer to behave properly with the pallet:: attributes for dev ux reasons. This was all discussed at length on paritytech/substrate#12333 and paritytech/substrate#12334 and the approach was approved without contest.

In the interim while we wait for rust-analyzer to fix their issue, it is true that we could move the docs for these stubs from frame_support_procedural to the pub use statements in frame_support and remove the extra docs entirely from the pallet macro (instead using doc links to link to all of the pallet:: attributes where we currently have that list of all the attributes), but this would result in a broken experience for users of rust-analyzer for the small benefit that the docs at least aren't duplicated in two places, so I say definitely not worth it for now.

I'm watching this closely and just want to keep this issue open until rust-analyzer releases their fix for this, and I have no problem taking responsibility for watching for that and managing this.

[bkchr]

I don't assume that rust-analyzer will fix this in the near future. It is said that this requires some deeper changes in rust-analyzer. I can not imagine what their todo list looks like. However, we should not wait. Rust docs is more important. We could either copy the docs for now, if hover support is really that important (I doubt, because a lot of stuff is breaking all the time in rust analyzer because of the amount of macros etc we use). Or we just add some dummy docs mentioning to the user that this currently doesn't work in rust-analyzer and where they find the real docs.

[sam0x17]

Right but we already did the copy that was part of paritytech/substrate#12334 so we should be good for now just wanted to describe this issue somewhere