Pallet dispatchable+storage doc module.

[kianenigma]

hopefully closes #12398, but needs more work.
step toward #13299

Description

The goal of this PR is to make docs for dispatchables and storages on pallets more accessible and discoverable when browsing the generated docs. The PR accomplishes this by adding two auto-generated modules to all pallets: dispatchables and storage_types. These modules are only produced when building docs and otherwise do not exist or interfere with pallets. The storage_types module is auto-populated with documented structs representing each storage type from the parent pallet. The dispatchables module is auto-populated with uncallable auto-generated functions matching the signature of each Call variant. Right now a link to the corresponding Call enum variant is also included, though this may be removed. A note is included specifying that these cannot be called and are doc-only.

Modules added to pallet module:

Detail view of storage_types module:

Detail view of of dispatchables module:

Detail view of a particular dispatchable:

Also hides docs for Instance2-Instance16 on the pallet module and adds a doc comment to Instance1 explaining this:

[sam0x17]

Figuring out how we want to handle generics in fn signatures in pallets aka situations like this:

		#[pallet::call_index(2)]
		#[pallet::weight(T::WeightInfo::force_transfer())]
		pub fn force_transfer(
			origin: OriginFor<T>,
			source: AccountIdLookupOf<T>,
			dest: AccountIdLookupOf<T>,
			#[pallet::compact] value: T::Balance,
		) -> DispatchResultWithPostInfo {

are causing this error:

error[E0412]: cannot find type `T` in this scope
   --> frame/balances/src/lib.rs:372:28
    |
369 |         pub fn force_transfer(
    |                              - help: you might be missing a type parameter: `<T>`
...
372 |             dest: AccountIdLookupOf<T>,
    |                                     ^ not found in this scope

As far as I can tell that is the only outstanding thing not working

[kianenigma]

Figuring out how we want to handle generics in fn signatures in pallets aka situations like this:

		#[pallet::call_index(2)]
		#[pallet::weight(T::WeightInfo::force_transfer())]
		pub fn force_transfer(
			origin: OriginFor<T>,
			source: AccountIdLookupOf<T>,
			dest: AccountIdLookupOf<T>,
			#[pallet::compact] value: T::Balance,
		) -> DispatchResultWithPostInfo {

are causing this error:

error[E0412]: cannot find type `T` in this scope
   --> frame/balances/src/lib.rs:372:28
    |
369 |         pub fn force_transfer(
    |                              - help: you might be missing a type parameter: `<T>`
...
372 |             dest: AccountIdLookupOf<T>,
    |                                     ^ not found in this scope

As far as I can tell that is the only outstanding thing not working

Maybe put a <T: Config> on every function? or better, wrap them all in

/// Some doc saying that this struct does not exist in reality
pub struct Dispatchables<T: Config>(Phantom(T));
impl<T: Config> Dispatchables {
  // This is now 100% identical to `Pallet` struct and should work. 
}

and then we export struct Dispatchables, which is good enough.

[bkchr]

This clearly requires some screenshots on how that looks in the docs. I'm not 100% sure that adding fake items to the docs is such a great idea.

[sam0x17]

These when clicked take you to the relevant docs. I think the best dev UX would be to have the docs here verbatim but alas that doesn't quite work in rustdoc with re-exports. I actually ran into a literal compiler bug when I tried to do anything other than re-exporting for this one, btw.

This one works better because they are not re-exports:

[bkchr]

Did you tried using pub type StorageItem = super::StorageItem instead of the re-export.

But with the calls people will then not know how to use them. From the docs it looks like there is some module, which doesn't really exists 🤷

[sam0x17]

Did you tried using pub type StorageItem = super::StorageItem instead of the re-export.

But with the calls people will then not know how to use them. From the docs it looks like there is some module, which doesn't really exists 🤷

Yeah in that case I think the current solution is the best compromise because at least it points them to the right place for the types.

As far as accessibility, I will add a comment saying these types and functions are available on the pallet itself or something to that effect

[kianenigma]

Suggestion: For each dispatchable, link to the actually function in struct Pallet and the corresponding enum Call variant as well, emphasizing that "the real thing is there, this is just a fake" list.

For storage items, I think this is great and already a massive improvement.

I will try and help a bit with the auto-generated documents, making sure they are not confusing, especially for the dispatch stuff.

[kianenigma]

I think it is a good step forward, but perhaps we can chose the doc strings a bit better.

Can't approve as I opened the PR, but has my blessing.

[sam0x17]

Hmm I wonder if we could instead just add the docs to the Call enum variants as an alternative to the dispatchables module? thoughts?

[KiChjang]

As with everything doc-related, it's hard to give comments if we don't know how it looks like on the UI, especially when it is about grouping similar items together, which is what the entire underlying issue is about.

[sam0x17]

As with everything doc-related, it's hard to give comments if we don't know how it looks like on the UI, especially when it is about grouping similar items together, which is what the entire underlying issue is about.

i.e. what if we emit doc strings on these variants instead of the dispatchables module shown above:

[KiChjang]

I still haven't seen how the docs on the enum variants look like, but I'm assuming it's similar to https://docs.rs/syn/latest/syn/enum.Type.html, in which case, it's clearly an improvement. We might want to write some doc comments on the Call enum itself so that people know how to click into it and look for docs on the dispatchables.

[sam0x17]

I still haven't seen how the docs on the enum variants look like, but I'm assuming it's similar to https://docs.rs/syn/latest/syn/enum.Type.html, in which case, it's clearly an improvement. We might want to write some doc comments on the Call enum itself so that people know how to click into it and look for docs on the dispatchables.

Yeah I haven't seen it yet either as it will require moving parts of the macro around to a whole different part of the pallet expansion. And yeah, I think this would be better dev UX since the Call enum is the actual thing that gets generated

[sam0x17]

lol so we've come full circle. I scrolled down the page on the Call enum only to realize the variants are already documented:

Do we maybe just want to surface this better with a link to Call on the main docs page for each pallet? Looks like this already perfectly documents the dispatchables for any given pallet..

[sam0x17]

I guess the intent though of what @kianenigma was doing with the dispatchables was to render them in the docs as actual functions instead of enum variants. Given that, I think it might be better to leave it the way he set it up, but I'm going to add doc links to the actual variants like he suggested on each one

[sam0x17]

OK so yeah latest commit adds this preface to the docs for the auto-generated dispatchables stub functions:

Notably, the NOTE actually links to the proper enum variant corresponding with the function.

[sam0x17]

Have also added a revamp of the very ugly Instance1, Instance2, Instance3... Instance16 stuff in the pallet module based on @ggwpez's feedback:

[ggwpez]

Using structs for the storage types works. We dont actually need the concrete storage type. If we really want we could still link to the frame-support::storage::... type but for high-level this looks good to me.
Before:

After:

[ggwpez]

Looks much better now, thanks!

[sam0x17]

@KiChjang this should be good for a final look now, screenshots at the top are updated

[sam0x17]

bot rebase

[paritytech-processbot]

Rebased

[sam0x17]

bot merge