From 963ee8a37130f6d76b2859e76164a01607a306cf Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Tue, 18 Jan 2022 06:21:19 +0900 Subject: [PATCH] Add feature: `no-metadata-doc` which removes doc from metadata and `full-metadata` which build metadata with all doc (#10493) * add features to remove or add doc * fmt * add test for event/error/call * fmt --- .gitlab-ci.yml | 2 +- frame/support/Cargo.toml | 6 ++++ frame/support/procedural/Cargo.toml | 1 + .../procedural/src/pallet/expand/call.rs | 4 ++- .../procedural/src/pallet/expand/constants.rs | 4 ++- .../procedural/src/pallet/expand/error.rs | 5 ++- .../procedural/src/pallet/expand/event.rs | 6 ++-- .../procedural/src/pallet/expand/storage.rs | 3 +- .../support/src/storage/types/counted_map.rs | 12 +++++-- frame/support/src/storage/types/double_map.rs | 2 ++ frame/support/src/storage/types/map.rs | 2 ++ frame/support/src/storage/types/nmap.rs | 2 ++ frame/support/src/storage/types/value.rs | 2 ++ frame/support/test/Cargo.toml | 1 + frame/support/test/tests/pallet.rs | 32 +++++++++++++++---- .../test/tests/pallet_compatibility.rs | 4 +++ .../tests/pallet_compatibility_instance.rs | 4 +++ 17 files changed, 76 insertions(+), 16 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 43674da4627a5..b68dd303841d8 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -450,7 +450,7 @@ test-linux-stable: &test-linux script: # this job runs all tests in former runtime-benchmarks, frame-staking and wasmtime tests - time cargo test --workspace --locked --release --verbose --features runtime-benchmarks --manifest-path bin/node/cli/Cargo.toml - - time cargo test -p frame-support-test --features=conditional-storage --manifest-path frame/support/test/Cargo.toml --test pallet # does not reuse cache 1 min 44 sec + - time cargo test -p frame-support-test --features=conditional-storage,no-metadata-docs --manifest-path frame/support/test/Cargo.toml --test pallet # does not reuse cache 1 min 44 sec - SUBSTRATE_TEST_TIMEOUT=1 time cargo test -p substrate-test-utils --release --verbose --locked -- --ignored timeout - sccache -s diff --git a/frame/support/Cargo.toml b/frame/support/Cargo.toml index efc7caab1bd4e..a6bbc73f9ddb6 100644 --- a/frame/support/Cargo.toml +++ b/frame/support/Cargo.toml @@ -63,3 +63,9 @@ std = [ ] runtime-benchmarks = [] try-runtime = [] +# By default some types have documentation, `no-metadata-docs` allows to reduce the documentation +# in the metadata. +no-metadata-docs = ["frame-support-procedural/no-metadata-docs"] +# By default some types have documentation, `full-metadata-docs` allows to add documentation to +# more types in the metadata. +full-metadata-docs = ["scale-info/docs"] diff --git a/frame/support/procedural/Cargo.toml b/frame/support/procedural/Cargo.toml index 640fe436b1855..4e9618b5bc167 100644 --- a/frame/support/procedural/Cargo.toml +++ b/frame/support/procedural/Cargo.toml @@ -24,3 +24,4 @@ syn = { version = "1.0.82", features = ["full"] } [features] default = ["std"] std = [] +no-metadata-docs = [] diff --git a/frame/support/procedural/src/pallet/expand/call.rs b/frame/support/procedural/src/pallet/expand/call.rs index 58c4804558b0e..355d4f87d3db9 100644 --- a/frame/support/procedural/src/pallet/expand/call.rs +++ b/frame/support/procedural/src/pallet/expand/call.rs @@ -137,6 +137,8 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream { let count = COUNTER.with(|counter| counter.borrow_mut().inc()); let macro_ident = syn::Ident::new(&format!("__is_call_part_defined_{}", count), span); + let capture_docs = if cfg!(feature = "no-metadata-docs") { "never" } else { "always" }; + quote::quote_spanned!(span => #[doc(hidden)] pub mod __substrate_call_check { @@ -164,7 +166,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream { )] #[codec(encode_bound())] #[codec(decode_bound())] - #[scale_info(skip_type_params(#type_use_gen), capture_docs = "always")] + #[scale_info(skip_type_params(#type_use_gen), capture_docs = #capture_docs)] #[allow(non_camel_case_types)] pub enum #call_ident<#type_decl_bounded_gen> #where_clause { #[doc(hidden)] diff --git a/frame/support/procedural/src/pallet/expand/constants.rs b/frame/support/procedural/src/pallet/expand/constants.rs index 3f853902010f9..1f9526f9cebe0 100644 --- a/frame/support/procedural/src/pallet/expand/constants.rs +++ b/frame/support/procedural/src/pallet/expand/constants.rs @@ -79,7 +79,9 @@ pub fn expand_constants(def: &mut Def) -> proc_macro2::TokenStream { let const_type = &const_.type_; let ident_str = format!("{}", const_.metadata_name.unwrap_or(const_.ident)); - let doc = const_.doc.clone().into_iter(); + let no_docs = vec![]; + let doc = if cfg!(feature = "no-metadata-docs") { &no_docs } else { &const_.doc }; + let default_byte_impl = &const_.default_byte_impl; quote::quote!({ diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 3e6247e77f8cc..4cf572c1797f2 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -58,12 +58,15 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { }; error_item.variants.insert(0, phantom_variant); + + let capture_docs = if cfg!(feature = "no-metadata-docs") { "never" } else { "always" }; + // derive TypeInfo for error metadata error_item .attrs .push(syn::parse_quote!( #[derive(#frame_support::scale_info::TypeInfo)] )); error_item.attrs.push(syn::parse_quote!( - #[scale_info(skip_type_params(#type_use_gen), capture_docs = "always")] + #[scale_info(skip_type_params(#type_use_gen), capture_docs = #capture_docs)] )); if get_doc_literals(&error_item.attrs).is_empty() { diff --git a/frame/support/procedural/src/pallet/expand/event.rs b/frame/support/procedural/src/pallet/expand/event.rs index 79a7acaf66a5a..acd60ab959c61 100644 --- a/frame/support/procedural/src/pallet/expand/event.rs +++ b/frame/support/procedural/src/pallet/expand/event.rs @@ -117,9 +117,11 @@ pub fn expand_event(def: &mut Def) -> proc_macro2::TokenStream { )] )); - // skip requirement for type params to implement `TypeInfo`, and require docs capture + let capture_docs = if cfg!(feature = "no-metadata-docs") { "never" } else { "always" }; + + // skip requirement for type params to implement `TypeInfo`, and set docs capture event_item.attrs.push(syn::parse_quote!( - #[scale_info(skip_type_params(#event_use_gen), capture_docs = "always")] + #[scale_info(skip_type_params(#event_use_gen), capture_docs = #capture_docs)] )); let deposit_event = if let Some(deposit_event) = &event.deposit_event { diff --git a/frame/support/procedural/src/pallet/expand/storage.rs b/frame/support/procedural/src/pallet/expand/storage.rs index 9d936dd50344e..f45223c1cc842 100644 --- a/frame/support/procedural/src/pallet/expand/storage.rs +++ b/frame/support/procedural/src/pallet/expand/storage.rs @@ -234,7 +234,8 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { let pallet_ident = &def.pallet_struct.pallet; let entries_builder = def.storages.iter().map(|storage| { - let docs = &storage.docs; + let no_docs = vec![]; + let docs = if cfg!(feature = "no-metadata-docs") { &no_docs } else { &storage.docs }; let ident = &storage.ident; let gen = &def.type_use_generics(storage.attr_span); diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 341dedaef0bac..0f98b13282a74 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -417,7 +417,11 @@ where fn build_metadata(docs: Vec<&'static str>, entries: &mut Vec) { ::Map::build_metadata(docs, entries); CounterFor::::build_metadata( - vec![&"Counter for the related counted storage map"], + if cfg!(feature = "no-metadata-docs") { + vec![] + } else { + vec![&"Counter for the related counted storage map"] + }, entries, ); } @@ -1054,7 +1058,11 @@ mod test { modifier: StorageEntryModifier::Default, ty: StorageEntryType::Plain(scale_info::meta_type::()), default: vec![0, 0, 0, 0], - docs: vec!["Counter for the related counted storage map"], + docs: if cfg!(feature = "no-metadata-docs") { + vec![] + } else { + vec!["Counter for the related counted storage map"] + }, }, ] ); diff --git a/frame/support/src/storage/types/double_map.rs b/frame/support/src/storage/types/double_map.rs index 1a4d979d98f8f..07da0f2239b3b 100644 --- a/frame/support/src/storage/types/double_map.rs +++ b/frame/support/src/storage/types/double_map.rs @@ -530,6 +530,8 @@ where MaxValues: Get>, { fn build_metadata(docs: Vec<&'static str>, entries: &mut Vec) { + let docs = if cfg!(feature = "no-metadata-docs") { vec![] } else { docs }; + let entry = StorageEntryMetadata { name: Prefix::STORAGE_PREFIX, modifier: QueryKind::METADATA, diff --git a/frame/support/src/storage/types/map.rs b/frame/support/src/storage/types/map.rs index e769daa490367..d0b82e1a84e5b 100644 --- a/frame/support/src/storage/types/map.rs +++ b/frame/support/src/storage/types/map.rs @@ -352,6 +352,8 @@ where MaxValues: Get>, { fn build_metadata(docs: Vec<&'static str>, entries: &mut Vec) { + let docs = if cfg!(feature = "no-metadata-docs") { vec![] } else { docs }; + let entry = StorageEntryMetadata { name: Prefix::STORAGE_PREFIX, modifier: QueryKind::METADATA, diff --git a/frame/support/src/storage/types/nmap.rs b/frame/support/src/storage/types/nmap.rs index 30e6fb5637882..03f15e335389b 100755 --- a/frame/support/src/storage/types/nmap.rs +++ b/frame/support/src/storage/types/nmap.rs @@ -451,6 +451,8 @@ where MaxValues: Get>, { fn build_metadata(docs: Vec<&'static str>, entries: &mut Vec) { + let docs = if cfg!(feature = "no-metadata-docs") { vec![] } else { docs }; + let entry = StorageEntryMetadata { name: Prefix::STORAGE_PREFIX, modifier: QueryKind::METADATA, diff --git a/frame/support/src/storage/types/value.rs b/frame/support/src/storage/types/value.rs index a368988f378bd..f145e9fb30414 100644 --- a/frame/support/src/storage/types/value.rs +++ b/frame/support/src/storage/types/value.rs @@ -210,6 +210,8 @@ where OnEmpty: crate::traits::Get + 'static, { fn build_metadata(docs: Vec<&'static str>, entries: &mut Vec) { + let docs = if cfg!(feature = "no-metadata-docs") { vec![] } else { docs }; + let entry = StorageEntryMetadata { name: Prefix::STORAGE_PREFIX, modifier: QueryKind::METADATA, diff --git a/frame/support/test/Cargo.toml b/frame/support/test/Cargo.toml index 975e0830ab0e4..f7275cbe2e853 100644 --- a/frame/support/test/Cargo.toml +++ b/frame/support/test/Cargo.toml @@ -52,3 +52,4 @@ try-runtime = ["frame-support/try-runtime"] conditional-storage = [] # Disable ui tests disable-ui-tests = [] +no-metadata-docs = ["frame-support/no-metadata-docs"] diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index 59b581eda58e4..5f9e886dbb672 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -1100,6 +1100,14 @@ fn migrate_from_pallet_version_to_storage_version() { fn metadata() { use frame_support::metadata::*; + fn maybe_docs(doc: Vec<&'static str>) -> Vec<&'static str> { + if cfg!(feature = "no-metadata-docs") { + vec![] + } else { + doc + } + } + let pallets = vec![ PalletMetadata { index: 1, @@ -1269,7 +1277,7 @@ fn metadata() { modifier: StorageEntryModifier::Default, ty: StorageEntryType::Plain(meta_type::()), default: vec![0, 0, 0, 0], - docs: vec!["Counter for the related counted storage map"], + docs: maybe_docs(vec!["Counter for the related counted storage map"]), }, StorageEntryMetadata { name: "Unbounded", @@ -1287,13 +1295,13 @@ fn metadata() { name: "MyGetParam", ty: meta_type::(), value: vec![10, 0, 0, 0], - docs: vec![" Some comment", " Some comment"], + docs: maybe_docs(vec![" Some comment", " Some comment"]), }, PalletConstantMetadata { name: "MyGetParam2", ty: meta_type::(), value: vec![11, 0, 0, 0], - docs: vec![" Some comment", " Some comment"], + docs: maybe_docs(vec![" Some comment", " Some comment"]), }, PalletConstantMetadata { name: "MyGetParam3", @@ -1305,19 +1313,19 @@ fn metadata() { name: "some_extra", ty: meta_type::(), value: vec![100, 0, 0, 0, 0, 0, 0, 0], - docs: vec![" Some doc", " Some doc"], + docs: maybe_docs(vec![" Some doc", " Some doc"]), }, PalletConstantMetadata { name: "some_extra_extra", ty: meta_type::(), value: vec![0, 0, 0, 0, 0, 0, 0, 0], - docs: vec![" Some doc"], + docs: maybe_docs(vec![" Some doc"]), }, PalletConstantMetadata { name: "SomeExtraRename", ty: meta_type::(), value: vec![0, 0, 0, 0, 0, 0, 0, 0], - docs: vec![" Some doc"], + docs: maybe_docs(vec![" Some doc"]), }, ], error: Some(PalletErrorMetadata { ty: meta_type::>() }), @@ -1351,7 +1359,7 @@ fn metadata() { modifier: StorageEntryModifier::Default, ty: StorageEntryType::Plain(meta_type::()), default: vec![0, 0, 0, 0], - docs: vec!["Counter for the related counted storage map"], + docs: maybe_docs(vec!["Counter for the related counted storage map"]), }, ], }), @@ -1362,6 +1370,16 @@ fn metadata() { }, ]; + let empty_doc = pallets[0].event.as_ref().unwrap().ty.type_info().docs().is_empty() && + pallets[0].error.as_ref().unwrap().ty.type_info().docs().is_empty() && + pallets[0].calls.as_ref().unwrap().ty.type_info().docs().is_empty(); + + if cfg!(feature = "no-metadata-docs") { + assert!(empty_doc) + } else { + assert!(!empty_doc) + } + let extrinsic = ExtrinsicMetadata { ty: meta_type::(), version: 4, diff --git a/frame/support/test/tests/pallet_compatibility.rs b/frame/support/test/tests/pallet_compatibility.rs index dc76f1fcbf036..339a8b6b09248 100644 --- a/frame/support/test/tests/pallet_compatibility.rs +++ b/frame/support/test/tests/pallet_compatibility.rs @@ -15,6 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +// Old macros don't support the flag `no-metadata-docs` so the result differs when the feature is +// activated. +#![cfg(not(feature = "no-metadata-docs"))] + use frame_support::traits::{ConstU32, ConstU64}; pub trait SomeAssociation { diff --git a/frame/support/test/tests/pallet_compatibility_instance.rs b/frame/support/test/tests/pallet_compatibility_instance.rs index 80ab3c2267fdc..4fe577f520fa3 100644 --- a/frame/support/test/tests/pallet_compatibility_instance.rs +++ b/frame/support/test/tests/pallet_compatibility_instance.rs @@ -15,6 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +// Old macros don't support the flag `no-metadata-docs` so the result differs when the feature is +// activated. +#![cfg(not(feature = "no-metadata-docs"))] + use frame_support::traits::{ConstU32, ConstU64}; mod pallet_old {