Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add skip_type_params attribute #96

Merged
merged 51 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
39ee6b4
Add new top-level attribute `scale_info(bounds(T: SomeTrait + OtherTr…
dvdplm May 1, 2021
b0eda68
Fmt
ascjones May 4, 2021
bd6dd5d
cleanup
dvdplm May 6, 2021
171cd4c
Merge branch 'dp-custom-bound' of github.com:paritytech/scale-info in…
dvdplm May 6, 2021
030d407
Merge branch 'master' into dp-custom-bound
ascjones May 27, 2021
b937c9a
Skip type params prototype
ascjones May 28, 2021
725ceca
Merge branch 'master' into aj-skip-type-params
ascjones Jun 7, 2021
56edb9c
Fmt
ascjones Jun 7, 2021
f503781
Clippy
ascjones Jun 7, 2021
1743b48
Satisfy clippy
ascjones Jun 7, 2021
bf9cf6d
Fix skip type params
ascjones Jun 7, 2021
2d4f1e8
Fix UI test
ascjones Jun 7, 2021
9c4f6d6
Add some more tests
ascjones Jun 11, 2021
100dfcf
Add failing test for type params with default values
ascjones Jun 11, 2021
c92041b
Fix for type params with defaults, compare on ident
ascjones Jun 11, 2021
a23cd47
Merge branch 'master' into aj-skip-type-params
ascjones Jun 14, 2021
3678114
Merge branch 'master' into aj-skip-type-params
ascjones Jun 14, 2021
4cdf0c5
WIP use bounds attribute for skipping type params
ascjones Jun 14, 2021
99cb9d3
Add required 'static bounds to all type params
ascjones Jun 15, 2021
c39167d
Fmt
ascjones Jun 15, 2021
835efe3
Satisfy clippy
ascjones Jun 15, 2021
75d25ad
Add UI test for skipping bounds
ascjones Jun 15, 2021
3012932
WIP docs
ascjones Jun 15, 2021
20ff837
Revert "Use bounds attribute for skipping type params"
ascjones Jun 15, 2021
7cdd440
WIP dual attribute parsing
ascjones Jun 15, 2021
6d97c49
Use new attribute parsing
ascjones Jun 16, 2021
7cab3e3
Fix up attribute parsing
ascjones Jun 16, 2021
1f84f41
Fmt
ascjones Jun 16, 2021
0716333
Reorder impls
ascjones Jun 16, 2021
a2d239b
Refactor attribute handling
ascjones Jun 16, 2021
ecf21f3
Fmt
ascjones Jun 16, 2021
f876d22
Add docs to attributes
ascjones Jun 16, 2021
b58904f
Add `'static` bounds for all type params, add some ui tests
ascjones Jun 16, 2021
9f64f74
Merge remote-tracking branch 'origin/master' into aj-skip-type-params
ascjones Jun 16, 2021
d61f75d
Check for duplicate attributes
ascjones Jun 16, 2021
a1741d2
Add helpful error message for type params in not in cuatom bounds and…
ascjones Jun 16, 2021
2da8e3f
Improve error message where a type param is missing from the bounds, …
ascjones Jun 16, 2021
2dc9484
Fix test and fmt
ascjones Jun 16, 2021
39e40b1
Refactor and validate missing skip type params
ascjones Jun 17, 2021
44c2af2
Update validation UI test
ascjones Jun 17, 2021
91799c7
Error message formatting
ascjones Jun 17, 2021
e4d570d
Merge remote-tracking branch 'origin/master' into aj-skip-type-params
ascjones Jun 22, 2021
271c2b5
Fix compilation after merge
ascjones Jun 22, 2021
a7e1e6b
Merge remote-tracking branch 'origin/master' into aj-skip-type-params
ascjones Jun 23, 2021
411e50c
Add TypeParameter struct and optional type
ascjones Jun 24, 2021
c5f2bd1
Add ui test for skipping type params
ascjones Jun 25, 2021
d453e69
Add example to named_type_params macro
ascjones Jun 25, 2021
b228318
Type hint for named_type_params with MetaForm
ascjones Jun 25, 2021
f060e59
Add bounds attribute docs
ascjones Jun 25, 2021
7bfa773
Add some docs attribute usage
ascjones Jun 25, 2021
eb41e0b
Fmt
ascjones Jun 25, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,6 @@ extern crate proc_macro;
mod trait_bounds;
mod utils;

use alloc::{
string::{
String,
ToString,
},
vec::Vec,
};
use proc_macro::TokenStream;
use proc_macro2::{
Span,
Expand All @@ -51,7 +44,7 @@ use syn::{
Variant,
};

#[proc_macro_derive(TypeInfo)]
#[proc_macro_derive(TypeInfo, attributes(scale_info))]
pub fn type_info(input: TokenStream) -> TokenStream {
match generate(input.into()) {
Ok(output) => output.into(),
Expand All @@ -68,21 +61,36 @@ fn generate(input: TokenStream2) -> Result<TokenStream2> {
fn generate_type(input: TokenStream2) -> Result<TokenStream2> {
let ast: DeriveInput = syn::parse2(input.clone())?;

utils::check_attributes(&ast)?;

let scale_info = crate_name_ident("scale-info")?;
let parity_scale_codec = crate_name_ident("parity-scale-codec")?;

let ident = &ast.ident;

let (impl_generics, ty_generics, _) = ast.generics.split_for_impl();
let type_params: Vec<_> =
if let Some(skip_type_params) = utils::skipped_type_params(&ast.attrs) {
ast.generics
.type_params()
.filter(|tp| !skip_type_params.iter().any(|skip| *skip == **tp))
.cloned()
.collect()
} else {
ast.generics.type_params().into_iter().cloned().collect()
};

let where_clause = trait_bounds::make_where_clause(
ident,
&ast.generics,
&type_params,
&ast.data,
&scale_info,
&parity_scale_codec,
)?;

let generic_type_ids = ast.generics.type_params().map(|ty| {
let (impl_generics, ty_generics, _) = ast.generics.split_for_impl();

let type_params_meta_types = type_params.iter().map(|ty| {
let ty_ident = &ty.ident;
quote! {
:: #scale_info ::meta_type::<#ty_ident>()
Expand All @@ -102,7 +110,7 @@ fn generate_type(input: TokenStream2) -> Result<TokenStream2> {
fn type_info() -> :: #scale_info ::Type {
:: #scale_info ::Type::builder()
.path(:: #scale_info ::Path::new(::core::stringify!(#ident), ::core::module_path!()))
.type_params(:: #scale_info ::prelude::vec![ #( #generic_type_ids ),* ])
.type_params(:: #scale_info ::prelude::vec![ #( #type_params_meta_types ),* ])
.docs(&[ #( #docs ),* ])
.#build_type
}
Expand Down
12 changes: 9 additions & 3 deletions derive/src/trait_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use syn::{
Generics,
Result,
Type,
TypeParam,
TypePath,
WhereClause,
};
Expand All @@ -34,9 +35,12 @@ use crate::utils;
/// Generates a where clause for a `TypeInfo` impl, adding `TypeInfo + 'static` bounds to all
/// relevant generic types including associated types (e.g. `T::A: TypeInfo`), correctly dealing
/// with self-referential types.
///
/// Ignores any type parameters not included in `type_params`.
pub fn make_where_clause<'a>(
input_ident: &'a Ident,
generics: &'a Generics,
type_params: &[TypeParam],
data: &'a syn::Data,
scale_info: &Ident,
parity_scale_codec: &Ident,
Expand All @@ -53,8 +57,8 @@ pub fn make_where_clause<'a>(
.push(parse_quote!(#lifetime: 'static))
}

let type_params = generics.type_params();
let ty_params_ids = type_params
let ty_params_ids = generics
.type_params()
.map(|type_param| type_param.ident.clone())
.collect::<Vec<Ident>>();

Expand Down Expand Up @@ -84,7 +88,9 @@ pub fn make_where_clause<'a>(
generics.type_params().into_iter().for_each(|type_param| {
let ident = type_param.ident.clone();
let mut bounds = type_param.bounds.clone();
bounds.push(parse_quote!(:: #scale_info ::TypeInfo));
if type_params.iter().any(|tp| *tp == *type_param) {
bounds.push(parse_quote!(:: #scale_info ::TypeInfo));
}
bounds.push(parse_quote!('static));
where_clause
.predicates
Expand Down
106 changes: 90 additions & 16 deletions derive/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,14 @@ use alloc::{
use proc_macro2::TokenStream;
use quote::quote;
use syn::{
parse::Parse,
parse_quote,
punctuated::Punctuated,
spanned::Spanned,
token,
AttrStyle,
Attribute,
DeriveInput,
Lit,
Meta,
NestedMeta,
Expand Down Expand Up @@ -55,6 +59,41 @@ pub fn get_doc_literals(attrs: &[syn::Attribute]) -> Vec<syn::Lit> {
.collect()
}

/// Trait bounds.
pub type TypeParams = Punctuated<syn::TypeParam, token::Comma>;

/// Parse `name(T, N)` as a custom trait bound.
struct SkipTypeParams<N> {
_name: N,
_paren_token: token::Paren,
params: TypeParams,
}

impl<N: Parse> Parse for SkipTypeParams<N> {
fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
let content;
let _name = input.parse()?;
let _paren_token = syn::parenthesized!(content in input);
let params = content.parse_terminated(syn::TypeParam::parse)?;
Ok(Self {
_name,
_paren_token,
params,
})
}
}

syn::custom_keyword!(skip_type_params);

/// Look for a `#[scale_info(skip_type_params(…))]`in the given attributes.
///
/// If found, do not register the given type params or require `TypeInfo` bounds for them.
ascjones marked this conversation as resolved.
Show resolved Hide resolved
pub fn skipped_type_params(attrs: &[Attribute]) -> Option<TypeParams> {
scale_info_meta_item(attrs.iter(), |meta: SkipTypeParams<skip_type_params>| {
Some(meta.params)
})
}

/// Look for a `#[codec(index = $int)]` attribute on a variant. If no attribute
/// is found, fall back to the discriminant or just the variant index.
pub fn variant_index(v: &Variant, i: usize) -> TokenStream {
Expand All @@ -77,7 +116,7 @@ pub fn maybe_index(variant: &Variant) -> Option<u8> {
.iter()
.filter(|attr| attr.style == AttrStyle::Outer);

find_meta_item(outer_attrs, |meta| {
codec_meta_item(outer_attrs, |meta| {
if let NestedMeta::Meta(Meta::NameValue(ref nv)) = meta {
if nv.path.is_ident("index") {
if let Lit::Int(ref v) = nv.lit {
Expand All @@ -99,7 +138,7 @@ pub fn is_compact(field: &syn::Field) -> bool {
.attrs
.iter()
.filter(|attr| attr.style == AttrStyle::Outer);
find_meta_item(outer_attrs, |meta| {
codec_meta_item(outer_attrs, |meta| {
if let NestedMeta::Meta(Meta::Path(ref path)) = meta {
if path.is_ident("compact") {
return Some(())
Expand All @@ -113,7 +152,7 @@ pub fn is_compact(field: &syn::Field) -> bool {

/// Look for a `#[codec(skip)]` in the given attributes.
pub fn should_skip(attrs: &[Attribute]) -> bool {
find_meta_item(attrs.iter(), |meta| {
codec_meta_item(attrs.iter(), |meta| {
if let NestedMeta::Meta(Meta::Path(ref path)) = meta {
if path.is_ident("skip") {
return Some(path.span())
Expand All @@ -125,22 +164,57 @@ pub fn should_skip(attrs: &[Attribute]) -> bool {
.is_some()
}

fn find_meta_item<'a, F, R, I>(itr: I, pred: F) -> Option<R>
fn codec_meta_item<'a, F, R, I, M>(itr: I, pred: F) -> Option<R>
where
F: Fn(&NestedMeta) -> Option<R> + Clone,
F: FnMut(M) -> Option<R> + Clone,
I: Iterator<Item = &'a Attribute>,
M: Parse,
{
itr.filter_map(|attr| {
if attr.path.is_ident("codec") {
if let Meta::List(ref meta_list) = attr
.parse_meta()
.expect("scale-info: Bad index in `#[codec(index = …)]`, see `parity-scale-codec` error")
{
return meta_list.nested.iter().filter_map(pred.clone()).next()
}
}
find_meta_item("codec", itr, pred)
}

None
fn scale_info_meta_item<'a, F, R, I, M>(itr: I, pred: F) -> Option<R>
where
F: FnMut(M) -> Option<R> + Clone,
I: Iterator<Item = &'a Attribute>,
M: Parse,
{
find_meta_item("scale_info", itr, pred)
}

fn find_meta_item<'a, F, R, I, M>(kind: &str, mut itr: I, mut pred: F) -> Option<R>
where
F: FnMut(M) -> Option<R> + Clone,
I: Iterator<Item = &'a Attribute>,
M: Parse,
{
itr.find_map(|attr| {
attr.path
.is_ident(kind)
.then(|| pred(attr.parse_args().ok()?))
.flatten()
})
.next()
}

/// Ensure attributes are correctly applied. This *must* be called before using
/// any of the attribute finder methods or the macro may panic if it encounters
/// misapplied attributes.
/// `#[scale_info(bounds())]` is the only accepted attribute.
pub fn check_attributes(input: &DeriveInput) -> syn::Result<()> {
for attr in &input.attrs {
check_top_attribute(attr)?;
}
Ok(())
}

// Only `#[scale_info(bounds())]` is a valid top attribute.
fn check_top_attribute(attr: &Attribute) -> syn::Result<()> {
if attr.path.is_ident("scale_info") {
match attr.parse_args::<SkipTypeParams<skip_type_params>>() {
Ok(_) => Ok(()),
Err(e) => Err(syn::Error::new(attr.span(), format!("Invalid attribute: {:?}. Only `#[scale_info(bounds(…))]` is a valid top attribute", e)))
}
} else {
Ok(())
}
}
9 changes: 3 additions & 6 deletions src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,14 +266,11 @@ impl TypeInfo for String {
}
}

impl<T> TypeInfo for PhantomData<T>
where
T: TypeInfo + ?Sized + 'static,
{
type Identity = Self;
impl<T> TypeInfo for PhantomData<T> {
type Identity = PhantomData<()>;

fn type_info() -> Type {
TypeDefPhantom::new(MetaType::new::<T>()).into()
TypeDefPhantom.into()
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ fn prelude_items() {
)
)
);
assert_type!(PhantomData<i32>, TypeDefPhantom::new(meta_type::<i32>()));
assert_type!(PhantomData<i32>, TypeDefPhantom);
assert_type!(
Cow<u128>,
Type::builder()
Expand Down
37 changes: 3 additions & 34 deletions src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ pub enum TypeDef<T: Form = MetaForm> {
/// A type using the [`Compact`] encoding
Compact(TypeDefCompact<T>),
/// A PhantomData type.
Phantom(TypeDefPhantom<T>),
Phantom(TypeDefPhantom),
}

impl IntoPortable for TypeDef {
Expand All @@ -211,7 +211,7 @@ impl IntoPortable for TypeDef {
TypeDef::Tuple(tuple) => tuple.into_portable(registry).into(),
TypeDef::Primitive(primitive) => primitive.into(),
TypeDef::Compact(compact) => compact.into_portable(registry).into(),
TypeDef::Phantom(phantom) => phantom.into_portable(registry).into(),
TypeDef::Phantom(phantom) => phantom.into(),
}
}
}
Expand Down Expand Up @@ -453,35 +453,4 @@ where
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg_attr(any(feature = "std", feature = "decode"), derive(scale::Decode))]
#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Encode, Debug)]
pub struct TypeDefPhantom<T: Form = MetaForm> {
/// The PhantomData type parameter
#[cfg_attr(feature = "serde", serde(rename = "type"))]
type_param: T::Type,
}

impl IntoPortable for TypeDefPhantom {
type Output = TypeDefPhantom<PortableForm>;

fn into_portable(self, registry: &mut Registry) -> Self::Output {
TypeDefPhantom {
type_param: registry.register_type(&self.type_param),
}
}
}

impl TypeDefPhantom {
/// Creates a new phantom type definition.
pub fn new(type_param: MetaType) -> Self {
Self { type_param }
}
}

impl<T> TypeDefPhantom<T>
where
T: Form,
{
/// Returns the type parameter type of the phantom type.
pub fn type_param(&self) -> &T::Type {
&self.type_param
}
}
pub struct TypeDefPhantom;
30 changes: 30 additions & 0 deletions test_suite/tests/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,36 @@ fn doc_capture_works() {
assert_type!(S, ty);
}

#[test]
fn skip_type_params() {
#[allow(unused)]
#[derive(TypeInfo)]
#[scale_info(skip_type_params(T))]
struct Hey<T, N> {
ciao: Greet<T>,
ho: N,
}
ascjones marked this conversation as resolved.
Show resolved Hide resolved

#[derive(TypeInfo)]
#[scale_info(skip_type_params(T))]
struct Greet<T> {
marker: PhantomData<T>,
}

struct SomeType;

let ty = Type::builder()
.path(Path::new("Hey", "derive"))
.type_params(tuple_meta_type!(u16))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems to me more correct to me to have the first type param marked as skipped than ignored, otherwise when looking at the metadata and the doc of the rust type we can't easily tell which one is skipped and which one is kept

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean like .composite(Fields::named().field(|f| f.ty::<Greet<SomeType>>().name("ciao").type_name("Greet<T>").skipped_type_params::<T>())?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually I'm not sure anymore, I don't understand enough the usecase for type_params.

But what I meant is that currently the Hey<SomeType, u16> type information about type params would be u16, whereas I would have expected "skipped", u16.

Copy link
Contributor Author

@ascjones ascjones Jun 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point, we are losing some information by removing it entirely. I have addressed this by adding a TypeParameter struct with Option<MetaType> which would be None for skipped type params. Addressed in 411e50c

.composite(
Fields::named()
.field(|f| f.ty::<Greet<SomeType>>().name("ciao").type_name("Greet<T>"))
.field(|f| f.ty::<u16>().name("ho").type_name("N")),
);

assert_type!(Hey<SomeType, u16>, ty);
}

#[rustversion::nightly]
#[test]
fn ui_tests() {
Expand Down
4 changes: 1 addition & 3 deletions test_suite/tests/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,7 @@ fn test_builtins() {
assert_json_for_type::<String>(json!({ "def": { "primitive": "str" } }));
assert_json_for_type::<str>(json!({ "def": { "primitive": "str" } }));
// PhantomData
assert_json_for_type::<PhantomData<bool>>(
json!({ "def": { "phantom": { "type": 0 } }, }),
)
assert_json_for_type::<PhantomData<bool>>(json!({ "def": { "phantom": null }, }))
}

#[test]
Expand Down
Loading