-
Notifications
You must be signed in to change notification settings - Fork 30
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
+785
−185
Merged
Changes from 48 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 b0eda68
Fmt
ascjones bd6dd5d
cleanup
dvdplm 171cd4c
Merge branch 'dp-custom-bound' of github.com:paritytech/scale-info in…
dvdplm 030d407
Merge branch 'master' into dp-custom-bound
ascjones b937c9a
Skip type params prototype
ascjones 725ceca
Merge branch 'master' into aj-skip-type-params
ascjones 56edb9c
Fmt
ascjones f503781
Clippy
ascjones 1743b48
Satisfy clippy
ascjones bf9cf6d
Fix skip type params
ascjones 2d4f1e8
Fix UI test
ascjones 9c4f6d6
Add some more tests
ascjones 100dfcf
Add failing test for type params with default values
ascjones c92041b
Fix for type params with defaults, compare on ident
ascjones a23cd47
Merge branch 'master' into aj-skip-type-params
ascjones 3678114
Merge branch 'master' into aj-skip-type-params
ascjones 4cdf0c5
WIP use bounds attribute for skipping type params
ascjones 99cb9d3
Add required 'static bounds to all type params
ascjones c39167d
Fmt
ascjones 835efe3
Satisfy clippy
ascjones 75d25ad
Add UI test for skipping bounds
ascjones 3012932
WIP docs
ascjones 20ff837
Revert "Use bounds attribute for skipping type params"
ascjones 7cdd440
WIP dual attribute parsing
ascjones 6d97c49
Use new attribute parsing
ascjones 7cab3e3
Fix up attribute parsing
ascjones 1f84f41
Fmt
ascjones 0716333
Reorder impls
ascjones a2d239b
Refactor attribute handling
ascjones ecf21f3
Fmt
ascjones f876d22
Add docs to attributes
ascjones b58904f
Add `'static` bounds for all type params, add some ui tests
ascjones 9f64f74
Merge remote-tracking branch 'origin/master' into aj-skip-type-params
ascjones d61f75d
Check for duplicate attributes
ascjones a1741d2
Add helpful error message for type params in not in cuatom bounds and…
ascjones 2da8e3f
Improve error message where a type param is missing from the bounds, …
ascjones 2dc9484
Fix test and fmt
ascjones 39e40b1
Refactor and validate missing skip type params
ascjones 44c2af2
Update validation UI test
ascjones 91799c7
Error message formatting
ascjones e4d570d
Merge remote-tracking branch 'origin/master' into aj-skip-type-params
ascjones 271c2b5
Fix compilation after merge
ascjones a7e1e6b
Merge remote-tracking branch 'origin/master' into aj-skip-type-params
ascjones 411e50c
Add TypeParameter struct and optional type
ascjones c5f2bd1
Add ui test for skipping type params
ascjones d453e69
Add example to named_type_params macro
ascjones b228318
Type hint for named_type_params with MetaForm
ascjones f060e59
Add bounds attribute docs
ascjones 7bfa773
Add some docs attribute usage
ascjones eb41e0b
Fmt
ascjones File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,202 @@ | ||
// Copyright 2019-2021 Parity Technologies (UK) Ltd. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
use syn::{ | ||
parse::{ | ||
Parse, | ||
ParseBuffer, | ||
}, | ||
punctuated::Punctuated, | ||
spanned::Spanned, | ||
Token, | ||
}; | ||
|
||
const SCALE_INFO: &str = "scale_info"; | ||
|
||
mod keywords { | ||
syn::custom_keyword!(scale_info); | ||
syn::custom_keyword!(bounds); | ||
syn::custom_keyword!(skip_type_params); | ||
} | ||
|
||
/// Parsed and validated set of `#[scale_info(...)]` attributes for an item. | ||
pub struct Attributes { | ||
bounds: Option<BoundsAttr>, | ||
skip_type_params: Option<SkipTypeParamsAttr>, | ||
} | ||
|
||
impl Attributes { | ||
/// Extract out `#[scale_info(...)]` attributes from an item. | ||
pub fn from_ast(item: &syn::DeriveInput) -> syn::Result<Self> { | ||
let mut bounds = None; | ||
let mut skip_type_params = None; | ||
|
||
let attributes_parser = |input: &ParseBuffer| { | ||
let attrs: Punctuated<ScaleInfoAttr, Token![,]> = | ||
input.parse_terminated(ScaleInfoAttr::parse)?; | ||
Ok(attrs) | ||
}; | ||
|
||
for attr in &item.attrs { | ||
if !attr.path.is_ident(SCALE_INFO) { | ||
continue | ||
} | ||
let scale_info_attrs = attr.parse_args_with(attributes_parser)?; | ||
|
||
for scale_info_attr in scale_info_attrs { | ||
// check for duplicates | ||
match scale_info_attr { | ||
ScaleInfoAttr::Bounds(parsed_bounds) => { | ||
if bounds.is_some() { | ||
return Err(syn::Error::new( | ||
attr.span(), | ||
"Duplicate `bounds` attributes", | ||
)) | ||
} | ||
bounds = Some(parsed_bounds); | ||
} | ||
ScaleInfoAttr::SkipTypeParams(parsed_skip_type_params) => { | ||
if skip_type_params.is_some() { | ||
return Err(syn::Error::new( | ||
attr.span(), | ||
"Duplicate `skip_type_params` attributes", | ||
)) | ||
} | ||
skip_type_params = Some(parsed_skip_type_params); | ||
} | ||
} | ||
} | ||
} | ||
|
||
// validate type params which do not appear in custom bounds but are not skipped. | ||
if let Some(ref bounds) = bounds { | ||
for type_param in item.generics.type_params() { | ||
if !bounds.contains_type_param(type_param) { | ||
let type_param_skipped = skip_type_params | ||
.as_ref() | ||
.map(|skip| skip.skip(type_param)) | ||
.unwrap_or(false); | ||
if !type_param_skipped { | ||
let msg = format!( | ||
"Type parameter requires a `TypeInfo` bound, so either: \n \ | ||
- add it to `#[scale_info(bounds({}: TypeInfo))]` \n \ | ||
- skip it with `#[scale_info(skip_type_params({}))]`", | ||
type_param.ident, type_param.ident | ||
); | ||
return Err(syn::Error::new(type_param.span(), msg)) | ||
} | ||
} | ||
} | ||
} | ||
|
||
Ok(Self { | ||
bounds, | ||
skip_type_params, | ||
}) | ||
} | ||
|
||
/// Get the `#[scale_info(bounds(...))]` attribute, if present. | ||
pub fn bounds(&self) -> Option<&BoundsAttr> { | ||
self.bounds.as_ref() | ||
} | ||
|
||
/// Get the `#[scale_info(skip_type_params(...))]` attribute, if present. | ||
pub fn skip_type_params(&self) -> Option<&SkipTypeParamsAttr> { | ||
self.skip_type_params.as_ref() | ||
} | ||
} | ||
|
||
/// Parsed representation of the `#[scale_info(bounds(...))]` attribute. | ||
#[derive(Clone)] | ||
pub struct BoundsAttr { | ||
predicates: Punctuated<syn::WherePredicate, Token![,]>, | ||
} | ||
|
||
impl Parse for BoundsAttr { | ||
fn parse(input: &ParseBuffer) -> syn::Result<Self> { | ||
input.parse::<keywords::bounds>()?; | ||
let content; | ||
syn::parenthesized!(content in input); | ||
let predicates = content.parse_terminated(syn::WherePredicate::parse)?; | ||
Ok(Self { predicates }) | ||
} | ||
} | ||
|
||
impl BoundsAttr { | ||
/// Add the predicates defined in this attribute to the given `where` clause. | ||
pub fn extend_where_clause(&self, where_clause: &mut syn::WhereClause) { | ||
where_clause.predicates.extend(self.predicates.clone()); | ||
} | ||
|
||
/// Returns true if the given type parameter appears in the custom bounds attribute. | ||
pub fn contains_type_param(&self, type_param: &syn::TypeParam) -> bool { | ||
self.predicates.iter().any(|p| { | ||
if let syn::WherePredicate::Type(ty) = p { | ||
if let syn::Type::Path(ref path) = ty.bounded_ty { | ||
path.path.get_ident() == Some(&type_param.ident) | ||
} else { | ||
false | ||
} | ||
} else { | ||
false | ||
} | ||
}) | ||
} | ||
} | ||
|
||
/// Parsed representation of the `#[scale_info(skip_type_params(...))]` attribute. | ||
#[derive(Clone)] | ||
pub struct SkipTypeParamsAttr { | ||
type_params: Punctuated<syn::TypeParam, Token![,]>, | ||
} | ||
|
||
impl Parse for SkipTypeParamsAttr { | ||
fn parse(input: &ParseBuffer) -> syn::Result<Self> { | ||
input.parse::<keywords::skip_type_params>()?; | ||
let content; | ||
syn::parenthesized!(content in input); | ||
let type_params = content.parse_terminated(syn::TypeParam::parse)?; | ||
Ok(Self { type_params }) | ||
} | ||
} | ||
|
||
impl SkipTypeParamsAttr { | ||
/// Returns `true` if the given type parameter should be skipped. | ||
pub fn skip(&self, type_param: &syn::TypeParam) -> bool { | ||
self.type_params | ||
.iter() | ||
.any(|tp| tp.ident == type_param.ident) | ||
} | ||
} | ||
|
||
/// Parsed representation of one of the `#[scale_info(..)]` attributes. | ||
pub enum ScaleInfoAttr { | ||
Bounds(BoundsAttr), | ||
SkipTypeParams(SkipTypeParamsAttr), | ||
} | ||
|
||
impl Parse for ScaleInfoAttr { | ||
fn parse(input: &ParseBuffer) -> syn::Result<Self> { | ||
let lookahead = input.lookahead1(); | ||
if lookahead.peek(keywords::bounds) { | ||
let bounds = input.parse()?; | ||
Ok(Self::Bounds(bounds)) | ||
} else if lookahead.peek(keywords::skip_type_params) { | ||
let skip_type_params = input.parse()?; | ||
Ok(Self::SkipTypeParams(skip_type_params)) | ||
} else { | ||
Err(input.error("Expected either `bounds` or `skip_type_params`")) | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the above, I will not get
TypeInfo
bounds auto-generated forT
(becausebounds()
override all auto generating machinery) but I will get the bounds I manually specified forU
(becausebounds(…)
trumpsskip_type_params
). Is that correct?A few examples like above would be good! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. It trumps it in respect to the generated
where
clause. Howeverskip_type_params
still has its effect on the generated list oftype_params
for the type.I am in the process of adding more rustdocs on the topic, in the meantime I have added some more ui tests and have updated the PR description.