diff --git a/prdoc/pr_3505.prdoc b/prdoc/pr_3505.prdoc new file mode 100644 index 0000000000000000000000000000000000000000..08ad909739c7d761371e0e1e40d575794fc4e217 --- /dev/null +++ b/prdoc/pr_3505.prdoc @@ -0,0 +1,36 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Removes `as [disambiguation_path]` from the required syntax in `derive_impl` + +doc: + - audience: Runtime Dev + description: | + This PR removes the need to specify `as [disambiguation_path]` for cases where the trait + definition resides within the same scope as default impl path. + + For example, in the following macro invocation + ```rust + #[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)] + impl frame_system::Config for Runtime { + ... + } + ``` + the trait `DefaultConfig` lies within the `frame_system` scope and `TestDefaultConfig` impls + the `DefaultConfig` trait. + + Using this information, we can compute the disambiguation path internally, thus removing the + need of an explicit specification: + ```rust + #[derive_impl(frame_system::config_preludes::TestDefaultConfig)] + impl frame_system::Config for Runtime { + ... + } + ``` + + In cases where the trait lies outside this scope, we would still need to specify it explicitly, + but this should take care of most (if not all) uses of `derive_impl` within FRAME's context. + +crates: + - name: frame-support-procedural + - name: pallet-default-config-example diff --git a/substrate/frame/examples/default-config/src/lib.rs b/substrate/frame/examples/default-config/src/lib.rs index cd1653e6c764358165c8306d4810c1d4c9bea3b4..69eca055965eb2c1270af6d1e069b788ef3dfa75 100644 --- a/substrate/frame/examples/default-config/src/lib.rs +++ b/substrate/frame/examples/default-config/src/lib.rs @@ -149,7 +149,7 @@ pub mod tests { } ); - #[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)] + #[derive_impl(frame_system::config_preludes::TestDefaultConfig)] impl frame_system::Config for Runtime { // these items are defined by frame-system as `no_default`, so we must specify them here. type Block = Block; diff --git a/substrate/frame/support/procedural/src/derive_impl.rs b/substrate/frame/support/procedural/src/derive_impl.rs index d6d5bf68efd5689af2e96250e24cef3cd7faf47b..8740ccd401abedcbcb27e19c77ef3e885207342a 100644 --- a/substrate/frame/support/procedural/src/derive_impl.rs +++ b/substrate/frame/support/procedural/src/derive_impl.rs @@ -172,6 +172,33 @@ fn combine_impls( final_impl } +/// Computes the disambiguation path for the `derive_impl` attribute macro. +/// +/// When specified explicitly using `as [disambiguation_path]` in the macro attr, the +/// disambiguation is used as is. If not, we infer the disambiguation path from the +/// `foreign_impl_path` and the computed scope. +fn compute_disambiguation_path( + disambiguation_path: Option<Path>, + foreign_impl: ItemImpl, + default_impl_path: Path, +) -> Result<Path> { + match (disambiguation_path, foreign_impl.clone().trait_) { + (Some(disambiguation_path), _) => Ok(disambiguation_path), + (None, Some((_, foreign_impl_path, _))) => + if default_impl_path.segments.len() > 1 { + let scope = default_impl_path.segments.first(); + Ok(parse_quote!(#scope :: #foreign_impl_path)) + } else { + Ok(foreign_impl_path) + }, + _ => Err(syn::Error::new( + default_impl_path.span(), + "Impl statement must have a defined type being implemented \ + for a defined type such as `impl A for B`", + )), + } +} + /// Internal implementation behind [`#[derive_impl(..)]`](`macro@crate::derive_impl`). /// /// `default_impl_path`: the module path of the external `impl` statement whose tokens we are @@ -194,18 +221,11 @@ pub fn derive_impl( let foreign_impl = parse2::<ItemImpl>(foreign_tokens)?; let default_impl_path = parse2::<Path>(default_impl_path)?; - // have disambiguation_path default to the item being impl'd in the foreign impl if we - // don't specify an `as [disambiguation_path]` in the macro attr - let disambiguation_path = match (disambiguation_path, foreign_impl.clone().trait_) { - (Some(disambiguation_path), _) => disambiguation_path, - (None, Some((_, foreign_impl_path, _))) => foreign_impl_path, - _ => - return Err(syn::Error::new( - foreign_impl.span(), - "Impl statement must have a defined type being implemented \ - for a defined type such as `impl A for B`", - )), - }; + let disambiguation_path = compute_disambiguation_path( + disambiguation_path, + foreign_impl.clone(), + default_impl_path.clone(), + )?; // generate the combined impl let combined_impl = combine_impls( @@ -257,3 +277,27 @@ fn test_runtime_type_with_doc() { } } } + +#[test] +fn test_disambugation_path() { + let foreign_impl: ItemImpl = parse_quote!(impl SomeTrait for SomeType {}); + let default_impl_path: Path = parse_quote!(SomeScope::SomeType); + + // disambiguation path is specified + let disambiguation_path = compute_disambiguation_path( + Some(parse_quote!(SomeScope::SomePath)), + foreign_impl.clone(), + default_impl_path.clone(), + ); + assert_eq!(disambiguation_path.unwrap(), parse_quote!(SomeScope::SomePath)); + + // disambiguation path is not specified and the default_impl_path has more than one segment + let disambiguation_path = + compute_disambiguation_path(None, foreign_impl.clone(), default_impl_path.clone()); + assert_eq!(disambiguation_path.unwrap(), parse_quote!(SomeScope::SomeTrait)); + + // disambiguation path is not specified and the default_impl_path has only one segment + let disambiguation_path = + compute_disambiguation_path(None, foreign_impl.clone(), parse_quote!(SomeType)); + assert_eq!(disambiguation_path.unwrap(), parse_quote!(SomeTrait)); +} diff --git a/substrate/frame/support/procedural/src/lib.rs b/substrate/frame/support/procedural/src/lib.rs index 5840377d72259a3fa82d850f5c23b15c46c8bbc0..ed920d6a1da45d4d3f69155c7d050a9258d05723 100644 --- a/substrate/frame/support/procedural/src/lib.rs +++ b/substrate/frame/support/procedural/src/lib.rs @@ -639,6 +639,11 @@ pub fn storage_alias(attributes: TokenStream, input: TokenStream) -> TokenStream /// default to `A` from the `impl A for B` part of the default impl. This is useful for scenarios /// where all of the relevant types are already in scope via `use` statements. /// +/// In case the `default_impl_path` is scoped to a different module such as +/// `some::path::TestTraitImpl`, the same scope is assumed for the `disambiguation_path`, i.e. +/// `some::A`. This enables the use of `derive_impl` attribute without having to specify the +/// `disambiguation_path` in most (if not all) uses within FRAME's context. +/// /// Conversely, the `default_impl_path` argument is required and cannot be omitted. /// /// Optionally, `no_aggregated_types` can be specified as follows: