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

Robust crate names (alternative to #61) #62

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .rustfmt.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ fn_single_line = false
where_single_line = false
imports_indent = "Block"
imports_layout = "Vertical" # changed
merge_imports = true # changed
imports_granularity = "Crate" # changed
reorder_imports = true
reorder_modules = true
reorder_impl_items = false
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Added

### Changed

### Fixed
- Derive: use known crate name aliases [(#61)](https://github.com/paritytech/scale-info/pull/61)

## [0.5.0] - 2021-01-27
### Added
Expand Down
2 changes: 2 additions & 0 deletions derive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ proc-macro = true
quote = "1.0"
syn = { version = "1.0", features = ["derive", "visit", "visit-mut"] }
proc-macro2 = "1.0"
proc-macro-crate = "0.1.5"
once_cell = "1.5"
43 changes: 0 additions & 43 deletions derive/src/impl_wrapper.rs

This file was deleted.

56 changes: 42 additions & 14 deletions derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
extern crate alloc;
extern crate proc_macro;

mod impl_wrapper;
mod trait_bounds;

use alloc::{
Expand All @@ -28,7 +27,10 @@ use alloc::{
vec::Vec,
};
use proc_macro::TokenStream;
use proc_macro2::TokenStream as TokenStream2;
use proc_macro2::{
Span,
TokenStream as TokenStream2,
};
use quote::quote;
use syn::{
parse::{
Expand All @@ -48,6 +50,7 @@ use syn::{
ExprLit,
Field,
Fields,
Ident,
Lifetime,
Lit,
Meta,
Expand All @@ -56,6 +59,26 @@ use syn::{
Variant,
};

use once_cell::sync::Lazy;

struct IdentHelper(Lazy<String>);

impl quote::ToTokens for IdentHelper {
fn to_tokens(&self, tokens: &mut TokenStream2) {
Ident::new(&self.0, Span::call_site()).to_tokens(tokens)
}
}
/// Get the name of the scale-info crate, to be robust against renamed dependencies.
static SCALE_INFO: IdentHelper = IdentHelper(Lazy::new(|| {
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a (probably irrational) aversion to global singletons unless absolutely necessary, though there is nothing wrong with them per se they are often a code smell.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Conceptually these aren't really globals of the bad kind though; they are constants and not really "state" (and obviously not mutable state).
Imagine a parallel universe where an Ident as actually a &'static str and proc-macro-crate::crate_name was a const fn, wouldn't it be more natural to have const SCALE_INFO: Ident = proc-macro-crate::crate_name("scale-info").expect("wut?!");? The crate name used is a value that does not vary, it's constant.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand your point, but like I said it's irrational 😁

proc_macro_crate::crate_name("scale-info")
.expect("Missing scale-info dependency. Please add it to your Cargo.toml")
}));
/// Get the name of the parity-scale-codec crate, to be robust against renamed dependencies.
static CODEC: IdentHelper = IdentHelper(Lazy::new(|| {
proc_macro_crate::crate_name("parity-scale-codec")
.expect("Missing parity-scale-codec dependency. Please add it to your Cargo.toml")
}));

#[proc_macro_derive(TypeInfo)]
pub fn type_info(input: TokenStream) -> TokenStream {
match generate(input.into()) {
Expand Down Expand Up @@ -85,7 +108,7 @@ fn generate_type(input: TokenStream2) -> Result<TokenStream2> {
let generic_type_ids = ast.generics.type_params().map(|ty| {
let ty_ident = &ty.ident;
quote! {
::scale_info::meta_type::<#ty_ident>()
:: #SCALE_INFO ::meta_type::<#ty_ident>()
}
});

Expand All @@ -97,18 +120,23 @@ fn generate_type(input: TokenStream2) -> Result<TokenStream2> {
};
let generic_types = ast.generics.type_params();
let type_info_impl = quote! {
impl <#( #generic_types ),*> ::scale_info::TypeInfo for #ident #ty_generics #where_clause {
impl <#( #generic_types ),*> :: #SCALE_INFO ::TypeInfo for #ident #ty_generics #where_clause {
type Identity = Self;
fn type_info() -> ::scale_info::Type {
::scale_info::Type::builder()
.path(::scale_info::Path::new(stringify!(#ident), module_path!()))
.type_params(::scale_info::prelude::vec![ #( #generic_type_ids ),* ])
fn type_info() -> :: #SCALE_INFO ::Type {
:: #SCALE_INFO ::Type::builder()
.path(:: #SCALE_INFO ::Path::new(stringify!(#ident), module_path!()))
.type_params(:: #SCALE_INFO ::prelude::vec![ #( #generic_type_ids ),* ])
.#build_type
}
}
};

Ok(impl_wrapper::wrap(ident, "TYPE_INFO", type_info_impl))
Ok(quote! {
#[allow(non_upper_case_globals, unused_attributes, unused_qualifications)]
const _: () = {
#type_info_impl;
};
})
}

type FieldsList = Punctuated<Field, Comma>;
Expand Down Expand Up @@ -198,7 +226,7 @@ fn generate_composite_type(data_struct: &DataStruct) -> TokenStream2 {
}
};
quote! {
composite(::scale_info::build::Fields::#fields)
composite(:: #SCALE_INFO ::build::Fields::#fields)
}
}

Expand Down Expand Up @@ -228,7 +256,7 @@ fn generate_c_like_enum_def(variants: &VariantList) -> TokenStream2 {
});
quote! {
variant(
::scale_info::build::Variants::fieldless()
:: #SCALE_INFO ::build::Variants::fieldless()
#( #variants )*
)
}
Expand Down Expand Up @@ -257,7 +285,7 @@ fn generate_variant_type(data_enum: &DataEnum) -> TokenStream2 {
quote! {
.variant(
#v_name,
::scale_info::build::Fields::named()
:: #SCALE_INFO ::build::Fields::named()
#( #fields)*
)
}
Expand All @@ -267,7 +295,7 @@ fn generate_variant_type(data_enum: &DataEnum) -> TokenStream2 {
quote! {
.variant(
#v_name,
::scale_info::build::Fields::unnamed()
:: #SCALE_INFO ::build::Fields::unnamed()
#( #fields)*
)
}
Expand All @@ -281,7 +309,7 @@ fn generate_variant_type(data_enum: &DataEnum) -> TokenStream2 {
});
quote! {
variant(
::scale_info::build::Variants::with_fields()
:: #SCALE_INFO ::build::Variants::with_fields()
#( #variants)*
)
}
Expand Down
12 changes: 8 additions & 4 deletions derive/src/trait_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use super::{
CODEC,
SCALE_INFO,
};
use alloc::vec::Vec;
use proc_macro2::Ident;
use syn::{
Expand Down Expand Up @@ -57,21 +61,21 @@ pub fn make_where_clause<'a>(
if is_compact {
where_clause
.predicates
.push(parse_quote!(#ty : ::scale::HasCompact));
.push(parse_quote!(#ty : :: #CODEC ::HasCompact));
where_clause
.predicates
.push(parse_quote!(<#ty as ::scale::HasCompact>::Type : ::scale_info::TypeInfo + 'static));
.push(parse_quote!(<#ty as :: #CODEC ::HasCompact>::Type : :: #SCALE_INFO ::TypeInfo + 'static));
} else {
where_clause
.predicates
.push(parse_quote!(#ty : ::scale_info::TypeInfo + 'static));
.push(parse_quote!(#ty : :: #SCALE_INFO ::TypeInfo + 'static));
}
});

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));
bounds.push(parse_quote!(:: #SCALE_INFO ::TypeInfo));
bounds.push(parse_quote!('static));
where_clause
.predicates
Expand Down
5 changes: 1 addition & 4 deletions test_suite/tests/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@
#![cfg_attr(not(feature = "std"), no_std)]

use pretty_assertions::assert_eq;
use scale::{
Compact,
Encode,
};
use scale::Encode;
use scale_info::{
build::*,
prelude::{
Expand Down